Skip to content

feat: add tooltip support to vaadin-side-nav-item #10008

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Aug 21, 2025
Merged

feat: add tooltip support to vaadin-side-nav-item #10008

merged 14 commits into from
Aug 21, 2025

Conversation

HerbertsVaadin
Copy link
Collaborator

@HerbertsVaadin HerbertsVaadin commented Aug 15, 2025 •
edited by web-padawan
Loading

Description

Adds better support for adding vaadin-tooltip to vaadin-side-nav-item, by providing a slot for it, and attaching the tooltip to only parent content.
This prevents tooltip appearing when hovering over items children.

Part of vaadin/flow-components#7615

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/pr
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.
  • I have not completed some of the steps above and my pull request can be closed immediately.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.
    (no Acceptance Criteria, but scope was defined)

@HerbertsVaadin HerbertsVaadin mentioned this pull request Aug 18, 2025
Merged
10 tasks
web-padawan
web-padawan reviewed Aug 18, 2025
View reviewed changes
web-padawan
web-padawan reviewed Aug 18, 2025
View reviewed changes
web-padawan
web-padawan reviewed Aug 18, 2025
View reviewed changes
@HerbertsVaadin HerbertsVaadin force-pushed the feat/sd-side-nav-item-tooltip branch from 2d495d4 to 296e591 Compare August 18, 2025 10:43
@web-padawan web-padawan force-pushed the feat/sd-side-nav-item-tooltip branch from 2d8c09f to eb2c3de Compare August 18, 2025 11:32
web-padawan
web-padawan reviewed Aug 18, 2025
View reviewed changes
Copy link
Member

@web-padawan web-padawan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the PR and wasn’t able to make tooltip announced in VoiceOver or NVDA it neither when setting aria-describedby on the slotted item content element, nor on the item itself.

When setting it on the item itself, at least JAWS announces it:

jaws-tooltip

So I will update the PR accordingly. We probably should also document this as an a11y limitation.

@web-padawan web-padawan force-pushed the feat/sd-side-nav-item-tooltip branch from 4f0048d to fdf7c29 Compare August 18, 2025 11:57
@web-padawan web-padawan changed the title feat: Tooltip support for vaadin-side-nav-item feat: add tooltip support to vaadin-side-nav-item Aug 18, 2025
@web-padawan
Copy link
Member

Updated to place <slot name="tooltip"> inside the link after the default slot - this makes it announced:

VoiceOver

Screenshot 2025-08-19 at 13 18 23

NVDA

Screenshot 2025-08-19 at 13 19 34

JAWS

Screenshot 2025-08-19 at 13 18 58

Note: in VoiceOver and NVDA there is now a duplicate announcement of the link and a button - in this case, "Layouts" and "Toggle child items" are announced twice. On the positive side, tooltip text is announced. JAWS works properly.

@web-padawan
Copy link
Member

Some observations from the internal discussion by @sissbruecker:

In VoiceOver the experience is still pretty broken:

  • When tabbing between nav items, the tooltip is only announced when it is visible. For me it only works if I focus some item and wait until the tooltip becomes visible, and then quickly tab and tab back. If I slowly tab through items tooltips are not announced. Which makes sense because the tooltip is hidden until the item becomes focused.
  • As soon as the tooltip is visible, it becomes navigatable as its own element when using the VoiceOver cursor

In NVDA the tooltip is not announced when the link is not clickable and you use cursor navigation.

@web-padawan web-padawan force-pushed the feat/sd-side-nav-item-tooltip branch from e8b923e to 1c63e9a Compare August 20, 2025 08:01
@web-padawan
Copy link
Member

Updated to use the approach with sr-only element which works in all screen readers:

VoiceOver

Screenshot 2025-08-20 at 10 54 39 Screenshot 2025-08-20 at 10 58 17

NVDA

Screenshot 2025-08-20 at 10 57 55

JAWS

Screenshot 2025-08-20 at 10 59 07

sissbruecker
sissbruecker reviewed Aug 20, 2025
View reviewed changes
@web-padawan web-padawan mentioned this pull request Aug 20, 2025
Merged
@web-padawan web-padawan force-pushed the feat/sd-side-nav-item-tooltip branch from a6c8e42 to 97d1d62 Compare August 20, 2025 13:52
@web-padawan web-padawan force-pushed the feat/sd-side-nav-item-tooltip branch from 97d1d62 to dbc16e3 Compare August 20, 2025 14:05
@web-padawan
Copy link
Member

Rebased after merging #10045.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@web-padawan web-padawan merged commit 267620c into main Aug 21, 2025
9 checks passed
@web-padawan web-padawan deleted the feat/sd-side-nav-item-tooltip branch August 21, 2025 06:58
@vaadin-bot
Copy link
Collaborator

Hi @HerbertsVaadin and @web-padawan, when i performed cherry-pick to this commit to 24.9, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 267620c
error: could not apply 267620c... feat: add tooltip support to vaadin-side-nav-item (#10008)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@web-padawan
Copy link
Member

I'll cherry-pick #10045 first and then this PR manually.

web-padawan added a commit that referenced this pull request Aug 21, 2025
@HerbertsVaadin @web-padawan
feat: add tooltip support to vaadin-side-nav-item (#10008)
9b031a4 
Co-authored-by: Serhii Kulykov <iamkulykov@gmail.com>
@web-padawan web-padawan mentioned this pull request Aug 21, 2025
Merged
web-padawan added a commit that referenced this pull request Aug 21, 2025
@web-padawan @HerbertsVaadin
feat: add tooltip support to vaadin-side-nav-item (#10008) (#10050)
b4031cf 
Co-authored-by: Herberts <80950643+HerbertsVaadin@users.noreply.github.com>
@web-padawan web-padawan mentioned this pull request Aug 26, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@web-padawan web-padawan web-padawan left review comments

@sissbruecker sissbruecker sissbruecker approved these changes

Assignees
No one assigned
Labels
need to pick manually 24.9 target/24.9
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@HerbertsVaadin @web-padawan @vaadin-bot @sissbruecker