rdk icon indicating copy to clipboard operation
rdk copied to clipboard

GOOD-94: migrate popperjs to floating-ui

Open shaswat-indian opened this issue 1 year ago • 2 comments

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, api changes) - change in the `usePosition` hook signature
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Same as the new behavior - tested with existing stories.

Issue Number: N/A

What is the new behavior?

https://github.com/reaviz/rdk/assets/33908100/f2892558-5c02-47e3-88a4-9940a2f1cf49

Does this PR introduce a breaking change?

[x] Yes
[ ] No

This PR migrate the dependency from PopperJS to floating-UI. This may be a breaking change for certain use cases where the usePosition hook is used, however this should not impact the use cases which are included in the storybook as the behavior looks identical even after the change.

Other information

shaswat-indian avatar Jan 17 '24 17:01 shaswat-indian

Just a heads up, I tested this locally with reablocks and Tooltip seems to be working with different positionings straight outta the box 🎉

But when testing with reaviz, which is using reablocks Tooltip under the hood, the chart tooltips are all in the top left of the screen. I don't think any adjusting necessarily needs to be done to this PR, but I can't tell why it's happening so something to be aware of. The fix might need to be made here or in reaviz to get it all working together, it's unclear

ghsteff avatar Jan 17 '24 17:01 ghsteff

@shaswat-indian - We need to make sure we cover the line / area chart scenario in reaviz.

amcdnl avatar Jan 17 '24 19:01 amcdnl