[$500] Login - App displays half or no animation on login with suspended email on mWeb

[izarutskaya]

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. If login then logout
3. Enter any suspended email eg: bhansalidhruval+email4@gmail.com
4. Continue and observe that in mWeb, animation below text is either displayed partially or not displayed at all

Expected Result:

App should display login page animation below suspended email text as it does on native apps

Actual Result:

App either displays half or no animation below suspended email text on login

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.3.75-8

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

Notes/Photos/Videos: Any additional supporting documentation

android.chrome.half.animation.on.login.with.suspended.email.mp4

ios.native.safari.login.with.suspended.email.mov

Screen_Recording_20231001_192352_Chrome.mp4

Rpreplay.Final1696240886.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696070722641579

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015ad25d04cd0c007c
• Upwork Job ID: 1708785780120797184
• Last Price Increase: 2023-10-16

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015ad25d04cd0c007c

[melvin-bot]

Triggered auto assignment to @maddylewis (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 - @mollfpr (External)

[ishpaul777]

Proposal

Problem

App displays half or no animation on login with suspended email on mWeb

Root cause

This happens because on smallscreen device we have set the flex1 for parent container of the sign in content that limits the container to its min-height.

Changes

We can safely remove the flex1 from the styles here.

Alternatively

We can increase the minHeight to a larger number when there is a suspended email error, we'll use a prop shouldShowEmailDeliveryFailurePage passed from upstream component to SignInPageLayout and use a different variable for container minHeight can be decided by design team.

[yh-0218]

Proposal

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

App displays half or no animation on login with suspended email on mWeb

What is the root cause of that problem?

The cause of issue is that we have style.flex1 and minHeight together.
So we get half image on web and no image on mWeb.

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

We need to remove style.flex1 and style.flexColumn from

App/src/pages/signin/SignInPageLayout/index.js

Line 156 in 3e9fdf6

<View style={[styles.flex1, styles.flexColumn, styles.overflowHidden, StyleUtils.getMinimumHeight(Math.max(variables.signInContentMinHeight, containerHeight))]}>

What alternative solutions did you explore? (Optional)

[mollfpr]

@ishpaul777 @yh-0218 Why it's working fine on native app?

[yh-0218]

@mollfpr Thanks for your reply.
Because windowHeight, insets.top, insets.bottom values are different between native app and mobile browser.
This means minHeight is different between native app and mobile browser.
When remove styles.flex1, styles.flexColumn, app will work fine on all devices.

[melvin-bot]

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

[mollfpr]

When remove styles.flex1, styles.flexColumn, app will work fine on all devices.

@yh-0218 Are this will make the property you mention set in the right value?

[ishpaul777]

Hey @mollfpr removing flexColumn is Unnecessary removing flex1 is enough to get this fixed and i proposed that first

[mollfpr]

@ishpaul777 I still don't find the answer to why removing the flex1 is fixing the issue.

[ishpaul777]

As far as I understand having flex1 with minHeight dont let the container height grow if there's minHeight.

Screen.Recording.2023-10-09.at.11.26.20.PM.mov

[yh-0218]

Hi, @mollfpr I tested that on all devices.

[mollfpr]

Even though removing flex is fixing the issue, I still don't think we can find the root cause. The same style is wrapped to the LoginForm, but there's not issue with the LoginForm.

This happens because on smallscreen device we have set the flex1 for parent container of the sign in content that limits the container to its min-height.

The cause of issue is that we have style.flex1 and minHeight together.
So we get half image on web and no image on mWeb.

Both proposals doesn't explain the why flex1 is the culprit.

@ishpaul777 @yh-0218

[yh-0218]

@mollfpr
flex: 1 means
flex-grow : 1; ➜ The div will grow in same proportion as the window-size
flex-shrink : 1; ➜ The div will shrink in same proportion as the window-size
flex-basis : 0; ➜ The div does not have a starting value as such and will

We have flex: 1 in ScrollView as contentContainerStyle so first child(has flex: 1) on mobile browser.

App/src/pages/signin/SignInPageLayout/index.js

Line 157 in 562f9d8

<View style={[styles.flex1, styles.flexColumn, styles.overflowHidden, StyleUtils.getMinimumHeight(Math.max(variables.signInContentMinHeight, containerHeight))]}>

But on native version we have flexGrow1 in ScrollView as contentContainerStyle
So this works correctly.
Thanks.

[maddylewis]

@mollfpr - will you provide an update on where we are with this one? are we still looking for a sufficient proposal?

[mollfpr]

Thanks for the explanation @yh-0218!

@maddylewis I think we can move forward with one of the proposals, but both proposals are mostly the same.

@ishpaul777 is proposing it first, and @yh-0218 is proposing the same solution with the addition of removing the flexColumn, but that doesn't change anything important.

[melvin-bot]

@mollfpr @maddylewis 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]

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

[mollfpr]

I'm okay with the solution to remove the flex1, and @ishpaul777 is proposing it first. So, let's go with their proposal.

🎀 👀 🎀 C+ reviewed!

@maddylewis Please help with the Engineer label. Thank you!

[ishpaul777]

@maddylewis gentle bump

[melvin-bot]

@mollfpr @maddylewis this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot]

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

[melvin-bot]

@mollfpr, @maddylewis Still overdue 6 days?! Let's take care of this!

[mollfpr]

Friendly bump @maddylewis to re-add the engineering label.

[ishpaul777]

@mollfpr Can you comment the 🎀 👀 🎀 C+ reviewed! stamp again so that melvin assign a engineer to review

[mollfpr]

@ishpaul777 It won't work because I'm not in the c+team temporarily. So Melvin doesn't recognize my comment as a c+.

[0xmiros]

We can close this as not reproducible anymore
https://expensify.slack.com/archives/C01GTK53T8Q/p1698438580870269?thread_ts=1698433995.864499&cid=C01GTK53T8Q