[Hold for Payment 2024-01-16] [Theme Switching] 🎨 Light theme - "+" icon in distance tab is white on white background

[lanitochka17]

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.4.13-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:

1. Access staging.new.expensify.com
2. Sign into a valid account
3. Put the Site to "Light mode" in preferences
4. Click Fab > Request > Distance Request > Add two waypoints and observe the "Add stop" button

Expected Result:

User expects the "+" to be either green or black since the app is in Light theme, it does not make sense to have white on white background

Actual Result:

The "+" sign in "White" in Light theme

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

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016c510bd37c1f74c7
• Upwork Job ID: 1736722830453690368
• Last Price Increase: 2023-12-18

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

[melvin-bot]

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

[mkhutornyi]

Proposal

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

Light theme - "+" icon in distance tab is white on white background

What is the root cause of that problem?

icon fill color is hardcoded as theme.textLight here:

App/src/components/Button/index.tsx

Line 224 in bd25112

fill={iconFill ?? theme.textLight}

App/src/components/Button/index.tsx

Line 235 in bd25112

fill={iconFill ?? theme.textLight}

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

update it to theme.icon which is default icon color

Additionally, when large or success prop is set to true, we should use color from styles.buttonLargeText or styles.buttonSuccessText to be compatible with background and consistent with text color

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

[grgia]

@shawnborton @dannymcclain should the button be green here?

[grgia]

This is not a deploy blocker

[dannymcclain]

That button should use our standard "primary" button styles (in both light and dark mode).

They look like this:

They map to our Figma variables like this:
Background: button
Text: text
Icon: icons

[grgia]

@mkhutornyi can you confirm that your proposal matches the linked mocks?

[grgia]

Thank you @dannymcclain !

[mkhutornyi]

Let me check

[mkhutornyi]

Does this look good? (updated proposal accordingly)

[shawnborton]

That seems right to me at least.

[dannymcclain]

Looks right to me.

[melvin-bot]

@luacmartins, @grgia Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@luacmartins, @grgia Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mkhutornyi]

As approved by design team, can I be assigned?

[luacmartins]

all yours @mkhutornyi

[melvin-bot]

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

Job added to Upwork: https://www.upwork.com/jobs/~016c510bd37c1f74c7

[melvin-bot]

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

[mkhutornyi]

PR is ready for review

[melvin-bot]

This issue has not been updated in over 15 days. @eVoloshchak, @sonialiap, @luacmartins, @grgia, @mkhutornyi eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[grgia]

@sonialiap we can close this out once we ensure payments are made to @eVoloshchak (C+ review) and @mkhutornyi for #33252

[sonialiap]

Deployed to production Jan 9, so it's ready for payment on Jan 16 (today 🎉 )

@eVoloshchak review $500 - please request in NewDot and complete the checklist
@mkhutornyi fix $500 - paid ✔️

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: there isn't a PR that caused this, this is a detail that was missed during the [Theme Switching Migration]
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: this is a very minor detail, I don't think an additional discussion is needed
• Determine if we should create a regression test for this bug.
  Is it easy to test for this bug? Yes
  Is the bug related to an important user flow? No
  Is it an impactful bug? No
  This is a very minor bug that doesn't prevent user from using the app (just the button color is different), I don't think a regression test is necessary here

[JmillsExpensify]

$500 approved for @eVoloshchak based on summary above.