feat(tooltip): add overflow tooltip directive - #2353
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the 'SiTooltipOverflowDirective', which automatically displays a tooltip containing the host element's text when it overflows and is truncated. It includes unit tests and updates the example application to showcase the new directive. Feedback was provided regarding the execution of 'updateTooltip()' outside the Angular zone, which prevents automatic change detection for the tooltip's event handlers. It is recommended to run the state updates and tooltip creation/destruction inside the Angular zone using 'NgZone.run()'.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the SiTooltipOverflowDirective to automatically display a tooltip when host element text overflows, along with supporting updates to SiTooltipService to accept a canShow callback. However, refactoring SiTooltipDirective to use this callback for isDisabled removes the property from the effect dependency tracking, causing a regression where dynamically disabling a visible tooltip will not immediately hide or destroy it.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the SiTooltipOverflowDirective to automatically display tooltips for truncated text, refactors SiTooltipDirective to instantiate tooltip references directly using a computed canShow signal, and updates SiTooltipService to support this conditional visibility. Feedback on the changes highlights a critical runtime error (NG0600) in SiTooltipOverflowDirective caused by updating a signal inside an effect's execution path. The reviewer recommends replacing the tooltipText signal with a direct getter function to resolve this issue and subsequently removing the unused signal import.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
d81880d to
3a2ad8e
Compare
Add a directive that automatically renders a tooltip if the host text is truncated.
3a2ad8e to
941bb05
Compare
Usage
Problem to solve
We have many places where text is eventually truncated if there is not enough space.
Since we do not know upfront if the text is truncated, we need to apply a tooltip conditionally.
Currently, we simply do not apply any tooltips in those cases.
The first place where I plan to use this directive is the breadcrumb. There we currently violate a11y by attaching a menu in case text is truncated. Also the truncation itself is implemented in a poor way.
Approach
Whenever the target element is hovered / focused we check if the text is truncated and update the tooltip content
based on the
innerText.Since we check the truncation and
innerTextalways immediately before showing, we don't need any listener for changes.Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: