Skip to content

Commit

Permalink
fix(dialog): toggle focus-trap on DOM connect and disconnect (#11429)
Browse files Browse the repository at this point in the history
**Related Issue:** #10731 

## Summary

Fixes an issue where a `focus-trap` instance might be left active after
the dialog is removed.

### Relevant changes

* Adds `useFocusTrap` controller
* Extracts and exports function to create default `focus-trap` options
  • Loading branch information
jcfranco authored Feb 3, 2025
1 parent bdd39b8 commit 5607902
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 40 deletions.
55 changes: 20 additions & 35 deletions packages/calcite-components/src/components/dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,6 @@ import { PropertyValues } from "lit";
import { createRef } from "lit-html/directives/ref.js";
import { createEvent, h, JsxNode, LitElement, method, property, state } from "@arcgis/lumina";
import { focusFirstTabbable, isPixelValue } from "../../utils/dom";
import {
activateFocusTrap,
connectFocusTrap,
deactivateFocusTrap,
FocusTrap,
FocusTrapComponent,
updateFocusTrapElements,
} from "../../utils/focusTrapComponent";
import {
componentFocusable,
LoadableComponent,
Expand All @@ -28,6 +20,7 @@ import { HeadingLevel } from "../functional/Heading";
import type { OverlayPositioning } from "../../utils/floating-ui";
import { useT9n } from "../../controllers/useT9n";
import type { Panel } from "../panel/panel";
import { useFocusTrap } from "../../controllers/useFocusTrap";
import T9nStrings from "./assets/t9n/messages.en.json";
import {
CSS,
Expand Down Expand Up @@ -66,10 +59,7 @@ let initialDocumentOverflowStyle: string = "";
* @slot footer-end - A slot for adding a trailing footer custom content. Should not be used with the `"footer"` slot.
* @slot footer-start - A slot for adding a leading footer custom content. Should not be used with the `"footer"` slot.
*/
export class Dialog
extends LitElement
implements OpenCloseComponent, FocusTrapComponent, LoadableComponent
{
export class Dialog extends LitElement implements OpenCloseComponent, LoadableComponent {
// #region Static Members

static override styles = styles;
Expand All @@ -80,7 +70,21 @@ export class Dialog

private dragPosition: DialogDragPosition = { ...initialDragPosition };

focusTrap: FocusTrap;
focusTrap = useFocusTrap<Dialog>({
triggerProp: "open",
focusTrapOptions: {
// scrim closes on click, so we let it take over
clickOutsideDeactivates: () => !this.modal,
escapeDeactivates: (event) => {
if (!event.defaultPrevented && !this.escapeDisabled) {
this.open = false;
event.preventDefault();
}

return false;
},
},
})(this);

private ignoreOpenChange = false;

Expand Down Expand Up @@ -268,7 +272,7 @@ export class Dialog
/** Updates the element(s) that are used within the focus-trap of the component. */
@method()
async updateFocusTrapElements(): Promise<void> {
updateFocusTrapElements(this);
this.focusTrap.updateContainerElements();
}

// #endregion
Expand Down Expand Up @@ -296,29 +300,11 @@ export class Dialog

override connectedCallback(): void {
this.mutationObserver?.observe(this.el, { childList: true, subtree: true });
connectFocusTrap(this, {
focusTrapOptions: {
// scrim closes on click, so we let it take over
clickOutsideDeactivates: () => !this.modal,
escapeDeactivates: (event) => {
if (!event.defaultPrevented && !this.escapeDisabled) {
this.open = false;
event.preventDefault();
}

return false;
},
},
});
this.setupInteractions();
}

async load(): Promise<void> {
setUpLoadableComponent(this);
// when dialog initially renders, if active was set we need to open as watcher doesn't fire
if (this.open) {
this.openDialog();
}
}

override willUpdate(changes: PropertyValues<this>): void {
Expand Down Expand Up @@ -359,7 +345,6 @@ export class Dialog
override disconnectedCallback(): void {
this.removeOverflowHiddenClass();
this.mutationObserver?.disconnect();
deactivateFocusTrap(this);
this.embedded = false;
this.cleanupInteractions();
}
Expand All @@ -381,7 +366,7 @@ export class Dialog

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

onBeforeClose(): void {
Expand All @@ -390,7 +375,7 @@ export class Dialog

onClose(): void {
this.calciteDialogClose.emit();
deactivateFocusTrap(this);
this.focusTrap.deactivate();
}

private toggleDialog(value: boolean): void {
Expand Down
96 changes: 96 additions & 0 deletions packages/calcite-components/src/controllers/useFocusTrap.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { makeGenericController } from "@arcgis/components-controllers";
import { createFocusTrap, FocusTrap, Options as FocusTrapOptions } from "focus-trap";
import { LitElement } from "@arcgis/lumina";
import { createFocusTrapOptions } from "../utils/focusTrapComponent";

export interface UseFocusTrap {
/**
* Activates the focus trap.
*
* @see https://github.com/focus-trap/focus-trap#trapactivate
*/
activate: (options?: Parameters<FocusTrap["activate"]>[0]) => void;

/**
* Deactivates the focus trap.
*
* @see https://github.com/focus-trap/focus-trap#trapdeactivate
*/
deactivate: (options?: Parameters<FocusTrap["deactivate"]>[0]) => void;

/**
* By default, the host element will be used as the focus-trap element, but if the focus-trap element needs to be a different element, use this method prior to activating to set the focus-trap element.
*/
overrideFocusTrapEl: (el: HTMLElement) => void;

/**
* Updates focusable elements within the trap.
*
* @see https://github.com/focus-trap/focus-trap#trapupdatecontainerelements
*/
updateContainerElements: () => void;
}

interface UseFocusTrapOptions<T extends LitElement = LitElement> {
/**
* The name of the prop that will trigger the focus trap to activate.
*/
triggerProp: keyof T;

/**
* Options to pass to the focus-trap library.
*/
focusTrapOptions?: FocusTrapOptions;
}

/**
* A controller for managing focus traps.
*
* Note: traps will be deactivated automatically when the component is disconnected.
*
* @param options
*/
export const useFocusTrap = <T extends LitElement>(
options: UseFocusTrapOptions<T>,
): ReturnType<typeof makeGenericController<UseFocusTrap, T>> => {
return makeGenericController<UseFocusTrap, T>((component, controller) => {
let focusTrap: FocusTrap;
let focusTrapEl: HTMLElement;
const { focusTrapOptions } = options;

controller.onConnected(() => {
if (component[options.triggerProp] && focusTrap) {
focusTrap.activate();
}
});
controller.onDisconnected(() => focusTrap?.deactivate());

return {
activate: (options?: Parameters<FocusTrap["activate"]>[0]) => {
const targetEl = focusTrapEl || component.el;

if (!targetEl.isConnected) {
return;
}

if (!focusTrap) {
focusTrap = createFocusTrap(targetEl, createFocusTrapOptions(targetEl, focusTrapOptions));
}

focusTrap.activate(options);
},
deactivate: (options?: Parameters<FocusTrap["deactivate"]>[0]) => focusTrap?.deactivate(options),
overrideFocusTrapEl: (el: HTMLElement) => {
if (focusTrap) {
throw new Error("Focus trap already created");
}

focusTrapEl = el;
},
updateContainerElements: () => {
const targetEl = focusTrapEl || component.el;
return focusTrap?.updateContainerElements(targetEl);
},
};
});
};
20 changes: 15 additions & 5 deletions packages/calcite-components/src/utils/focusTrapComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,32 @@ export function connectFocusTrap(component: FocusTrapComponent, options?: Connec
return;
}

const focusTrapOptions: FocusTrapOptions = {
component.focusTrap = createFocusTrap(focusTrapNode, createFocusTrapOptions(el, options?.focusTrapOptions));
}

/**
* Helper to create the FocusTrap options.
*
* @param hostEl
* @param options
*/
export function createFocusTrapOptions(hostEl: HTMLElement, options?: FocusTrapOptions): FocusTrapOptions {
const focusTrapNode = options?.fallbackFocus || hostEl;

return {
clickOutsideDeactivates: true,
fallbackFocus: focusTrapNode,
setReturnFocus: (el) => {
focusElement(el as FocusableElement);
return false;
},
...options?.focusTrapOptions,
...options,

// the following options are not overridable
document: el.ownerDocument,
document: hostEl.ownerDocument,
tabbableOptions,
trapStack: focusTrapStack,
};

component.focusTrap = createFocusTrap(focusTrapNode, focusTrapOptions);
}

/**
Expand Down

0 comments on commit 5607902

Please sign in to comment.