Skip to content

Commit c5f55e9

Browse files
committed
fix(modal): no longer loses focus trap after clicking inside the component
* restores changes reverted by #6483 * bumps focus-trap dep
1 parent 858a01d commit c5f55e9

File tree

6 files changed

+105
-32
lines changed

6 files changed

+105
-32
lines changed

package-lock.json

Lines changed: 15 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
"@stencil/core": "2.20.0",
7575
"@types/color": "3.0.3",
7676
"color": "4.2.3",
77-
"focus-trap": "7.2.0",
77+
"focus-trap": "7.3.1",
7878
"form-request-submit-polyfill": "2.0.0",
7979
"lodash-es": "4.17.21",
8080
"sortablejs": "1.15.0"

src/components/modal/modal.e2e.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,47 @@ describe("calcite-modal accessibility checks", () => {
389389
expect(await isElementFocused(page, `#${button1Id}`)).toBe(true);
390390
});
391391

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);
431+
});
432+
392433
describe("setFocus", () => {
393434
const createModalHTML = (contentHTML?: string, attrs?: string) =>
394435
`<calcite-modal open ${attrs}>${contentHTML}</calcite-modal>`;

src/components/popover/popover.e2e.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,4 +734,48 @@ 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+
});
737781
});

src/utils/focusTrapComponent.spec.ts

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

1313
connectFocusTrap(fakeComponent);
1414

15-
expect(fakeComponent.focusTrapEl.tabIndex).toBe(-1);
1615
expect(fakeComponent.focusTrap).toBeDefined();
1716
expect(fakeComponent.focusTrap.active).toBe(false);
1817

@@ -34,14 +33,4 @@ describe("focusTrapComponent", () => {
3433
deactivateFocusTrap(fakeComponent);
3534
expect(deactivateSpy).toHaveBeenCalledTimes(1);
3635
});
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-
});
4736
});

src/utils/focusTrapComponent.ts

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

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

0 commit comments

Comments
 (0)