-
Notifications
You must be signed in to change notification settings - Fork 13.4k
fix(datetime): ensure datetime is shown when intersection observer fails to report visibility #30793
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts
Outdated
Show resolved
Hide resolved
core/src/components/datetime-button/test/overlays/datetime-button.e2e.ts
Outdated
Show resolved
Hide resolved
|
@ShaneK The dev build does correctly resolve the problem for me 👍 Thanks providing a fix 🙇 |
thetaPC
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
brandyscarney
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small request to link the follow-up investigative ticket & I also left some other feedback on the Jira ticket. Looks good though!
Co-authored-by: Brandy Smith <[email protected]>
Issue number: resolves #30706
What is the current behavior?
Due to some recent unknown changes, the intersection observer for date time no longer reliably fires, especially in mobile views.
What is the new behavior?
In this PR, we're adding a visibility check after everything has had a chance to render to make sure we're setting up properly even if the intersection observer has failed to trigger for some reason.
Does this introduce a breaking change?
Other information
Since the intersection observer is being set up after a
raf, it's possible something got introduced to make the initial setup slower for some reason, causing timing issues. I think we should do a more thorough investigation into the cause of this problem when we have more time.This PR also adds tests to verify the new fallback works properly.
Current dev build: