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

Fixed Panels animations not working on docs site #2370

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

r100-stack
Copy link
Member

@r100-stack r100-stack commented Dec 11, 2024

Changes

I believe the animations were not working on the docs site because the examples were placed in a shadow DOM. Thus, calling getElementById("…").scrollIntoView() wouldn't work since getElementById() would not return the Panel that is in the shadow DOM. Exact code:

ref.current?.ownerDocument.getElementById(newActiveId)?.scrollIntoView({

To fix it, this PR no longer uses the ownerDocument. Since the panels are always within the PanelsWrapper, we can instead use search for the Panel within the ref of the the PanelsWrapper:

- ref.current?.ownerDocument.getElementById(newActiveId)?.scrollIntoView()
+ ref.current?.querySelector(`[id='${newActiveId}']`)?.scrollIntoView()

For the selector, I didn't use `#${newActiveId}` since React's useId() gives ids starting with ":" that gives an error if used in querySelector().

Testing

Confirmed that the animations now work on the docs site.

Docs

Added a changeset. But since this issue might be rare, I could remove the changeset if any reviewer feels so.

@r100-stack r100-stack self-assigned this Dec 11, 2024
@r100-stack r100-stack marked this pull request as ready for review December 11, 2024 23:52
@r100-stack r100-stack requested a review from a team as a code owner December 11, 2024 23:52
@r100-stack r100-stack requested review from mayank99 and smmr-dn and removed request for a team December 11, 2024 23:52
Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work on diagnosing this as a shadow DOM issue

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.

3 participants