diff --git a/packages/calcite-components/src/components.d.ts b/packages/calcite-components/src/components.d.ts index 8751465866a..45c00f705de 100644 --- a/packages/calcite-components/src/components.d.ts +++ b/packages/calcite-components/src/components.d.ts @@ -393,10 +393,6 @@ export namespace Components { * When `true`, the component is expanded. */ "expanded": boolean; - /** - * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. - */ - "flipPlacements": FlipPlacement[]; /** * Accessible name for the component. */ @@ -406,10 +402,18 @@ export namespace Components { * @deprecated Use the `layout` property on the component's parent instead. */ "layout": Extract<"horizontal" | "vertical" | "grid", Layout>; + /** + * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. + */ + "menuFlipPlacements": FlipPlacement[]; /** * When `true`, the `calcite-action-menu` is open. */ "menuOpen": boolean; + /** + * Determines where the action menu will be positioned. + */ + "menuPlacement": LogicalPlacement; /** * Use this property to override individual strings used by the component. */ @@ -422,10 +426,6 @@ export namespace Components { * Determines the type of positioning to use for the overlaid content. Using `"absolute"` will work for most cases. The component will be positioned inside of overflowing parent containers and will affect the container's layout. `"fixed"` should be used to escape an overflowing parent container, or when the reference element's `position` CSS property is `"fixed"`. */ "overlayPositioning": OverlayPositioning; - /** - * Determines where the action menu will be positioned. - */ - "placement": LogicalPlacement; /** * Specifies the size of the `calcite-action-menu`. */ @@ -632,10 +632,6 @@ export namespace Components { * When `true`, displays a drag handle in the header. */ "dragHandle": boolean; - /** - * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. - */ - "flipPlacements": FlipPlacement[]; /** * The component header text. */ @@ -660,6 +656,14 @@ export namespace Components { * When `true`, a busy indicator is displayed. */ "loading": boolean; + /** + * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. + */ + "menuFlipPlacements": FlipPlacement[]; + /** + * Determines where the action menu will be positioned. + */ + "menuPlacement": LogicalPlacement; /** * Use this property to override individual strings used by the component. */ @@ -676,10 +680,6 @@ export namespace Components { * Determines the type of positioning to use for the overlaid content. Using `"absolute"` will work for most cases. The component will be positioned inside of overflowing parent containers and will affect the container's layout. `"fixed"` should be used to escape an overflowing parent container, or when the reference element's `position` CSS property is `"fixed"`. */ "overlayPositioning": OverlayPositioning; - /** - * Determines where the action menu will be positioned. - */ - "placement": LogicalPlacement; /** * Sets focus on the component's first tabbable element. */ @@ -3880,10 +3880,6 @@ export namespace Components { * When `true`, interaction is prevented and the component is displayed with lower opacity. */ "disabled": boolean; - /** - * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. - */ - "flipPlacements": FlipPlacement[]; /** * The component header text. */ @@ -3896,10 +3892,18 @@ export namespace Components { * When `true`, a busy indicator is displayed. */ "loading": boolean; + /** + * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. + */ + "menuFlipPlacements": FlipPlacement[]; /** * When `true`, the action menu items in the `header-menu-actions` slot are open. */ "menuOpen": boolean; + /** + * Determines where the action menu will be positioned. + */ + "menuPlacement": LogicalPlacement; /** * Use this property to override individual strings used by the component. */ @@ -3912,10 +3916,6 @@ export namespace Components { * Determines the type of positioning to use for the overlaid content. Using `"absolute"` will work for most cases. The component will be positioned inside of overflowing parent containers and will affect the container's layout. `"fixed"` should be used to escape an overflowing parent container, or when the reference element's `position` CSS property is `"fixed"`. */ "overlayPositioning": OverlayPositioning; - /** - * Determines where the action menu will be positioned. - */ - "placement": LogicalPlacement; /** * Specifies the size of the component. */ @@ -8349,10 +8349,6 @@ declare namespace LocalJSX { * When `true`, the component is expanded. */ "expanded"?: boolean; - /** - * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. - */ - "flipPlacements"?: FlipPlacement[]; /** * Accessible name for the component. */ @@ -8362,10 +8358,18 @@ declare namespace LocalJSX { * @deprecated Use the `layout` property on the component's parent instead. */ "layout"?: Extract<"horizontal" | "vertical" | "grid", Layout>; + /** + * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. + */ + "menuFlipPlacements"?: FlipPlacement[]; /** * When `true`, the `calcite-action-menu` is open. */ "menuOpen"?: boolean; + /** + * Determines where the action menu will be positioned. + */ + "menuPlacement"?: LogicalPlacement; /** * Use this property to override individual strings used by the component. */ @@ -8378,10 +8382,6 @@ declare namespace LocalJSX { * Determines the type of positioning to use for the overlaid content. Using `"absolute"` will work for most cases. The component will be positioned inside of overflowing parent containers and will affect the container's layout. `"fixed"` should be used to escape an overflowing parent container, or when the reference element's `position` CSS property is `"fixed"`. */ "overlayPositioning"?: OverlayPositioning; - /** - * Determines where the action menu will be positioned. - */ - "placement"?: LogicalPlacement; /** * Specifies the size of the `calcite-action-menu`. */ @@ -8595,10 +8595,6 @@ declare namespace LocalJSX { * When `true`, displays a drag handle in the header. */ "dragHandle"?: boolean; - /** - * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. - */ - "flipPlacements"?: FlipPlacement[]; /** * The component header text. */ @@ -8623,6 +8619,14 @@ declare namespace LocalJSX { * When `true`, a busy indicator is displayed. */ "loading"?: boolean; + /** + * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. + */ + "menuFlipPlacements"?: FlipPlacement[]; + /** + * Determines where the action menu will be positioned. + */ + "menuPlacement"?: LogicalPlacement; /** * Use this property to override individual strings used by the component. */ @@ -8660,10 +8664,6 @@ declare namespace LocalJSX { * Determines the type of positioning to use for the overlaid content. Using `"absolute"` will work for most cases. The component will be positioned inside of overflowing parent containers and will affect the container's layout. `"fixed"` should be used to escape an overflowing parent container, or when the reference element's `position` CSS property is `"fixed"`. */ "overlayPositioning"?: OverlayPositioning; - /** - * Determines where the action menu will be positioned. - */ - "placement"?: LogicalPlacement; /** * Displays a status-related indicator icon. * @deprecated Use `icon-start` instead. @@ -12032,10 +12032,6 @@ declare namespace LocalJSX { * When `true`, interaction is prevented and the component is displayed with lower opacity. */ "disabled"?: boolean; - /** - * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. - */ - "flipPlacements"?: FlipPlacement[]; /** * The component header text. */ @@ -12048,10 +12044,18 @@ declare namespace LocalJSX { * When `true`, a busy indicator is displayed. */ "loading"?: boolean; + /** + * Specifies the component's fallback menu `placement` when it's initial or specified `placement` has insufficient space available. + */ + "menuFlipPlacements"?: FlipPlacement[]; /** * When `true`, the action menu items in the `header-menu-actions` slot are open. */ "menuOpen"?: boolean; + /** + * Determines where the action menu will be positioned. + */ + "menuPlacement"?: LogicalPlacement; /** * Use this property to override individual strings used by the component. */ @@ -12076,10 +12080,6 @@ declare namespace LocalJSX { * Determines the type of positioning to use for the overlaid content. Using `"absolute"` will work for most cases. The component will be positioned inside of overflowing parent containers and will affect the container's layout. `"fixed"` should be used to escape an overflowing parent container, or when the reference element's `position` CSS property is `"fixed"`. */ "overlayPositioning"?: OverlayPositioning; - /** - * Determines where the action menu will be positioned. - */ - "placement"?: LogicalPlacement; /** * Specifies the size of the component. */ diff --git a/packages/calcite-components/src/components/dialog/dialog.e2e.ts b/packages/calcite-components/src/components/dialog/dialog.e2e.ts index 1dc66aeba94..456eda76160 100644 --- a/packages/calcite-components/src/components/dialog/dialog.e2e.ts +++ b/packages/calcite-components/src/components/dialog/dialog.e2e.ts @@ -430,6 +430,7 @@ describe("calcite-dialog", () => { await page.setContent(` `); + await skipAnimations(page); const dialog = await page.find("calcite-dialog"); await page.$eval( @@ -444,7 +445,7 @@ describe("calcite-dialog", () => { await page.keyboard.press("Escape"); await page.waitForChanges(); - await page.waitForChanges(); + expect(mockCallBack).toHaveBeenCalledTimes(2); expect(await page.find(`calcite-dialog >>> .${CSS.containerOpen}`)).toBeNull(); }); @@ -691,12 +692,14 @@ describe("calcite-dialog", () => { const page = await newE2EPage(); await page.setContent(``); await skipAnimations(page); + const openedEvent = page.waitForEvent("calciteDialogOpen"); const dialog = await page.find("calcite-dialog"); const container = await page.find(`calcite-dialog >>> .${CSS.container}`); dialog.setProperty("open", true); await page.waitForChanges(); + await openedEvent; expect(await dialog.isVisible()).toBe(true); await page.keyboard.press("Escape"); @@ -711,22 +714,21 @@ describe("calcite-dialog", () => { it("closes when Escape key is pressed and dialog is open on page load", async () => { const page = await newE2EPage(); - await page.setContent(``); + await page.setContent(``); + const openedEvent = page.waitForEvent("calciteDialogOpen"); const dialog = await page.find("calcite-dialog"); await page.waitForChanges(); expect(dialog).toHaveAttribute("open"); - expect(dialog).toHaveAttribute("open"); + await openedEvent; await page.keyboard.press("Escape"); await page.waitForChanges(); expect(dialog).not.toHaveAttribute("open"); - expect(dialog).not.toHaveAttribute("open"); dialog.setProperty("open", true); await page.waitForChanges(); expect(dialog).toHaveAttribute("open"); - expect(dialog).toHaveAttribute("open"); }); it("closes and allows re-opening when Close button is clicked", async () => { @@ -735,9 +737,12 @@ describe("calcite-dialog", () => { await skipAnimations(page); const dialog = await page.find("calcite-dialog"); const container = await page.find(`calcite-dialog >>> .${CSS.container}`); + const openedEvent = page.waitForEvent("calciteDialogOpen"); + await page.waitForChanges(); dialog.setProperty("open", true); await page.waitForChanges(); + await openedEvent; expect(await container.isVisible()).toBe(true); const closeButton = await page.find(`calcite-dialog >>> calcite-panel >>> #${PanelIDS.close}`); diff --git a/packages/calcite-components/src/components/dialog/dialog.tsx b/packages/calcite-components/src/components/dialog/dialog.tsx index 71f7e4c388c..421e18dfe26 100644 --- a/packages/calcite-components/src/components/dialog/dialog.tsx +++ b/packages/calcite-components/src/components/dialog/dialog.tsx @@ -5,7 +5,6 @@ import { EventEmitter, h, Host, - Listen, Method, Prop, State, @@ -231,7 +230,14 @@ export class Dialog this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); connectLocalized(this); connectMessages(this); - connectFocusTrap(this); + connectFocusTrap(this, { + focusTrapOptions: { + // Scrim has it's own close handler, allow it to take over. + clickOutsideDeactivates: false, + escapeDeactivates: this.escapeDeactivates, + onDeactivate: this.focusTrapDeactivates, + }, + }); this.setupInteractions(); } @@ -379,19 +385,17 @@ export class Dialog this.handleMutationObserver(), ); - //-------------------------------------------------------------------------- - // - // Event Listeners - // - //-------------------------------------------------------------------------- - - @Listen("keydown", { target: "window" }) - handleEscape(event: KeyboardEvent): void { - if (this.open && !this.escapeDisabled && event.key === "Escape" && !event.defaultPrevented) { - this.open = false; - event.preventDefault(); + private escapeDeactivates = (event: KeyboardEvent): boolean => { + if (event.defaultPrevented || this.escapeDisabled) { + return false; } - } + event.preventDefault(); + return true; + }; + + private focusTrapDeactivates = (): void => { + this.open = false; + }; //-------------------------------------------------------------------------- // diff --git a/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx b/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx index 3abe5824478..101a129d177 100644 --- a/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx +++ b/packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx @@ -397,6 +397,10 @@ export class InputDatePicker this.commitValue(); }; + private focusTrapDeactivates = (): void => { + this.open = false; + }; + //-------------------------------------------------------------------------- // // Events @@ -963,10 +967,6 @@ export class InputDatePicker this.open = true; this.focusOnOpen = true; event.preventDefault(); - } else if (key === "Escape") { - this.open = false; - event.preventDefault(); - this.restoreInputFocus(); } }; @@ -998,8 +998,12 @@ export class InputDatePicker connectFocusTrap(this, { focusTrapEl: el, focusTrapOptions: { + allowOutsideClick: true, + // Allow outside click and let the popover manager take care of closing the popover. + clickOutsideDeactivates: false, initialFocus: false, setReturnFocus: false, + onDeactivate: this.focusTrapDeactivates, }, }); }; diff --git a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx index 84260c9a7f8..d4372d5178e 100644 --- a/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx +++ b/packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx @@ -11,7 +11,6 @@ import { VNode, Watch, } from "@stencil/core"; -import { FocusTrap } from "focus-trap"; import dayjs from "dayjs/esm"; import customParseFormat from "dayjs/esm/plugin/customParseFormat"; import localeData from "dayjs/esm/plugin/localeData"; @@ -48,12 +47,6 @@ import { numberStringFormatter, SupportedLocale, } from "../../utils/locale"; -import { - activateFocusTrap, - connectFocusTrap, - deactivateFocusTrap, - FocusTrapComponent, -} from "../../utils/focusTrapComponent"; import { formatTimePart, formatTimeString, @@ -159,7 +152,6 @@ interface DayjsTimeParts { export class InputTimePicker implements FloatingUIComponent, - FocusTrapComponent, FormComponent, InteractiveComponent, LabelableComponent, @@ -195,15 +187,6 @@ export class InputTimePicker */ @Prop({ reflect: true }) focusTrapDisabled = false; - @Watch("focusTrapDisabled") - handleFocusTrapDisabled(focusTrapDisabled: boolean): void { - if (!this.open) { - return; - } - - focusTrapDisabled ? deactivateFocusTrap(this) : activateFocusTrap(this); - } - /** * The `id` of the form that will be associated with the component. * @@ -375,10 +358,6 @@ export class InputTimePicker private calciteTimePickerEl: HTMLCalciteTimePickerElement; - private focusOnOpen = false; - - focusTrap: FocusTrap; - private localeConfig: ILocale; /** whether the value of the input was changed as a result of user typing or not */ @@ -552,15 +531,6 @@ export class InputTimePicker private popoverOpenHandler = (event: CustomEvent): void => { event.stopPropagation(); this.calciteInputTimePickerOpen.emit(); - - activateFocusTrap(this, { - onActivate: () => { - if (this.focusOnOpen) { - this.calciteTimePickerEl.setFocus(); - this.focusOnOpen = false; - } - }, - }); }; private popoverBeforeCloseHandler = (event: CustomEvent): void => { @@ -571,13 +541,6 @@ export class InputTimePicker private popoverCloseHandler = (event: CustomEvent): void => { event.stopPropagation(); this.calciteInputTimePickerClose.emit(); - - deactivateFocusTrap(this, { - onDeactivate: () => { - this.calciteInputEl.setFocus(); - this.focusOnOpen = false; - }, - }); this.open = false; }; @@ -727,12 +690,7 @@ export class InputTimePicker } } else if (key === "ArrowDown") { this.open = true; - this.focusOnOpen = true; - event.preventDefault(); - } else if (key === "Escape" && this.open) { - this.open = false; event.preventDefault(); - this.calciteInputEl.setFocus(); } }; @@ -859,13 +817,6 @@ export class InputTimePicker private setCalciteTimePickerEl = (el: HTMLCalciteTimePickerElement): void => { this.calciteTimePickerEl = el; - connectFocusTrap(this, { - focusTrapEl: el, - focusTrapOptions: { - initialFocus: false, - setReturnFocus: false, - }, - }); }; private setInputValue = (newInputValue: string): void => { @@ -983,7 +934,6 @@ export class InputTimePicker disconnectLabel(this); disconnectForm(this); disconnectLocalized(this); - deactivateFocusTrap(this); disconnectMessages(this); } @@ -1025,7 +975,7 @@ export class InputTimePicker { @@ -149,7 +149,7 @@ describe("calcite-modal", () => { const mockCallBack = jest.fn(); await page.exposeFunction("beforeClose", mockCallBack); await page.setContent(` - + `); const modal = await page.find("calcite-modal"); await page.$eval( @@ -159,13 +159,15 @@ describe("calcite-modal", () => { window as GlobalTestProps<{ beforeClose: HTMLCalciteModalElement["beforeClose"] }> ).beforeClose), ); + await skipAnimations(page); await page.waitForChanges(); + modal.setProperty("open", true); await page.waitForChanges(); expect(await modal.getProperty("opened")).toBe(true); + await page.keyboard.press("Escape"); await page.waitForChanges(); - await page.waitForChanges(); expect(mockCallBack).toHaveBeenCalledTimes(1); expect(await modal.getProperty("opened")).toBe(false); }); @@ -391,36 +393,45 @@ describe("calcite-modal", () => { it("closes and allows re-opening when Escape key is pressed", async () => { const page = await newE2EPage(); - await page.setContent(``); + await page.setContent(``); await skipAnimations(page); const modal = await page.find("calcite-modal"); - await modal.setProperty("open", true); + const openedEvent = page.waitForEvent("calciteModalOpen"); + + modal.setProperty("open", true); await page.waitForChanges(); expect(await modal.isVisible()).toBe(true); + await page.keyboard.press("Escape"); await page.waitForChanges(); expect(await modal.isVisible()).toBe(false); expect(await modal.getProperty("open")).toBe(false); - await modal.setProperty("open", true); + + modal.setProperty("open", true); await page.waitForChanges(); + await openedEvent; expect(await modal.isVisible()).toBe(true); }); it("closes when Escape key is pressed and modal is open on page load", async () => { const page = await newE2EPage(); - await page.setContent(``); + await page.setContent(``); + await skipAnimations(page); + const openedEvent = page.waitForEvent("calciteModalOpen"); + const modal = await page.find("calcite-modal"); await page.waitForChanges(); + await openedEvent; expect(modal).toHaveAttribute("open"); - expect(modal).toHaveAttribute("open"); + await page.keyboard.press("Escape"); await page.waitForChanges(); + await waitForAnimationFrame(); expect(modal).not.toHaveAttribute("open"); - expect(modal).not.toHaveAttribute("open"); + await modal.setProperty("open", true); await page.waitForChanges(); expect(modal).toHaveAttribute("open"); - expect(modal).toHaveAttribute("open"); }); it("closes and allows re-opening when Close button is clicked", async () => { diff --git a/packages/calcite-components/src/components/modal/modal.tsx b/packages/calcite-components/src/components/modal/modal.tsx index cb3bd1d009c..aa8a2c4ebd6 100644 --- a/packages/calcite-components/src/components/modal/modal.tsx +++ b/packages/calcite-components/src/components/modal/modal.tsx @@ -5,7 +5,6 @@ import { EventEmitter, h, Host, - Listen, Method, Prop, State, @@ -198,7 +197,14 @@ export class Modal connectConditionalSlotComponent(this); connectLocalized(this); connectMessages(this); - connectFocusTrap(this); + connectFocusTrap(this, { + focusTrapOptions: { + // Scrim has it's own close handler, allow it to take over. + clickOutsideDeactivates: false, + escapeDeactivates: this.escapeDeactivates, + onDeactivate: this.focusTrapDeactivates, + }, + }); } disconnectedCallback(): void { @@ -395,20 +401,6 @@ export class Modal @State() defaultMessages: ModalMessages; - //-------------------------------------------------------------------------- - // - // Event Listeners - // - //-------------------------------------------------------------------------- - - @Listen("keydown", { target: "window" }) - handleEscape(event: KeyboardEvent): void { - if (this.open && !this.escapeDisabled && event.key === "Escape" && !event.defaultPrevented) { - this.open = false; - event.preventDefault(); - } - } - //-------------------------------------------------------------------------- // // Events @@ -603,4 +595,16 @@ export class Modal private contentBottomSlotChangeHandler = (event: Event): void => { this.hasContentBottom = slotChangeHasAssignedElement(event); }; + + private escapeDeactivates = (event: KeyboardEvent) => { + if (event.defaultPrevented || this.escapeDisabled) { + return false; + } + event.preventDefault(); + return true; + }; + + private focusTrapDeactivates = () => { + this.open = false; + }; } diff --git a/packages/calcite-components/src/components/popover/popover.e2e.ts b/packages/calcite-components/src/components/popover/popover.e2e.ts index 29e8a2b11c2..2975e571b29 100644 --- a/packages/calcite-components/src/components/popover/popover.e2e.ts +++ b/packages/calcite-components/src/components/popover/popover.e2e.ts @@ -11,6 +11,7 @@ import { t9n, themed, } from "../../tests/commonTests"; +import { skipAnimations } from "../../tests/utils"; import { FloatingCSS } from "../../utils/floating-ui"; import { CSS } from "./resources"; @@ -524,35 +525,26 @@ describe("calcite-popover", () => { Hello World
Button
`, ); + await skipAnimations(page); const popover = await page.find("calcite-popover"); - expect(await popover.getProperty("open")).toBe(true); const ref = await page.find("#ref"); - await ref.click(); - await page.waitForChanges(); - expect(await popover.getProperty("open")).toBe(true); + await page.waitForChanges(); const outsideNode = await page.find("#outsideNode"); - await outsideNode.click(); - await page.waitForChanges(); - expect(await popover.getProperty("open")).toBe(true); popover.setProperty("triggerDisabled", false); - await page.waitForChanges(); - await ref.click(); - await page.waitForChanges(); - expect(await popover.getProperty("open")).toBe(false); }); @@ -631,8 +623,7 @@ describe("calcite-popover", () => {
referenceElement
`, ); - - await page.waitForChanges(); + await skipAnimations(page); const popover = await page.find(`calcite-popover`); const ref = await page.find("#ref"); diff --git a/packages/calcite-components/src/components/popover/popover.tsx b/packages/calcite-components/src/components/popover/popover.tsx index 70c614380a8..6a9ccf4fde4 100644 --- a/packages/calcite-components/src/components/popover/popover.tsx +++ b/packages/calcite-components/src/components/popover/popover.tsx @@ -293,7 +293,14 @@ export class Popover this.setFilteredPlacements(); connectLocalized(this); connectMessages(this); - connectFocusTrap(this); + connectFocusTrap(this, { + focusTrapEl: this.el, + focusTrapOptions: { + allowOutsideClick: true, + clickOutsideDeactivates: this.clickOutsideDeactivates, + onDeactivate: this.focusTrapDeactivates, + }, + }); // we set up the ref element in the next frame to ensure PopoverManager // event handlers are invoked after connect (mainly for `components` output target) @@ -522,6 +529,22 @@ export class Popover this.reposition(true); }; + private clickOutsideDeactivates = (event: MouseEvent): boolean => { + const path = event.composedPath(); + const isReferenceElementInPath = + this.effectiveReferenceElement instanceof EventTarget && + path.includes(this.effectiveReferenceElement); + + const outsideClick = !path.includes(this.el); + const shouldCloseOnOutsideClick = this.autoClose && outsideClick; + + return shouldCloseOnOutsideClick && (this.triggerDisabled || isReferenceElementInPath); + }; + + private focusTrapDeactivates = (): void => { + this.open = false; + }; + // -------------------------------------------------------------------------- // // Render Methods diff --git a/packages/calcite-components/src/components/sheet/sheet.e2e.ts b/packages/calcite-components/src/components/sheet/sheet.e2e.ts index 2c9408b547f..56b6f4421b3 100644 --- a/packages/calcite-components/src/components/sheet/sheet.e2e.ts +++ b/packages/calcite-components/src/components/sheet/sheet.e2e.ts @@ -186,11 +186,13 @@ describe("calcite-sheet properties", () => { window as GlobalTestProps<{ beforeClose: HTMLCalciteSheetElement["beforeClose"] }> ).beforeClose), ); - await page.waitForChanges(); + await skipAnimations(page); + await page.waitForEvent("calciteSheetOpen"); expect(await sheet.getProperty("opened")).toBe(true); + await page.keyboard.press("Escape"); await page.waitForChanges(); - await page.waitForChanges(); + expect(mockCallBack).toHaveBeenCalledTimes(1); expect(await sheet.getProperty("opened")).toBe(false); }); diff --git a/packages/calcite-components/src/components/sheet/sheet.tsx b/packages/calcite-components/src/components/sheet/sheet.tsx index 8cc68f2dbf1..ee6b9c80445 100644 --- a/packages/calcite-components/src/components/sheet/sheet.tsx +++ b/packages/calcite-components/src/components/sheet/sheet.tsx @@ -5,7 +5,6 @@ import { EventEmitter, h, Host, - Listen, Method, Prop, VNode, @@ -152,7 +151,14 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo connectedCallback(): void { this.mutationObserver?.observe(this.el, { childList: true, subtree: true }); - connectFocusTrap(this); + connectFocusTrap(this, { + focusTrapOptions: { + // Scrim has it's own close handler, allow it to take over. + clickOutsideDeactivates: false, + escapeDeactivates: this.escapeDeactivates, + onDeactivate: this.focusTrapDeactivates, + }, + }); } disconnectedCallback(): void { @@ -218,20 +224,6 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo this.handleMutationObserver(), ); - //-------------------------------------------------------------------------- - // - // Event Listeners - // - //-------------------------------------------------------------------------- - - @Listen("keydown", { target: "window" }) - handleEscape(event: KeyboardEvent): void { - if (this.open && !this.escapeDisabled && event.key === "Escape" && !event.defaultPrevented) { - this.open = false; - event.preventDefault(); - } - } - //-------------------------------------------------------------------------- // // Events @@ -355,4 +347,16 @@ export class Sheet implements OpenCloseComponent, FocusTrapComponent, LoadableCo private handleMutationObserver(): void { this.updateFocusTrapElements(); } + + private escapeDeactivates = (event: KeyboardEvent) => { + if (event.defaultPrevented || this.escapeDisabled) { + return false; + } + event.preventDefault(); + return true; + }; + + private focusTrapDeactivates = (): void => { + this.open = false; + }; } diff --git a/packages/calcite-components/src/tests/stackedFocusTrap.e2e.ts b/packages/calcite-components/src/tests/stackedFocusTrap.e2e.ts new file mode 100644 index 00000000000..a8a5f2ea795 --- /dev/null +++ b/packages/calcite-components/src/tests/stackedFocusTrap.e2e.ts @@ -0,0 +1,131 @@ +import { E2EElement, newE2EPage, E2EPage } from "@stencil/core/testing"; +import { camelCase } from "change-case"; +import { html } from "../../support/formatting"; +import { skipAnimations } from "../tests/utils"; + +describe("stacked focus-trap components", () => { + const componentStack = html` + + + + Open Modal from Sheet + + + + +

The small dialog is perfect for short confirmation dialogs or very compact interfaces with few elements.

+ Back +
+ + +
+

This is an example modal that opens from a Sheet.

+
+ Open Another Modal +
+ + +
+

+ This is an example of a another modal that opens from a modal. This modal an input date picker, a popover and + a tooltip. +

+ +
+

Example Popover.

+ + Input Date Picker + + + + Input Time Picker + + +
+
+ Example Popover + auto +
+
+ `; + + it("closes a stack of open components sequentially in visual order", async () => { + const page = await newE2EPage(); + await page.setContent(componentStack); + await skipAnimations(page); + + async function testStackEscapeSequence(page: E2EPage, pickerType: string): Promise { + async function openAndCheckVisibility(element: E2EElement): Promise { + const elTagNameCamelCased = camelCase(element.tagName); + const openEvent = page.waitForEvent(`${elTagNameCamelCased}Open`); + + element.setProperty("open", true); + await page.waitForChanges(); + await openEvent; + } + + async function testEscapeAndAssertOpenState(focusTrapOrderElements: E2EElement[]): Promise { + for (let i = 0; i < focusTrapOrderElements.length; i++) { + const elTagNameCamelCased = camelCase(focusTrapOrderElements[i].tagName); + const closeEvent = page.waitForEvent(`${elTagNameCamelCased}Close`); + + const activeElementId = await page.evaluate(() => document.activeElement?.id); + + // 'input-time-picker', 'input-date-picker' retain focus after pressing esc + if (activeElementId) { + if (activeElementId === "input-time-picker") { + await page.keyboard.press("Tab"); + } else if (activeElementId === "input-date-picker") { + await page.keyboard.down("Shift"); + await page.keyboard.press("Tab"); + } + } + await page.waitForChanges(); + + const activeElementIdAfterTab = await page.evaluate(() => document.activeElement?.id); + + // sheet itself is not focusable, so we check for `sheet-button` as the element focus returns to it after closing the previous component + const expectedElementId = + focusTrapOrderElements[i].id === "sheet" + ? "sheet-button" + : focusTrapOrderElements[i].id || document.body.id; + expect(activeElementIdAfterTab).toBe(expectedElementId); + + await page.keyboard.press("Escape"); + await page.waitForChanges(); + await closeEvent; + + expect(await focusTrapOrderElements[i].getProperty("open")).toBe(false); + + for (let j = 0; j < focusTrapOrderElements.length; j++) { + const expectedOpenState = j > i; + expect(await focusTrapOrderElements[j].getProperty("open")).toBe(expectedOpenState); + } + } + } + + const sheet = await page.find("#sheet"); + const dialog = await page.find("#dialog"); + const firstModal = await page.find("#example-modal"); + const secondModal = await page.find("#another-modal"); + const popover = await page.find("#popover"); + const inputPicker = await page.find(pickerType); + + await openAndCheckVisibility(sheet); + await openAndCheckVisibility(dialog); + await openAndCheckVisibility(firstModal); + await openAndCheckVisibility(secondModal); + await openAndCheckVisibility(popover); + + await inputPicker.click(); + + await testEscapeAndAssertOpenState([inputPicker, popover, secondModal, firstModal, dialog, sheet]); + } + + await testStackEscapeSequence(page, "calcite-input-time-picker"); + await testStackEscapeSequence(page, "calcite-input-date-picker"); + }); +}); diff --git a/packages/calcite-components/src/utils/focusTrapComponent.ts b/packages/calcite-components/src/utils/focusTrapComponent.ts index 877c3dc04f3..45fee2ce35f 100644 --- a/packages/calcite-components/src/utils/focusTrapComponent.ts +++ b/packages/calcite-components/src/utils/focusTrapComponent.ts @@ -58,8 +58,6 @@ export function connectFocusTrap(component: FocusTrapComponent, options?: Connec } const focusTrapOptions: FocusTrapOptions = { - clickOutsideDeactivates: true, - escapeDeactivates: false, fallbackFocus: focusTrapNode, setReturnFocus: (el) => { focusElement(el as FocusableElement);