[$500] MacOS/Chrome - Background color is stuck on right click/long press on year in year search

[izarutskaya]

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 the app
2. Open settings->profile->personal details->date of birth
3. Click on year to open year search
4. Right click on any unselected value and click on another value to close right click popup or long press on any value on mWeb
5. Hover on other messages in web and observe that background color is stuck on value used in step 4

Expected Result:

App should remove background color as soon as we hover on other value

Actual Result:

Background color is stuck on right click/long press on year in year search even on hovering on other value

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: v1.3.71-8

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

Notes/Photos/Videos: Any additional supporting documentation

background.color.stuck.on.year.search.1.mov

2023-09-19.21.52.17.mov

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant Dattaram Sawant

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01790aaa0a663bd0f8
• Upwork Job ID: 1704394005281148928
• Last Price Increase: 2023-09-20
• Automatic offers:
• bernhardoj | Contributor | 26982141
• dhanashree-sawant | Reporter | 26982143

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01790aaa0a663bd0f8

[melvin-bot]

Triggered auto assignment to @mallenexpensify (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

[melvin-bot]

Triggered auto assignment to @miljakljajic (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[studentofcoding]

Proposal

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

What is the root cause of that problem?

The hover state is not being updated correctly when right-clicking on an element. This is because the PressableWithoutFeedback component does not have an event handler for right-click events. As a result, the hover state remains stuck when right-clicking on the element.

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

To fix the issue and make the hover state active when hovering over another element, you can add an onContextMenu event handler to the PressableWithoutFeedback component. This event handler will update the hoveredDay state to null when a right-click event occurs. and also keep tracking onMouseEnter & Leave to track the hovered state

Then on hovered, make the background change

<PressableWithoutFeedback
    key={`${index}_day-${day}`}
    disabled={isDisabled}
    onPress={() => this.onDayPressed(day)}
    onContextMenu={() => this.setState({ hoveredDay: null })}
    style={styles.calendarDayRoot}
    accessibilityLabel={day ? day.toString() : undefined}
    focusable={Boolean(day)}
    accessible={Boolean(day)}
    dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
    onMouseEnter={() => this.setState({ hoveredDay: day })}
    onMouseLeave={() => this.setState({ hoveredDay: null })}
>
    {({ hovered, pressed }) => (
        <View
            style={[
                styles.calendarDayContainer,
                isSelected ? styles.calendarDayContainerSelected : {},
                !isDisabled ? StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered || day === this.state.hoveredDay, pressed)) : {},
            ]}
        >
            <Text style={isDisabled ? styles.buttonOpacityDisabled : styles.dayText}>{day}</Text>
        </View>
    )}
</PressableWithoutFeedback>

What alternative solutions did you explore? (Optional)

Instead of using theonMouseEnter and onMouseLeave event handlers, we can use the onMouseOver event handler to update the hoveredDay state. The onMouseOver event will be triggered when the mouse pointer enters or leaves the element or any of its child elements.

<PressableWithoutFeedback
    key={`${index}_day-${day}`}
    disabled={isDisabled}
    onPress={() => this.onDayPressed(day)}
    onContextMenu={() => this.setState({ hoveredDay: null })}
    style={styles.calendarDayRoot}
    accessibilityLabel={day ? day.toString() : undefined}
    focusable={Boolean(day)}
    accessible={Boolean(day)}
    dataSet={{[CONST.SELECTION_SCRAPER_HIDDEN_ELEMENT]: true}}
    onMouseOver={() => this.setState({ hoveredDay: day })}
>
    {({ hovered, pressed }) => (
        <View
            style={[
                styles.calendarDayContainer,
                isSelected ? styles.calendarDayContainerSelected : {},
                !isDisabled ? StyleUtils.getButtonBackgroundColorStyle(getButtonState(hovered || day === this.state.hoveredDay, pressed)) : {},
            ]}
        >
            <Text style={isDisabled ? styles.buttonOpacityDisabled : styles.dayText}>{day}</Text>
        </View>
    )}
</PressableWithoutFeedback>

[dukenv0307]

Proposal

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

Background color is stuck on right click/long press on year in year search even on hovering on other value

What is the root cause of that problem?

Here, we'll show focused background color if the Pressable is focused.

When right click and dismiss the context menu, the onBlur of the Pressable is not triggered so focused is still true, although we already click away from the Pressable. This is the same root cause that causes us to rely on both onMouseLeave and onBlur for the Hoverable here.

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

We should call .blur() in onHoverOut of the Pressable to make sure it's blurred properly when moving outside of the Pressable

We can add below this line

onHoverOut={(e) => {
    if (props.onHoverOut) {
        props.onHoverOut(e);
    }
    if (innerRef && innerRef.current) {
        innerRef.current.blur();
    }
}}

The innerRef should be defined as the ref of the Pressable

What alternative solutions did you explore? (Optional)

NA

[dhanashree-sawant]

Hi @mallenexpensify, @thesahindia whenever you are available, can you change the reported by to my correct handle? currently I am not getting emails due to it.
Correct handle: @dhanashree-sawant

[miljakljajic]

@dhanashree-sawant has reported another issue with the background colour being stuck, I think we should combine the two into one issue? @thesahindia what do you think?

https://expensify.slack.com/archives/C049HHMV9SM/p1695223040847299

[dhanashree-sawant]

They are 2 different components with different cause I guess.

[miljakljajic]

@thesahindia I'd be curious for your input here as although they're different components, the behaviour is basically identical, so perhaps there is just one root cause that we can identify here.

[bernhardoj]

Proposal

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

The background color of the year list is stuck on a focused/highlighted color when we right-click/long-press it. This happens also on the workspace member page (and many lists).

What is the root cause of that problem?

The list item on the year page is RadioListItem. In RadioListItem, we set the focusStyle to be styles.hoveredComponentBG.

App/src/components/SelectionList/RadioListItem.js

Line 21 in 6836eaf

focusStyle={styles.hoveredComponentBG}

So, if the item is focused by right-clicking or long-pressing it, the background color will change to the hovered background color. To remove the hovered effect color, we can press on anywhere.

This was previously fixed in #24577 (comment) and #20585, but looks like it gets reintroduced by a bad merge.

this issue happens on UserListItem too

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

Remove focusStyle from both RadioListItem and UserListItem

[thesahindia]

@bernhardoj's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @MonilBhavsar, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[thesahindia]

@thesahindia I'd be curious for your input here as although they're different components, the behaviour is basically identical, so perhaps there is just one root cause that we can identify here.

We should keep them separated. They are different.

[dukenv0307]

Remove focusStyle from both RadioListItem and UserListItem

@thesahindia @bernhardoj I don't think this fixes the issue at the root, the background is not highlighted any more but the option is still focused and if you press enter, it will still select that option (which is confusing to the user because there's no indication that the option will be selected if pressing enter).

We should either remove the focus in that case, or keep the focus but also keep the highlight so the user knows what will happen if they press enter.

cc @MonilBhavsar

[bernhardoj]

To be fair, that happens to every part of the app (except the one that doesn't accept focus).

[MonilBhavsar]

I see we fixed it here #24577
Where was it updated recently and there's also no blame? anyone has any idea

[bernhardoj]

@MonilBhavsar In this PR #24577, we previously fixed this by removing the focusStyle from RadioListItem which is located in components/SelectionListRadio. Then, in #22622, the SelectionListRadio folder is renamed to SelectionList, and the RadioListItem in the new folder contains the old code, i.e. with focusStyle still exists.

We can see here that the removed RadioListItem doesn't have a focusStyle,

but the new one has a focusStyle.

[MonilBhavsar]

Welp thanks! That makes sense 👍
It was deploy last month, so let's handle it separately

[MonilBhavsar]

I don't think this fixes the issue at the root, the background is not highlighted any more but the option is still focused and if you press enter, it will still select that option (which is confusing to the user because there's no indication that the option will be selected if pressing enter).

We should either remove the focus in that case, or keep the focus but also keep the highlight so the user knows what will happen if they press enter.

@dukenv0307 sorry I didn't get the concern here regarding expected behavior. The highlighted background displays the option user is currently viewing and about to select. so on pressing enter, it is expected that it will selected, no?

[dukenv0307]

The highlighted background displays the option user is currently viewing and about to select. so on pressing enter, it is expected that it will selected, no?

@MonilBhavsar So there're 2 behaviors in this case:

1. Should background be highlighted for item that was right clicked?
2. Should Enter press select that item?

IMO, either both should be "yes", or both should be "no". But the selected proposal from @bernhardoj will mean 1 is no and 2 is yes.

That's why I raised the concern, which combination do you think is correct here?

[MonilBhavsar]

It also works like this in other sections of the app, so I'm fine 👍 cc @miljakljajic @mallenexpensify would like to have your opinion on this

[miljakljajic]

Also fine by me

[melvin-bot]

@mallenexpensify, @miljakljajic, @MonilBhavsar, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MonilBhavsar]

Thanks! We're good to go then 👍

[melvin-bot]

📣 @thesahindia Please request via NewDot manual requests for the Reviewer role ($500)

[melvin-bot]

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[bernhardoj]

Just rechecked and it's no longer reproducible as the focusStyle is removed in this PR #27767

[MonilBhavsar]

Right! In that case, I think we close this issue? @miljakljajic

[melvin-bot]

@mallenexpensify @miljakljajic @MonilBhavsar @bernhardoj @thesahindia 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!

[mallenexpensify]

Also unable to reproduce, closing. Comment and/ore reopen if you disagree or if you're able to reproduce