refactor(tooltip): improve tooltip show timing#1514
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a change to improve the tooltip display timing by differentiating between focus events triggered by keyboard and those by mouse clicks. It adds a pointerdown event listener and a flag to delay showing the tooltip on click, making the behavior consistent with hover.
The implementation is generally good, but there's a potential edge case where the isPointerDown flag could get stuck in a true state, affecting subsequent interactions. I've added a suggestion to make the flag's state transient and avoid this issue. Otherwise, the changes look correct and address the intended goal.
| protected pointerDown(): void { | ||
| this.isPointerDown = true; | ||
| } |
There was a problem hiding this comment.
The isPointerDown flag is set here but only reset when the tooltip is shown or hidden. If a pointerdown event occurs without a corresponding show/hide action (e.g., on a non-focusable element), the flag can get stuck as true. This could cause future keyboard focus events to be incorrectly delayed.
To prevent this, the flag should be reset automatically after the event sequence. Using setTimeout ensures the flag is transient, covering the pointerdown -> focus sequence without affecting later, unrelated focus events.
| protected pointerDown(): void { | |
| this.isPointerDown = true; | |
| } | |
| protected pointerDown(): void { | |
| this.isPointerDown = true; | |
| setTimeout(() => (this.isPointerDown = false), 0); | |
| } |
|
Documentation. Coverage Reports: |
|
Closing this in favor of #1517. |
Adds
pointerdownevent handling to improve tooltip display timing.