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

chore(docs): fix page-loaded class #4012

Merged
merged 5 commits into from
May 15, 2024

Conversation

evwilkin
Copy link
Contributor

@evwilkin evwilkin commented May 9, 2024

Towards #4009

This PR adds extra logic to check if page has already finished loading (and if so, adds the page-loaded class) before adding window.load event listener.

  • This fixes the race case condition where the page-loaded class is sometimes not added in incognito windows where the listener is never triggered because it is added after the page has already loaded.

@patternfly-build
Copy link
Contributor

patternfly-build commented May 9, 2024

mcoker
mcoker previously approved these changes May 9, 2024
Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LGTM! I did pull it down and test with a big image on an example and the event is firing before the image has fully loaded, but I also added this to the <head> and as the last thing in <body> and got the same results, so I think the change in this PR is good, though we'll have to see if we still have any issues with images loading

    <script>
      window.onload = function() {
        console.log("window.load fired");
      };
    </script>

Copy link
Collaborator

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

I haven't been able to replicate the issue here but this change looks like it should be a great fix 🚀

@evwilkin evwilkin dismissed stale reviews from wise-king-sullyman and mcoker via 919fc85 May 14, 2024 20:55
Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LGTM! Verified on a handful of examples it's firing and looks to be at the right time

@evwilkin evwilkin merged commit b35cdb3 into patternfly:main May 15, 2024
4 checks passed
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.

[Documentation framework] - "page loaded" class doesn't always fire
4 participants