[ARGG-1157][BpkTooltip]: Migrate tooltip to floating-ui #3485
Conversation
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
Browser supportIf this is a visual change, make sure you've tested it in multiple browsers. |
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
1 similar comment
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
olliecurtis
force-pushed
the
ARGG-1157-tooltip-to-floating-ui
branch
from
dbb5c17
to
35587fb
Compare
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
olliecurtis
force-pushed
the
ARGG-1157-tooltip-to-floating-ui
branch
from
35587fb
to
1b0b280
Compare
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
olliecurtis
force-pushed
the
ARGG-1157-tooltip-to-floating-ui
branch
from
1b0b280
to
6ec9e40
Compare
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
| */ | ||
| ariaLabel: string; | ||
| /** | ||
| * "target" should be a DOM element with a "ref" attached to it. |
There was a problem hiding this comment.
Yes this is still the case, we will still want people to pass through a ReactElement and not a function like document.getElementById which is what the popover did, but the tooltip was never built to support that only
<div>
My target
</div>
There was a problem hiding this comment.
I mean the ref part with a "ref" attached to it. . I see we're using floating ui to set the ref so I don't think the consumer has to? https://github.com/Skyscanner/backpack/pull/3485/files#diff-2702cb346943a8761bb30c82e1a61efeae40297ed8cee8028da4376b719dc73cR132-R138
Also interesting that you can clone an element and attach a ref to it 😮 I think that's new-ish, I think a few years ago it was not possible
There was a problem hiding this comment.
Ah gotcha! Let me do a test with removing that target ref and see what magic happens!
Also interesting that you can clone an element and attach a ref to it 😮 I think that's new-ish, I think a few years ago it was not possible
Interesting didn't realise it was a newer feature! I guess it is just a property to a DOM element so is easy to add this prop as any other?
There was a problem hiding this comment.
Ah gotcha! Let me do a test with removing that target ref and see what magic happens!
Cool, have you checked that?
Interesting didn't realise it was a newer feature! I guess it is just a property to a DOM element so is easy to add this prop as any other?
Yeah, I remember it wasn't possible before, but not sure why 🤔
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3485 to see this build running in a browser. |
Migrating the underlying library of the
BpkTooltipto use the maintainers replacement libraryfloating-ui, a much more lightweight library and has a lot of functionality built in that means we can remove our use of custom built react Portals!This is one PR of a few to update components away from popperjs to floating-ui so can be used as a base!
Changes here are as follows:
TooltipPortal, as we no longer need to use our custom React Portal implementation as this is handled via floating-uirenderTargetprop as this is longer needed to apply where in the DOM the tooltip should be and is located byfloating-uirelative to the targetportalStyleandportalClassNameas these are no longer required as we aren't using a custom portalpopperModifiersproperty as these no longer have an effect on the componentclassNameproperty to prevent overriding of componentMigration guide:
The main changes for this component is removing properties that have minor to no impact to the current functionality of the component.
So remove use of the following properties:
renderTarget,portalStyle,portalClassName,popperModifiersandclassName.Should the design/style not be fit for purpose please align with design to make sure they use the component as expected before getting in touch.
Remember to include the following changes:
[KOA-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md