Skip to content

Commit bb9cf28

Browse files
makafsalriddhybansalariellalgilmore
authored
fix(modal): resolve focus trap issue (#20157)
* fix(modal): resolve focus trap issue * fix(modal): implement a prop to disable focus trap * refactor(modal): remove disableFocusTrap property --------- Co-authored-by: Riddhi Bansal <41935566+riddhybansal@users.noreply.github.com> Co-authored-by: Ariella Gilmore <ariellalgilmore@gmail.com>
1 parent d53c886 commit bb9cf28

File tree

4 files changed

+80
-150
lines changed

4 files changed

+80
-150
lines changed

packages/web-components/src/components/modal/modal-body.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,31 @@ import { carbonElement as customElement } from '../../globals/decorators/carbon-
1717
*/
1818
@customElement(`${prefix}-modal-body`)
1919
class CDSModalBody extends LitElement {
20+
private userDefinedTabindex: string | null = null;
21+
22+
connectedCallback() {
23+
super.connectedCallback?.();
24+
// Store the tabindex if user set it initially
25+
if (this.hasAttribute('tabindex')) {
26+
this.userDefinedTabindex = this.getAttribute('tabindex');
27+
}
28+
}
29+
30+
checkScroll() {
31+
const hasScroll = this.scrollHeight > this.clientHeight;
32+
33+
// Respect user-defined tabindex
34+
if (this.userDefinedTabindex !== null) return;
35+
36+
if (hasScroll) {
37+
this.setAttribute('tabindex', '0');
38+
} else {
39+
this.removeAttribute('tabindex');
40+
}
41+
}
42+
2043
render() {
21-
return html` <slot></slot> `;
44+
return html` <slot @slotchange=${this.checkScroll}></slot> `;
2245
}
2346

2447
static styles = styles;

packages/web-components/src/components/modal/modal.stories.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,8 +337,8 @@ export const WithAILabel = {
337337
render: () => html`
338338
<cds-modal open has-scrolling-content>
339339
<cds-modal-header>
340-
<cds-ai-label alignment="bottom-left"> ${content}${actions}</cds-ai-label>
341340
<cds-modal-close-button></cds-modal-close-button>
341+
<cds-ai-label alignment="bottom-left"> ${content}${actions}</cds-ai-label>
342342
<cds-modal-label>Account resources</cds-modal-label>
343343
<cds-modal-heading>Add a custom domain</cds-modal-heading>
344344
</cds-modal-header>

packages/web-components/src/components/modal/modal.ts

Lines changed: 53 additions & 148 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import { classMap } from 'lit/directives/class-map.js';
99
import { LitElement, html } from 'lit';
10-
import { property, query } from 'lit/decorators.js';
10+
import { property } from 'lit/decorators.js';
1111
import { prefix } from '../../globals/settings';
1212
import HostListener from '../../globals/decorators/host-listener';
1313
import HostListenerMixin from '../../globals/mixins/host-listener';
@@ -18,42 +18,6 @@ import { carbonElement as customElement } from '../../globals/decorators/carbon-
1818

1919
export { MODAL_SIZE };
2020

21-
const PRECEDING =
22-
Node.DOCUMENT_POSITION_PRECEDING | Node.DOCUMENT_POSITION_CONTAINS;
23-
24-
const FOLLOWING =
25-
Node.DOCUMENT_POSITION_FOLLOWING | Node.DOCUMENT_POSITION_CONTAINED_BY;
26-
27-
/**
28-
* Tries to focus on the given elements and bails out if one of them is successful.
29-
*
30-
* @param elems The elements.
31-
* @param reverse `true` to go through the list in reverse order.
32-
* @returns `true` if one of the attempts is successful, `false` otherwise.
33-
*/
34-
function tryFocusElems(elems: NodeListOf<HTMLElement>, reverse = false) {
35-
if (!reverse) {
36-
for (let i = 0; i < elems.length; ++i) {
37-
const elem = elems[i];
38-
elem.focus();
39-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- https://github.com/carbon-design-system/carbon/issues/20452
40-
if (elem.ownerDocument!.activeElement === elem) {
41-
return true;
42-
}
43-
}
44-
} else {
45-
for (let i = elems.length - 1; i >= 0; --i) {
46-
const elem = elems[i];
47-
elem.focus();
48-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- https://github.com/carbon-design-system/carbon/issues/20452
49-
if (elem.ownerDocument!.activeElement === elem) {
50-
return true;
51-
}
52-
}
53-
}
54-
return false;
55-
}
56-
5721
/**
5822
* Modal.
5923
*
@@ -71,18 +35,6 @@ class CDSModal extends HostListenerMixin(LitElement) {
7135
*/
7236
private _launcher: Element | null = null;
7337

74-
/**
75-
* Node to track focus going outside of modal content.
76-
*/
77-
@query('#start-sentinel')
78-
private _startSentinelNode!: HTMLAnchorElement;
79-
80-
/**
81-
* Node to track focus going outside of modal content.
82-
*/
83-
@query('#end-sentinel')
84-
private _endSentinelNode!: HTMLAnchorElement;
85-
8638
/**
8739
* Handles `click` event on this element.
8840
*
@@ -102,92 +54,28 @@ class CDSModal extends HostListenerMixin(LitElement) {
10254
};
10355

10456
/**
105-
* Handles `blur` event on this element.
57+
* Handle the keydown event.
58+
* Trap the focus inside the side-panel by tracking keydown.key == `Tab`
10659
*
107-
* @param event The event.
108-
* @param event.target The event target.
109-
* @param event.relatedTarget The event relatedTarget.
60+
* @param {KeyboardEvent} event The keyboard event object.
11061
*/
111-
@HostListener('shadowRoot:focusout')
112-
// eslint-disable-next-line @typescript-eslint/ban-ts-comment -- https://github.com/carbon-design-system/carbon/issues/20452
113-
// @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
114-
private _handleBlur = async ({ target, relatedTarget }: FocusEvent) => {
115-
const {
116-
open,
117-
_startSentinelNode: startSentinelNode,
118-
_endSentinelNode: endSentinelNode,
119-
} = this;
120-
const oldContains = target !== this && this.contains(target as Node);
121-
const currentContains =
122-
relatedTarget !== this &&
123-
(this.contains(relatedTarget as Node) ||
124-
(this.shadowRoot?.contains(relatedTarget as Node) &&
125-
relatedTarget !== (endSentinelNode as Node)));
126-
127-
// Performs focus wrapping if _all_ of the following is met:
128-
// * This modal is open
129-
// * The viewport still has focus
130-
// * Modal body used to have focus but no longer has focus
131-
const { selectorTabbable: selectorTabbableForModal } = this
132-
.constructor as typeof CDSModal;
133-
if (open && relatedTarget && oldContains && !currentContains) {
134-
const comparisonResult = (target as Node).compareDocumentPosition(
135-
relatedTarget as Node
136-
);
62+
@HostListener('keydown')
63+
protected _handleHostKeydown = (event: KeyboardEvent) => {
64+
if (event.key === 'Tab') {
65+
const { first: _firstElement, last: _lastElement } = this.getFocusable();
13766

138-
if (relatedTarget === startSentinelNode || comparisonResult & PRECEDING) {
139-
await (this.constructor as typeof CDSModal)._delay();
140-
if (
141-
!tryFocusElems(
142-
this.querySelectorAll(selectorTabbableForModal),
143-
true
144-
) &&
145-
relatedTarget !== this
146-
) {
147-
this.focus();
148-
}
149-
} else if (
150-
relatedTarget === endSentinelNode ||
151-
comparisonResult & FOLLOWING
67+
if (
68+
event.shiftKey &&
69+
(this.shadowRoot?.activeElement === _firstElement ||
70+
document.activeElement === _firstElement)
15271
) {
153-
await (this.constructor as typeof CDSModal)._delay();
154-
if (!tryFocusElems(this.querySelectorAll(selectorTabbableForModal))) {
155-
this.focus();
156-
}
157-
}
158-
}
72+
event.preventDefault();
15973

160-
// Adjust scroll if needed so that element with focus is not obscured by gradient
161-
const modal = document.querySelector(`${prefix}-modal`);
162-
const modalContent = document.querySelector(`${prefix}-modal-body`);
163-
if (
164-
!modal ||
165-
!modal.hasAttribute('has-scrolling-content') ||
166-
!modalContent ||
167-
!relatedTarget ||
168-
!modalContent.contains(relatedTarget as Node)
169-
) {
170-
return;
171-
}
74+
_lastElement?.focus();
75+
} else if (!event.shiftKey && document.activeElement === _lastElement) {
76+
event.preventDefault();
17277

173-
const lastContent = modalContent.children[modalContent.children.length - 1];
174-
const gradientSpacing =
175-
modalContent.scrollHeight -
176-
(lastContent as HTMLElement).offsetTop -
177-
(lastContent as HTMLElement).clientHeight;
178-
179-
for (const elem of modalContent.children) {
180-
if (elem.contains(relatedTarget as Node)) {
181-
const spaceBelow =
182-
modalContent.clientHeight -
183-
(elem as HTMLElement).offsetTop +
184-
modalContent.scrollTop -
185-
(elem as HTMLElement).clientHeight;
186-
if (spaceBelow < gradientSpacing) {
187-
modalContent.scrollTop =
188-
modalContent.scrollTop + (gradientSpacing - spaceBelow);
189-
}
190-
break;
78+
_firstElement?.focus();
19179
}
19280
}
19381
};
@@ -201,6 +89,38 @@ class CDSModal extends HostListenerMixin(LitElement) {
20189
}
20290
};
20391

92+
/**
93+
* Get focusable elements.
94+
*
95+
* Querying all tabbable items.
96+
*
97+
* @returns {{first: HTMLElement, last: HTMLElement, all: HTMLElement[]}} Returns an object with various elements.
98+
*/
99+
private getFocusable(): {
100+
first: HTMLElement | undefined;
101+
last: HTMLElement | undefined;
102+
all: HTMLElement[];
103+
} {
104+
const elements: HTMLElement[] = [];
105+
106+
// Add tabbable elements inside light DOM
107+
const tabbableItems = this.querySelectorAll<HTMLElement>(selectorTabbable);
108+
if (tabbableItems?.length) {
109+
elements.push(...tabbableItems);
110+
}
111+
112+
// Flatten NodeList arrays and filter for focusable items
113+
const all = elements?.filter(
114+
(el): el is HTMLElement => typeof el?.focus === 'function'
115+
);
116+
117+
return {
118+
first: all[0],
119+
last: all[all.length - 1],
120+
all,
121+
};
122+
}
123+
204124
/**
205125
* Handles `click` event on the modal container.
206126
*
@@ -345,11 +265,6 @@ class CDSModal extends HostListenerMixin(LitElement) {
345265
...containerClass,
346266
});
347267
return html`
348-
<a
349-
id="start-sentinel"
350-
class="${prefix}--visually-hidden"
351-
href="javascript:void 0"
352-
role="navigation"></a>
353268
<div
354269
aria-label=${ariaLabel}
355270
part="dialog"
@@ -362,11 +277,6 @@ class CDSModal extends HostListenerMixin(LitElement) {
362277
? html` <div class="cds--modal-content--overflow-indicator"></div> `
363278
: ``}
364279
</div>
365-
<a
366-
id="end-sentinel"
367-
class="${prefix}--visually-hidden"
368-
href="javascript:void 0"
369-
role="navigation"></a>
370280
`;
371281
}
372282

@@ -383,15 +293,10 @@ class CDSModal extends HostListenerMixin(LitElement) {
383293
// For cases where a `carbon-web-components` component (e.g. `<cds-button>`) being `primaryFocusNode`,
384294
// where its first update/render cycle that makes it focusable happens after `<cds-modal>`'s first update/render cycle
385295
(primaryFocusNode as HTMLElement).focus();
386-
} else if (
387-
!tryFocusElems(
388-
this.querySelectorAll(
389-
(this.constructor as typeof CDSModal).selectorTabbable
390-
),
391-
true
392-
)
393-
) {
394-
this.focus();
296+
} else {
297+
const { first } = this.getFocusable();
298+
299+
first?.focus();
395300
}
396301
} else if (
397302
this._launcher &&

packages/web-components/src/globals/settings.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const selectorTabbable = `
1818
iframe, object, embed, *[tabindex]:not([tabindex='-1']), *[contenteditable=true],
1919
${prefix}-accordion-item,
2020
${prefix}-actionable-notification-button,
21+
${prefix}-ai-label,
2122
${prefix}-button,
2223
${prefix}-breadcrumb-link,
2324
${prefix}-checkbox,
@@ -36,6 +37,7 @@ const selectorTabbable = `
3637
${prefix}-number-input,
3738
${prefix}-modal,
3839
${prefix}-modal-close-button,
40+
${prefix}-modal-footer-button,
3941
${prefix}-multi-select,
4042
${prefix}-inline-notification,
4143
${prefix}-toast-notification,

0 commit comments

Comments
 (0)