Skip to content
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

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

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
33 changes: 33 additions & 0 deletions packages/calcite-components/src/components/dialog/dialog.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1160,4 +1160,37 @@ describe("calcite-dialog", () => {
expect(await dialog.getProperty("open")).toBe(true);
});
});

describe("focusTrap", () => {
it("closes when Escape key is pressed and focusTrapDisabled=true", async () => {
const page = await newE2EPage();
await page.setContent(html` <calcite-dialog focus-trap-disabled open closable></calcite-dialog> `);
await skipAnimations(page);
await page.waitForChanges();

const dialog = await page.find("calcite-dialog");
const button = await page.find("calcite-dialog >>> calcite-action");

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

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

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

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

dialog.setAttribute("modal", "true");
button.callMethod("setFocus");

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

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

expect(await dialog.isVisible()).toBe(false);
});
});
});
20 changes: 19 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`, prevents focus trapping. */
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
@property({ reflect: true }) focusTrapDisabled = false;

/** When `true`, displays and positions the component. */
@property({ reflect: true })
get open(): boolean {
Expand Down Expand Up @@ -366,6 +369,19 @@ export class Dialog
// #endregion

// #region Private Methods

private handleFocusTrapDisabled(focusTrapDisabled: boolean): void {
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
if (!this.open) {
jcfranco marked this conversation as resolved.
Show resolved Hide resolved
return;
}

if (focusTrapDisabled) {
deactivateFocusTrap(this);
} else {
activateFocusTrap(this);
}
}

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

onOpen(): void {
this.calciteDialogOpen.emit();
activateFocusTrap(this);
if (this.modal || (this.modal === false && this.focusTrapDisabled === false)) {
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
activateFocusTrap(this);
}
}

onBeforeClose(): void {
Expand Down
Loading