Skip to content

Commit 68f2c0e

Browse files
jcfrancojcfranco
authored
revert(focus-trap): drop focus-trap options causing inconsistent focus behavior (#6483)
**Related Issue:** #6281 #6454 ## Summary Reverts 764609d and df144dc to give us more time to look into other scenarios where focus is being blocked after updating focus-trap configuration.
1 parent c273315 commit 68f2c0e

File tree

5 files changed

+53
-165
lines changed

5 files changed

+53
-165
lines changed

‎src/components/modal/modal.e2e.ts

Lines changed: 37 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import { E2EPage, newE2EPage } from "@stencil/core/testing";
1+
import { newE2EPage } from "@stencil/core/testing";
22
import { focusable, renders, slots, hidden, t9n } from "../../tests/commonTests";
33
import { html } from "../../../support/formatting";
44
import { CSS, SLOTS, DURATIONS } from "./resources";
5-
import { isElementFocused, newProgrammaticE2EPage, skipAnimations, waitForAnimationFrame } from "../../tests/utils";
5+
import { newProgrammaticE2EPage, skipAnimations } from "../../tests/utils";
66

77
describe("calcite-modal properties", () => {
88
it("renders", () => renders("calcite-modal", { display: "flex", visible: false }));
@@ -303,131 +303,83 @@ describe("opening and closing behavior", () => {
303303

304304
describe("calcite-modal accessibility checks", () => {
305305
it("traps focus within the modal when open", async () => {
306-
const button1Id = "button1";
307-
const button2Id = "button2";
308306
const page = await newE2EPage();
309307
await page.setContent(
310-
html`<calcite-modal>
308+
`<calcite-modal>
311309
<div slot="content">
312-
<button id="${button1Id}">Focus1</button>
313-
<button id="${button2Id}">Focus2</button>
310+
<button class="btn-1">Focus1</button>
311+
<button class="btn-2">Focus1</button>
314312
</div>
315313
</calcite-modal>`
316314
);
317-
await skipAnimations(page);
318-
await page.waitForChanges();
319315
const modal = await page.find("calcite-modal");
320-
modal.setProperty("open", true);
316+
let $button1;
317+
let $button2;
318+
let $close;
319+
await page.$eval(".btn-1", (elm) => ($button1 = elm));
320+
await page.$eval(".btn-2", (elm) => ($button2 = elm));
321+
await page.$eval("calcite-modal", (elm) => {
322+
$close = elm.shadowRoot.querySelector(".close");
323+
});
324+
await modal.setProperty("open", true);
321325
await page.waitForChanges();
322-
323-
expect(await isElementFocused(page, `.${CSS.close}`, { shadowed: true })).toBe(true);
326+
expect(document.activeElement).toEqual($close);
324327
await page.keyboard.press("Tab");
325-
expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
328+
expect(document.activeElement).toEqual($button1);
326329
await page.keyboard.press("Tab");
327-
expect(await isElementFocused(page, `#${button2Id}`)).toBe(true);
328-
330+
expect(document.activeElement).toEqual($button2);
329331
await page.keyboard.press("Tab");
330-
expect(await isElementFocused(page, `.${CSS.close}`, { shadowed: true })).toBe(true);
332+
expect(document.activeElement).toEqual($close);
331333
await page.keyboard.down("Shift");
332334
await page.keyboard.press("Tab");
333-
expect(await isElementFocused(page, `#${button2Id}`)).toBe(true);
334-
335+
expect(document.activeElement).toEqual($button2);
335336
await page.keyboard.press("Tab");
336-
expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
337+
expect(document.activeElement).toEqual($button1);
337338
});
338339

339340
it("restores focus to previously focused element when closed", async () => {
340-
const initiallyFocusedId = "initially-focused";
341-
const initiallyFocusedIdSelector = `#${initiallyFocusedId}`;
342341
const page = await newE2EPage();
343-
await page.setContent(
344-
html`
345-
<button id="${initiallyFocusedId}">Focus</button>
346-
<calcite-modal></calcite-modal>
347-
`
348-
);
349-
await skipAnimations(page);
342+
await page.setContent(`<button>Focus</button><calcite-modal></calcite-modal>`);
350343
const modal = await page.find("calcite-modal");
351-
await page.$eval(initiallyFocusedIdSelector, (button: HTMLButtonElement) => {
352-
button.focus();
344+
let $button;
345+
await page.$eval("button", (elm: any) => {
346+
$button = elm;
347+
$button.focus();
353348
});
354349
await modal.setProperty("open", true);
355350
await page.waitForChanges();
356351
await modal.setProperty("open", false);
357352
await page.waitForChanges();
358-
expect(await isElementFocused(page, initiallyFocusedIdSelector)).toBe(true);
353+
expect(document.activeElement).toEqual($button);
359354
});
360355

361356
it("traps focus within the modal when open and disabled close button", async () => {
362-
const button1Id = "button1";
363-
const button2Id = "button2";
364357
const page = await newE2EPage();
365358
await page.setContent(
366-
html`<calcite-modal close-button-disabled>
359+
`<calcite-modal close-button-disabled>
367360
<div slot="content">
368-
<button id="${button1Id}">Focus1</button>
369-
<button id="${button2Id}">Focus2</button>
361+
<button class="btn-1">Focus1</button>
362+
<button class="btn-2">Focus1</button>
370363
</div>
371364
</calcite-modal>`
372365
);
373-
await skipAnimations(page);
374366
const modal = await page.find("calcite-modal");
367+
let $button1;
368+
let $button2;
369+
await page.$eval(".btn-1", (elm) => ($button1 = elm));
370+
await page.$eval(".btn-2", (elm) => ($button2 = elm));
375371

376372
await modal.setProperty("open", true);
377373
await page.waitForChanges();
378-
expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
379-
380374
await page.keyboard.press("Tab");
381-
expect(await isElementFocused(page, `#${button2Id}`)).toBe(true);
382-
375+
expect(document.activeElement).toEqual($button1);
383376
await page.keyboard.press("Tab");
384-
expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
377+
expect(document.activeElement).toEqual($button2);
385378
await page.keyboard.down("Shift");
386379
await page.keyboard.press("Tab");
387-
expect(await isElementFocused(page, `#${button2Id}`)).toBe(true);
380+
expect(document.activeElement).toEqual($button2);
388381
await page.keyboard.press("Tab");
389-
expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
390-
});
391-
392-
it("traps focus within the modal when user clicks inside the modal", async () => {
393-
const page = await newE2EPage();
394-
await page.setContent(
395-
html`
396-
<calcite-modal aria-labelledby="modal-title" close-button-disabled open>
397-
<div slot="header" id="modal-title">Modal title</div>
398-
<div slot="content">
399-
<p>Modal content.</p>
400-
<calcite-button icon-start="plus" id="plus" round></calcite-button>
401-
</div>
402-
<calcite-button slot="back" color="neutral" width="full">Back</calcite-button>
403-
<calcite-button slot="secondary" width="full" appearance="outline">Cancel</calcite-button>
404-
<calcite-button slot="primary" width="full">Save</calcite-button>
405-
</calcite-modal>
406-
`
407-
);
408-
await skipAnimations(page);
409-
410-
const contentEl = await page.find(`div[slot="content"]`);
411-
const backButtonEl = await page.find(`calcite-button[slot="back"]`);
412-
413-
await page.keyboard.press("Tab");
414-
expect(await isElementFocused(page, `#plus`)).toBe(true);
415-
416-
await page.keyboard.press("Tab");
417-
expect(await isElementFocused(page, `[slot="back"]`)).toBe(true);
418-
419-
await page.keyboard.press("Tab");
420-
expect(await isElementFocused(page, `[slot="secondary"]`)).toBe(true);
421-
422-
await page.keyboard.press("Tab");
423-
expect(await isElementFocused(page, `[slot="primary"]`)).toBe(true);
424-
425-
await contentEl.click();
426-
await page.waitForChanges();
427-
await backButtonEl.click();
428-
await page.waitForChanges();
429-
430-
expect(await isElementFocused(page, `[slot="back"]`)).toBe(true);
382+
expect(document.activeElement).toEqual($button1);
431383
});
432384

433385
describe("setFocus", () => {

‎src/components/popover/popover.e2e.ts

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -734,48 +734,4 @@ describe("calcite-popover", () => {
734734
shadowFocusTargetSelector: `.${CSS.closeButton}`
735735
}));
736736
});
737-
738-
it("should focus input element in the page with popover when user click", async () => {
739-
// refer to https://github.com/Esri/calcite-components/issues/5993 for context
740-
const page = await newE2EPage();
741-
await page.setContent(html` <calcite-shell content-behind>
742-
<calcite-shell-panel slot="panel-end">
743-
<calcite-label>
744-
value
745-
<calcite-input-number value="0" min="0" max="10" id="shell-input"></calcite-input-number>
746-
</calcite-label>
747-
<calcite-button id="button">open popover</calcite-button>
748-
</calcite-shell-panel>
749-
</calcite-shell>
750-
751-
<calcite-popover reference-element="button">
752-
<calcite-panel heading="popover panel header" closable="true" style="height: 400px">
753-
<calcite-input-number value="5" min="0" max="10" id="popover-input"></calcite-input-number>
754-
</calcite-panel>
755-
</calcite-popover>`);
756-
757-
const popover = await page.find("calcite-popover");
758-
expect(await popover.getProperty("open")).toBe(false);
759-
760-
const referenceElement = await page.find("calcite-button#button");
761-
await referenceElement.click();
762-
await page.waitForChanges();
763-
expect(await popover.getProperty("open")).toBe(true);
764-
765-
const inputElInPopover = await page.find("calcite-input-number#popover-input");
766-
await inputElInPopover.click();
767-
expect(await page.evaluate(() => document.activeElement.id)).toBe("popover-input");
768-
769-
await page.keyboard.press("Backspace");
770-
await page.keyboard.type("12345");
771-
expect(await inputElInPopover.getProperty("value")).toBe("12345");
772-
773-
const inputElInShell = await page.find("calcite-input-number#shell-input");
774-
await inputElInShell.click();
775-
expect(await page.evaluate(() => document.activeElement.id)).toBe("shell-input");
776-
777-
await page.keyboard.press("Backspace");
778-
await page.keyboard.type("12345");
779-
expect(await inputElInShell.getProperty("value")).toBe("12345");
780-
});
781737
});

‎src/tests/utils.ts

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -271,35 +271,3 @@ export async function skipAnimations(page: E2EPage): Promise<void> {
271271
content: `:root { --calcite-duration-factor: 0; }`
272272
});
273273
}
274-
275-
interface MatchesFocusedElementOptions {
276-
/**
277-
* Set this to true when the focused element is expected to reside in the shadow DOM
278-
*/
279-
shadowed: boolean;
280-
}
281-
282-
/**
283-
* This util helps determine if a selector matches the currently focused element.
284-
*
285-
* @param page – the E2E page
286-
* @param selector – selector of element to match
287-
* @param options - options to customize the utility behavior
288-
*/
289-
export async function isElementFocused(
290-
page: E2EPage,
291-
selector: string,
292-
options?: MatchesFocusedElementOptions
293-
): Promise<boolean> {
294-
const shadowed = options?.shadowed;
295-
296-
return page.evaluate(
297-
(selector: string, shadowed: boolean): boolean => {
298-
const targetDoc = shadowed ? document.activeElement?.shadowRoot : document;
299-
300-
return !!targetDoc?.activeElement?.matches(selector);
301-
},
302-
selector,
303-
shadowed
304-
);
305-
}

‎src/utils/focusTrapComponent.spec.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ describe("focusTrapComponent", () => {
1212

1313
connectFocusTrap(fakeComponent);
1414

15+
expect(fakeComponent.focusTrapEl.tabIndex).toBe(-1);
1516
expect(fakeComponent.focusTrap).toBeDefined();
1617
expect(fakeComponent.focusTrap.active).toBe(false);
1718

@@ -33,4 +34,14 @@ describe("focusTrapComponent", () => {
3334
deactivateFocusTrap(fakeComponent);
3435
expect(deactivateSpy).toHaveBeenCalledTimes(1);
3536
});
37+
38+
it("focusTrapEl with tabIndex`", () => {
39+
const fakeComponent = {} as any;
40+
fakeComponent.focusTrapEl = document.createElement("div");
41+
fakeComponent.focusTrapEl.tabIndex = 0;
42+
43+
connectFocusTrap(fakeComponent);
44+
expect(fakeComponent.focusTrapEl.tabIndex).toBe(0);
45+
expect(fakeComponent.focusTrap).toBeDefined();
46+
});
3647
});

‎src/utils/focusTrapComponent.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,12 @@ export function connectFocusTrap(component: FocusTrapComponent): void {
4242
return;
4343
}
4444

45+
if (focusTrapEl.tabIndex == null) {
46+
focusTrapEl.tabIndex = -1;
47+
}
48+
4549
const focusTrapOptions: FocusTrapOptions = {
46-
allowOutsideClick: true,
47-
clickOutsideDeactivates: (event: MouseEvent | TouchEvent) => {
48-
return !event.composedPath().find((el) => (el as HTMLElement) === focusTrapEl);
49-
},
50+
clickOutsideDeactivates: true,
5051
document: focusTrapEl.ownerDocument,
5152
escapeDeactivates: false,
5253
fallbackFocus: focusTrapEl,

0 commit comments

Comments
 (0)