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

🎉 add allGraphersLoaded Event #3096

Merged
merged 1 commit into from
Jan 30, 2024
Merged

🎉 add allGraphersLoaded Event #3096

merged 1 commit into from
Jan 30, 2024

Conversation

ikesau
Copy link
Member

@ikesau ikesau commented Jan 12, 2024

Takes #3087 further by adding an event that will dispatch on any page to confirm that all graphers have loaded.

This is necessary for this site-screenshots PR so that we can reliably take a screenshot once the page is ready instead of sleeping an arbitrary amount of time that will either be too long or too short for most pages.

I can imagine we'll forget this code exists and accidentally break it from time to time, but once we have a proper site screenshots test on every page type, we'll be able to make our CI tests fail until it's resolved (or confidently vetoed, like we do with the SVG tester)

Copy link

This PR has had no activity within the last two weeks. It is considered stale and will be closed in 3 days if no further activity is detected.

@github-actions github-actions bot added the stale label Jan 27, 2024
Copy link
Contributor

@danyx23 danyx23 left a comment

Choose a reason for hiding this comment

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

Huh, I thought I had marked this already as approved - sorry for the delay, LGTM!

@github-actions github-actions bot removed the stale label Jan 30, 2024
@ikesau ikesau merged commit 11ea553 into master Jan 30, 2024
22 checks passed
@ikesau ikesau deleted the graphers-loaded-event branch January 30, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants