Skip to content

Comments

Status Chart: Fix mobile Safari#2975

Merged
StephanTLavavej merged 5 commits intomicrosoft:gh-pagesfrom
StephanTLavavej:gh-pages
Aug 3, 2022
Merged

Status Chart: Fix mobile Safari#2975
StephanTLavavej merged 5 commits intomicrosoft:gh-pagesfrom
StephanTLavavej:gh-pages

Conversation

@StephanTLavavej
Copy link
Member

My previous PR #2967 which attempted to address #2963 appeared to solve the problem for desktop Chrome, but broke mobile Safari such that the charts wouldn't even load.

I don't understand the root cause, but I suspect that it has something to do with how es-module-shims has to do actual work for mobile Safari (so perhaps the window is considered "loaded" before the JS finishes executing, and the addEventListener did nothing).

This PR uses a different technique that appears to work reliably in both environments. Now I run a function, check_css(), that detects whether Primer CSS has loaded. If it hasn't, we'll wait 50ms and check again. (Chosen semi-arbitrarily - we don't need to check every frame, but we do want to notice it quickly.)

This also updates dependencies slightly, including the Node version mentioned in the README.

As usual, this contains a "DROP BEFORE MERGING: Regen built/status_chart.mjs." commit to make the live preview work.

📉 📱 Live preview: stephantlavavej.github.io/STL/

@StephanTLavavej StephanTLavavej added bug Something isn't working documentation Related to documentation or comments labels Jul 30, 2022
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner July 30, 2022 23:30
@StephanTLavavej StephanTLavavej merged commit f1cb36c into microsoft:gh-pages Aug 3, 2022
@h-vetinari
Copy link

I didn't check the page between #2967 and this PR was merged, but now dark mode on firefox has no visible charts anymore (and refreshing doesn't help as it did pre-#2967).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Related to documentation or comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants