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

Workspace - Inconsistency with hovering styles #30952

Closed
6 tasks done
kbecciv opened this issue Nov 7, 2023 · 8 comments
Closed
6 tasks done

Workspace - Inconsistency with hovering styles #30952

kbecciv opened this issue Nov 7, 2023 · 8 comments
Assignees
Labels
Engineering Reviewing Has a PR in review Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Nov 7, 2023

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.3.96.0
Reproducible in staging?: y
Reproducible in production?: n
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:

Precondition: Have at least one workspace.

  1. Navigate to staging.new.expensify.com.
  2. Go to Settings.
  3. Click on Workspaces.

Expected Result:

There is no outline feedback around workspace avatars when clicking on Workspaces.

Actual Result:

There is outline feedback around workspace avatars when clicking on Workspaces.

Workaround:

Unknown

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

Bug6266781_1699326006460.20231107_104300.mp4

View all open jobs on GitHub

@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Nov 7, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Nov 7, 2023

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Nov 7, 2023

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

@DylanDylann
Copy link
Contributor

DylanDylann commented Nov 7, 2023

Proposal

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

Workspace - Outline feedback around workspace avatars when clicking on Workspaces

What is the root cause of that problem?

When displaying the avatar we are using isHovered and pressed states of parent Component (Workspaces MenuItem) to get the style for it

<MultipleAvatars
isHovered={isHovered}
isPressed={pressed}
icons={props.floatRightAvatars}
size={props.floatRightAvatarSize || fallbackAvatarSize}
fallbackIcon={defaultWorkspaceAvatars.WorkspaceBuilding}
shouldStackHorizontally={props.shouldStackHorizontally}

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

We can wrap MultipleAvatars component by Hoverable and PressableWithSecondaryInteraction separately and using isHovered and pressed states itself like this

<Hoverable>
            {(isHovered) => (
                <PressableWithSecondaryInteraction
                    ......
                >
                    {({pressed}) => (
                                <MultipleAvatars
                                            isHovered={isHovered}
                                            isPressed={pressed}
                                            icons={props.floatRightAvatars}
                                            size={props.floatRightAvatarSize || fallbackAvatarSize}
                                            fallbackIcon={defaultWorkspaceAvatars.WorkspaceBuilding}
                                            shouldStackHorizontally={props.shouldStackHorizontally}
                                        />

We also make the same fix to other child elements

What alternative solutions did you explore? (Optional)

We can create new props called shouldApplyHoverAndPressStateToChildElement and we only pass isHovered and pressed states of parent Component to child element if shouldApplyHoverAndPressStateToChildElement is true

@situchan
Copy link
Contributor

situchan commented Nov 7, 2023

@DylanDylann For deploy blockers, offending PR should be identified

@bernhardoj
Copy link
Contributor

A regression from #28858 where we change the menu item hover color.

@MonilBhavsar
Copy link
Contributor

Also hovering style on various workspace menu items is affected

Screen.Recording.2023-11-07.at.4.52.06.PM.mov

I'm leaning towards reverting that PR and coming up with new approach and testing plan

@MonilBhavsar MonilBhavsar changed the title Workspace - Outline feedback around workspace avatars when clicking on Workspaces Workspace - Inconsistency with hovering styles Nov 7, 2023
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Nov 7, 2023
@MonilBhavsar
Copy link
Contributor

@mountiny mountiny removed the DeployBlockerCash This issue or pull request should block deployment label Nov 7, 2023
@mountiny
Copy link
Contributor

mountiny commented Nov 7, 2023

Fixed in staging, nobody needs to be paid, closing

@mountiny mountiny closed this as completed Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants