Skip to content

Surface useOverlayPosition's updatePosition from usePopover #3872

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

Closed
wants to merge 1 commit into from

Conversation

BenBeattieHood
Copy link
Contributor

The 'updatePosition' is still important when using popovers, eg if using react transitiongroup for a popover, as the position cannot be calculated while the contents is not mounted. This PR surfaces the callback.

✅ 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

The 'updatePosition' is still important when using popovers, eg if using react transitiongroup for a popover, as the position cannot be calculated while the contents is not mounted. This commit surfaces the callback.
@BenBeattieHood
Copy link
Contributor Author

As this PR is only to surface a property already public, I don't think there's any tests/docs to update - but happy to if needed. 👍

@ktabors
Copy link
Member

ktabors commented Jun 9, 2023

Thanks for the PR. For the build to pass you'll need to sign the CLA, then close and re-open this PR to let it refresh.

@ktabors ktabors added the waiting Waiting on Issue Author label Jun 9, 2023
@ktabors
Copy link
Member

ktabors commented Jun 23, 2023

Please reopen this when the CLA is signed.

@ktabors ktabors closed this Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting Waiting on Issue Author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants