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

docs: move color-mode script #37658

Merged
merged 5 commits into from Dec 18, 2022
Merged

docs: move color-mode script #37658

merged 5 commits into from Dec 18, 2022

Conversation

XhmikosR
Copy link
Member

@XhmikosR XhmikosR commented Dec 16, 2022

Doesn't make any sense to have the script before charset and the other tags.

@mdo: Does the order need to be specific? If so, which one is it? Like, does this script need to be before CSS?

Also

  1. can't we asynchronously load the script or does it need to be blocking?
  2. can't /shouldn't we use DOMContentLoaded instead of window.load?

@XhmikosR
Copy link
Member Author

We also need to move that script along with the other scripts. Right now it's being output in /js which is moot.

@XhmikosR XhmikosR marked this pull request as ready for review December 16, 2022 16:19
@XhmikosR XhmikosR requested a review from a team as a code owner December 16, 2022 16:19
@mdo
Copy link
Member

mdo commented Dec 16, 2022

I think it needs to be that high up to avoid the white flash on page load when in dark mode. Comparing this deploy preview to our staging, this looks broken.

@XhmikosR
Copy link
Member Author

Weird, I don't see any flash here on Firefox. I also tried it with limited network connection (2G) and I don't get any flash. Which browser are you using?

The culprit is probably async loading, but it'd help me if you reverted one by one the patches and saw which one triggers the effect you are seeing.

@mdo
Copy link
Member

mdo commented Dec 16, 2022

I think the async revert fixed it. I definitely saw repeated flashing on my iPhone before that. Will look more when I'm back at a computer.

@julien-deramond
Copy link
Member

julien-deramond commented Dec 17, 2022

Tried it on macOS (Firefox & Chrome - latest versions) with some throttling (2G/3G). Everything is rendered at the same time except the big logo on the main page and the Algolia search bar that are rendered after (but it was OK in terms of UX).
Let's wait for mdo's tests to confirm there's no flashing anymore.

@mdo mdo merged commit e1315d1 into main Dec 18, 2022
@mdo mdo deleted the xmr/docs-color-script branch December 18, 2022 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants