[$500] Web -Split bill- Square appears over checkmark when deselect participant and than use keyboard arrow #32400

kbecciv · 2023-12-02T15:06:57Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 1.4.7-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Navigate to http://staging.new.expensify.com/
Open Group DM
Click on "+" near message composer
Click on "Split bill"
Insert amount
Click "Next"
Click with mouse on any member to deselect it
Use the keyboard up/down arrows to go through the list

Expected Result:

There should be no visual errors while scrolling through members list

Actual Result:

White square appears over checkmark when deselect participant and than use keyboard arrow

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

Bug6298629_1701521884927.Recording__1447.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~017a562fd6464d4101
Upwork Job ID: 1730966804029255680
Last Price Increase: 2023-12-23

melvin-bot · 2023-12-02T15:07:02Z

Job added to Upwork: https://www.upwork.com/jobs/~017a562fd6464d4101

melvin-bot · 2023-12-02T15:07:03Z

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-12-02T15:07:06Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

melvin-bot · 2023-12-02T15:07:08Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)

tienifr · 2023-12-02T15:48:50Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Square appears over checkmark when deselect participant and press arrow key

What is the root cause of that problem?

A div's tabIndex is set with 0 by default.

What changes do you think we should make in order to solve the problem?

Set tabIndex to -1 of the View wrapping circle icon:

App/src/components/SelectCircle.tsx

Line 21 in 1682be4

    
           <View style={[styles.selectCircle, styles.alignSelfCenter, selectCircleStyles]}>

However, the circle border is still focusable:

In case we want to completely forbid focusing by key on Icon, we can set tabIndex={-1} to the Views inside Icon component here and here. Or we can introduce tabIndex prop for Icon so we can manually control the focus behavior here.

What alternative solutions did you explore? (Optional)

NA

bernhardoj · 2023-12-02T16:37:00Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

The check(circle) icon of money request confirmation list outline is squared instead of circle.

What is the root cause of that problem?

The money request confirmation list currently uses OptionsSelector which renders OptionRow as the item. In OptionRow, we render the check(circle) with a Pressable and SelectCircle component.

App/src/components/OptionRow.js

Lines 268 to 275 in 1682be4

    
           <PressableWithFeedback 
        
               onPress={() => props.onSelectedStatePressed(props.option)} 
        
               disabled={isDisabled} 
        
               role={CONST.ACCESSIBILITY_ROLE.CHECKBOX} 
        
               accessibilityLabel={CONST.ACCESSIBILITY_ROLE.CHECKBOX} 
        
           > 
        
               <SelectCircle isChecked={props.isSelected} /> 
        
           </PressableWithFeedback>

SelectCircle is the one that renders the circle and the checked icon. The pressable itself doesn't have any style, so when we focus it with keyboard, we see a squared outline. Also, if we look carefully, the outline has an extra space on the left that is caused by a margin-left (styles.selectCircle) that is applied in SelectCircle.

App/src/components/SelectCircle.tsx

Lines 21 to 28 in 1682be4

    
           <View style={[styles.selectCircle, styles.alignSelfCenter, selectCircleStyles]}> 
        
               {isChecked && ( 
        
                   <Icon 
        
                       src={Expensicons.Checkmark} 
        
                       fill={theme.iconSuccessFill} 
        
                   /> 
        
               )} 
        
           </View>

What changes do you think we should make in order to solve the problem?

First, we need to set the SelectCircle margin-left to 0,

<SelectCircle isChecked={props.isSelected} selectCircleStyles={styles.ml0} />

and move it to the pressable. Next, apply the same border-radius + 1 (padding) style of SelectCircle to the pressable similar to what we did in Checkbox,

App/src/components/Checkbox.tsx

Line 91 in 1682be4

    
           style={[StyleUtils.getCheckboxPressableStyle(containerBorderRadius + 2), style]} // to align outline on focus, border-radius of pressable should be 2px more than Checkbox

and also add padding 1 so the blue outline is visible (similar to Checkbox where we add 2 padding).

<PressableWithFeedback
    style={{borderRadius: (variables.componentSizeSmall / 2) + 1, padding: 1, ...styles.ml2}}

if we don't add the padding, the blue outline won't show because it's covered by the outline of SelectCircle

tanmai-kms · 2023-12-02T16:56:14Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

The square appears over checkmark when deselecting participant not only uses keyboard arrow but also clicks on checkbox directly

What is the root cause of that problem?

The root cause is that it's impacted from global style :focus-visible

App/web/index.html

Line 60 in 1682be4

:focus-visible {

What changes do you think we should make in order to solve the problem?

We shouldn't fix it inside SelectCircle component only because the issue is happening with all Pressable -> GenericPressable components

We should add new 1 more global style for [data-tag="pressable"]:focus-visible with box-shadow: none in the web/index.html. This is to make all "pressable" component correct.

What alternative solutions did you explore? (Optional)

We can fix by adding new style for Pressable component

melvin-bot · 2023-12-02T16:56:18Z

📣 @tanmai-kms! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

Make sure you've read and understood the contributing guidelines.
Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

tanmai-kms · 2023-12-02T17:06:16Z

Contributor details
Your Expensify account email: duytan.learning.agu@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01b376159894de44f5

melvin-bot · 2023-12-02T17:06:19Z

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

suneox · 2023-12-02T18:15:12Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Web -Split bill- Square appears over checkmark when deselect participant and than use keyboard arrow

What is the root cause of that problem?

By default, we have config focus-visible to set box-shadow when lose focus

What changes do you think we should make in order to solve the problem?

For this case OptionRow has handled up/down key to select an item so on PressableWithFeedback at this line we just add style styles.noSelect and remove accessibility when use tab instead of up/down key

    <PressableWithFeedback
        onPress={() => props.onSelectedStatePressed(props.option)}
        disabled={isDisabled}
        role={CONST.ACCESSIBILITY_ROLE.CHECKBOX}
        accessibilityLabel={CONST.ACCESSIBILITY_ROLE.CHECKBOX}
+       style={styles.noSelect}
+       tabIndex={-1}
    >
        <SelectCircle isChecked={props.isSelected} />
    </PressableWithFeedback>

What alternative solutions did you explore? (Optional)

dylanexpensify · 2023-12-05T19:57:45Z

@eVoloshchak to review proposals!

dragnoir · 2023-12-07T11:10:15Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Web -Split bill- Focus from browser appears over checkmark and over the row when deselect participant or select roo, than use keyboard arrow up/down

What is the root cause of that problem?

The issue is bigger than just a focus style applied to a selected HTML element.
When we click on a row, in the dom it's a button or a div with aria-label="checkbox", the browser apply a focus on it, wish next can trigger actions by the keyboard.

004.mp4

If we follow this flow:
1- With mouse, click on the last element under "split with" (not the checkbox), the row
2- move to the upper row with keyboard (now the previous row is focused but the new row have the beackground effect wish visually seems it's the one focused)
3- from keyboard, push "space", and here appear the true bug, the action happen on the previous row, wish is wrong.
⇒ This happen because browser is keeping focus on the element we interacted with mouse
The <ArrowKeyFocusManager> change the state of foocusedIndex buit does not manupulate or mutate the space or change the focus on the browser.

Even if we execute the previous proposals, changing focus style on index html to to box-shadow: none; or outline:0;, or adding the tabIndex={-1} , the main issue I described will persist, just the visual focus effect will diseppear.

What changes do you think we should make in order to solve the problem?

Once we click on a PressableWithFeedback we need to use the blur() method. This HTMLElement.blur() method removes keyboard focus from the clicked element.

App/src/components/OptionRow.js

Lines 164 to 166 in 10727a7

    
           <PressableWithFeedback 
        
               ref={(el) => (pressableRef.current = el)} 
        
               onPress={(e) => {

We can apply it on onPress

<PressableWithFeedback
	ref={(el) => (pressableRef.current = el)}
	onPress={(e) => {
	// ....

	pressableRef.current.blur();

}}

We need also to apply the same blur method to the checkbox and this solve boath issues.

App/src/components/OptionRow.js

Lines 268 to 269 in 10727a7

    
           <PressableWithFeedback 
        
               onPress={() => props.onSelectedStatePressed(props.option)}

change to:

<PressableWithFeedback
	ref={pressableRowRef}
	onPress={() => {
		props.onSelectedStatePressed(props.option);
		pressableRowRef.current.blur();
	}}

Result (video)

005.mp4

melvin-bot · 2023-12-09T16:00:07Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

dylanexpensify · 2023-12-11T09:26:13Z

@eVoloshchak to review proposals!

dylanexpensify · 2023-12-13T08:48:38Z

Bump @eVoloshchak

eVoloshchak · 2023-12-13T22:17:11Z

The issue turned out to be more complex, we have 3 distinct problems with the participants list

Blue outline appears over the checkmark when selecting participant and then using a keyboard arrow
Select a participant using arrow keys, press "Space" to add a participant. Notice the wrong participant is selected (@dragnoir has described this problem in their proposal)
Whether you can select a user by pressing "Space" is not consistent. It's possible only if you click on the list of participants (which defeats the purpose of keyboard navigation), and if you click outside the list, it stops working

Screen.Recording.2023-12-13.at.23.14.39.mov

A proposal to resolve all of the issues above would be ideal, we'd fix the whole keyboard navigation flow on this page

bernhardoj · 2023-12-14T04:40:10Z

Keyboard navigation should be out of the scope of this issue. The issue here is simply about the focus outline of the circle.

There is an issue here that is trying to fix the inconsistent keyboard navigation (looking at the PR, they will prioritize the row that has the browser focus).

Whether you can select a user by pressing "Space" is not consistent. It's possible only if you click on the list of participants (which defeats the purpose of keyboard navigation), and if you click outside the list, it stops working

You can only select by pressing Space when the element is focused. This is the native behavior of the web. Currently, we only have a custom enter listener. Having a new custom space key listener would be a new feature.

melvin-bot · 2023-12-16T14:15:18Z

@eVoloshchak @dylanexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

bernhardoj · 2024-01-24T05:02:36Z

PR is ready

cc: @shubham1206agra

dragnoir · 2024-01-25T12:47:50Z

@shubham1206agra the PR submitted still have the issue I mentioned.

I think my proposal is better, it remove the issue.

@dragnoir solution works in this case but will have a problem with keyboard selection and we can still see misaligned blue box if we try to go with keyboard (tab/shift+tab).

Also about keyboard keyboard (tab/shift+tab), pls check here https://expensify.slack.com/archives/C01GTK53T8Q/p1668604822482759 we are not supporting this type of navigation I guess

dylanexpensify · 2024-01-29T11:38:28Z

@bernhardoj @shubham1206agra what's holding up our merge?

shubham1206agra · 2024-01-29T13:04:40Z

@dylanexpensify I am OOO right now. Will review the PR tomorrow.

shubham1206agra · 2024-01-30T03:54:12Z

@shubham1206agra the PR submitted still have the issue I mentioned.

I think my proposal is better, it remove the issue.

Sorry but your changes will fall under keyboard navigation changes, which is not allowed. The proposal by @bernhardoj will fix the visual inconsistencies, and will also fix the area of pressable action of checkbox.

@dragnoir solution works in this case but will have a problem with keyboard selection and we can still see misaligned blue box if we try to go with keyboard (tab/shift+tab).

Also about keyboard keyboard (tab/shift+tab), pls check here https://expensify.slack.com/archives/C01GTK53T8Q/p1668604822482759 we are not supporting this type of navigation I guess

About support, I have asked the same internally https://expensify.slack.com/archives/C02NK2DQWUX/p1706050548700389?thread_ts=1705396204.297479&cid=C02NK2DQWUX

dragnoir · 2024-01-30T06:46:36Z

@shubham1206agra thank you for your reply.

Please test the PR when you have time, if you do this:

Select or deselect an option with mouse
navigate with arrow key top or down
click on space or enter

then the wrong option will be actioned (selected/deselected) not the one highlighted.

shubham1206agra · 2024-01-30T12:28:27Z

@shubham1206agra thank you for your reply.

Please test the PR when you have time, if you do this:
* Select or deselect an option with mouse

* navigate with arrow key top or down

* click on space or enter
then the wrong option will be actioned (selected/deselected) not the one highlighted.

Highlighted by what?
I think there are 2 different highlighting occurring at the same time, which is causing these problems. I will open up a discussion for the same later.

dylanexpensify · 2024-02-07T10:30:20Z

@bernhardoj @shubham1206agra can we get an update please

shubham1206agra · 2024-02-07T10:35:42Z

PR is merged already, and is deployed to prod too.

dragnoir · 2024-02-09T10:00:16Z

@shubham1206agra as I mentioned before, the issue is not fixed, the silution implemented I think it's just a workaround.

Please check the video demonstrationg the issue after the PR.

20240209_105351.mp4

Currently just the focus style is changed from square to round. But when you navigate and try to select an element from the list, the action happen the the previous element where the focus is not where the new element selected

@dylanexpensify As you see from the video and the screenshot below, when I navigate to (2) and I click on space or enter on the keyboard, the item that should be selected/deselected is (2) not (1) as happening now.

shubham1206agra · 2024-02-09T10:11:19Z

@dragnoir I understand what you are saying, but I am saying that this issue requires changes to keyboard navigation structure. So, the issue is out of scope of the current issue. You may want to open another issue for this issue for a detailed discussion for this.

dragnoir · 2024-02-09T10:20:41Z

@shubham1206agra we already support this type of navigation in other component like search, LHN, .... also keeping the focus actif on an item, for the user, means the ability to interact with it with keyboard or with other navigation tools. I think just changing the style to round and don't care about what element is trully focused it very bad UX and not a fix for such an app.

dylanexpensify · 2024-02-13T20:20:05Z

@shubham1206agra mind confirming where we're at

shubham1206agra · 2024-02-14T02:04:52Z

@dylanexpensify The PR was deployed a week ago. So we can get ready for payment.

shubham1206agra · 2024-02-14T02:06:20Z

For #32400 (comment), we need to open another issue to start the discussion about Keyboard Nav.

cc @dylanexpensify

dylanexpensify · 2024-02-21T10:45:26Z

Got it - will do today!

shubham1206agra · 2024-02-26T15:07:49Z

@dylanexpensify Can you start payment here?

dylanexpensify · 2024-02-28T08:52:08Z

Yes!

Payment summary:

Contributor: @bernhardoj $500
Contributor+: @shubham1206agra $500

Please request/apply!

shubham1206agra · 2024-02-28T08:54:41Z

@dylanexpensify You need to send us an offer.

dylanexpensify · 2024-02-28T08:59:22Z

I know - in the works!

dylanexpensify · 2024-02-28T10:45:30Z

Offers sent!

dylanexpensify · 2024-02-28T13:11:02Z

done!

kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 2, 2023

melvin-bot bot changed the title ~~Web -Split bill- Square appears over checkmark when deselect participant and than use keyboard arrow~~ [$500] Web -Split bill- Square appears over checkmark when deselect participant and than use keyboard arrow Dec 2, 2023

melvin-bot bot assigned dylanexpensify Dec 2, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 2, 2023

melvin-bot bot assigned eVoloshchak Dec 2, 2023

melvin-bot bot added the Overdue label Dec 5, 2023

melvin-bot bot removed the Overdue label Dec 5, 2023

melvin-bot bot added the Overdue label Dec 8, 2023

melvin-bot bot removed the Overdue label Dec 11, 2023

AndrewGable assigned bernhardoj Jan 24, 2024

bernhardoj mentioned this issue Jan 24, 2024

Fix split bill participant option select circle outline #35028

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Jan 24, 2024

kbecciv mentioned this issue Feb 4, 2024

Deploy Checklist: New Expensify 2024-02-02 #35673

Closed

dylanexpensify closed this as completed Feb 28, 2024

bernhardoj mentioned this issue Sep 24, 2024

[HOLD for payment 2024-10-25] [Hold for Payment 2024-10-23][$125] Web - Blue border appears on a radio button when pressing a keyboard #49501

Closed

6 tasks

[$500] Web -Split bill- Square appears over checkmark when deselect participant and than use keyboard arrow #32400

[$500] Web -Split bill- Square appears over checkmark when deselect participant and than use keyboard arrow #32400

Comments

kbecciv commented Dec 2, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 2, 2023

melvin-bot bot commented Dec 2, 2023

melvin-bot bot commented Dec 2, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Dec 2, 2023

tienifr commented Dec 2, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

bernhardoj commented Dec 2, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

tanmai-kms commented Dec 2, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Dec 2, 2023

tanmai-kms commented Dec 2, 2023

melvin-bot bot commented Dec 2, 2023

suneox commented Dec 2, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

dylanexpensify commented Dec 5, 2023

dragnoir commented Dec 7, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

melvin-bot bot commented Dec 9, 2023

dylanexpensify commented Dec 11, 2023

dylanexpensify commented Dec 13, 2023

eVoloshchak commented Dec 13, 2023

bernhardoj commented Dec 14, 2023

melvin-bot bot commented Dec 16, 2023

bernhardoj commented Jan 24, 2024

dragnoir commented Jan 25, 2024

dylanexpensify commented Jan 29, 2024

shubham1206agra commented Jan 29, 2024

shubham1206agra commented Jan 30, 2024

dragnoir commented Jan 30, 2024

shubham1206agra commented Jan 30, 2024

dylanexpensify commented Feb 7, 2024

shubham1206agra commented Feb 7, 2024 • edited Loading

dragnoir commented Feb 9, 2024

shubham1206agra commented Feb 9, 2024

dragnoir commented Feb 9, 2024

dylanexpensify commented Feb 13, 2024

shubham1206agra commented Feb 14, 2024

shubham1206agra commented Feb 14, 2024

dylanexpensify commented Feb 21, 2024

shubham1206agra commented Feb 26, 2024

dylanexpensify commented Feb 28, 2024

shubham1206agra commented Feb 28, 2024

dylanexpensify commented Feb 28, 2024

dylanexpensify commented Feb 28, 2024

dylanexpensify commented Feb 28, 2024

kbecciv commented Dec 2, 2023 •

edited by melvin-bot bot

Loading

tienifr commented Dec 2, 2023 •

edited

Loading

bernhardoj commented Dec 2, 2023 •

edited

Loading

suneox commented Dec 2, 2023 •

edited

Loading

dragnoir commented Dec 7, 2023 •

edited

Loading

shubham1206agra commented Feb 7, 2024 •

edited

Loading