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

Focus scope shadowdom #2542

Closed
wants to merge 5 commits into from
Closed

Conversation

padmkris123
Copy link

@padmkris123 padmkris123 commented Nov 10, 2021

Partial work on 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:

Create FocusScope under a shadowDom. The focus management should continue to work.

🧢 Your Project:

Adobe Acrobat Sign

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

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

So in general this makes sense and addresses the first bullet in the issue description. I think this can go in as is, but it doesn't completely close the issue, so I'm going to edit the PR description a little to reflect that.

render(<Test />);
});

afterEach(cleanup);
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't need cleanup, it's called automatically by default

@dannify
Copy link
Member

dannify commented Apr 28, 2022

Thanks for working on this. I am going to close this PR for now since it seems to be stale. Please feel free to reopen when you get a chance to pick it up again.

@dannify dannify closed this Apr 28, 2022
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

4 participants