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
[Popover] Fix PaperProps.ref breaking positioning #26560
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work.
A couple of minor things:
- Did you run
yarn prettier
and commit the changes? - If you're up to it, please write a separate test without
openPopover
.
It was the right decision to reach for existing tools but this particular test is pre-dates our new testing approach. Your change hightlights this issue perfectly where a single added test now affects all existing test. Debugging existing tests became harder and tests should usually not change over time unless breaking changes are introduced.
Yeah thanks for pointing out, I forgot to run For (2) - I will try to separate out the test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #26538
Fixed by making use of
useForkRef
utility as described in the issue comments.