Skip to content

Commit 9f4b14b

Browse files
authored
fix(popover): prevent disabled reference elements from toggling popover (#8983)
**Related Issue:** #7732 ## Summary This changes `PopoverManager` to no longer rely on the capture phase for toggling popovers, so clicks from disabled elements are ignored properly. **Note**: this also introduces a new DOM util to help determine if a click event was triggered via keyboard or not.
1 parent 2728d4f commit 9f4b14b

File tree

4 files changed

+66
-23
lines changed

4 files changed

+66
-23
lines changed

packages/calcite-components/src/components/popover/PopoverManager.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ReferenceElement } from "../../utils/floating-ui";
22
import { isActivationKey } from "../../utils/key";
3+
import { isKeyboardTriggeredClick } from "../../utils/dom";
34

45
export default class PopoverManager {
56
// --------------------------------------------------------------------------
@@ -71,7 +72,7 @@ export default class PopoverManager {
7172
Array.from(this.registeredElements.values()).forEach((popover) => (popover.open = false));
7273
}
7374

74-
private keyHandler = (event: KeyboardEvent): void => {
75+
private keyDownHandler = (event: KeyboardEvent): void => {
7576
if (event.defaultPrevented) {
7677
return;
7778
}
@@ -84,16 +85,20 @@ export default class PopoverManager {
8485
};
8586

8687
private clickHandler = (event: PointerEvent): void => {
88+
if (isKeyboardTriggeredClick(event)) {
89+
return;
90+
}
91+
8792
this.togglePopovers(event);
8893
};
8994

9095
private addListeners(): void {
91-
window.addEventListener("click", this.clickHandler, { capture: true });
92-
window.addEventListener("keydown", this.keyHandler, { capture: true });
96+
window.addEventListener("click", this.clickHandler);
97+
window.addEventListener("keydown", this.keyDownHandler);
9398
}
9499

95100
private removeListeners(): void {
96-
window.removeEventListener("click", this.clickHandler, { capture: true });
97-
window.removeEventListener("keydown", this.keyHandler, { capture: true });
101+
window.removeEventListener("click", this.clickHandler);
102+
window.removeEventListener("keydown", this.keyDownHandler);
98103
}
99104
}

packages/calcite-components/src/components/popover/popover.e2e.ts

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -258,27 +258,25 @@ describe("calcite-popover", () => {
258258
const page = await newE2EPage();
259259

260260
await page.setContent(
261-
`<calcite-popover placement="auto" reference-element="ref">content</calcite-popover><div id="ref">referenceElement</div>`,
261+
html`<calcite-popover placement="auto" reference-element="ref">content</calcite-popover>
262+
<div id="ref" tabindex="0">referenceElement</div>`,
262263
);
263264

264265
await page.waitForChanges();
265266

266267
const popover = await page.find(`calcite-popover`);
268+
const ref = await page.find("#ref");
267269

268270
expect(await popover.isVisible()).toBe(false);
269271

270-
await page.evaluate(() => {
271-
document.getElementById("ref").dispatchEvent(new KeyboardEvent("keydown", { key: "Enter" }));
272-
});
273-
272+
await ref.focus();
273+
await page.keyboard.press("Enter");
274274
await page.waitForChanges();
275275

276276
expect(await popover.isVisible()).toBe(true);
277277

278-
await page.evaluate(() => {
279-
document.getElementById("ref").dispatchEvent(new KeyboardEvent("keydown", { key: "Enter" }));
280-
});
281-
278+
await ref.focus();
279+
await page.keyboard.press("Enter");
282280
await page.waitForChanges();
283281

284282
expect(await popover.isVisible()).toBe(false);
@@ -288,27 +286,25 @@ describe("calcite-popover", () => {
288286
const page = await newE2EPage();
289287

290288
await page.setContent(
291-
`<calcite-popover placement="auto" reference-element="ref">content</calcite-popover><div id="ref">referenceElement</div>`,
289+
html`<calcite-popover placement="auto" reference-element="ref">content</calcite-popover>
290+
<div id="ref" tabindex="0">referenceElement</div>`,
292291
);
293292

294293
await page.waitForChanges();
295294

296295
const popover = await page.find(`calcite-popover`);
296+
const ref = await page.find("#ref");
297297

298298
expect(await popover.isVisible()).toBe(false);
299299

300-
await page.evaluate(() => {
301-
document.getElementById("ref").dispatchEvent(new KeyboardEvent("keydown", { key: " " }));
302-
});
303-
300+
await ref.focus();
301+
await page.keyboard.press(" ");
304302
await page.waitForChanges();
305303

306304
expect(await popover.isVisible()).toBe(true);
307305

308-
await page.evaluate(() => {
309-
document.getElementById("ref").dispatchEvent(new KeyboardEvent("keydown", { key: " " }));
310-
});
311-
306+
await ref.focus();
307+
await page.keyboard.press(" ");
312308
await page.waitForChanges();
313309

314310
expect(await popover.isVisible()).toBe(false);
@@ -559,6 +555,22 @@ describe("calcite-popover", () => {
559555
expect(await popover.getProperty("open")).toBe(false);
560556
});
561557

558+
it("should not toggle popovers when the ref element (component) is disabled", async () => {
559+
const page = await newE2EPage();
560+
await page.setContent(
561+
html` <calcite-popover reference-element="ref"> Hello World</calcite-popover>
562+
<calcite-button id="ref" disabled>Button</calcite-button>`,
563+
);
564+
const popover = await page.find("calcite-popover");
565+
const ref = await page.find("#ref");
566+
567+
expect(await popover.getProperty("open")).toBe(false);
568+
569+
await ref.click();
570+
await page.waitForChanges();
571+
expect(await popover.getProperty("open")).toBe(false);
572+
});
573+
562574
describe("owns a floating-ui", () => {
563575
floatingUIOwner(
564576
`<calcite-popover placement="auto" reference-element="ref">content</calcite-popover><div id="ref">referenceElement</div>`,

packages/calcite-components/src/utils/dom.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
slotChangeHasTextContent,
1919
slotChangeHasContent,
2020
isBefore,
21+
isKeyboardTriggeredClick,
2122
} from "./dom";
2223
import { guidPattern } from "./guid.spec";
2324

@@ -594,4 +595,16 @@ describe("dom", () => {
594595
expect(isBefore(div1, div2)).toBe(false);
595596
});
596597
});
598+
599+
describe("isKeyboardTriggeredClick", () => {
600+
it("should return true if click is triggered by keyboard", () => {
601+
const event = new MouseEvent("click", { detail: 0 });
602+
expect(isKeyboardTriggeredClick(event)).toBe(true);
603+
});
604+
605+
it("should return false if click is triggered by mouse/pointer", () => {
606+
const event = new MouseEvent("click", { detail: 1 });
607+
expect(isKeyboardTriggeredClick(event)).toBe(false);
608+
});
609+
});
597610
});

packages/calcite-components/src/utils/dom.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,19 @@ export function isPrimaryPointerButton(event: PointerEvent): boolean {
584584
return !!(event.isPrimary && event.button === 0);
585585
}
586586

587+
/**
588+
* This helper returns true if the mouse event was triggered by a keyboard click.
589+
*
590+
* @param {MouseEvent} event The mouse event.
591+
* @returns {boolean} The value.
592+
*/
593+
export function isKeyboardTriggeredClick(event: MouseEvent): boolean {
594+
// we assume event.detail = 0 is a keyboard click
595+
// see https://www.w3.org/TR/uievents/#event-type-click
596+
// see https://developer.mozilla.org/en-US/docs/Web/API/Element/click_event#usage_notes
597+
return event.detail === 0;
598+
}
599+
587600
export type FocusElementInGroupDestination = "first" | "last" | "next" | "previous";
588601

589602
/**

0 commit comments

Comments
 (0)