-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[HOLD for payment 2025-01-28] [$250] Improve visibility of tooltips #54775
Comments
cc @Expensify/design - want to make sure you all get a chance to weigh in here before we make this one External. Right now in the mock above, I am doing this: Light mode:
Dark mode:
Figma is here. |
Current assignee @shawnborton is eligible for the Design assigner, not assigning anyone new. |
Edited by proposal-police: This proposal was edited at 2025-01-03 09:42:19 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
to:
App/src/styles/theme/themes/light.ts Line 83 in 04cd7b0
Line 4016 in 04cd7b0
We need to update the
to:
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?What alternative solutions did you explore? (Optional) |
Edited by proposal-police: This proposal was edited at 2025-01-03 09:51:08 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Improve visibility of tooltips What is the root cause of that problem?Improvement What changes do you think we should make in order to solve the problem?We need to update the tooltip color here BG (dark mode) color App/src/styles/theme/themes/dark.ts Line 83 in 04cd7b0
to: BG (light mode) color App/src/styles/theme/themes/light.ts Line 83 in 04cd7b0
And also we need to update the text color as well Line 4016 in 04cd7b0
And we also need to increase the width and the height of the pointer pointer App/src/styles/utils/generators/TooltipStyleUtils/index.ts Lines 18 to 22 in 04cd7b0
App/src/styles/utils/generators/TooltipStyleUtils/index.ts Lines 255 to 257 in 04cd7b0
We can change the POINTER_WIDTH to 16 and POINTER_HEIGHT to 8 instead of changing directly here App/src/styles/utils/generators/TooltipStyleUtils/index.ts Lines 255 to 257 in 04cd7b0
Because we use that variables in some places when displaying the tooltip like here: App/src/styles/utils/generators/TooltipStyleUtils/index.ts Lines 144 to 145 in 04cd7b0
and here App/src/styles/utils/generators/TooltipStyleUtils/index.ts Lines 198 to 199 in 04cd7b0
And also we don't need to update the icon color because it's already using App/src/styles/theme/themes/dark.ts Line 84 in 04cd7b0
App/src/styles/theme/themes/light.ts Line 84 in 04cd7b0
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?What alternative solutions did you explore? (Optional) |
jeff Your proposal will be dismissed because you did not follow the proposal template. |
@shawnborton Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Posted in the thread! |
Alright, sounds like we are cool with the higher contrast version. I updated the original post, let's get this assigned and ready for proposal review. |
Job added to Upwork: https://www.upwork.com/jobs/~021876672988734265232 |
Triggered auto assignment to @garrettmknight ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ishpaul777 ( |
Please re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
Update in App/src/styles/theme/themes/dark.ts:
Update in App/src/styles/theme/themes/light.ts:
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
What alternative solutions did you explore? (Optional)
|
Triggered auto assignment to @justinpersaud, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @ishpaul777 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @truph01 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@ishpaul777 PR is ready |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.87-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-01-28. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@garrettmknight I found a regression on Android caused by this, where the tooltip tip's width continues to increase indefinitely, which I previously reported here. android-tooltip-width.mp4 |
@linhvovan29546 our PR does not cause such an issue it's not reproducible on my dev. app and deployed production app Screen.Recording.2025-01-27.at.12.55.24.PM.mov |
@ishpaul777 I can reproduce the issue in dev app(debug mode), but I was unable to reproduce it in the production app |
@linhvovan29546 try reverting our PR and retest on main, we did not changes any of the logic related to positoning or size of tooltip so its not clear to me how this can be caused by our if this was a bug QA would have caught it in staging. @truph01 if you can test this it would be great |
I couldn't reproduce either on staging so I think we're good here? |
@ishpaul777 in the meantime, can you complete the checklist? |
Regression Test Proposal : steps:
Do we agree 👍 or 👎 |
Cool, I think we're good to go here. |
Problem: Our current educational tooltips use a very small pointer arrow, and they have very low contrast with the background color of the app in light mode. As a result, it's quite hard to tell exactly what a tooltip is pointing at.
Solution: Let's update our tooltips to use a higher-contrast BG color, use a larger pointer, and make them theme-dependent. The result would be something like this:

Light mode:
Dark mode:
Pointer:
Convo: https://expensify.slack.com/archives/C07HPDRELLD/p1735250426325589
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @garrettmknightThe text was updated successfully, but these errors were encountered: