[HOLD for payment 2023-02-20] [$4000] Android - New room - The page darkens and the drop-down menu is poorly visible

[kbecciv]

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. Open the app
2. Login with any account
3. Click on FAB button -> New Room
4. Open any drop-down window (Workspace or Visibility)

Expected Result:

The page doesn't go dark, and the drop-down menu is visible well

Actual Result:

The page darkens and the drop-down menu is poorly visible

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [ ] Android / native

Version Number: 1.2.47.0

Reproducible in staging?: Yes

Reproducible in production?: no

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://user-images.githubusercontent.com/93399543/210285310-8c197643-db76-438e-8c29-e6784fffdabb.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01959d86ee4a1f08b9
• Upwork Job ID: 1610336595939086336
• Last Price Increase: 2023-02-04

[OSBotify]

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

[deetergp]

I'm pretty sure this PR is the one that introduced the bug (cc @sketchydroide)

[jasperhuangg]

@deetergp This actually happens for any dropdown menus you pull-up on Android, it isn't unique to the New Room Page. Here's a screenshot after opening the language selector dropdown on the login page. So I don't actually think the PR you linked has anything to do with this–it might be coming from elsewhere since it happens on all dropdowns.

[jasperhuangg]

@deetergp To add to this I don't think this needs to be a deploy blocker since no behavior is actually broken. This is purely a visual glitch.

cc @grgia @Luke9389 could this have anything to do with this dark theme PR?

[sketchydroide]

I'm pretty sure https://github.com/Expensify/App/pull/13859 is the one that introduced the bug (cc @sketchydroide)

Not sure how this could be caused by that PR, it just checks if the language was changed and makes the componenent update if it did... Am I missing something?

[melvin-bot[bot]]

Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01959d86ee4a1f08b9

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[syedsaroshfarrukhdot]

Proposal :-

To fix it we need to set color to colorBackgroundFloating to fix the issue in styles.xml because applying background color dark to android app also causes Modals to be Dark.

diff --git a/android/app/src/main/res/values/styles.xml b/android/app/src/main/res/values/styles.xml
index a804c14ab..b30501c5b 100644
--- a/android/app/src/main/res/values/styles.xml
+++ b/android/app/src/main/res/values/styles.xml
@@ -10,6 +10,7 @@
         <item name="android:statusBarColor">#061B09</item>
         <item name="colorAccent">@color/accent</item>
         <item name="android:editTextBackground">@drawable/rn_edit_text_material</item>
+        <item name="colorBackgroundFloating">@color/white</item>
     </style>

     <!-- Themes unique to the boot splash page -->

After Fix :-

https://user-images.githubusercontent.com/81307212/210307867-943023cb-54eb-4854-a483-69785fe08d55.mp4

[aimane-chnaif]

@syedsaroshfarrukhdot your proposal only changes background color in dark theme. so ripple color is still white and user feels no ripple effect.

[grgia]

cc @0xmiroslav since you mentioned a fix on your PR

[ali-thowfeek]

Proposal

In the picker component,

   <RNPickerSelect
      onValueChange={this.onInputChange}

      // We add a text color to prevent white text on white background dropdown items on Windows
-    items={_.map(this.props.items, item => ({...item, color: themeColors.pickerOptionsTextColor}))}
+    items={_.map(this.props.items, item => ({...item, color: Platform.OS === 'android' ? themeColors.text : themeColors.pickerOptionsTextColor}))}

[0xmiros]

@ali-thowfeek I already proposed solution here https://github.com/Expensify/App/pull/13874#issuecomment-1369634855 in case we wanna keep dark theme for native picker in android. Otherwise, we need to update android native picker style to light theme, while app is fully in dark mode. This is the case when we wanna update picker style to dark theme in all platforms at once.

[ali-thowfeek]

@0xmiroslav Instead of fixing the other way around i agree with you on this:

So the solution is to set picker text color to white in android (like video above), and still keep light mode (white background, dark text) in all other platforms. But eventually, we need to fix iOS and web too to apply dark theme to picker, either customizing this package or replace with other good package. I will raise PR if this is agreed.

And can someone explain what exactly is the fix we are looking for here?

[aimane-chnaif]

I suggest to use this GH to apply dark theme to picker in all platforms if it's not that difficult and time consuming. Applying light theme and then go back to dark theme is not ideal. cc: @deetergp @grgia

[deetergp]

I think you're right @aimane-chnaif. We definitely want parity, so let's say the goal here is to apply the dark theme to the picker on all platforms.

[syedsaroshfarrukhdot]

Updated Proposal :- We could decide upon ripple effect color which should be used.

diff --git a/android/app/src/main/res/values/styles.xml b/android/app/src/main/res/values/styles.xml
index a804c14ab..b66042132 100644
--- a/android/app/src/main/res/values/styles.xml
+++ b/android/app/src/main/res/values/styles.xml
@@ -10,8 +10,14 @@
         <item name="android:statusBarColor">#061B09</item>
         <item name="colorAccent">@color/accent</item>
         <item name="android:editTextBackground">@drawable/rn_edit_text_material</item>
+        <item name="colorBackgroundFloating">@color/white</item>
+        <item name="colorControlHighlight">@color/gray4</item>
     </style>

     <!-- Themes unique to the boot splash page -->
     <style name="BootTheme" parent="AppTheme">
         <item name="android:background">@drawable/bootsplash</item>

After Fix :-

https://user-images.githubusercontent.com/81307212/210423245-4e350861-5a31-4e6f-baea-e3fa5f547009.mp4

[aimane-chnaif]

@syedsaroshfarrukhdot did you read https://github.com/Expensify/App/issues/13951#issuecomment-1370103954?

[syedsaroshfarrukhdot]

@syedsaroshfarrukhdot did you read #13951 (comment)?

Sorry missed it initially was updating proposal have just read it now after you highlighted it. Will look for a possible solution to implement darkmode for react native picker on all platforms. Thanks for highlighting.

[ali-thowfeek]

Updated Proposal

We are able to handle Android and iOS in this regard. But web and macOS always end up using the system default. I don't see any ways to customize that. Even Github hasn't figured it out, their website follows the user's sytem defaults even if you set to dark mode in Github settings. So I guess its better for us settle with the mobile platforms and web on windows.

/src/components/Picker/index.js

   <RNPickerSelect
      onValueChange={this.onInputChange}

-      // We add a text color to prevent white text on white background dropdown items on Windows
-      items={_.map(this.props.items, item => ({...item, color: themeColors.pickerOptionsTextColor}))}
+      items={_.map(this.props.items, item => ({...item, color: themeColors.text}))}

/src/components/Picker/pickerStyles/index.ios.js

 const pickerStyles = isDisabled => ({
     ...styles.picker(isDisabled),
     inputIOS: styles.picker(isDisabled).inputNative,
+    done: {
+        color: '#ffffff',
+    },
+    doneDepressed: {
+        fontSize: 17,
+        color: '#aaaaaa',
+    },
+    modalViewMiddle: {
+        borderTopWidth: 0,
+        backgroundColor: '#232323',
+    },
+    modalViewBottom: {
+        backgroundColor: '#252525',
+    },
 });

 export default pickerStyles;

/src/styles/styles.js b/src/styles/styles.js

 const picker = {
-    backgroundColor: themeColors.transparent,
     color: themeColors.text,
     fontFamily: fontFamily.EXP_NEUE,
     fontSize: variables.fontSizeNormal,
@@ -571,7 +570,6 @@ const styles = {
             height: 26,
             opacity: 1,
             cursor: 'pointer',
-            backgroundColor: 'transparent',
         },
         inputAndroid: {
             fontFamily: fontFamily.EXP_NEUE,
@@ -595,6 +593,20 @@ const styles = {
             width: variables.iconSizeExtraSmall,
             height: variables.iconSizeExtraSmall,
         },
+        done: {
+            color: '#ffffff',
+        },
+        doneDepressed: {
+            fontSize: 17,
+            color: '#aaaaaa',
+        },
+        modalViewMiddle: {
+            borderTopWidth: 0,
+            backgroundColor: '#232323',
+        },
+        modalViewBottom: {
+            backgroundColor: '#252525',
+        },
     },

Android

iOS

Windows Web

[aimane-chnaif]

@ali-thowfeek web solution is missing in your proposal. We'll only accept proposal which works on all platforms, especially on windows.

[ali-thowfeek]

@aimane-chnaif by the way, when you say windows do you mean desktop(mac)? Because I didn't if know the app was targeted to windows as the Readme only had macOS mentioned. As for the issue currently there seems to be no way to achieve this on web and desktop. (Unless someone comes up with a solution)

And as for macOS if the user has set the system setting to dark mode then this is what we get in our app, which is what we want:

And on the web

But if the user switches the system setting to light mode then I guess we are left with no options as we cant be switching the system setting of the user.

We will wait and see what others come up with

[Luke9389]

@jasperhuangg to answer your earlier question, I've never seen this before. Kinda strange, it doesn't happen in the android emulator.

[aimane-chnaif]

@ali-thowfeek This should be dark theme. i.e. you can see github dropdown is set to dark theme after setting dark theme in Appearance setting even if user switches system setting to light mode.

[roryabraham]

FWIW I think this is a regression probably caused by https://github.com/Expensify/App/pull/13874 and the closest proposal to what we want is probably https://github.com/Expensify/App/issues/13951#issuecomment-1370123025, except switching the colors out from light to dark.

Reason being – https://github.com/Expensify/App/pull/13874 hard-coded the dark theme on Android, so it's fine to hard-code a dark theme for native modals too.

[roryabraham]

Though, I tried doing that suggestion, just with different colors, and then the modal didn't open at all for me on a physical Pixel 4a

[ali-thowfeek]

@aimane-chnaif acutally github hasn't figured it out either. I'll attach some reference images: MacOS Chrome System setting: light mode Github setting: dark mode

---

As you can see it is still rendered as light picker (follows the system setting)

If we switch it other around like System setting: dark mode Github setting: light mode

Once again it simply follows the system setting regardless of what mode github is in.

---

As for mobile web:

Android Chrome System setting: light mode Github setting: dark mode

Follows system setting again:

---

iOS Safari System setting: light mode Github setting: dark mode

Same here:

[aimane-chnaif]

@ali-thowfeek try on Windows chrome, not mac

[ali-thowfeek]

@aimane-chnaif yep, you were right. Was able to get it to work on Windows web. Updated my proposal.

[syedsaroshfarrukhdot]

Though, I tried doing that suggestion, just with different colors, and then the modal didn't open at all for me on a physical Pixel 4a

It must be due to the issue which I reported we are currently facing Issue on opening picker on android on Physical Device. There is a PR up for it. It should be resolved soon.

[0xmiros]

Proposal

Common: (this fixes android issue automatically)

1. update picker text color https://github.com/Expensify/App/blob/8f5e4f291d1571b5b60112228d6e6247ed8a6b5b/src/styles/themes/default.js#L62

   +    pickerOptionsTextColor: colors.white,

Web:

2. update background color https://github.com/Expensify/App/blob/8f5e4f291d1571b5b60112228d6e6247ed8a6b5b/src/styles/styles.js#L23-L24

   +    backgroundColor: themeColors.appBG,

3. update background color in small picker https://github.com/Expensify/App/blob/8f5e4f291d1571b5b60112228d6e6247ed8a6b5b/src/styles/styles.js#L581

   +            backgroundColor: themeColors.appBG,

4. fix label, dropdown arrow hidden issue when background set

   pickerLabel: {
       position: 'absolute',
       left: 11,
       top: 6,
   +       zIndex: 1,
   +       ...pointerEventsNone,
   },
   picker: (disabled = false) => ({
       iconContainer: {
           top: Math.round(variables.inputHeight * 0.5) - 11,
           right: 10,
   -           zIndex: -1,
   +           ...pointerEventsNone,
       },

iOS: (fully customizable)

+    done: {
+        color: themeColors.text,
+    },
+    modalViewMiddle: {
+        backgroundColor: themeColors.border,
+        borderTopWidth: 0,
+    },
+    modalViewBottom: {
+        backgroundColor: themeColors.highlightBG,
+    },

Add these styles inside these: 5. https://github.com/Expensify/App/blob/8f5e4f291d1571b5b60112228d6e6247ed8a6b5b/src/styles/styles.js#L552

6. import themeColors from '../../../styles/themes/default'; https://github.com/Expensify/App/blob/8f5e4f291d1571b5b60112228d6e6247ed8a6b5b/src/components/Picker/pickerStyles/index.ios.js#L4-L7

Demo

Web (Windows chrome): Web (Windows firefox):

NOTE: Mac browser picker depends on Mac OS theme and not customizable

Android:

iOS:

[aimane-chnaif]

@ali-thowfeek your web solution breaks style on this page so 👎

Dropdown background color should be same as page background color (appBG)

@0xmiroslav's proposal makes sense to me. Need confirmation from design team if all screenshots look good in their proposal. cc: @shawnborton

[ali-thowfeek]

@aimane-chnaif Its just the colors, the approach stays the same. We can choose the appropriate colors from the design team. But never the less, we should consider this upcoming PR as well,

for which i propose this fix in the index.html file within the style tag to fix the windows issue on top of my previous proposal. Because with the above PR's design changes the background color of the <Picker> component has to be set as 'transparent' as it is now, which in turn breaks the windows theming as it is now. So the below fix promises future compatibility.

+ select option {
+ color: rgb(231,236,233); // acutally we have to include this as well, again color to be decided
+ background-color: rgba(6,27,9,0.92); // to be replaced with the suitable color
+ }

[0xmiros]

@ali-thowfeek only setting background color of select option will not work on Firefox.

I took major time of my research on this issue to fix label, arrow hidden in dropdown after setting background.

[ali-thowfeek]

@0xmiroslav honestly I've been putting time into this as well 😀. On Firefox also, my last suggestion works, but there is a slight border, which I think we might have to consider a trade off, taking into account this PR

[0xmiros]

slight border is just the issue I meant. I don't think this is acceptable solution.

[ali-thowfeek]

@aimane-chnaif If not we will have to pass a prop containing the background color to the picker component manually on each and every place it is used just to sort out this windows issue. And this color has to go hand in hand with the bg of where it is used. That way we can ensure it all would look the same. I wouldn't not recommend this approach, if were to get this windows issue sorted out no matter the cost this is the only way. For instance, <LocalePicker /> component encompasses the Picker component, and it is used in a couple of places, so we will have to edit LocalPicker once again to take in a prop which will then pass on to picker component. As you can see, I see this as the ultimate solution, but it involves a lot of moving parts, so it's hard to recommend. So Im just putting this out there, for the team to decide on this.

And by the way @0xmiroslav solution does not have a significant difference to my approach, its the same, with only change of colors 🤷‍♂️ which will be ultimately has to be decided the by design team. Also, note that I was the first one find the issue in windows was with passing 'transaprent' as the color. And first to come up with the solution for iOS.

[ali-thowfeek]

slight border is just the issue I meant. I don't think this is acceptable solution.

But It does work fine on all the major players in browsers

[aimane-chnaif]

We'll only accept solutions which work on all platforms. The challenging part of this issue was windows web responsive.

@0xmiroslav already proposed working solution in android first time. And then @ali-thowfeek proposed working solution on iOS (not web). Finally @0xmiroslav proposed working solution on all platforms.

@ali-thowfeek can you share link which proposed working solution on web before @0xmiroslav's last proposal?

-            backgroundColor: 'transparent',

Its just the colors, the approach stays the same. We can choose the appropriate colors from the design team.

I won't consider this is a solution because when backgroundColor set to specific color, it will hide some layouts (or hardly visible) as @0xmiroslav stated:

[ali-thowfeek]

@aimane-chnaif Even with @0xmiroslav solution he is setting a background color. So the best bet, considering the way forward is this solution If not once this PR merges, it will once again set the color to be transparent and we have to start all over again.

[ali-thowfeek]

And here is how it looks with this solution:

[aimane-chnaif]

@ali-thowfeek I don't follow that solution since it breaks on Firefox and also you proposed this after @0xmiroslav's proposal.

Anyway, I will decide after getting @shawnborton's design opinion.

[aimane-chnaif]

I suggest to hold this issue (pause receiving proposals) until https://github.com/Expensify/App/pull/12447 is merged

[adelekennedy]

@deetergp @aimane-chnaif are we agreed to HOLD this issue until we merge #12447?

[aimane-chnaif]

@deetergp @aimane-chnaif are we agreed to HOLD this issue until we merge #12447?

I think so because dropdown designs are updated after that PR and thus it will affect dropdown menu items.

[deetergp]

Currently on hold. Going to bump this down to weekly till the hold is removed.

[mallenexpensify]

Removed Help Wanted because job is on hold, please add back when it's off hold

[deetergp]

Still on hold