fix: Tooltip: Don't close if user hovers on opener then tooltip content #5320
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
| @@ -449,6 +449,9 @@ class Tooltip extends RtlMixin(LitElement) { | |||
| this._isHovering = false; | |||
| this._resizeRunSinceTruncationCheck = false; | |||
| this._viewportMargin = defaultViewportMargin; | |||
|
|
|||
| this.#isHoveringTooltip = false; | |||
| this.#leftTooltip = false; | |||
There was a problem hiding this comment.
Open to a better name for this
There was a problem hiding this comment.
Hmmm yeah, I can see it being a little confusing. Idk, maybe mouseLeftTooltip would be a bit clearer. Or perhaps wasHoveringTooltip, or mouseLeavingTooltip?
margaree
changed the title
| @@ -855,7 +867,7 @@ class Tooltip extends RtlMixin(LitElement) { | |||
| _onTargetMouseLeave() { | |||
| clearTimeout(this._hoverTimeout); | |||
| this._isHovering = false; | |||
| this._updateShowing(); | |||
| setTimeout(this._updateShowing.bind(this), 100); // delay to allow for mouseenter to fire if hovering on tooltip | |||
There was a problem hiding this comment.
Nit: arrow functions allow you to avoid bind
| setTimeout(this._updateShowing.bind(this), 100); // delay to allow for mouseenter to fire if hovering on tooltip | |
| setTimeout(() => this._updateShowing(), 100); // delay to allow for mouseenter to fire if hovering on tooltip |
| setTimeout(() => { | ||
| this.#leftTooltip = false; | ||
| this._updateShowing(); | ||
| }, 100); // delay to allow for mouseenter to fire if hovering on target |
There was a problem hiding this comment.
Is there a weird race condition if they leave but then come back and leave again within this time period? Might be more obvious if you increase it to like 10s while testing. If this is a problem, you could capture the timeout identifier (the thing returned from setTimeout) and cancel the previous one using clearTimeout.
Jira ticket
See here to test out
A user should be able to go back and forth (hovering) between tooltip opener and content. Previously the tooltip would close as soon as the mouse left the opener. This is a WCAG violation (see ticket for more details).
Ideally the gap between the opener and tooltip (the offset) would be a hover zone but I was unable to get that working in a way that wasn't overly complex given the complexity of tooltip positioning.
This implementation uses a timeout to delay the closing. Jeff and I both felt like the 100ms time was a good balance of enough time for a user to generally get to the tooltip without being detectable on actual close.
This should be fine in general usecases (though let me know if there's something I'm missing). There could be some issues if something weird is being done with offset. I was able to go between opener and tooltip with larger offsets, but someone with low dexterity could have problems, so if those cases come up then we should dig into those more.