Skip to content

Provide access to portalContainer of react-aria-components popover element - #5381

Merged
snowystinger merged 9 commits into
adobe:mainfrom
BenBeattieHood:rac-expose-overlay-portalcontainer
Nov 23, 2023
Merged

snowystinger merged 9 commits into
adobe:mainfrom
BenBeattieHood:rac-expose-overlay-portalcontainer

Conversation

@BenBeattieHood

@BenBeattieHood BenBeattieHood commented Nov 8, 2023 •

Copy link
Copy Markdown
Contributor

#5382

Also:

✅ 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

🧢 Your Project:

PageUp/GEL

@reidbarber reidbarber left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR!

If you sign the CLA, then close and re-open this PR, it should re-build and pass CI.

@BenBeattieHood

Copy link
Copy Markdown
Contributor Author

Thanks @reidbarber, all done 👍

@BenBeattieHood

Copy link
Copy Markdown
Contributor Author

@devongovett @dannify any chance this can be included in the November release? It's a small change that exposes RAC functionality already provided by existing code.

@snowystinger

Copy link
Copy Markdown
Member

Hey, we're already done with testing for this release, so it won't be in this one. The team has some concerns about assumptions we've made with regards to the portalContainer, so we'll be wanting to spend a little time focused on discussions about this. Hopefully we'll have a chance to look at it after we complete the current release.
Thank you for your patience and understanding.

@BenBeattieHood

Copy link
Copy Markdown
Contributor Author

Thanks @snowystinger 👍

@LFDanLu LFDanLu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, tested locally and verified that the portalcontainer is being set properly. Will approve when the conflict is updated

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.

4 participants