From 5edf2bc1e7bb59250259968aeab0a0ffb1991eac Mon Sep 17 00:00:00 2001 From: JC Franco Date: Thu, 13 Feb 2025 17:04:11 -0800 Subject: [PATCH] fix(dialog, modal, sheet): preserve focus-trap extra containers across internal trap updates --- .../src/components/dialog/dialog.tsx | 3 ++- .../src/components/modal/modal.tsx | 3 ++- .../src/components/sheet/sheet.tsx | 3 ++- .../src/controllers/useFocusTrap.ts | 23 ++++++++++++------- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/packages/calcite-components/src/components/dialog/dialog.tsx b/packages/calcite-components/src/components/dialog/dialog.tsx index fd092194727..512e213533e 100644 --- a/packages/calcite-components/src/components/dialog/dialog.tsx +++ b/packages/calcite-components/src/components/dialog/dialog.tsx @@ -286,7 +286,8 @@ export class Dialog extends LitElement implements OpenCloseComponent { async updateFocusTrapElements( extraContainers?: FocusTrapOptions["extraContainers"], ): Promise { - this.focusTrap.updateContainerElements(extraContainers); + this.focusTrap.setExtraContainers(extraContainers); + this.focusTrap.updateContainerElements(); } /** When defined, provides a condition to disable focus trapping. When `true`, prevents focus trapping. */ diff --git a/packages/calcite-components/src/components/modal/modal.tsx b/packages/calcite-components/src/components/modal/modal.tsx index 45e8deae1dc..b54284a7497 100644 --- a/packages/calcite-components/src/components/modal/modal.tsx +++ b/packages/calcite-components/src/components/modal/modal.tsx @@ -248,7 +248,8 @@ export class Modal extends LitElement implements OpenCloseComponent { async updateFocusTrapElements( extraContainers?: FocusTrapOptions["extraContainers"], ): Promise { - this.focusTrap.updateContainerElements(extraContainers); + this.focusTrap.setExtraContainers(extraContainers); + this.focusTrap.updateContainerElements(); } // #endregion diff --git a/packages/calcite-components/src/components/sheet/sheet.tsx b/packages/calcite-components/src/components/sheet/sheet.tsx index 3480b65b345..5a7854bf204 100644 --- a/packages/calcite-components/src/components/sheet/sheet.tsx +++ b/packages/calcite-components/src/components/sheet/sheet.tsx @@ -229,7 +229,8 @@ export class Sheet extends LitElement implements OpenCloseComponent { async updateFocusTrapElements( extraContainers?: FocusTrapOptions["extraContainers"], ): Promise { - this.focusTrap.updateContainerElements(extraContainers); + this.focusTrap.setExtraContainers(extraContainers); + this.focusTrap.updateContainerElements(); } // #endregion diff --git a/packages/calcite-components/src/controllers/useFocusTrap.ts b/packages/calcite-components/src/controllers/useFocusTrap.ts index 672b83198d2..446713f38b4 100644 --- a/packages/calcite-components/src/controllers/useFocusTrap.ts +++ b/packages/calcite-components/src/controllers/useFocusTrap.ts @@ -24,11 +24,16 @@ export interface UseFocusTrap { overrideFocusTrapEl: (el: HTMLElement) => void; /** - * Updates focusable elements within the trap. + * Sets the extra containers to be used in the focus trap. * * @see https://github.com/focus-trap/focus-trap#trapupdatecontainerelements */ - updateContainerElements: (extraContainers?: FocusTrapOptions["extraContainers"]) => void; + setExtraContainers: (extraContainers?: FocusTrapOptions["extraContainers"]) => void; + + /** + * Updates focusable elements within the trap. + */ + updateContainerElements: () => void; } interface UseFocusTrapOptions { @@ -100,6 +105,7 @@ export const useFocusTrap = ( return makeGenericController((component, controller) => { let focusTrap: FocusTrap; let focusTrapEl: HTMLElement; + let effectiveContainers: FocusTrapOptions["extraContainers"]; const internalFocusTrapOptions = options.focusTrapOptions; controller.onConnected(() => { @@ -123,11 +129,9 @@ export const useFocusTrap = ( ...internalFocusTrapOptions, ...component.focusTrapOptions, }; + effectiveContainers ||= getEffectiveContainerElements(targetEl, component); - focusTrap = createFocusTrap( - getEffectiveContainerElements(targetEl, component), - createFocusTrapOptions(targetEl, effectiveFocusTrapOptions), - ); + focusTrap = createFocusTrap(effectiveContainers, createFocusTrapOptions(targetEl, effectiveFocusTrapOptions)); } if ( @@ -146,9 +150,12 @@ export const useFocusTrap = ( focusTrapEl = el; }, - updateContainerElements: (extraContainers?: FocusTrapOptions["extraContainers"]) => { + setExtraContainers: (extraContainers?: FocusTrapOptions["extraContainers"]) => { const targetEl = focusTrapEl || component.el; - return focusTrap?.updateContainerElements(getEffectiveContainerElements(targetEl, component, extraContainers)); + effectiveContainers = getEffectiveContainerElements(targetEl, component, extraContainers); + }, + updateContainerElements: () => { + return focusTrap?.updateContainerElements(effectiveContainers); }, }; });