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 #4013

Merged
merged 3 commits into from
May 14, 2024

Conversation

evwilkin
Copy link
Contributor

@evwilkin evwilkin commented May 9, 2024

Towards #4009

This PR wraps the window.load listener in a check to see if the page has already completed loading before adding that listener (and if so, adds the page-loaded class).

  • This fixes the race condition where that event listener sometimes does not fire in incognito when it is added after the page has loaded.
  • Mirrors PR chore(docs): fix page-loaded class #4012 going into main branch.

@patternfly-build
Copy link
Contributor

patternfly-build commented May 9, 2024

@evwilkin
Copy link
Contributor Author

@mcoker @wise-king-sullyman FYI pushed one additional change, to wrap this code in useEffect as an additional check to delay execution until the Example has loaded into the DOM

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.

Manually applied to my local dev server (until docs framework full page examples are fixed) - LGTM!

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 remember having some funky behavior when I tried using a useEffect for this in the initial approach, but I also didn't have the readyState check which I'm guessing resolves that issue since if it isn't done doing the initial paint when the effect runs it should add the listener, and when it is it should just load the class.

I do wonder if there might be an edge case here if the effect starts running before painting is complete, but not in time for the listener to catch the event, but we can always cross that bridge if/when we get there.

@evwilkin evwilkin merged commit 3826a11 into patternfly:v6 May 14, 2024
4 checks passed
@evwilkin evwilkin deleted the chore/4009-page-loaded-v6 branch May 14, 2024 20:58
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