Skip to content

Commit a5e32ba

Browse files
Mil4n0rMil4n0r
authored
Merge pull request #2125 from dxc-technology/gomezivann/focus-lock-issue
Updates on the FocusLock and Dialog
2 parents d80caba + dbce556 commit a5e32ba

File tree

9 files changed

+155
-78
lines changed

9 files changed

+155
-78
lines changed

‎apps/website/screens/components/dialog/code/DialogCodePage.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const sections = [
2525
</thead>
2626
<tbody>
2727
<tr>
28-
<td>isCloseVisible</td>
28+
<td>closable</td>
2929
<td>
3030
<TableCode>boolean</TableCode>
3131
</td>

‎apps/website/screens/components/dialog/code/examples/backgroundClick.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const code = `() => {
1111
<DxcInset space="2rem">
1212
<DxcButton label="Enter your data" onClick={handleClick} />
1313
{isDialogVisible && (
14-
<DxcDialog onBackgroundClick={handleClick} isCloseVisible={false}>
14+
<DxcDialog onBackgroundClick={handleClick} closable={false}>
1515
<DxcInset space="1.5rem">
1616
Please enter your personal information.
1717
</DxcInset>

‎apps/website/screens/theme-generator/ImportDialog.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const ImportDialog = ({ customThemeSchema, setCustomTheme, setDialogVisible }: I
5353
};
5454

5555
return (
56-
<DxcDialog isCloseVisible={false} onBackgroundClick={closeDialog}>
56+
<DxcDialog closable={false} onBackgroundClick={closeDialog}>
5757
<ImportDialogContainer>
5858
<DxcHeading text="Import theme" level={2} weight="normal" />
5959
<DxcTextarea

‎packages/lib/src/dialog/Dialog.accessibility.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ describe("Dialog component accessibility tests", () => {
1111
});
1212
it("Should not have basic accessibility issues for close button not visible", async () => {
1313
// baseElement is needed when using React Portals
14-
const { baseElement } = render(<DxcDialog isCloseVisible={false}>Dialog text</DxcDialog>);
14+
const { baseElement } = render(<DxcDialog closable={false}>Dialog text</DxcDialog>);
1515
const results = await axe(baseElement);
1616
expect(results).toHaveNoViolations();
1717
});

‎packages/lib/src/dialog/Dialog.stories.tsx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,7 @@ export const DialogWithInputs = () => (
255255
semantic="error"
256256
title="Error"
257257
message={{
258-
text:
259-
"User: arn:aws:xxx::xxxxxxxxxxxx:assumed-role/assure-sandbox-xxxx-xxxxxxxxxxxxxxxxxxxxxxxxxx/sandbox-xxxx-xxxxxxxxxxxxxxxxxx is not authorized to perform: lambda:xxxxxxxxxxxxxx on resource: arn:aws:lambda:us-east-1:xxxxxxxxxxxx:function:sandbox-xxxx-xx-xxxxxxx-xxxxxxx-lambda because no identity-based policy allows the lambda:xxxxxxxxxxxxxx action",
258+
text: "User: arn:aws:xxx::xxxxxxxxxxxx:assumed-role/assure-sandbox-xxxx-xxxxxxxxxxxxxxxxxxxxxxxxxx/sandbox-xxxx-xxxxxxxxxxxxxxxxxx is not authorized to perform: lambda:xxxxxxxxxxxxxx on resource: arn:aws:lambda:us-east-1:xxxxxxxxxxxx:function:sandbox-xxxx-xx-xxxxxxx-xxxxxxx-lambda because no identity-based policy allows the lambda:xxxxxxxxxxxxxx action",
260259
}}
261260
/>
262261
<DxcFlex justifyContent="flex-end" gap="0.5rem">
@@ -317,7 +316,7 @@ export const DialogWithoutOverlay = () => (
317316
export const DialogCloseVisibleFalse = () => (
318317
<ExampleContainer expanded={true}>
319318
<Title title="Dialog Close Visible" theme="dark" level={4} />
320-
<DxcDialog isCloseVisible={false}>
319+
<DxcDialog closable={false}>
321320
<DxcInset space="1.5rem">
322321
<DxcParagraph>
323322
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi egestas luctus porttitor. Donec massa magna,

‎packages/lib/src/dialog/Dialog.test.tsx‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,13 @@ import DxcSwitch from "../switch/Switch";
1212
import DxcTextInput from "../text-input/TextInput";
1313
import DxcTextarea from "../textarea/Textarea";
1414
import DxcDialog from "./Dialog";
15+
import DxcTooltip from "../tooltip/Tooltip";
16+
import DxcAlert from "../alert/Alert";
1517

18+
(global as any).globalThis = global;
19+
(global as any).DOMRect = {
20+
fromRect: () => ({ top: 0, left: 0, bottom: 0, right: 0, width: 0, height: 0 }),
21+
};
1622
(global as any).ResizeObserver = class ResizeObserver {
1723
observe() {}
1824
unobserve() {}
@@ -35,13 +41,13 @@ describe("Dialog component tests", () => {
3541
});
3642

3743
test("Dialog renders without close button", () => {
38-
const { queryByRole } = render(<DxcDialog isCloseVisible={false}>dialog-text</DxcDialog>);
44+
const { queryByRole } = render(<DxcDialog closable={false}>dialog-text</DxcDialog>);
3945
expect(queryByRole("button")).toBeFalsy();
4046
});
4147

4248
test("Dialog renders with aria-modal false when overlay is not used", () => {
4349
const { getByRole } = render(
44-
<DxcDialog isCloseVisible={false} overlay={false}>
50+
<DxcDialog closable={false} overlay={false}>
4551
dialog-text
4652
</DxcDialog>
4753
);
@@ -264,7 +270,7 @@ describe("Dialog component: Focus lock tests", () => {
264270
const { getAllByRole } = render(
265271
<>
266272
<DxcTextInput label="Name" />
267-
<DxcDialog isCloseVisible={false}>
273+
<DxcDialog closable={false}>
268274
<h2>Policy agreement</h2>
269275
<p>Sample text.</p>
270276
</DxcDialog>
@@ -280,4 +286,47 @@ describe("Dialog component: Focus lock tests", () => {
280286
fireEvent.keyDown(dialog, { key: "Tab", shiftKey: true });
281287
expect(document.activeElement).not.toEqual(inputs[0]);
282288
});
289+
test("Focus travels correctly in a complex tab sequence", async () => {
290+
const { getAllByRole, queryByRole, getByRole } = render(
291+
<DxcDialog>
292+
<DxcSelect label="Accept" options={options} />
293+
<DxcDateInput label="Older age" />
294+
<DxcTooltip label="Text input tooltip label">
295+
<DxcTextInput label="Name" />
296+
</DxcTooltip>
297+
<DxcAlert
298+
semantic="error"
299+
title="Error"
300+
message={{
301+
text: "User: arn:aws:xxx::xxxxxxxxxxxx:assumed-role/assure-sandbox-xxxx-xxxxxxxxxxxxxxxxxxxxxxxxxx/sandbox-xxxx-xxxxxxxxxxxxxxxxxx is not authorized to perform: lambda:xxxxxxxxxxxxxx on resource: arn:aws:lambda:us-east-1:xxxxxxxxxxxx:function:sandbox-xxxx-xx-xxxxxxx-xxxxxxx-lambda because no identity-based policy allows the lambda:xxxxxxxxxxxxxx action",
302+
}}
303+
/>
304+
<DxcButton label="Cancel" />
305+
<DxcButton label="Save" />
306+
</DxcDialog>
307+
);
308+
const select = getAllByRole("combobox")[0];
309+
expect(document.activeElement).toEqual(select);
310+
fireEvent.keyDown(select, { key: "ArrowDown", code: "ArrowDown", keyCode: 40, charCode: 40 });
311+
expect(queryByRole("listbox")).toBeTruthy();
312+
userEvent.tab();
313+
userEvent.tab();
314+
await userEvent.keyboard('{Enter}');
315+
expect(getAllByRole("dialog")[1]).toBeTruthy();
316+
userEvent.click(getAllByRole("dialog")[0]);
317+
userEvent.tab();
318+
userEvent.tab();
319+
userEvent.tab();
320+
userEvent.tab();
321+
expect(document.activeElement).toEqual(getByRole("button", { name: "Close alert" }));
322+
userEvent.tab();
323+
userEvent.tab();
324+
expect(document.activeElement).toEqual(getByRole("button", { name: "Save" }));
325+
userEvent.tab();
326+
userEvent.tab();
327+
expect(document.activeElement).toEqual(select);
328+
userEvent.tab({ shift: true });
329+
userEvent.tab({ shift: true });
330+
expect(getByRole("button", { name: "Save" })).toBeTruthy();
331+
});
283332
});

‎packages/lib/src/dialog/Dialog.tsx‎

Lines changed: 62 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -8,66 +8,6 @@ import useTranslatedLabels from "../useTranslatedLabels";
88
import FocusLock from "../utils/FocusLock";
99
import DialogPropsType from "./types";
1010

11-
const DxcDialog = ({
12-
isCloseVisible = true,
13-
onCloseClick,
14-
children,
15-
overlay = true,
16-
onBackgroundClick,
17-
tabIndex = 0,
18-
}: DialogPropsType): JSX.Element => {
19-
const colorsTheme = useTheme();
20-
const translatedLabels = useTranslatedLabels();
21-
22-
useEffect(() => {
23-
const handleKeyDown = (event: KeyboardEvent) => {
24-
if (event.key === "Escape") {
25-
event.preventDefault();
26-
onCloseClick?.();
27-
}
28-
};
29-
30-
document.addEventListener("keydown", handleKeyDown);
31-
return () => {
32-
document.removeEventListener("keydown", handleKeyDown);
33-
};
34-
}, [onCloseClick]);
35-
36-
return (
37-
<ThemeProvider theme={colorsTheme.dialog}>
38-
<BodyStyle />
39-
{createPortal(
40-
<DialogContainer>
41-
{overlay && (
42-
<Overlay
43-
onClick={() => {
44-
onBackgroundClick?.();
45-
}}
46-
/>
47-
)}
48-
<Dialog role="dialog" aria-modal={overlay} isCloseVisible={isCloseVisible} aria-label="Dialog">
49-
<FocusLock>
50-
{children}
51-
{isCloseVisible && (
52-
<CloseIconAction
53-
onClick={() => {
54-
onCloseClick?.();
55-
}}
56-
aria-label={translatedLabels.dialog.closeIconAriaLabel}
57-
tabIndex={tabIndex}
58-
>
59-
<DxcIcon icon="close" />
60-
</CloseIconAction>
61-
)}
62-
</FocusLock>
63-
</Dialog>
64-
</DialogContainer>,
65-
document.body
66-
)}
67-
</ThemeProvider>
68-
);
69-
};
70-
7111
const BodyStyle = createGlobalStyle`
7212
body {
7313
overflow: hidden;
@@ -91,14 +31,14 @@ const Overlay = styled.div`
9131
background-color: ${(props) => props.theme.overlayColor};
9232
`;
9333

94-
const Dialog = styled.div<{ isCloseVisible: DialogPropsType["isCloseVisible"] }>`
34+
const Dialog = styled.div<{ closable: DialogPropsType["closable"] }>`
9535
position: relative;
9636
box-sizing: border-box;
9737
max-width: 80%;
9838
min-width: 696px;
9939
border-radius: 4px;
10040
background-color: ${(props) => props.theme.backgroundColor};
101-
${(props) => props.isCloseVisible && "min-height: 72px;"}
41+
${(props) => props.closable && "min-height: 72px;"}
10242
box-shadow: ${(props) =>
10343
`${props.theme.boxShadowOffsetX} ${props.theme.boxShadowOffsetY} ${props.theme.boxShadowBlur} ${props.theme.boxShadowColor}`};
10444
z-index: 2147483647;
@@ -142,4 +82,64 @@ const CloseIconAction = styled.button`
14282
}
14383
`;
14484

85+
const DxcDialog = ({
86+
closable = true,
87+
onCloseClick,
88+
children,
89+
overlay = true,
90+
onBackgroundClick,
91+
tabIndex = 0,
92+
}: DialogPropsType): JSX.Element => {
93+
const colorsTheme = useTheme();
94+
const translatedLabels = useTranslatedLabels();
95+
96+
useEffect(() => {
97+
const handleKeyDown = (event: KeyboardEvent) => {
98+
if (event.key === "Escape") {
99+
event.preventDefault();
100+
onCloseClick?.();
101+
}
102+
};
103+
104+
document.addEventListener("keydown", handleKeyDown);
105+
return () => {
106+
document.removeEventListener("keydown", handleKeyDown);
107+
};
108+
}, [onCloseClick]);
109+
110+
return (
111+
<ThemeProvider theme={colorsTheme.dialog}>
112+
<BodyStyle />
113+
{createPortal(
114+
<DialogContainer>
115+
{overlay && (
116+
<Overlay
117+
onClick={() => {
118+
onBackgroundClick?.();
119+
}}
120+
/>
121+
)}
122+
<Dialog role="dialog" aria-modal={overlay} closable={closable} aria-label="Dialog">
123+
<FocusLock>
124+
{children}
125+
{closable && (
126+
<CloseIconAction
127+
onClick={() => {
128+
onCloseClick?.();
129+
}}
130+
aria-label={translatedLabels.dialog.closeIconAriaLabel}
131+
tabIndex={tabIndex}
132+
>
133+
<DxcIcon icon="close" />
134+
</CloseIconAction>
135+
)}
136+
</FocusLock>
137+
</Dialog>
138+
</DialogContainer>,
139+
document.body
140+
)}
141+
</ThemeProvider>
142+
);
143+
};
144+
145145
export default DxcDialog;

‎packages/lib/src/dialog/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ type Props = {
22
/**
33
* If true, the close button will be visible.
44
*/
5-
isCloseVisible?: boolean;
5+
closable?: boolean;
66
/**
77
* This function will be called when the user clicks the close button.
88
* The responsibility of hiding the dialog lies with the user.

‎packages/lib/src/utils/FocusLock.tsx‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ const attemptFocus = (element: HTMLElement): boolean => {
4545
* @param element: HTMLElement
4646
* @returns boolean: true if element is contained inside a Radix Portal, false otherwise.
4747
*/
48-
const radixPortalContains = (activeElement: Element): boolean => {
48+
const radixPortalContains = (activeElement: Node): boolean => {
4949
const radixPortals = document.querySelectorAll("[data-radix-portal]");
5050
const radixPoppers = document.querySelectorAll("[data-radix-popper-content-wrapper]");
5151
return (
@@ -69,7 +69,7 @@ const useFocusableElements = (ref: React.MutableRefObject<HTMLDivElement>): HTML
6969
const observer = new MutationObserver(() => {
7070
setFocusableElements(getFocusableElements(ref.current));
7171
});
72-
observer.observe(ref.current, { childList: true, subtree: true, attributes: true });
72+
observer.observe(ref.current, { childList: true, subtree: true });
7373
return () => {
7474
observer.disconnect();
7575
};
@@ -90,23 +90,52 @@ const useFocusableElements = (ref: React.MutableRefObject<HTMLDivElement>): HTML
9090
const FocusLock = ({ children }: { children: React.ReactNode }): JSX.Element => {
9191
const childrenContainerRef = useRef<HTMLDivElement>();
9292
const focusableElements = useFocusableElements(childrenContainerRef);
93+
const initialFocus = useRef(false);
9394

9495
const focusFirst = useCallback(() => {
9596
if (focusableElements?.length === 0) childrenContainerRef.current?.focus();
9697
else if (focusableElements?.length > 0) focusableElements.some((element) => attemptFocus(element));
9798
}, [focusableElements]);
9899

99100
const focusLast = () => {
100-
focusableElements?.reverse()?.some((element) => attemptFocus(element));
101+
focusableElements
102+
?.slice()
103+
.reverse()
104+
?.some((element) => attemptFocus(element));
101105
};
102106

103107
const focusLock = (event: React.KeyboardEvent<HTMLDivElement>) => {
104-
if (event.key === "Tab") focusableElements.length === 0 && event.preventDefault();
108+
if (event.key === "Tab" && focusableElements.length === 0) event.preventDefault();
105109
};
106110

107111
useEffect(() => {
108-
if (!childrenContainerRef.current?.contains(document.activeElement) && !radixPortalContains(document.activeElement))
112+
if (focusableElements !== undefined && !initialFocus.current) {
113+
initialFocus.current = true;
109114
focusFirst();
115+
}
116+
}, [focusableElements, focusFirst]);
117+
118+
useEffect(() => {
119+
const focusGuardHandler = (event: FocusEvent) => {
120+
const target = event.relatedTarget as Node | null;
121+
const container = childrenContainerRef.current;
122+
123+
if (
124+
target &&
125+
!(
126+
container?.contains(target) ||
127+
container?.nextElementSibling?.contains(target) ||
128+
container?.previousElementSibling?.contains(target) ||
129+
radixPortalContains(target)
130+
)
131+
)
132+
focusFirst();
133+
};
134+
135+
document.addEventListener("focusout", focusGuardHandler);
136+
return () => {
137+
document.removeEventListener("focusout", focusGuardHandler);
138+
};
110139
}, [focusFirst]);
111140

112141
return (

0 commit comments

Comments
 (0)