Skip to content

Commit

Permalink
WB-1796: Change modal default focus to be the Close (X) button (#2383)
Browse files Browse the repository at this point in the history
## Summary:

Today, the default for a modal is to focus the first focusable element, but we
exclude the top close button. What this often means is that the first button in
the footer area of the modal is focused.

This PR changes the default focus to be the top close button. This is a more
consistent experience as it allows to focus in the first focusable element in
the dialog content area.

Also had to modify the `IconButton` styles to make sure the focus ring in close
button is visible when the focus is applied programmatically.

Note that we could use `node.focus({focusVisible: true})` to make the focus
visible, but this is not supported in all browsers. So, we are using the
`:focus` pseudo-class to style the focus ring in these cases.


Issue: https://khanacademy.atlassian.net/browse/WB-1796

## Test plan:

Navigate to the `ModalLauncher` docs: /?path=/docs/packages-modal-modallauncher--__docs__

1. Open the modal by clicking the "Open Modal" button.
2. Verify that the dismiss button is focused when the modal is opened.

<img width="1122" alt="Screenshot 2024-12-06 at 5 35 38 PM" src="https://github.com/user-attachments/assets/40211b3f-434e-4e7c-af07-0030f9b33a19">


Also verify that the focus state works as expected in the `IconButton` stories.

Author: jandrade

Reviewers: jandrade, beaesguerra, marcysutton

Required Reviewers:

Approved By: beaesguerra

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️  dependabot, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #2383
  • Loading branch information
jandrade authored Dec 11, 2024
1 parent 7fe9d75 commit 0955be7
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 43 deletions.
7 changes: 7 additions & 0 deletions .changeset/kind-buckets-poke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@khanacademy/wonder-blocks-modal": patch
---

- ModalBackdrop: Change initial focus behavior. Focus on the dismiss button (X) by default.

- CloseButton: Override `:focus` styles on the dismiss button to make it visually distinct when the focus is set programmatically.
5 changes: 5 additions & 0 deletions .changeset/perfect-cameras-protect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-icon-button": patch
---

Fix focus styles: drop Safari v14 support.
3 changes: 2 additions & 1 deletion __docs__/wonder-blocks-modal/modal-launcher.argtypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export default {
control: {type: "text"},
description: `The selector for the element that will be focused
when the dialog shows. When not set, the first tabbable element
within the dialog will be used.`,
within the dialog will be used, which usually is the dismiss button
(X).`,
table: {
type: {summary: "string"},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,22 +353,9 @@ const _generateStyles = (
backgroundColor: "transparent",
},
},
// Provide basic, default focus styles on older browsers (e.g.
// Safari 14)
":focus": {
boxShadow: `0 0 0 ${theme.border.width.default}px ${defaultStrokeColor}`,
borderRadius: theme.border.radius.default,
},
// Remove default focus styles for mouse users ONLY if
// :focus-visible is supported on this platform.
":focus:not(:focus-visible)": {
boxShadow: "none",
},
// Provide focus styles for keyboard users on modern browsers.

// :focus-visible -> Provide focus styles for keyboard users only.
":focus-visible": {
// Reset default focus styles
boxShadow: "none",
// Apply modern focus styles
outlineWidth: theme.border.width.default,
outlineColor: defaultStrokeColor,
outlineOffset: 1,
Expand Down Expand Up @@ -397,17 +384,6 @@ const _generateStyles = (
// For order reference: https://css-tricks.com/snippets/css/link-pseudo-classes-in-order/
":hover": {...disabledStatesStyles, outline: "none"},
":active": {...disabledStatesStyles, outline: "none"},
// Provide basic, default focus styles on older browsers (e.g.
// Safari 14)
":focus": {
boxShadow: `0 0 0 ${theme.border.width.default}px ${disabledStrokeColor}`,
borderRadius: theme.border.radius.default,
},
// Remove default focus styles for mouse users ONLY if
// :focus-visible is supported on this platform.
":focus:not(:focus-visible)": {
boxShadow: "none",
},
":focus-visible": disabledStatesStyles,
},
} as const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe("ModalBackdrop", () => {
);

const firstFocusableElement = await screen.findByRole("button", {
name: "first focusable button",
name: "Close modal",
});

// Assert
Expand All @@ -207,7 +207,7 @@ describe("ModalBackdrop", () => {
await waitFor(() => expect(firstFocusableElement).toHaveFocus());
});

test("If no initialFocusId is set, we focus the first button in the modal", async () => {
test("If no initialFocusId is set, we focus the dismiss button in the modal", async () => {
// Arrange
render(
<ModalBackdrop onCloseModal={() => {}}>
Expand All @@ -217,7 +217,7 @@ describe("ModalBackdrop", () => {

// Act
const firstFocusableElement = await screen.findByRole("button", {
name: "first focusable button",
name: "Close modal",
});

// Assert
Expand All @@ -228,7 +228,14 @@ describe("ModalBackdrop", () => {
// Arrange
render(
<ModalBackdrop onCloseModal={() => {}}>
{exampleModal}
<OnePaneDialog
content={<div data-testid="example-modal-content" />}
title="Title"
footer={<div data-testid="example-modal-footer" />}
testId="example-modal-test-id"
// Ensure that there are no focusable elements
closeButtonVisible={false}
/>
</ModalBackdrop>,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ describe("ModalLauncher", () => {
expect(onClose).not.toHaveBeenCalled();
});

test("if modal is launched, move focus inside the modal", async () => {
test("if modal is launched, move focus to first focusable element inside dialog", async () => {
// Arrange
render(
<ModalLauncher
Expand All @@ -263,23 +263,20 @@ describe("ModalLauncher", () => {
</ModalLauncher>,
);

const modalOpener = await screen.findByRole("button", {
name: "Open modal",
});
// force focus
modalOpener.focus();
// focus on the open modal button
await userEvent.tab();

// Act
// Launch the modal.
await userEvent.type(modalOpener, "{enter}");
await userEvent.keyboard("{enter}");

// wait until the modal is open
await screen.findByRole("dialog");

// Assert
await waitFor(async () =>
expect(
await screen.findByRole("button", {name: "Button in modal"}),
await screen.findByRole("button", {name: "Close modal"}),
).toHaveFocus(),
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ type Props = {
onCloseModal: () => unknown;
/**
* The selector for the element that will be focused when the dialog shows.
* When not set, the first tabbable element within the dialog will be used.
* When not set, the first tabbable element within the dialog will be used,
* which usually is the dismiss button (X).
*/
initialFocusId?: string;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ type Props = Readonly<{
backdropDismissEnabled?: boolean;
/**
* The selector for the element that will be focused when the dialog shows.
* When not set, the first tabbable element within the dialog will be used.
* When not set, the first tabbable element within the dialog will be used,
* which usually is the dismiss button (X).
*/
initialFocusId?: string;
/**
Expand Down
11 changes: 11 additions & 0 deletions packages/wonder-blocks-modal/src/components/modal-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,17 @@ const themedStylesFn: ThemedStylesFn<ModalDialogThemeContract> = (theme) => ({
// This is to allow the button to be tab-ordered before the modal
// content but still be above the header and content.
zIndex: 1,

// NOTE: IconButton uses :focus-visible, which is not supported for
// programmatic focus. This is a workaround to make sure the focus
// outline is visible when this control is focused.
":focus": {
outlineWidth: theme.border.width,
outlineColor: theme.border.color,
outlineOffset: 1,
outlineStyle: "solid",
borderRadius: theme.border.radius,
},
},

dark: {
Expand Down
2 changes: 2 additions & 0 deletions packages/wonder-blocks-modal/src/themes/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ const theme = {
},
border: {
radius: tokens.border.radius.medium_4,
width: tokens.border.width.thin,
color: tokens.color.blue,
},
spacing: {
dialog: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
* List of elements that can be focused
* @see https://www.w3.org/TR/html5/editing.html#can-be-focused
*/
const FOCUSABLE_ELEMENTS =
'a[href], details, input, textarea, select, button:not([aria-label^="Close"])';
const FOCUSABLE_ELEMENTS = "a[href], details, input, textarea, select, button";

export function findFocusableNodes(
root: HTMLElement | Document,
Expand Down

0 comments on commit 0955be7

Please sign in to comment.