From 0955be7eadb5b36dff7084bfd0d05d838fc5d919 Mon Sep 17 00:00:00 2001 From: Juan Andrade Date: Wed, 11 Dec 2024 11:05:16 -0500 Subject: [PATCH] WB-1796: Change modal default focus to be the Close (X) button (#2383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. Screenshot 2024-12-06 at 5 35 38 PM 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: https://github.com/Khan/wonder-blocks/pull/2383 --- .changeset/kind-buckets-poke.md | 7 +++++ .changeset/perfect-cameras-protect.md | 5 ++++ .../modal-launcher.argtypes.ts | 3 +- .../src/components/icon-button-core.tsx | 28 ++----------------- .../__tests__/modal-backdrop.test.tsx | 15 +++++++--- .../__tests__/modal-launcher.test.tsx | 13 ++++----- .../src/components/modal-backdrop.tsx | 3 +- .../src/components/modal-launcher.tsx | 3 +- .../src/components/modal-panel.tsx | 11 ++++++++ .../wonder-blocks-modal/src/themes/default.ts | 2 ++ .../src/util/find-focusable-nodes.ts | 3 +- 11 files changed, 50 insertions(+), 43 deletions(-) create mode 100644 .changeset/kind-buckets-poke.md create mode 100644 .changeset/perfect-cameras-protect.md diff --git a/.changeset/kind-buckets-poke.md b/.changeset/kind-buckets-poke.md new file mode 100644 index 000000000..b59dbb28f --- /dev/null +++ b/.changeset/kind-buckets-poke.md @@ -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. \ No newline at end of file diff --git a/.changeset/perfect-cameras-protect.md b/.changeset/perfect-cameras-protect.md new file mode 100644 index 000000000..457031621 --- /dev/null +++ b/.changeset/perfect-cameras-protect.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-icon-button": patch +--- + +Fix focus styles: drop Safari v14 support. diff --git a/__docs__/wonder-blocks-modal/modal-launcher.argtypes.ts b/__docs__/wonder-blocks-modal/modal-launcher.argtypes.ts index 48e1ab57e..5c8851826 100644 --- a/__docs__/wonder-blocks-modal/modal-launcher.argtypes.ts +++ b/__docs__/wonder-blocks-modal/modal-launcher.argtypes.ts @@ -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"}, }, diff --git a/packages/wonder-blocks-icon-button/src/components/icon-button-core.tsx b/packages/wonder-blocks-icon-button/src/components/icon-button-core.tsx index 65fe14342..07b0d872c 100644 --- a/packages/wonder-blocks-icon-button/src/components/icon-button-core.tsx +++ b/packages/wonder-blocks-icon-button/src/components/icon-button-core.tsx @@ -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, @@ -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; diff --git a/packages/wonder-blocks-modal/src/components/__tests__/modal-backdrop.test.tsx b/packages/wonder-blocks-modal/src/components/__tests__/modal-backdrop.test.tsx index f716ae358..def288e42 100644 --- a/packages/wonder-blocks-modal/src/components/__tests__/modal-backdrop.test.tsx +++ b/packages/wonder-blocks-modal/src/components/__tests__/modal-backdrop.test.tsx @@ -197,7 +197,7 @@ describe("ModalBackdrop", () => { ); const firstFocusableElement = await screen.findByRole("button", { - name: "first focusable button", + name: "Close modal", }); // Assert @@ -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( {}}> @@ -217,7 +217,7 @@ describe("ModalBackdrop", () => { // Act const firstFocusableElement = await screen.findByRole("button", { - name: "first focusable button", + name: "Close modal", }); // Assert @@ -228,7 +228,14 @@ describe("ModalBackdrop", () => { // Arrange render( {}}> - {exampleModal} + } + title="Title" + footer={
} + testId="example-modal-test-id" + // Ensure that there are no focusable elements + closeButtonVisible={false} + /> , ); diff --git a/packages/wonder-blocks-modal/src/components/__tests__/modal-launcher.test.tsx b/packages/wonder-blocks-modal/src/components/__tests__/modal-launcher.test.tsx index 97d8185cf..5aa249ab0 100644 --- a/packages/wonder-blocks-modal/src/components/__tests__/modal-launcher.test.tsx +++ b/packages/wonder-blocks-modal/src/components/__tests__/modal-launcher.test.tsx @@ -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( { , ); - 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"); @@ -279,7 +276,7 @@ describe("ModalLauncher", () => { // Assert await waitFor(async () => expect( - await screen.findByRole("button", {name: "Button in modal"}), + await screen.findByRole("button", {name: "Close modal"}), ).toHaveFocus(), ); }); diff --git a/packages/wonder-blocks-modal/src/components/modal-backdrop.tsx b/packages/wonder-blocks-modal/src/components/modal-backdrop.tsx index 808fe50cf..f1196ca3f 100644 --- a/packages/wonder-blocks-modal/src/components/modal-backdrop.tsx +++ b/packages/wonder-blocks-modal/src/components/modal-backdrop.tsx @@ -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; /** diff --git a/packages/wonder-blocks-modal/src/components/modal-launcher.tsx b/packages/wonder-blocks-modal/src/components/modal-launcher.tsx index d4c469aa8..2b5458ab1 100644 --- a/packages/wonder-blocks-modal/src/components/modal-launcher.tsx +++ b/packages/wonder-blocks-modal/src/components/modal-launcher.tsx @@ -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; /** diff --git a/packages/wonder-blocks-modal/src/components/modal-panel.tsx b/packages/wonder-blocks-modal/src/components/modal-panel.tsx index 5c84fac75..842a6352f 100644 --- a/packages/wonder-blocks-modal/src/components/modal-panel.tsx +++ b/packages/wonder-blocks-modal/src/components/modal-panel.tsx @@ -175,6 +175,17 @@ const themedStylesFn: ThemedStylesFn = (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: { diff --git a/packages/wonder-blocks-modal/src/themes/default.ts b/packages/wonder-blocks-modal/src/themes/default.ts index 623c7eb8f..43694a8cd 100644 --- a/packages/wonder-blocks-modal/src/themes/default.ts +++ b/packages/wonder-blocks-modal/src/themes/default.ts @@ -15,6 +15,8 @@ const theme = { }, border: { radius: tokens.border.radius.medium_4, + width: tokens.border.width.thin, + color: tokens.color.blue, }, spacing: { dialog: { diff --git a/packages/wonder-blocks-modal/src/util/find-focusable-nodes.ts b/packages/wonder-blocks-modal/src/util/find-focusable-nodes.ts index 8fcb60625..2f394673a 100644 --- a/packages/wonder-blocks-modal/src/util/find-focusable-nodes.ts +++ b/packages/wonder-blocks-modal/src/util/find-focusable-nodes.ts @@ -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,