[CP Stg] Use theme.text for Android picker item color in high contrast mode#90860
Conversation
… themes BasePicker was using theme.textLight for Android native picker dialog items, assuming it would always be a light color. In the DARK_CONTRAST theme, textLight maps to #002E22 (a dark color), making picker items invisible on the dark native dialog background. Changed to use theme.text which is #E7ECE9 in both dark and dark-contrast themes. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
| /** | ||
| * We pass light text on Android, since Android Native alerts have a dark background in all themes for now. | ||
| * We use theme.text instead of theme.textLight because textLight maps to a dark color in high contrast themes, | ||
| * making picker items invisible on the dark native dialog background. | ||
| */ | ||
| const itemColor = useMemo(() => { | ||
| if (getOperatingSystem() === CONST.OS.ANDROID) { | ||
| return theme.textLight; | ||
| } | ||
|
|
||
| return theme.text; | ||
| }, [theme]); | ||
| const itemColor = useMemo(() => theme.text, [theme]); |
There was a problem hiding this comment.
@MelvinBot remove entire comment and remove useMemo as auto memoized by React compiler.
| /** | |
| * We pass light text on Android, since Android Native alerts have a dark background in all themes for now. | |
| * We use theme.text instead of theme.textLight because textLight maps to a dark color in high contrast themes, | |
| * making picker items invisible on the dark native dialog background. | |
| */ | |
| const itemColor = useMemo(() => { | |
| if (getOperatingSystem() === CONST.OS.ANDROID) { | |
| return theme.textLight; | |
| } | |
| return theme.text; | |
| }, [theme]); | |
| const itemColor = useMemo(() => theme.text, [theme]); | |
| const itemColor = theme.text; |
There was a problem hiding this comment.
Done — removed the comment and replaced useMemo with a plain assignment since React Compiler handles memoization automatically.
React Compiler auto-memoizes this, so useMemo is unnecessary. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
@MelvinBot add Tests/QA Steps and check off all items in author checklist as verified by me |
|
Updated the PR body: added Tests and QA Steps, and checked off all author checklist items as verified by |
|
@MelvinBot Tests / QA Steps is not correct. Update like this:
|
|
Updated the Tests and QA Steps sections in the PR description as requested. |
|
@MelvinBot mark as ready for review |
|
Marked as ready for review. |
|
@MelvinBot mark as ready for review. Still in draft |
|
Marked this PR as ready for review. |
|
@youssef-lr Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d7a558242
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| return theme.text; | ||
| }, [theme]); | ||
| const itemColor = theme.text; |
There was a problem hiding this comment.
Preserve light text for Android picker dialog items
This change makes Android picker options use theme.text for every theme, but the Android dialog background is hardcoded dark (@color/card_highlight_bg via android/app/src/main/res/values/styles.xml and android/app/src/main/res/values/colors.xml). In Light and Light Contrast themes, theme.text is #002E22 (src/styles/theme/themes/light.ts), which creates very low contrast against that dark dialog background, making option labels difficult to read. Before this commit, Android used theme.textLight, which avoided this regression for users in light themes opening any BasePicker on Android.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
BasePicker is only used in signin page which is always dark theme, so it's fine.
|
@roryabraham looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
tests were passing? |
…TextColor Use theme.text for Android picker item color in high contrast mode (cherry picked from commit 821ea3d) (cherry-picked to staging by roryabraham)
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Cherry-picked to staging by https://github.com/roryabraham in version: 9.3.75-12 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are needed for this PR. This is a bug fix that corrects the Android picker item text color in high contrast mode (changing |
|
🚀 Cherry-picked to staging by https://github.com/roryabraham in version: 9.3.75-14 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a purely internal bug fix that corrects the text color used for Android picker items in high contrast mode ( The existing help site article ( |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.75-14 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Cherry-picked to staging by https://github.com/roryabraham in version: 9.3.76-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.77-3 🚀
|
Explanation of Change
BasePickerwas usingtheme.textLightas the text color for picker items on Android, assuming it would always be a light color (since Android native alert dialogs have a dark background). However, in theDARK_CONTRASTtheme,textLightis overridden tocolors.productLight900(#002E22) — a very dark green. After PR #89600 started applyingDARK_CONTRASTon the sign-in page when the user had high contrast mode enabled, the language picker dropdown became invisible: dark text on a dark dialog background.The fix simplifies
itemColorto always usetheme.text, which is#E7ECE9(light) in bothDARKandDARK_CONTRASTthemes and works correctly on Android's dark native dialog background. This also removes the now-unusedgetOperatingSystemimport.Fixed Issues
$ #90846
PROPOSAL: #90846 (comment)
Tests
Offline tests
N/A — this change only affects theme color selection for picker items.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari