[$500] mWeb - Magic link - "Open in app" banner is dark themed on magic code page in light mode

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.15-1
Reproducible in staging?: Y
Reproducible in production?: Y
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. Go to https://staging.new.expensify.com/
2. Enter an email you have access to
3. Tap on "Continue"
4. Go to email inbox and copy the Magic link and change it to staging
5. Open an incognito tab and navigate to the staging magic link

Expected Result:

The "Open in app" banner should be in light mode

Actual Result:

The "Open in app" banner is in dark mode even if the app is in light mode.
It also happens if the app has 2FA enabled and on the "Magic code expired" page

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/~01f555a808f6be075d
• Upwork Job ID: 1737996800572018688
• Last Price Increase: 2024-01-05

[melvin-bot]

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

[melvin-bot]

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

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

[rushatgabhane]

Proposal

Root cause:

When logged out, the preferred theme from storage isn't set and defaults to dark mode.

App/src/hooks/useThemePreference.ts

Lines 13 to 17 in 760c67c

	const theme = preferredThemeFromStorage ?? CONST.THEME.DEFAULT;
	// If the user chooses to use the device theme settings, we need to set the theme preference to the system theme
	if (theme === CONST.THEME.SYSTEM) {
	setThemePreference(systemTheme ?? CONST.THEME.FALLBACK);

Solution

cc: @grgia @chrispader I'm pretty sure this is the expected behavior?

[tienifr]

Proposal

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

Smart App Banner is always in dark mode.

What is the root cause of that problem?

The problem here is the inconsistency.

The validate code page is supposed to always have dark mode (just like SignInPage) but it doesn't. In SignInPage, we manually set the theme to be dark here. But we didn't do it for ValidateCodeModal so it will fallback to system theme, which was light in OP.

In SignInPage, we disable the CustomStatusbar which determines the theme of smart app banner here. That's why even when the theme changes, the banner remains dark.

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

1. If we want ValidateCodeModal to be in dark, we can implement the same as SignInPage.
2. We can implement the smart app banner to follow whichever theme the page has

What alternative solutions did you explore? (Optional)

NA

[cubuspl42]

@rushatgabhane Please stick to the proposal template, read the contributing guidelines for more details.

😎

[cubuspl42]

@tienifr

We can implement the smart app banner to follow whichever theme the page has

Can we? Would you share some details?

[ikevin127]

@rushatgabhane is right about the persistence of the preferredTheme onyx variable upon logout, we already have an open issue #33160 that's pending review.

Having said that, the current issue can be fixed independent of that one because this one is regarding the system default theme logic compared to the other issue which is regarding the persistence of a chosen theme.

Proposal

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

The "Open in app" banner is in dark mode even if the app is in light mode.

It happens for all ValidateLoginPage screens:

• Here is your magic code
• 2FA
• Magic code expired
• Abracadabra, you are signed in!
• FullScreenLoadingIndicator

What is the root cause of that problem?

The root cause is that we hardcoded the theme to dark for the SignInPage ThemeProvider which is wrapping the nested CustomStatusBarAndBackground and because of this when the device system theme is light (system default) the custom status bar and background are always using the dark theme colors.

App/src/pages/signin/SignInPage.js

Lines 288 to 291 in 862b0ea

	<ThemeProvider theme={CONST.THEME.DARK}>
	<ThemeStylesProvider>
	<ColorSchemeWrapper>
	<CustomStatusBarAndBackground isNested />

Therefore the currentScreenBackgroundColor is always equal to theme.appBG which is #061B09 dark theme color.

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

To preserve the SignInPage always dark themed but not force it on the CustomStatusBarAndBackground we can wrap CustomStatusBarAndBackground with another ThemeProvider without theme prop which will fix the issue by using system default theme or onyx preferredTheme (if existing) instead of the hardcoded dark theme.

This way when the user reaches the above mentioned screens while in light mode, the background of the "Open in app" banner will use the correct theme.

Changed code

function SignInPage(props) {
    return (
        <ThemeProvider theme={CONST.THEME.DARK}>
            <ThemeStylesProvider>
                <ColorSchemeWrapper>
+                   {/* Re-wrap CustomStatusBarAndBackground to override the hardcoded dark theme set above to system default or onyx `preferredTheme` (if existing) */ }
+                   <ThemeProvider> 
+                       <CustomStatusBarAndBackground isNested />
+                   </ThemeProvider>
                    ...

What alternative solutions did you explore? (Optional)

Moving CustomStatusBarAndBackground outside of the ThemeProvider like stated here #33476 (comment) would work the same way.

Below I added videos for both cases - when system default theme is light or dark showing both the issue screen and sign in screen. I cannot get the banner to show on local development - it will only be seen once the fix is deployed to staging.

Note: When the theme is light, the status bar of the SignInPage will be white (including during the splash screen).

In order to keep the status bar dark while theme is light it would require overidding theme.PAGE_THEMES['Home'] with darkTheme.PAGE_THEMES['Home'] which would persist the dark themed status bar in the Home screen once logged in and I don't think we want that because it looks like broken theme.

Videos

iOS: mWeb Safari

Screen.Recording.2023-12-22.at.19.05.09.mov

[chrispader]

Changed code

function SignInPage(props) {
    return (
        <ThemeProvider theme={CONST.THEME.DARK}>
            <ThemeStylesProvider>
                <ColorSchemeWrapper>
+                   {/* Re-wrap CustomStatusBarAndBackground to override the hardcoded dark theme set above to system default or onyx `preferredTheme` (if existing) */ }
+                   <ThemeProvider> 
+                       <CustomStatusBarAndBackground isNested />
+                   </ThemeProvider>
                    ...

Instead of this, we can also just move the CustomStatusBarAndBackground element outside of the ThemeProvider, like this:

function SignInPage(props) {
    return (
+    <>
+      <CustomStatusBarAndBackground isNested />
        <ThemeProvider theme={CONST.THEME.DARK}>
            <ThemeStylesProvider>
                <ColorSchemeWrapper>
-                       <CustomStatusBarAndBackground isNested />
                    ...
+    </>

[melvin-bot]

@cubuspl42, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@cubuspl42, @dylanexpensify Eep! 4 days overdue now. Issues have feelings too...

[cubuspl42]

@grgia Would you comment on the Rushat's question?

[chrispader]

When logged out, the preferred theme from storage isn't set and defaults to dark mode.
cc: @grgia @chrispader I'm pretty sure this is the expected behavior?

Yes, if (previously) logged out all the data will be cleared and the default theme should be dark. Am i right? @grgia

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@cubuspl42, @dylanexpensify Huh... This is 4 days overdue. Who can take care of this?

[cubuspl42]

@dylanexpensify I think we should close this one. It's more like a design discussion than an apparent bug. Also, the impact seems to be negligible. The dark banner, arguably, composes well with the system bar.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@cubuspl42 @dylanexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@cubuspl42, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!