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

[$1000] Icons in VBA flow are very hard to see or white in dark mode #13132

Closed
kavimuru opened this issue Nov 29, 2022 · 29 comments
Closed

[$1000] Icons in VBA flow are very hard to see or white in dark mode #13132

kavimuru opened this issue Nov 29, 2022 · 29 comments
Assignees
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@kavimuru
Copy link

kavimuru commented Nov 29, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Access staging.new.expensify.com (Web)
  2. Sign into a valid account
  3. Click on Profile > Workspace > Connect bank > Connect manually (First issue here on Routing page)
  4. Enter routing information and proceed to the next page
  5. Scroll down to the "Incorporation date" section and locate the calendar icon on the right of the input field (Second issue here)

Expected Result:

  1. The background of the routing image should match the dark theme
  2. The calendar icon should be a color that is easily seen in the dark theme
  3. When the user taps the dropdown, all text should be clear and readable

Actual Result:

  1. The background of the routing image is white and does not match the current theme
  2. The calendar icon is black and very hard to see, an average user may miss this entirely if they do not know its there
  3. When the user taps the dropdown, all text is black on a dark background and hard to understand

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.2.33-1
Reproducible in staging?: y
Reproducible in production?: n
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
Screenshot 2022-11-29 at 9 55 30 AM

Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1669696874240429

View all open jobs on GitHub

Upwork Automation - Do Not Edit

@kavimuru kavimuru added the DeployBlockerCash This issue or pull request should block deployment label Nov 29, 2022
@OSBotify
Copy link
Contributor

👋 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
Copy link

melvin-bot bot commented Nov 29, 2022

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

@dhairyasenjaliya
Copy link
Contributor

**Proposal **

Proposal slack link - https://expensify.slack.com/archives/C049HHMV9SM/p1669731061170839?thread_ts=1669696874.240429&cid=C049HHMV9SM

Solution 1

  • We need to color-scheme to .expensify-datepicker::-webkit-calendar-picker-indicator CSS
.expensify-datepicker::-webkit-calendar-picker-indicator {
+   color-scheme: dark; 
}

Solution 2

  • We can also add to filter to .expensify-datepicker::-webkit-calendar-picker-indicator CSS

Note : We should add dark/light based on current theme once we have it we can make conditional to that

 .expensify-datepicker::-webkit-calendar-picker-indicator {
+   filter: invert(1);
} 

Result
Screenshot 2022-11-29 at 10 06 46 AM

@melvin-bot melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Nov 29, 2022
@luacmartins
Copy link
Contributor

Gonna reassign to myself since Maria might be out for the day.

@luacmartins luacmartins assigned luacmartins and unassigned MariaHCD Nov 29, 2022
@luacmartins
Copy link
Contributor

@dhairyasenjaliya solution one looks good! Can you get a PR up soon since this is a deploy blocker?

@dhairyasenjaliya
Copy link
Contributor

Hey @luacmartins sure creating PR now

@luacmartins
Copy link
Contributor

Cool, I'll assign you to this issue and get the process started! Thanks!

@luacmartins luacmartins added the External Added to denote the issue can be worked on by a contributor label Nov 29, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 29, 2022

Triggered auto assignment to @tjferriss (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot melvin-bot bot changed the title Icons in VBA flow are very hard to see or white in dark mode [$1000] Icons in VBA flow are very hard to see or white in dark mode Nov 29, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 29, 2022

Job added to Upwork: https://www.upwork.com/jobs/~01bad5604e9c4055b5

@melvin-bot
Copy link

melvin-bot bot commented Nov 29, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 29, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 29, 2022

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 29, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 29, 2022

📣 @dhairyasenjaliya You have been assigned to this job by @luacmartins!
Please apply to this job in Upwork 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 📖

@luacmartins
Copy link
Contributor

Going to speed this one up since it's a deploy blocker

@mvtglobally
Copy link

mvtglobally commented Nov 30, 2022

@luacmartins the dates are still hard to see on phone. Should we log a separate one?

IMG_6006

@dhairyasenjaliya
Copy link
Contributor

@mvtglobally
Copy link

@dhairyasenjaliya i saw it. Out team logged this collective issue earlier as multiple themes were not working yesterday. As we were clubbing multiple similar issues together. I am clarifying if now each should be separated

@luacmartins
Copy link
Contributor

@mvtglobally please log another issue for #13132 (comment)

@luacmartins
Copy link
Contributor

I'll remove the blockeer label from this one, since we fixed the calendar icon issue.

@luacmartins luacmartins added Reviewing Has a PR in review and removed DeployBlockerCash This issue or pull request should block deployment labels Nov 30, 2022
@tjferriss
Copy link
Contributor

@mollfpr and @dhairyasenjaliya remember to apply to the Upworks job: https://www.upwork.com/jobs/~01bad5604e9c4055b5.

@mollfpr
Copy link
Contributor

mollfpr commented Dec 1, 2022

@tjferriss Im haven't reviewed anything, so I’m not eligible for compensation

@dhairyasenjaliya
Copy link
Contributor

@tjferriss Is this eligible for a 50% bonus since PR merged within an hour

@tjferriss
Copy link
Contributor

@mollfpr my mistake.

@dhairyasenjaliya yes, you are eligible for the bonus. Can you please apply to the Upworks job?: https://www.upwork.com/jobs/~01bad5604e9c4055b5

@tjferriss
Copy link
Contributor

tjferriss commented Dec 1, 2022

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

@mollfpr mollfpr removed their assignment Dec 2, 2022
@dhairyasenjaliya
Copy link
Contributor

@tjferriss applie along with bug reporting bonus as well

@tjferriss
Copy link
Contributor

@dhairyasenjaliya you've been hired.

@luacmartins
Copy link
Contributor

@tjferriss checklist is done on my end.

@tjferriss
Copy link
Contributor

I've created the regression test issue and paid out @dhairyasenjaliya. I'll close this out once the regression test has been added.

@melvin-bot
Copy link

melvin-bot bot commented Dec 14, 2022

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

@tjferriss
Copy link
Contributor

Applause confirmed we don't need an additional regression test for this issue so we're good to close this out. Thanks everybody.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

8 participants