Skip to content
This repository has been archived by the owner on Jun 10, 2021. It is now read-only.

fix: sign-in screen flashed before showing requested page, during page reload - even for logged in users #30

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

sahanar11
Copy link
Contributor

Before changes:
When you reload the page, even if the user has alredy logged in the signin screen would flash before showing the requested page/screen.

Screen recording of the same
https://app.box.com/s/5yjtxm7hb10mzhujrdqxaijav1gv6m7i

After changes:
When you reload the page, the signup screen is no longer shown if the user is already logged in. The flash screen is now replaced with a loader, while the API call is in progress.

Screen recording of the same
https://app.box.com/s/uzpntrwy3ksg9v6baizlfrdxsh1fyp1e

top: '35%',
width: '100%',
textAlign: 'center',
}}>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Using inline-styles instead of defining these styles in a css/sass file & using a className.
This is because if we use css className approach, if there is a delay in loading the css styles,
the loader/spinner is initially rendered at top-left of the screen and then eventually moves to the center of the page causing a flicker.

Better approach would have been to use styled-components, but since the entire app doesn't amke use of styled-components - using inline-styles in this case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes total sense, though actually I think the CSS only wouldn't load before the content during local development due to the way webpack extracts the css during production bundles?

@kyle-ssg
Copy link
Member

kyle-ssg commented Aug 4, 2020

I have a feeling this only affects local development unless I've missed something here. Having said that thank you so much for looking into doing these PRs. It's reasons like this we wanted to push the open source initiative 🙌

@kyle-ssg kyle-ssg merged commit ff92ee3 into Flagsmith:master Aug 4, 2020
@sahanar11
Copy link
Contributor Author

Thanks @kyle-ssg

I verified again, this is happening both in local development and PROD env https://app.bullet-train.io/

Please find the recording of the same issue in production environment below.
https://app.box.com/s/9nm1j2y56intvvpz7kwkfrzhpm9chshr

@kyle-ssg
Copy link
Member

kyle-ssg commented Aug 4, 2020

Ah yep sorry @sahanar11 I should have replied to the first comment I meant using inline styles vs css is just relevant for local development as css in prod is within the head.

Anyway, regardless I can see the issue this fixes, looking forward to getting this into production. Thanks!

kyle-ssg added a commit that referenced this pull request Aug 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants