-
Notifications
You must be signed in to change notification settings - Fork 104
[PoC DO NOT MERGE] Floating #1114
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
base: develop
Are you sure you want to change the base?
Conversation
This reverts commit 63f87f7.
I love the deliberateness and thought of this approach - allowing us to tune the performance in the edge cases of large numbers of elements feels so helpful. |
|
For the first few examples with icons or icon + text. General recommendation is not to make non-interactive elements focusable (even though activating a tooltip may perhaps be considered as interaction?) Also Heydon says that, with recommendation not to use tooltips at all. And in his guide for tooltips, he really doesn't work with non-focusable elements. On the other hand, APG pattern clearly requests that
and it's the only way to make them accessible for keyboard users. Also, there are some resources that actually suggest making trigger elements focusable, even though they are not interactive in a typical sense
I don't know how reputable those are. But it suggest people use it in some ways. It seems that the main Heydon's issue with it is
which I've tried to address by labeling via describedby or labelledby, depending on context. That seems what is suggested in the APG pattern as well. This article https://www.a11y-collective.com/blog/tooltips-in-web-accessibility/ is quite interesting as it tries to balance many perspectives. With our current patterns, we will inevitably have to break one of the rules. So take it as some ideas that may perhaps help or not - it's probably best to just pragmatically try out and then evaluate whether it can add some value to the current experience, or not. |
|
Thank you @rtibbles. That's result of my despair after looking into our many use-cases 😁 |
|
Hey @MisRob! The amount of thought, information and effort that went into investigating possible implementation for this this new approach was simply astounding! 😍 Can't say I was surprised, being familiar with your thoroughness, but I was still 🙇🏽♀️ I may have some further thoughts on the sources you mentioned above, but given that we are navigating an overcharted territory where much more experienced a11y colleagues at APG have not managed to arrive to consensus after 9 years, and even without understanding all that happens under the hood, I very much appreciate the approach you've taken to accommodate various needs. Let's start with the test results of the NVDA output in latest Chrome and Firefox on Windows 10. KTooltip
Upon focusing, NVDA announces the Captions and subtitles On both Chrome and Firefox NVDA announces: For the best (expected) experience it should be Lazy KTooltipSame as for KTooltip on both Chrome and Firefox. KTooltip with delegated eventsNo live examples. KLabeledIcon with KTooltipOn both Chrome and Firefox NVDA only announces KIconButton with KTooltipOn both Chrome and Firefox NVDA correctly announces KTextbox with KTooltipOn both Chrome and Firefox NVDA announces: I believe a better experience would be: Note the different tooltip position between Chrome and Firefox (better imho)
Context menuClickable area is not focusable by keyboard, so not sure what is envisioned here to be the equivalent interaction workflow for the keyboard user... 🤔 Even with the mouse, left click does nothing. |
|
Thanks a lot for your time on this @radinamatic. High-level, that's good news - sounds we may benefit from some of those explorations. One of the first next steps will be a new version of I also appreciate detailed testing with screenreaders that are definitely more common than my Orca. As part of the new tooltip work, I will return to this part and see what we can do. Apologies for the context menu - that wasn't meant for a11y testing, but rather for preview for developers on how code would look like. I need to remember to be more explicit for you. |
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 clear writeup and demos Misha, it’s very helpful context. Overall the new composables (useKFloatingPosition, useKFloatingInteraction) make sense and feel like the right foundation and I agree with the proposed approach to start with Popper 2 and, depending on the results, downgrading if necessary, and keeping Floating UI in mind for later. Since these composables might be the base for all floating elements later, maybe we could test adding one other floating component (like a dropdown) using them just to be sure the APIs won’t need to change much down the road?
| * 'hover', 'touch', 'focus', 'keyboardfocus'. | ||
| * Default: ['hover']. | ||
| * | ||
| * @param {String} delegateTo Optional. 'root' or ID of the element to delegate |
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.
I like that this event delegation is optional, and that the events are automatically attached to the trigger element if this isn't specified
| Lazy rendering is for pages with many tooltips to prevent performance problems. Unlike | ||
| regular tooltips, lazy tooltips are not immediately present in the DOM, so using | ||
| <code>aria-labelledby</code> or <code>aria-describedby</code> does not work correctly. | ||
| Instead, <code>aria-label</code> or <code>aria-description</code> is a better choice. |
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.
The aria-describedby vs. aria-description pattern looks like a great approach, it would be nice to have this documented side by side in a ready-to-copy table if possible, so that devs can know exactly which to use for lazy vs. non-lazy tooltips at a glance.
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.
Definitely - I hope to use some of the drafts here for the final documentation
| Besides delegating to the root, ID of any element can be used as the delegation target. This | ||
| is useful when many tooltips are children of a container smaller than the | ||
| document—delegating to the container prevents unnecessary execution of handlers elsewhere in | ||
| the document. |
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.
I really like that the lazy rendering and event delegation are optional. It would be useful if we could define clear guidance in the docs, like:
“Use lazy if there are 100+ tooltips on a page.”
“Use delegation when tooltips share a parent container.”
I think this will help everyone stay consistent later when implementing tooltips.
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.
From my observations so far, this was very hard to generalize. There's more conditions to evaluate. Ultimately I think it's best to do a performance measurement of both options and choose the best in a given context.
In any case, I will think about it more. I hear that right now it's a very vague guidance. Perhaps I could suggest few example numbers that generally should work quite well, but still emphasize the recommendation to do measurements.
| * DOM node right next to the reference DOM node" | ||
| * https://popper.js.org/docs/v2/modifiers/event-listeners/ | ||
| */ | ||
| /* function updatePosition(floatingId) { |
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.
If this will eventually be exposed, it would be helpful to document exactly when it should be used (even if it is rarely needed)
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.
Makes sense
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.
A few comments - nothing is standing out to me as concerning in the approach.
| expect(_floatingInteractions['floating-2']).toBeUndefined(); | ||
| }); | ||
|
|
||
| it(`updates '_floatingCahe'`, async () => { |
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.
typo!
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.
thans!
|
|
||
| // If 'delegateTo' is 'root', this map is used to decide | ||
| // whether an event should be attached to Document or Window | ||
| const DELEGATE_ROOT = 'root'; |
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.
Just for my understanding, what is the use case for not delegating to root? Is it because delegating to a more proximate ancestor in the DOM tree will allow us to segment the floating elements more effectively? Does this give us any performance gains, or is it just a better developer experience?
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.
You could imagine a page where the first half of a page has many many tooltips, and another half has none tooltips while containing many elements. In that case it'd be best to delegate only to the element what wraps the first half, so that we don't trigger many mouse event listeners unnecessarily when interacting with the second half.
Anyway, I don't know if we have such a page :) I was just aware that generally this may be useful for balancing both sides, and architecture-wise it felt a bit more flexible compared to hardcoding window/document.
| function decreaseDelegateUsage(delegateTo, eventType) { | ||
| const usage = _delegateUsage[delegateTo]; | ||
| if (usage) { | ||
| usage[eventType] -= 1; |
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.
Do we need to do an existence check here as well?
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.
Yes, I should check on this everywhere in the final version
| // whether an event should be attached to Document or Window | ||
| const DELEGATE_ROOT = 'root'; | ||
| const DELEGATE_ROOT_TARGET = { | ||
| [MOUSEENTER]: 'document', |
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.
Is there a reason these couldn't just be direct references to document and window?
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.
Would you explain a bit more please? I don't get the question.
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.
@rtibbles can correct if I'm wrong but I think he's asking why strings as opposed to the window or document objects themselves - for example:
const DELEGATE_ROOT_TARGET = { [MOUSEENTER]: document, [FOCUS]: window, ... };
// Further down below
if (delegateTo === DELEGATE_ROOT) {
//const target = DELEGATE_ROOT_TARGET[eventType];
//return target === 'window' ? window : document;
// This instead:
return DELEGATE_ROOT_TARGET[event];| } | ||
| if (delegateTo === DELEGATE_ROOT) { | ||
| const target = DELEGATE_ROOT_TARGET[eventType]; | ||
| return target === 'window' ? window : document; |
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.
My question about the DELEGATE_ROOT_TARGET may mean that target would already be this?
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.
I still don't follow - let see if your answer in the related comment helps :)
| } | ||
|
|
||
| function areInteractionsValid(interactions) { | ||
| return interactions.every(i => SUPPORTED_INTERACTIONS.includes(i)); |
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.
Maybe check that interactions is even an Array first?
Not a huge deal, but I suppose SUPPORTED_INTERACTIONS could be a set, and we could just check that a set of interactions is a subset. Looks like isSubsetOf is polyfillable by core.js so we may be able to use it directly: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/isSubsetOf
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.
That sounds neat - I will have a look.
|
Thanks both for your time and feedback @LianaHarris360 @rtibbles.
Yes, I can add a quick experimental draft :) |
| // Determines if new listeners should be added to the trigger | ||
| // or the delegate elements associated with this floating element | ||
| onMounted(() => { | ||
| if (isNuxtServerSideRendering()) return; |
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.
Is this required for examples on the KDS site?
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.
Yes, that's a condition we generally add specifically because of the KDS site. I am not sure about the details, from what I observed, logic is mounted two times during building - and this condition prevents the first time mount attempt breaking the build.
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.
Fantastic - the demo looks/feels great overall and the tools themselves look like they'd be nice to use and they're very well documented. I have no suggestions for improvements or changes - I think that if I used them in some code I might start developing opinions 😅 - so I look forward to this being available!
Thank you for all of your hard work on this!
|
Thanks for taking time to preview it @nucleogenesis - it's good to hear that on some basic level it will be hopefully straightforward to use. Yes, I think after some time, we may come up with some tweaks ;) |


Description
Proposes solution for the next version of
KTooltipand also has broader exploration of how we could approach floating elements in the future.Not for merging. After this proof-of-concept stage, selected parts will be finalized through new PRs.
Reviewer guidance
useKFloatingInteraction,useKFloatingPosition,KTooltip/next,KIconButton, andv-focusable(majority is in this commit)As for a11y, @radinamatic would you please provide feedback whether some techniques I'm playing with in the live examples on this page could be an interesting direction for us and perhaps one step towards better experience? The APG tooltip pattern is so far just very basic and work in progress.
References
Related #745
Library choice and browser support
Oct 20 update: Note that based on the limitations mentioned below we decided to improve our browserlist and ability to test against it easily - which will change the way we think about the final choice. Feel free to skip this section.
Given our needs for customization, accessibility, performance, and maintainability, I think it'd be suitable to use a third-party library only for low-level position calculations. Candidates include Floating UI or its predecessor Popper v2 (or perhaps even v1).
Floating UI is a modern, actively maintained, and widely used library. However, its browser support is far away from our browserlist.
I previewed one instance of the next
KTooltipusinguseKFloatingPositionbuilt with both Floating UI and Popper 2 on Browserstack and didn't notice any differences between them. In the browsers below, they worked as expected, except one hiccup:*****However:
Based on this, one approach could be:
Once our browserlist allows, upgrading to Floating UI should be straightforward. In the meantime, we can prepare ground by removing other third-party dependencies in favor of
useKFloating....Performance
I looked into performance metrics such as repaints/reflows, JS heap size, number of DOM nodes, number of event listeners, and scripting time.
Number of DOM nodes
In the past, enabling
lazyonVTooltipsin Studio has reduced performance issues from excessive DOM nodes. Therefore, thelazyoption is supported in the nextKTooltip.Number of event listeners
For floating elements that can appear in large numbers, it's important to consider where event listeners controlling their visibility are attached. A common approach is to add listeners to each trigger element. For example, every icon showing a tooltip listening for
mouseoverand possible other events. On pages with many floating elements, this can lead to hundreds or thousands of listeners. Event delegation is a technique to reduce the number by attaching a single listener to a container or the window/document, handling events for its children. However, this means the handler runs for non-trigger elements too, which can increase scripting time, especially for frequent events likemouseenter.Even though
mouseenteris optimized and the handler is low-cost (exits unless the event target has thedata-floating-idattribute), on pages with only a few trigger elements (= smileys), event delegation still feels like unnecessary overhead:On the other hand, on pages with many trigger elements, event delegation often pays off, reducing the number of listeners by hundreds or even thousands:
For this reason,
useKFloatingInteractionand the nextKTooltip(or any other floating components) don't enforce a specific approach, but only provide an optionaldelegateToparameter. KDS documentation will offer guidance on how to choose the best approach.Measurements
In addition,
useKFloatingInteractionoptimizes other areas, such as using an elements cache and attaching leave event listeners only when needed. Here’s a test of the nextKTooltipon a page with 1000 tooltips:VTooltip, lazyKTooltip, lazy, events delegated to document/windowThese results were consistent across repeated measurements.
I tried testing on the aforementioned African Storybook page too, but due to many factors and thousands of listeners, it's nearly impossible to get clear numbers—e.g. repeated measurements of the current version show fluctuations in listener counts in the thousands for reasons unrelated to tooltips. One consistent finding was that, despite event delegation, scripting time was even better than before, suggesting that using
mouseoveron the whole window should be just fine on this page.Finally, after examining large channel pages, I believe that applying similar strategies to any elements appearing in large numbers, not just floating ones, will lead to many more improvements.