search

atomiks / tippyjs

Tooltip, popover, dropdown, and menu library
https://atomiks.github.io/tippyjs/
MIT License
12.03k stars 520 forks source link

Autoincrementing `id` causes issues with 2+ tippy modules and snapshot tests #939

Open dbolkensteyn opened 3 years ago

dbolkensteyn commented 3 years ago

I am running two Chrome extensions which both embed tippy.js. When both extensions are enabled, they seem to conflict with one another, whereas they work well individually.

I believe that there is a conflict due to both of them adding tippy-1 ID elements to the DOM, and both wanting to later delete them and so on.

Is there a way to change the prefix (tippy-) of those IDs to make them unique between the Chrome extensions? Thank you!

atomiks commented 3 years ago

The IDs aren't used for deletion, that's just to link aria attributes. What problem are you having exactly?

dbolkensteyn commented 3 years ago

The problem is quite subtle: When I slowly interact with the page, everything works., both tooltips show up at the same location and the one with higher zIndex wins.

However as I move faster between hoverable elements, eventually a few of them no longer get deleted and remain dangling, while most still work as expected: image

If I then scroll the page with the dangling tooltip on it, it moves to the top left of the screen: image

And here is its DOM element: (appendTo: document.body, in case this changes something): image

dbolkensteyn commented 3 years ago

Ah I just realized that there is also a Javascript error appearing: image

cscheffauer commented 3 years ago

Also to note - changing the id dynamically will cause issues while doing snapshot tests. Whenever a test will rerun, it id changes and isn't stable

atomiks commented 3 years ago

So we should use a different technique for IDs than an autoincrementing integer for 1) snapshot tests and 2) scenarios in which 2+ tippy modules are embedded (for accessibility reasons, at the least)

Moving to https://github.com/ai/nanoid will likely be needed, though since this lib technically supports IE11 with polyfills, you'll need to add a new one if we migrate to this which is a breaking change