Skip to content

feat(dialog): add focusTrapDisabled property for non-modal dialogs #11362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
0439c86
feat(dialog): add focusTrapDisabled attribute to for non-modal dialogs
Elijbet Jan 22, 2025
702b4c8
rework logic
Elijbet Jan 22, 2025
61cb7b2
WIP
Elijbet Jan 22, 2025
3fd589b
doc
Elijbet Jan 22, 2025
5225520
handleFocusTrap - deactivate when not open
Elijbet Jan 23, 2025
99aa605
WIP
Elijbet Jan 23, 2025
7e910f6
willUpdate changes
Elijbet Jan 23, 2025
a82272d
cleanup test and remove check from activateFocusTrap
Elijbet Jan 23, 2025
aa64339
more control in tests
Elijbet Jan 23, 2025
66736a4
adjust visibility check
Elijbet Jan 24, 2025
f4a4a99
cleanup
Elijbet Jan 24, 2025
bdf7a80
cleanup
Elijbet Jan 24, 2025
cf808df
Merge branch 'dev' into elijbet/10685-focusTrapDisabled-attribute-for…
Elijbet Jan 28, 2025
278464f
assert on the element focus in tests and add _focusTrapDisabledOverri…
Elijbet Jan 29, 2025
0e2ab22
typeError fix
Elijbet Jan 29, 2025
826d93f
refactor and cleanup
Elijbet Jan 31, 2025
0c7a4f9
consistency with focusTrapDisabled and Override
Elijbet Jan 31, 2025
2873183
corrected doc and condition, added spec test
Elijbet Jan 31, 2025
5c11de0
focusTrapDisabledOverride spec
Elijbet Jan 31, 2025
ee12e16
cleanup
Elijbet Jan 31, 2025
3cbdaa5
DRY spec test to use beforeEach, add more test coverage
Elijbet Feb 3, 2025
b6af077
merge conflicts: enforce focusTrapDisabled and Override on components…
Elijbet Feb 4, 2025
360898b
change type on focusTrapComponent interface for useFocusTrap controll…
Elijbet Feb 4, 2025
0a27896
Merge branch 'dev' into elijbet/10685-focusTrapDisabled-attribute-for…
Elijbet Feb 4, 2025
2187f3c
set modal and focusTrapDisabled properties
Elijbet Feb 5, 2025
b0792a6
e2e set focus
Elijbet Feb 5, 2025
11f8a0b
no need to react to the prop change outside of onOpen
Elijbet Feb 5, 2025
96a6d1d
group e2e tests based on whether it's a modal or non-modal, cleanup
Elijbet Feb 5, 2025
ff3413d
safeguard e2e
Elijbet Feb 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 56 additions & 1 deletion packages/calcite-components/src/components/dialog/dialog.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @ts-strict-ignore
import { newE2EPage, E2EPage } from "@arcgis/lumina-compiler/puppeteerTesting";
import { describe, expect, it, vi } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import {
accessible,
defaults,
Expand Down Expand Up @@ -1160,4 +1160,59 @@ describe("calcite-dialog", () => {
expect(await dialog.getProperty("open")).toBe(true);
});
});

describe("focusTrap", () => {
let page: E2EPage;

beforeEach(async () => {
page = await newE2EPage();
await page.setContent(html`
<calcite-dialog width-scale="s" focus-trap-disabled open closable
><button id="insideEl">inside</button></calcite-dialog
>
<button id="outsideEl">outside</button>
`);

await skipAnimations(page);
await page.waitForChanges();
});

it("can tab out of non-modal dialog when focusTrapDisabled=true", async () => {
const dialog = await page.find("calcite-dialog >>> .container");
const action = await page.find("calcite-dialog >>> calcite-action");
const outsideEl = await page.find("#outsideEl");

expect(await dialog.isVisible()).toBe(true);

await action.callMethod("setFocus");
await page.waitForChanges();

await page.keyboard.press("Tab");
await page.waitForChanges();
await page.keyboard.press("Tab");
await page.waitForChanges();
await page.keyboard.press("Tab");
await page.waitForChanges();

const activeElementId = await page.evaluate(() => document.activeElement.id);
expect(activeElementId).toBe(await outsideEl.getProperty("id"));
});

it("cannot tab out of dialog when modal=true and focusTrapDisabled=true", async () => {
const dialog = await page.find("calcite-dialog >>> .container");
const insideEl = await page.find("#insideEl");

expect(await dialog.isVisible()).toBe(true);

await page.keyboard.press("Tab");
await page.waitForChanges();
await page.keyboard.press("Tab");
await page.waitForChanges();
await page.keyboard.press("Tab");
await page.waitForChanges();

const activeElementId = await page.evaluate(() => document.activeElement.id);
expect(activeElementId).toBe(await insideEl.getProperty("id"));
});
});
});
22 changes: 21 additions & 1 deletion packages/calcite-components/src/components/dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ export class Dialog
/** When `true`, displays a scrim blocking interaction underneath the component. */
@property({ reflect: true }) modal = false;

/** When `true` and `modal` is `false`, prevents focus trapping. */
@property({ reflect: true }) focusTrapDisabled = false;

/** When `true`, displays and positions the component. */
@property({ reflect: true })
get open(): boolean {
Expand Down Expand Up @@ -270,6 +273,11 @@ export class Dialog
updateFocusTrapElements(this);
}

/** When defined, provides a condition to disable focus trapping. When `true`, prevents focus trapping. */
focusTrapDisabledOverride(): boolean {
return !this.modal && this.focusTrapDisabled;
}

// #endregion

// #region Events
Expand Down Expand Up @@ -328,6 +336,9 @@ export class Dialog
if (changes.has("modal") && (this.hasUpdated || this.modal !== false)) {
this.updateOverflowHiddenClass();
}
if ((changes.has("modal") || changes.has("focusTrapDisabled")) && this.hasUpdated) {
this.handleFocusTrapDisabled();
}

if (
(changes.has("open") && (this.hasUpdated || this.open !== false)) ||
Expand Down Expand Up @@ -366,6 +377,15 @@ export class Dialog
// #endregion

// #region Private Methods

private handleFocusTrapDisabled(): void {
if (!this.open) {
return;
}

activateFocusTrap(this);
}

private updateAssistiveText(): void {
const { messages } = this;
this.assistiveText =
Expand All @@ -380,7 +400,7 @@ export class Dialog

onOpen(): void {
this.calciteDialogOpen.emit();
activateFocusTrap(this);
this.handleFocusTrapDisabled();
}

onBeforeClose(): void {
Expand Down
33 changes: 33 additions & 0 deletions packages/calcite-components/src/utils/focusTrapComponent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,37 @@ describe("focusTrapComponent", () => {
expect(customFocusTrapStack).toHaveLength(1);
});
});
describe("focusTrapDisabledOverride", () => {
it("should activate focus trap when focusTrapDisabledOverride returns true", () => {
const fakeComponent = {} as FocusTrapComponent;
fakeComponent.el = document.createElement("div");

connectFocusTrap(fakeComponent);

const activateSpy = vi.fn();
fakeComponent.focusTrap.activate = activateSpy;

fakeComponent.focusTrapDisabledOverride = () => false;

activateFocusTrap(fakeComponent);

expect(activateSpy).toHaveBeenCalledTimes(1);
});

it("should deactivate focus trap when focusTrapDisabledOverride returns true", () => {
const fakeComponent = {} as FocusTrapComponent;
fakeComponent.el = document.createElement("div");

connectFocusTrap(fakeComponent);

const activateSpy = vi.fn();
fakeComponent.focusTrap.activate = activateSpy;

fakeComponent.focusTrapDisabledOverride = () => true;

activateFocusTrap(fakeComponent);

expect(activateSpy).toHaveBeenCalledTimes(0);
});
});
});
5 changes: 4 additions & 1 deletion packages/calcite-components/src/utils/focusTrapComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export interface FocusTrapComponent {
/** When `true`, prevents focus trapping. */
focusTrapDisabled?: boolean;

/** When defined, provides a condition to disable focus trapping. When `true`, prevents focus trapping. */
focusTrapDisabledOverride?: () => boolean;

/** The focus trap instance. */
focusTrap: FocusTrap;

Expand Down Expand Up @@ -73,7 +76,7 @@ export function activateFocusTrap(
component: FocusTrapComponent,
options?: Parameters<_FocusTrap["activate"]>[0],
): void {
if (!component.focusTrapDisabled) {
if (component.focusTrapDisabledOverride ? !component.focusTrapDisabledOverride() : !component.focusTrapDisabled) {
component.focusTrap?.activate(options);
}
}
Expand Down