[HOLD for payment 2024-02-20] [$500] iOS - Login - Background image moves downward when an error appears

[kbecciv]

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.25-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. Open New Expensify app.
2. Enter an email and tap on "Continue".
3. Enter 6 random numbers (to trigger incorrect magic code error).

Expected Result:

When the error appears, the background image should remain static.

Actual Result:

The background image moves downward.

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

Bug6343355_1705344453709.Aoje2064_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011bb5ec1c98737926
• Upwork Job ID: 1746986846306430976
• Last Price Increase: 2024-01-15
• Automatic offers:
• s77rt | Reviewer | 28130620
• shahinyan11 | Contributor | 28130621

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~011bb5ec1c98737926

[melvin-bot]

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

[melvin-bot]

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

[Ykumar1415]

I tried this issue and able to reproduce this issue .

[melvin-bot]

📣 @Ykumar1415! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[tienifr]

Proposal

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

The background image moves downward.

What is the root cause of that problem?

The signInContentMinHeight value that we currently set is too low (800). So initially the height will be set to that, then when the error appears, the total height of the content now is more than 800, so it will cause the background image to shift downwards because the background image occupies the area of the content container.

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

Set the signInContentMinHeight to a more reasonable amount (850 or 900) that takes into account the case where an error text must be displayed. This won't affect the UI, it will just make sure the background image is static in all cases of the sign in page.

What alternative solutions did you explore? (Optional)

We can rethink how BackgroundImage works, to me background image can be considered as something that's background and will always cover the full height and width of the parent element that's foreground. But the background image in our case is using an SVG, which has fixed size, while the foreground element will have dynamic size. To solve this, we can make the BackgroundImage have a fill color, that fill color can be understood as the general color of the SVG and will be used to fill in any gaps between the SVG and the size of the background image.

1. To do that we can update this to something like:

<View
    style={[
        styles.signInBackground,
        {
            width: '100%',
            height: '100%',
            backgroundColor: props.fillColor,
        }
    ]}
>
    <Image
        source={src}
        style={[styles.signInBackground, StyleUtils.getWidthStyle(props.width)]}
    />
</View>

(That's for iOS, but for other platforms it's similar change)
2. Remove this, since the background image is always full size of foreground element now so no need for position.
3. Then we can pass fillColor={theme.signInPage} here to specify the fillColor of the background image.

[shahinyan11]

Proposal

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

iOS - Login - Background image moves downward when an error appears

What is the root cause of that problem?

The background image has position: "absolute", bottom: 0 style, and its height is less than the height of the parent container. When the error message appears, the height of the parent container is increased and the image is moved down due to the bottom: 0 style.

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

Set the backgroundColor of the resizable container to the same as the Footer component. Then wrap the image in a new container whose height will be equal to windowHeight and place the image at the bottom of that container. Set this containers backgroundColor the sam as current background color of screen and position to absolute to cover current resizable container .

1. Add StyleUtils.getBackgroundColorStyle(theme.signInPage) in this array of styles
2. Wrap the Image in new container which will have following styles.

{
    // To cover a resizable container with the screen size
    position: 'absolute', 
    height: <windowHeight>
    top: 0,
    left: 0,
    // To keep the current background color of screen
    backgroundColor: colors.productDark200, 
    // So that the image is always at the bottom.
    justifyContent: "flex-end",  
}

3. Remove all image styles as they have been moved to the new container besides minHeight: 700.

Here is the PR where you can see all change

What alternative solutions did you explore? (Optional)

[pipolodev]

Proposal

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

iOS - Login - Background image moves downward when an error appears

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

The styles in SignInPageContent() include excessive mb and mt properties, causing the content to extend beyond the parent container and pushing the image downward. To resolve this issue, I removed one mb when the screen its small to create additional space.

Original Line here.
<View style={[styles.flexGrow2, styles.mb8]}>

Replacing that line with this:
<View style={[styles.flexGrow2, !isSmallScreenWidth ? styles.mb8 : null]}>

And effectively resolving the problem.

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

📣 @pipolodev! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[pipolodev]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01ae9cc052c388f8fd

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[s77rt]

@tienifr Thanks for the proposal. Your RCA makes sense. As for the solution can we find something more permanent? i.e. won't break if we add more text below the error message.

[s77rt]

@shahinyan11 Thanks for the proposal. Your RCA is correct. Regarding the solution won't using top instead of bottom just create a different kind of problems, i.e. when an error message appears an empty space will be created (when the parent height exceeds the background height)

[s77rt]

@pipolodev Thanks for the proposal. Unfortunately I don't your RCA is correct.

[MichaelBuhler]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01888d8c892c42b3b1

[melvin-bot]

⚠️ Invalid email. Please make sure to create an Expensify account with this email first here.

[MichaelBuhler]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01888d8c892c42b3b1

[melvin-bot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[MichaelBuhler]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01888d8c892c42b3b1

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[tienifr]

As for the solution can we find something more permanent? i.e. won't break if we add more text below the error message.

@s77rt I think if we ever add more text below the error message, we need to redesign the page, because the page content now (even without the error message) already occupies the whole screen so adding more things to it will push the SignInHero animation out of the screen and it will be another bug.

The key to keep the background image static here is to put in a reasonable min height that contains all the possible content in the page, I think 850 is enough to do that, otherwise we'll need to redesign as I mentioned above because the content would be too long.

The reason we put 800 earlier, I think, is just that we didn't take into account the error message (because it's not always visible), so changing to another proper value to cover that case is good enough to fix it.

[shahinyan11]

when an error message appears an empty space will be created

@s77rt Sorry, but I didn't understand where the empty space will be created ? I checked, there is no empty space.

[s77rt]

@tienifr I think that makes sense. Can you please check if that value is enough for Android as well?

[shahinyan11]

Proposal

Updated. Updated step 3

[s77rt]

@shahinyan11 Overall the solution looks good enough.

🎀 👀 🎀 C+ reviewed
Link to proposal

@rlinoz If you think this is worth fixing then we should be good to move forward

[melvin-bot]

Current assignee @rlinoz is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[rlinoz]

Sure, since we found a good solution this is some nice polish, thanks for the effort, lets do it!

Assigning @shahinyan11

[melvin-bot]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @shahinyan11 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.40-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Wrap the login page background image in a new container to fix it moving #34958

If no regressions arise, payment will be issued on 2024-02-20. 🎊

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment automatic offer (Reviewer)
• @shahinyan11 requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@s77rt] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@zanyrenney] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

• The PR that introduced the bug has been identified: n/a. This is more like a feature request
• The offending PR has been commented on: n/a
• A discussion in #expensify-bugs has been started: n/a
• Determine if we should create a regression test for this bug: No

[shahinyan11]

@zanyrenney Hi. When sending payment do not forget to leave feedback please

[zanyrenney]

payment summary

@s77rt requires payment automatic offer (Reviewer) - paid $500
@shahinyan11 requires payment automatic offer (Contributor) - paid $500