Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Hover effect on Sidebar rows #1463

Merged

Conversation

parasharrajat
Copy link
Member

Please review.

Details

Added existing Hoverable Component as a wrapper to the OptionRow Component and based on the hover state added a style with the Background color to show the feedback.
Now we have another prop on OptionRow component hoverStyle which takes value of styles.sidebarLinkHover and this prop is set to styles.modalLinkHover on the common ancestor of all Sidebar modals(NewChat, NewGroup,SearchView) OptionSelector.

Fixed Issues

Fixes #1439

Tests

Open the app and hover over the sidebar rows to see the feedback.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Web

screen-.3.mp4

Desktop

screen-.4.mp4

@parasharrajat parasharrajat requested a review from a team as a code owner February 11, 2021 20:12
@botify botify requested review from roryabraham and removed request for a team February 11, 2021 20:13
@shawnborton
Copy link
Contributor

Looks great!

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks pretty good – just a few small changes.

@roryabraham
Copy link
Contributor

Also, it looks like E2E tests are failing. They can be a bit flaky, so I would push the requested changes and if they're still failing then you might have to figure out what's going wrong

@parasharrajat
Copy link
Member Author

Updated.

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why optionHoveredStyle needs to be a prop of the OptionsList component. It looks like that prop is only used once, and we're passing a static value of styles.hoveredComponentBG. So instead of:

  1. Passing styles.hoveredComponentBG from OptionsSelector to OptionsList
  2. Then passing it from OptionsList to OptionRow

We should just pass styles.hoveredComponentBG from OptionsList to OptionRow. Does that make sense?

@parasharrajat
Copy link
Member Author

parasharrajat commented Feb 12, 2021

@roryabraham OptionList is a generic component and it is used in different locations where we need two different designs for OptionRow. So by default, we gave one and one we are passing from the OptionSelector Component which is the common ancestor.

Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, LGTM 👍

@roryabraham roryabraham merged commit 58bfd3d into Expensify:master Feb 12, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Feb 12, 2021
@parasharrajat parasharrajat deleted the parasharrajat/row-feedback branch November 4, 2022 19:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add hover feedback to chat rows found in Left Hand Navigation
3 participants