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

Add focus tracking support for shadow dom #5983

Closed
wants to merge 21 commits into from
Closed

Conversation

ritz078
Copy link
Contributor

@ritz078 ritz078 commented Mar 1, 2024

Part of #1472

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

ritz078 and others added 18 commits February 22, 2024 18:15
A new 'ShadowRoot' example is added in the useFocusRing.stories.tsx under the @react-aria/interactions package. The getOwnerDocument function in domHelpers.ts is also updated to return either a Document or a ShadowRoot, depending on the context. This helps illustrate focus management within Shadow DOM contexts.
The commit replaces the use of getOwnerDocument with getRootNode function in the FocusScope.tsx file. This change modifies how the active element is retrieved and how event listeners are added and removed, leading to better management of the focus scope. Additionally, getRootNode function has been expanded to handle Shadow DOM.
The function 'isElementInScope' in FocusScope.tsx has been updated to accept 'ShadowRoot' as an additional possible type for the 'element' parameter. This allows compatibility with more potential focusing scenarios.
@snowystinger snowystinger changed the title Add support for shadow dom Add focus tracking support for shadow dom Mar 10, 2024
@snowystinger
Copy link
Member

Thanks for the PR, I see it's failing Typescript right now. I also started to do an initial review, however, I don't understand the changes in this commit:

The commit replaces the use of getOwnerDocument with getRootNode function in the FocusScope.tsx file. This change modifies how the active element is retrieved and how event listeners are added and removed, leading to better management of the focus scope. Additionally, getRootNode function has been expanded to handle Shadow DOM.

Can you explain more about the changes in this commit? What was going "wrong" before the commit? Do we need tests for it?

@ritz078
Copy link
Contributor Author

ritz078 commented Mar 10, 2024

@snowystinger Thanks for reviewing. You can pause the review for now. We will get back to working on this PR or create a new one this week.

…nd get correct active element.

- Make `getFocusableTreeWalker` compatible with shadow root.
- Update `getOwnerWindow`.
- Fix `useFocus` - `useFocusVisible` - `useInteractionOutside` - `usePress`.
- Utilize `getDeepActiveElement`.
- Update usage of `e.target` in `useOverlay` to use `composedPath`.
@ritz078
Copy link
Contributor Author

ritz078 commented Mar 12, 2024

Closing this in favour of #6046

@ritz078 ritz078 closed this Mar 12, 2024
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.

None yet

3 participants