[$250] Web - Categories - Button icons turns black when hovering over the button

[IuliiaHerets]

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: v9.0.22-3
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Categories/Tags/Taxes/Members.
3. Hover over the buttons on the top right.

Expected Result:

The button icon will not turn black when hovering over the button.

Actual Result:

The button icons turns black when hovering over the button.
Issue is also reproducible with Show more and Add stop button

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6576295_1724089957083.20240820_015050.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01754de5ed278fbfd1
• Upwork Job ID: 1825680074639815229
• Last Price Increase: 2024-08-19
• Automatic offers:
• hoangzinh | Reviewer | 103605340

Issue Owner

Current Issue Owner: @hoangzinh

[melvin-bot]

Triggered auto assignment to @tylerkaraszewski (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[github-actions]

👋 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.

[IuliiaHerets]

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

[etCoderDysto]

Edited by proposal-police: This proposal was edited at 2024-08-19 19:06:14 UTC.

Proposal

Offending PR #47181

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

Button icons turns black when hovering over the button

What is the root cause of that problem?

We are not passing the appropriate theme color (theme.iconHovered) for iconHoverFill prop of button

App/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx

Lines 244 to 249 in c7a8d12

	<Button
	medium
	success
	onPress={navigateToCreateCategoryPage}
	icon={Expensicons.Plus}
	text={translate('workspace.categories.addCategory')}

Here is we use iconHoverFill when user hovers hover the button

App/src/components/Button/index.tsx

Lines 271 to 275 in c7a8d12

	<View style={[large ? styles.mr2 : styles.mr1, !text && styles.mr0, iconStyles]}>
	<Icon
	src={icon}
	hasText={!!text}
	fill={isHovered ? iconHoverFill : iconFill ?? (success || danger ? theme.textLight : theme.icon)}

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

We should pass the appropriate value to iconHoverFill prop in all instance where Button is used

Note: We will search through app to indetify all usages of Button component
When iconHoverFill is not passed to the Button component we can pass theme.textLight to fill prop of icon when danger or success prop is passed to the Button component and theme.icon when neither danger nor success is passed to Button component. This change will be applied here, here, and here. We can extract the fill style to function to avoid the redundancy

<Icon
    src={iconRight}
    fill={isHovered ? iconHoverFill ?? (success || danger ? theme.textLight : theme.icon) : iconFill ?? (success || danger ? theme.textLight : theme.icon)}

What alternative solutions did you explore? (Optional)

Result:

Screen.Recording.2024-08-19.at.10.23.14.at.night.mp4

[neonbhai]

Proposal

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

Button icons turns black when hovering over the button

What is the root cause of that problem?

We recently introduced iconHoverFill in #47181 to help style the statusbar on the search page. This modified the logic of the button to have hoverFill as undefined, if not explicitly passed.

This is different from the earlier behaviour where we defaulted to checking for success/danger props, and choosing theme.textLight or theme.icon:

App/src/components/Button/index.tsx

Line 266 in f9e6f5f

fill={iconFill ?? (success || danger ? theme.textLight : theme.icon)}

We should fallback to the same logic for a hovered icon.

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

We should update the logic here, here, and here to:

const defaultFill = success || danger ? theme.textLight : theme.icon;

fill={
  isHovered
    ? iconHoverFill ?? defaultFill
    : iconFill ?? defaultFill
}

Result

Works perfectly:

Screen.Recording.2024-08-20.at.12.42.53.AM.mov

[etCoderDysto]

Proposal updated

• Added few implementation detail

[etCoderDysto]

Proposal updated

• Added result

[tylerkaraszewski]

I've confirmed this happens on staging but not production. I'm going to mark external.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01754de5ed278fbfd1

[melvin-bot]

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

[melvin-bot]

📣 @mohamed-iyed! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[github-actions]

@user Your proposal will be dismissed because you did not follow the proposal template.

[mohamed-iyed]

Contributor details
Your Expensify account email: mohamediyedrhimita@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01e11da7d5ef514a1c

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[mohamed-iyed]

Proposal

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

Button icons turns black when hovering over the button.

What is the root cause of that problem?

The cause is here, here and here the iconHoverFill variable is undefined.

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

We should check if the icon is hovered and the iconHoverFill is not undefined (eg: isHovered && iconHoverFill).

const fill = isHovered && iconHoverFill ? iconHoverFill : iconFill ?? (success || danger ? theme.textLight : theme.icon)

   fill={fill}

I assigned the logic to a variable and pass it as a prop to the Icon component.

Result:

Screen.Recording.2024-08-20.at.7.26.30.AM.mov

[hoangzinh]

Thanks for proposals, everyone. All of you have pointed out the correct root cause, especially @neonbhai. But in my opinion, I prefer @mohamed-iyed's solution. He provided a solution that is backward compatible with previous logic. So, I think we should go ahead with his proposal.

Link to proposal #47652 (comment)

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @hoangzinh 🎉 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]

📣 @mohamed-iyed You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[mohamed-iyed]

the PR will be ready for review in 2 hours.

[hoangzinh]

@mohamed-iyed because it's a Deploy blocker, can you give us a quick update on it?

[mohamed-iyed]

@hoangzinh i am stuck in the 10th step about making the commit signed. here

[mohamed-iyed]

@hoangzinh is it possible to work on this the next day ? Because i got to test this on multiple platforms to see if there are any regressions.

[hoangzinh]

If it's a normal bug, then we're fine. But it's a Deploy blocker (Hourly), I'm afraid that we can't. We need to fix it as soon as possible. Can you comment your roadblock in this Slack thread https://expensify.slack.com/archives/C01GTK53T8Q/p1724164452341479? Thank you.

[hoangzinh]

FYI: the offending PR is going to be reverted here #47181 (comment)

[mohamed-iyed]

@hoangzinh can i fix the issue while testing it only on the web app ?

[rayane-djouah]

cc @luacmartins

[luacmartins]

Gonna close this issue since the offending PR is reverted. I'll work on a fix.

[melvin-bot]

@tylerkaraszewski @luacmartins @bfitzexpensify Be sure to fill out the Contact List!

[mohamed-iyed]

@hoangzinh this is the PR.