refactor(tooltip): only show tooltip on focus if :focus-visible matches#1517
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the tooltip to only appear on keyboard focus by checking for :focus-visible, which is a great enhancement for accessibility and user experience. The implementation is mostly correct and is well-tested with a new end-to-end test.
I've found one potential issue in the tooltip directive related to server-side rendering (SSR). The current logic could cause errors on the server because it attempts to render a tooltip in a non-browser environment. I've left a specific comment with a suggested fix for this.
Otherwise, the changes look good and the new test provides good coverage for the new behavior.
spike-rabbit
force-pushed
the
refactor/tooltip/only-show-on-focus-visible
branch
2 times, most recently
from
9bec8c0 to
4c3cfec
Compare
|
Documentation. Coverage Reports: |
dauriamarco
left a comment
dauriamarco
left a comment
There was a problem hiding this comment.
LGTM 👍 and I guess that I can also close #1514 as this one is a better solution, especially if we don't need to show the tooltip on programmatic focus.
…ches Currently, the tooltip is always shown, if the attached element receives focus. This behavior is not desired, as this previously also showed the tooltip if Element was programmatically focussing an element. For instance in the navbar after closing a flyout menu. With the new logic, a tooltip will only be shown on focus if the `:focus-visible` selector matches and thus the site is operated by a keyboard. This commit is labeled as a refactor, as the new behavior was introduced within the same release.
spike-rabbit
force-pushed
the
refactor/tooltip/only-show-on-focus-visible
branch
from
4c3cfec to
4a238f4
Compare
|
|
2 participants
Currently, the tooltip is always shown
if the attached element receives focus.
This behavior is not desired.
This previously also showed the tooltip
if Element was programmatically focussing an element. For instance in the navbar after closing a flyout menu.
With the new logic, a tooltip will only be shown on focus if the
:focus-visibleselector matches and thus the site is operated by a keyboard.This commit is labeled as a refactor,
as the new behavior was introduced within the same release.