[HOLD for payment 2023-06-29] Web - Split Bill - Money request confirmation list stays highlighted after press

[kbecciv]

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

---

Action Performed:

1. Open a group chat
2. Start a Split Bill and proceed until the confirmation page
3. Unchecks the items in the list

Expected Result:

The item is not highlighted if we don't hover it or is not focused

Actual Result:

The item stays highlighted after press and also the first item that is not clickable is highlighted if we click it

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.26.1

Reproducible in staging?: yes

Reproducible in production?: yes

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

Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-06-08.at.13.24.35.mov

Recording.3078.mp4

Expensify/Expensify Issue URL:

Issue reported by: @bernhardoj

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686202285251059

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0110ed1d7d5b0b56cb
• Upwork Job ID: 1670152822418042880
• Last Price Increase: 2023-06-17

[melvin-bot]

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

[melvin-bot]

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

[Christinadobrzyn]

I can reproduce but I don't see why this is a bug - asking for more information https://expensify.slack.com/archives/C049HHMV9SM/p1686604097827569?thread_ts=1686202285.251059&cid=C049HHMV9SM

[dukenv0307]

Proposal

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

Money request confirmation list stays highlighted after press

What is the root cause of that problem?

We add focusStyle as this.props.hoverStyle in OptionRow that is optionHoveredStyle props we passed to OptionsSelector. When we select a row, it's focused, and focusStyle is applied.

App/src/components/OptionRow.js

Line 185 in 93957f0

focusStyle={this.props.hoverStyle}

In money request confirmation list we pass optionHoveredStyle to OptionsSelector, so that when a row is selected it stays highlighted with style backgroundColor: themeColors.hoverComponentBG, focusIndex change to the next option in OptionsSelector and then we can see both options are highlighted with different bg color.

App/src/components/MoneyRequestConfirmationList.js

Line 302 in 93957f0

optionHoveredStyle={canModifyParticipants ? styles.hoveredComponentBG : {}}

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

I saw that optionHoveredStyle has a default style as optionHoveredStyle: styles.hoveredComponentBG. So I think we should define a new prop in optionsSelectorPropTypes like optionFocusedStyle to prevent focusStyle in some select pages in which we can select multiple and this issue doesn't occur in these pages.

What alternative solutions did you explore? (Optional)

[bernhardoj]

It's a regression from #19612 because of the focusStyle to solve this issue. The linked issue does not happen in the previous implementation because the hovered style is applied directly to the Pressable instead of the children. @Skalakid may know what's the best to do here.

[Christinadobrzyn]

Reached out here for some help - #19612 (comment)

[Skalakid]

@bernhardoj @Christinadobrzyn I reverted my changes for testing and still, I got the same bug (even in PRs merged before #19612). However, I think I know where is the problem. It is caused because of not disabling focusing in this list. I will write the proposal

[bernhardoj]

Hmm, that's strange. I removed focusStyle and the issue is gone. To have us in the same page, the first row should not be highlighted (ss taken from video above).

[Skalakid]

Proposal

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

The money request confirmation list stays highlighted after press

What is the root cause of that problem?

There are two root causes of this problem. The first one is that we don't disable focusing on this list. The second one is that we have two places that specify focus styles for PressableWithFeedback

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

We should pass the disableFocusOptions prop to the <OptionsList> component. This is the parent component of the user list where the bug exists. By passing this option the optionIsFocused prop in OptionRow will be always false.
After we change this, we should remove focus styles from the style prop in PressableWithFeedback because we can just use focusStyle prop.
Next, we should make focusStyle disabled when optionIsFocused is false, so we can change it to this.props.optionIsFocused ? styles.sidebarLinkActive : null}.

After we do these changes, the issue should be fixed, however we will have to fix again this issue. To do this we should change a little bit PressableWithFeedback structure and move the styles from OpacityView to the parent component GenericPressable. These changes will resolve both issues

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.