Skip to content

Conversation

@evwilkin
Copy link
Member

@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
Collaborator

patternfly-build commented May 9, 2024

@evwilkin
Copy link
Member 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
@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