Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] Android - Login – Login page blinks when open app #46232

Closed
1 of 6 tasks
lanitochka17 opened this issue Jul 25, 2024 · 20 comments
Closed
1 of 6 tasks

[$250] Android - Login – Login page blinks when open app #46232

lanitochka17 opened this issue Jul 25, 2024 · 20 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 25, 2024

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: 9.0.12-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
**Email or phone of affected tester (no customers):**[email protected]
Issue reported by: Applause - Internal Team

Action Performed:

  1. Sign out and kill the NewDot android app
  2. Open the NewDot app
  3. Take a note when login page opens

Expected Result:

App should not blink

Actual Result:

Login page blinks when open app

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

Bug6552998_1721934555623.Recording__3569.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~014fc22ce9eab9144e
  • Upwork Job ID: 1819152387493807005
  • Last Price Increase: 2024-08-01
  • Automatic offers:
    • ikevin127 | Reviewer | 103430665
Issue OwnerCurrent Issue Owner: @ikevin127
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 25, 2024
Copy link

melvin-bot bot commented Jul 25, 2024

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@lanitochka17
Copy link
Author

We think that this bug might be related to #vip-vsp

@lanitochka17
Copy link
Author

@kevinksullivan FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@bernhardoj
Copy link
Contributor

bernhardoj commented Jul 26, 2024

Proposal

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

Login page blinks in native.

What is the root cause of that problem?

The page blinks is caused by the sign in hand animation. In sign in hand animation, if the splash screen isn't hidden yet, we render a plain View with the same size as the lottie animation to prevent any jump because the sign in page is centered vertically..

const {isSplashHidden} = useSplashScreen();
// Prevents rendering of the Lottie animation until the splash screen is hidden
// by returning an empty view of the same size as the animation.
// See issue: https://github.com/Expensify/App/issues/34696
if (!isSplashHidden) {
return <View style={[styles.alignSelfCenter, imageSize]} />;
}
return (
<Lottie
source={LottieAnimations.Hands}
loop
autoPlay
style={[styles.alignSelfCenter, imageSize]}
webStyle={{...styles.alignSelfCenter, ...imageSize}}
/>
);

It was added in #35185 to improve the performance by delaying the lottie. When the splash screen is hidden, we start rendering the Lottie component. However, in Lottie component, we don't render the lottie animation immediately.

return animationFile ? (
<LottieView
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
source={animationFile}
ref={ref}
style={[aspectRatioStyle, props.style]}
webStyle={{...aspectRatioStyle, ...webStyle}}
onAnimationFailure={() => setIsError(true)}
/>
) : null;

The animationFile value is undefined initially and will be set in the useEffect.

const [animationFile, setAnimationFile] = useState<string | AnimationObject | {uri: string}>();
useEffect(() => {
setAnimationFile(source.file);
}, [setAnimationFile, source.file]);

It was added in #45211 to also improve the performance by delaying the lottie. But since we render nothing for a brief moment, the page moves down and up.

Placeholder view -> null -> Lottie

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

We need to return a placeholder view too when the animationFile isn't ready to prevent any jump. We already have that here

if (isError || appState.isBackground) {
return <View style={[aspectRatioStyle, props.style]} />;
}

so we just need to add || !animationFile to the existing condition.

I think we can also move the isSplashHidden condition to Lottie. Lmk if you agree!

What alternative solutions did you explore? (Optional)

Pass a new props to Lottie to set the animationFile immediately for sign in hand animation

OR

Wrap the Lottie with a View with the same size.

return (
<Lottie
source={LottieAnimations.Hands}
loop
autoPlay
style={[styles.alignSelfCenter, imageSize]}
webStyle={{...styles.alignSelfCenter, ...imageSize}}
/>
);

@melvin-bot melvin-bot bot added the Overdue label Jul 29, 2024
Copy link

melvin-bot bot commented Jul 29, 2024

@kevinksullivan Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Jul 31, 2024

@kevinksullivan Huh... This is 4 days overdue. Who can take care of this?

@kevinksullivan kevinksullivan added the External Added to denote the issue can be worked on by a contributor label Aug 1, 2024
@melvin-bot melvin-bot bot changed the title Android - Login – Login page blinks when open app [$250] Android - Login – Login page blinks when open app Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

Job added to Upwork: https://www.upwork.com/jobs/~014fc22ce9eab9144e

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 1, 2024
Copy link

melvin-bot bot commented Aug 1, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Aug 1, 2024
@kevinksullivan
Copy link
Contributor

asking about if this fits in newdot quality here

https://expensify.slack.com/archives/C05LX9D6E07/p1722554976014829

@ikevin127

This comment was marked as outdated.

@ikevin127
Copy link
Contributor

@bernhardoj's proposal looks good to me. The root cause was correctly identified and the main solution fixes the issue.

I think we can also move the isSplashHidden condition to Lottie. Lmk if you agree!

If you're talking about the isSplashHidden check from within the SignInHeroImage component I agree, I tested this and it makes sense to check all conditions in that one if within the Lottie component. Just make sure to adjust the comment above that if to explain the newly added conditions 👍

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Aug 3, 2024

Triggered auto assignment to @MariaHCD, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@ikevin127
Copy link
Contributor

cc @MariaHCD for final assignment review when you get a chance, thank you! 🧑‍💻

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

📣 @ikevin127 🎉 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 melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Aug 7, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @ikevin127

@ikevin127
Copy link
Contributor

ikevin127 commented Aug 21, 2024

⚠️ Production deploy automation failed here -> this should've been paid on 2024-08-20 according to production deploy confirmed in #46947 (comment) and deploy checklist.

Payment was due yesterday!

cc @kevinksullivan @MariaHCD

@bernhardoj
Copy link
Contributor

@kevinksullivan I think we need a payment summary here.

Requested in ND.

@JmillsExpensify
Copy link

Reporting for payment summary.

@kevinksullivan
Copy link
Contributor

Summary

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants