Skip to content

Commit 45f288b

Browse files
dancormiergiamir
andauthored
fix(tooltip): prevent hide when focus is within tooltip (#1811)
* fix(tooltip): prevent hide when focus is within tooltip * Add tests * formatting * prefer usage of waitFor and waitForElementToBeRemoved over sleep function * formatting * simplify the hiding logic when focus is in the popover element * remove unnecessary binding * Remove aria-hidden from tooltip docs --------- Co-authored-by: Giamir Buoncristiani <giamir.buoncristiani@gmail.com>
1 parent 763fb23 commit 45f288b

File tree

3 files changed

+127
-28
lines changed

3 files changed

+127
-28
lines changed

docs/product/components/popovers.html

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -580,39 +580,42 @@ <h1 class="s-page-title--header" role="presentation">Lorem ipsum</h1>
580580

581581
<div class="stacks-preview">
582582
{% highlight html %}
583-
<button class="s-btn" role="button"
584-
aria-describedby="tooltip-example"
585-
aria-expanded="false"
586-
data-controller="s-tooltip"
587-
data-s-tooltip-placement="top-start">
583+
<button
584+
class="s-btn"
585+
role="button"
586+
aria-describedby="tooltip-example"
587+
aria-expanded="false"
588+
data-controller="s-tooltip">
588589
589590
</button>
590-
<div class="s-popover s-popover__tooltip"
591-
id="tooltip-example"
592-
role="tooltip"
593-
aria-hidden="true">
591+
<div
592+
class="s-popover s-popover__tooltip"
593+
id="tooltip-example"
594+
role="tooltip">
594595
<div class="s-popover--arrow"></div>
595596
<div class="s-popover--content">
596597
597598
</div>
598599
</div>
599600
{% endhighlight %}
600601
<div class="stacks-preview--example bg-black-100">
601-
<button class="s-btn s-btn__filled" role="button"
602-
aria-describedby="tooltip-example"
603-
data-controller="s-tooltip"
604-
data-s-tooltip-placement="top-start">
602+
<button
603+
class="s-btn s-btn__filled"
604+
role="button"
605+
aria-describedby="tooltip-example"
606+
aria-expanded="false"
607+
data-controller="s-tooltip">
605608
Hover tooltip popover
606609
</button>
607-
<div class="s-popover s-popover__tooltip"
610+
<div
611+
class="s-popover s-popover__tooltip"
608612
id="tooltip-example"
609-
role="tooltip"
610-
aria-hidden="true">
613+
role="tooltip">
611614
<div class="s-popover--arrow"></div>
612615
<div class="s-popover--content">
613616
<div class="s-empty-state wmx2">
614617
{% spot "EmptyLg", "mb12" %}
615-
<p class="mb0">There’s no data associated with your account yet. Please check back tomorrow.</p>
618+
<p class="mb0">There’s no data associated with your account yet. Please visit our <a href="#" class="s-link">help page</a> for more information.</p>
616619
</div>
617620
</div>
618621
</div>

lib/components/popover/tooltip.test.ts

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { html, fixture, expect } from "@open-wc/testing";
2-
import { screen, waitForElementToBeRemoved } from "@testing-library/dom";
2+
import {
3+
screen,
4+
waitForElementToBeRemoved,
5+
waitFor,
6+
} from "@testing-library/dom";
37
import userEvent from "@testing-library/user-event";
48
import "../../index";
59

@@ -59,4 +63,73 @@ describe("tooltip", () => {
5963
await user.hover(svg);
6064
expect(tooltip).to.be.visible;
6165
});
66+
67+
it("should continue to show the tooltip if focus is moved to an element within the tooltip", async () => {
68+
await fixture(html`
69+
<button
70+
class="s-btn s-btn__filled"
71+
role="button"
72+
aria-describedby="tooltip-example"
73+
data-controller="s-tooltip"
74+
>
75+
Hover tooltip popover
76+
</button>
77+
<div
78+
class="s-popover s-popover__tooltip"
79+
id="tooltip-example"
80+
role="tooltip"
81+
data-testid="tooltip"
82+
>
83+
<div class="s-popover--arrow"></div>
84+
<div class="s-popover--content">
85+
<a href="#" data-testid="link">View more</a>
86+
</div>
87+
</div>
88+
`);
89+
90+
const tooltip = screen.getByTestId("tooltip");
91+
const link = screen.getByTestId("link");
92+
93+
await user.tab();
94+
await waitFor(() => expect(tooltip).to.have.class("is-visible")); // wait for the tooltip to appear
95+
await user.tab(); // tab to nested link within tooltip
96+
expect(link).to.have.focus; // link within tooltip is focused
97+
expect(tooltip).to.be.visible; // link within tooltip is focused, tooltip is visible
98+
});
99+
100+
it("should hide the tooltip if focus is moved to an element outside the tooltip and trigger", async () => {
101+
await fixture(html`
102+
<button
103+
class="s-btn s-btn__filled"
104+
role="button"
105+
aria-describedby="tooltip-example"
106+
data-controller="s-tooltip"
107+
>
108+
Hover tooltip popover
109+
</button>
110+
<div
111+
class="s-popover s-popover__tooltip"
112+
id="tooltip-example"
113+
role="tooltip"
114+
data-testid="tooltip"
115+
>
116+
<div class="s-popover--arrow"></div>
117+
<div class="s-popover--content">
118+
<a href="#">View more</a>
119+
</div>
120+
</div>
121+
<button data-testid="neutral-btn">Another button</button>
122+
`);
123+
124+
const tooltip = screen.getByTestId("tooltip");
125+
const neutralBtn = screen.getByTestId("neutral-btn");
126+
127+
await user.tab();
128+
await waitFor(() => expect(tooltip).to.have.class("is-visible")); // wait for the tooltip to appear
129+
await user.tab(); // tab to nested link within tooltip
130+
await user.tab(); // tab to button after tooltip
131+
expect(neutralBtn).to.have.focus; // button after tooltip is focused
132+
await waitForElementToBeRemoved(() => screen.queryByRole("tooltip")); // wait for the tooltip to hide
133+
expect(tooltip).not.to.have.class("is-visible"); // check that .is-visible class is removed
134+
});
62135
});

lib/components/popover/tooltip.ts

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export class TooltipController extends BasePopoverController {
1414
private boundHide!: () => void;
1515
private boundHideIfWithin!: () => void;
1616
private boundHideOnEscapeKeyEvent!: () => void;
17+
private boundHideIfNotFocused!: () => void;
1718
private boundClearActiveTimeout!: () => void;
1819
private activeTimeout!: number;
1920

@@ -193,6 +194,19 @@ export class TooltipController extends BasePopoverController {
193194
}
194195
}
195196

197+
/**
198+
* Hides the tooltip if the focus is not on the popover element.
199+
* @param event A focusout event object from the reference or popover element
200+
*/
201+
private hideIfNotFocused(event: FocusEvent) {
202+
if (
203+
!this.referenceElement.contains(event.relatedTarget as Node) &&
204+
!this.popoverElement.contains(event.relatedTarget as Node)
205+
) {
206+
this.scheduleHide();
207+
}
208+
}
209+
196210
private hideOnEscapeKeyEvent(event: KeyboardEvent) {
197211
if (event.key === "Escape") {
198212
this.scheduleHide();
@@ -204,15 +218,23 @@ export class TooltipController extends BasePopoverController {
204218
private bindKeyboardEvents() {
205219
this.boundScheduleShow =
206220
this.boundScheduleShow || this.scheduleShow.bind(this);
207-
this.boundHide = this.boundHide || this.scheduleHide.bind(this);
208221
this.boundHideOnEscapeKeyEvent =
209222
this.boundHideOnEscapeKeyEvent ||
210223
this.hideOnEscapeKeyEvent.bind(this);
211-
224+
this.boundHideIfNotFocused =
225+
this.boundHideIfNotFocused || this.hideIfNotFocused.bind(this);
212226
this.referenceElement.addEventListener("focus", this.boundScheduleShow);
213-
this.referenceElement.addEventListener("blur", this.boundHide);
227+
this.referenceElement.addEventListener(
228+
"focusout",
229+
this.boundHideIfNotFocused.bind(this)
230+
);
231+
this.popoverElement.addEventListener(
232+
"focusout",
233+
this.boundHideIfNotFocused.bind(this)
234+
);
214235
document.addEventListener("keyup", this.boundHideOnEscapeKeyEvent);
215236
}
237+
216238
/**
217239
* Unbinds all mouse events
218240
*/
@@ -221,7 +243,14 @@ export class TooltipController extends BasePopoverController {
221243
"focus",
222244
this.boundScheduleShow
223245
);
224-
this.referenceElement.removeEventListener("blur", this.boundHide);
246+
this.referenceElement.removeEventListener(
247+
"focusout",
248+
this.boundHideIfNotFocused
249+
);
250+
this.referenceElement.removeEventListener(
251+
"focusout",
252+
this.boundHideIfNotFocused
253+
);
225254
document.removeEventListener("keyup", this.boundHideOnEscapeKeyEvent);
226255
}
227256

@@ -234,7 +263,6 @@ export class TooltipController extends BasePopoverController {
234263
this.boundHide = this.boundHide || this.scheduleHide.bind(this);
235264
this.boundClearActiveTimeout =
236265
this.boundClearActiveTimeout || this.clearActiveTimeout.bind(this);
237-
238266
this.referenceElement.addEventListener(
239267
"mouseover",
240268
this.boundScheduleShow
@@ -256,11 +284,6 @@ export class TooltipController extends BasePopoverController {
256284
this.boundScheduleShow
257285
);
258286
this.referenceElement.removeEventListener("mouseout", this.boundHide);
259-
this.referenceElement.removeEventListener(
260-
"focus",
261-
this.boundScheduleShow
262-
);
263-
this.referenceElement.removeEventListener("blur", this.boundHide);
264287
this.popoverElement.removeEventListener(
265288
"mouseover",
266289
this.boundClearActiveTimeout

0 commit comments

Comments
 (0)