[Awaiting Payment 29th April] [$250] Web - Category - No hover effect for an already selected category

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.60-1
Reproducible in staging?: y
Reproducible in production?: y
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

• Create a collect workspace on OD and enable chat for newDot
• Go to tags settings on OD for the collect workspace and add a few tags

1. Navigate to staging.new.expensify.com
2. Open workspace chat page
3. Start a manual IOU flow and select a category
4. Click on tag and select one
5. Click on category
6. Press the keyboard down arrow a few times to shift the focus
7. Hover over the selected category
8. Go back and click on tags
9. Press the keyboard down arrow a few times
10. Hover over the selected tag value

Expected Result:

There should be a hover effect visible for the selected category as is the case for the selected tag

Actual Result:

There is no hover effect for the selected category after the focus has shifted from it which is different from the behavior seen for the selected independent tag value

Workaround:

n/a

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6438413_1712305224995.bandicam_2024-04-05_11-19-57-298.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0135176cb3aeb0eafc
• Upwork Job ID: 1776321147884154880
• Last Price Increase: 2024-04-05
• Automatic offers:
• jayeshmangwani | Reviewer | 0
• Krishna2323 | Contributor | 0

[melvin-bot]

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

[kbecciv]

@lschurr I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[kbecciv]

We think that this bug might be related to #wave-collect - Release 1

[bernhardoj]

Proposal

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

There is no hover effect when hovering selected category.

What is the root cause of that problem?

The category picker page uses SelectionList. The BaseListItem won't show the hover effect if the item is selected.

App/src/components/SelectionList/BaseListItem.tsx

Line 62 in 51c039d

hoverStyle={!item.isDisabled && !item.isSelected && styles.hoveredComponentBG}

That's why there is no hover effect on the selected item even after we move away the item focus.

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

Instead of check whether the item is selected or not, we should check whether the item is focused or not.

hoverStyle={!item.isDisabled && !isFocused && styles.hoveredComponentBG}

or just remove the selected condition

[Krishna2323]

Proposal

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

There is no hover effect when hovering selected category.

What is the root cause of that problem?

In BaseListItem we don't pass the hoverStyle when item is selected, it is because in TableListItem we want to use activeComponentBG when item is selected and we don't want to change the bg when item is hovered because we already have applied activeComponentBG.

App/src/components/SelectionList/BaseListItem.tsx

Line 62 in 51c039d

hoverStyle={!item.isDisabled && !item.isSelected && styles.hoveredComponentBG}

App/src/components/SelectionList/TableListItem.tsx

Line 45 in b10893a

pressableStyle={[[styles.selectionListPressableItemWrapper, item.isSelected && styles.activeComponentBG, isFocused && styles.sidebarLinkActive]]}

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

We should remove !item.isSelected check from BaseListItem and also accept a new prop called hoverStyle. In TableListItem we will pass hoverStyle={item.isSelected && styles.activeComponentBG}.

Update hoverStyle in BaseListItem.

hoverStyle={!item.isDisabled && [styles.hoveredComponentBG, hoverStyle]}

Note: We also need to make few more changes like adding type in BaseListItemProps but those are very minor.

Result

hover_issue_category_picker.mp4

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0135176cb3aeb0eafc

[melvin-bot]

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

[lschurr]

Hi @jayeshmangwani! Looks like we have a few proposals already. Could you please review when you get a chance? Thanks!

[jayeshmangwani]

Thanks for the proposals @bernhardoj @Krishna2323. I also feel we should remove the selected condition, but that was added in this PR and this commit intentionally,
For the output, let's confirm with the design team.

[jayeshmangwani]

The existing behaviour for the list items is if the list item is selected, we will not show the default hover effect (theme.hoveredComponentBG). Instead, the list uses the passed background color;

For this issue, the proposal is to remove the condition isSelected from the ListItem. This will change the behaviour for the categories list, Tags list and Taxes list:

Right now, on the workspace categories page, when we select any category, it will show the background color to theme.activeComponentBG, and then hover over other list items and again hover over to the selected category is not change the background color.

After removing the isSelected conditions, the categories list's behaviour will be like this:

If the List item is selected, then the background color will be theme.activeComponentBG; if hovering over the selected item, then the background color will be theme.hoverComponentBG

Adding videos below, before and after of this change:

It looks like this:

before.mov

After this change, it will look like this:

after.mov

@shawnborton We will need your suggestions on whether we should remove the condition of the list selected.

[Krishna2323]

@jayeshmangwani, can you pls check my proposal, it won't change the behaviour of the workspace list

[jayeshmangwani]

@Krishna2323 I have checked your proposal, but I feel that we should remove the is selected condition. Let's wait for the design team, and if we don’t want to change the existing behaviour of the categories list, then we can go with other proposals.

[shawnborton]

Hmm I would be curious for the @Expensify/design team's thoughts here. It sounds like the problem is that when a row in a table is selected, it doesn't have any kind of hover style. I'm not super convinced that's a major problem, but if we were to solve it, I would just make sure even a selected row would get our rowHover background color when you hover over it, which I think is basically the second video in @jayeshmangwani's comment here.

[dannymcclain]

I'm not super convinced that's a major problem, but if we were to solve it, I would just make sure even a selected row would get our rowHover background color when you hover over it

I agree with Shawn. Not totally convinced it's a problem, but agree with this solution if we deem we should change it.

Also, why does the keyboard highlight use the "selected" color state instead of just the "hover" color state?

[jayeshmangwani]

Also, why does the keyboard highlight use the "selected" color state instead of just the "hover" color state?

@dannymcclain I'm not sure why it was added, but I am tagging the @luacmartins and @burczu as this is coming from this PR and In that PR, we are setting it not to show the hover background color if the list item is selected.

[luacmartins]

We assumed that was the expected behavior. Happy to change it if we decide to show the hover effect on a selected item.

[melvin-bot]

Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[Krishna2323]

I believe our concern is to avoid changing the background on hover if an item is already focused or selected and has a background applied. However, the issue arises when there is no hover style applied to an item that doesn't have any background and is selectable. To observe this, try removing the focus from the selected category/option using the up/down arrow.

issue_hover_bg.mp4

[shawnborton]

However, the issue arises when there is no hover style applied to an item that doesn't have any background and is selectable.

Yup, that makes sense. I'd be down to fix that particular case.

[dannymcclain]

I'm fine with fixing that case too, but I don't feel too strongly.

[dubielzyk-expensify]

I actually think what we have here is working well. If there's already a background on a selected item, then don't do a hover style. If there isn't one, then use it. So I'm okay with this

[shawnborton]

Okay cool, sounds like design team agrees: let's move forward with @Krishna2323's idea here. @Krishna2323 - you might need to update your proposal to recapture the problem statement and solution though.

[Krishna2323]

@jayeshmangwani, can you pls check #39716 (comment) and review my proposal again?

[lschurr]

Commenting to remove overdue since this one is still moving.

[jayeshmangwani]

@Krishna2323's proposal will work if we have to add a hover effect to only those selected list items that have no background color. @luacmartins , if we agree on this, then we can proceed with the proposal.

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @Krishna2323 🎉 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 📖

[Krishna2323]

@jayeshmangwani, PR ready for review.

[Krishna2323]

@lschurr, PR was deployed to production on 22nd April, this is ready for the payments process.

Sorry @trjExpensify for the bump.

[trjExpensify]

Easily done! :)

[lschurr]

Thanks for the bump on this one!

Payment summary:

• Contributor: @Krishna2323 $250 (paid in Upwork)
• Reviewer: @jayeshmangwani $250 (paid in Upwork)