From f71d3101d6789bd5a45eac390d16f7fb1f550e8b Mon Sep 17 00:00:00 2001 From: Yossi Saadi Date: Tue, 21 Jan 2025 19:50:10 +0200 Subject: [PATCH 1/7] feat(Modal): allow disabling autoFocus --- packages/core/src/components/Modal/Modal/Modal.tsx | 3 ++- packages/core/src/components/Modal/Modal/Modal.types.tsx | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/Modal/Modal/Modal.tsx b/packages/core/src/components/Modal/Modal/Modal.tsx index 9f313b4097..6dd14a878e 100644 --- a/packages/core/src/components/Modal/Modal/Modal.tsx +++ b/packages/core/src/components/Modal/Modal/Modal.tsx @@ -37,6 +37,7 @@ const Modal = forwardRef( closeButtonTheme, closeButtonAriaLabel, onClose = () => {}, + autoFocus = true, anchorElementRef, alertModal, container = document.body, @@ -114,7 +115,7 @@ const Modal = forwardRef( {createPortal( - +
void; + /** + * Determines if focus should automatically move to the first focusable element when the component mounts. + * When set to `false` - disables the automatic focus behavior. + */ + autoFocus?: boolean; /** * Additional action to render in the header area. */ From 1ed3acdc5914a0a8b0d1df08af661f8419f155f4 Mon Sep 17 00:00:00 2001 From: Yossi Saadi Date: Sun, 26 Jan 2025 13:24:46 +0200 Subject: [PATCH 2/7] feat(Modal): pass autoFocus value to ModalContent to prevent autofocus on content --- .../core/src/components/Modal/ModalContent/ModalContent.tsx | 4 +++- .../core/src/components/Modal/context/ModalContext.types.ts | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/Modal/ModalContent/ModalContent.tsx b/packages/core/src/components/Modal/ModalContent/ModalContent.tsx index 8f5812d4cd..69ca27f6fd 100644 --- a/packages/core/src/components/Modal/ModalContent/ModalContent.tsx +++ b/packages/core/src/components/Modal/ModalContent/ModalContent.tsx @@ -2,19 +2,21 @@ import React, { forwardRef } from "react"; import { getTestId } from "../../../tests/test-ids-utils"; import { ComponentDefaultTestId } from "../../../tests/constants"; import { ModalContentProps } from "./ModalContent.types"; +import { useModal } from "../context/ModalContext"; const ModalContent = forwardRef( ( { children, className, id, "data-testid": dataTestId }: ModalContentProps, ref: React.ForwardedRef ) => { + const { autoFocus } = useModal(); return (
{children}
diff --git a/packages/core/src/components/Modal/context/ModalContext.types.ts b/packages/core/src/components/Modal/context/ModalContext.types.ts index 4903b75bbd..42b6d1eda4 100644 --- a/packages/core/src/components/Modal/context/ModalContext.types.ts +++ b/packages/core/src/components/Modal/context/ModalContext.types.ts @@ -16,6 +16,10 @@ export type ModalProviderValue = { * Callback to set the description element ID for accessibility. */ setDescriptionId: (id: string) => void; + /** + * Passed from the Modal component, `true` by default, `false` if set by user. + */ + autoFocus?: boolean; }; export interface ModalProviderProps { From e5a6387d7cfb0280dc518ad25ac01c84bc375472 Mon Sep 17 00:00:00 2001 From: Yossi Saadi Date: Sun, 26 Jan 2025 13:26:00 +0200 Subject: [PATCH 3/7] feat(Modal): new autoFocus and onFocusAttempt functionalities to manage focus --- .../core/src/components/Modal/Modal/Modal.tsx | 26 +++++- .../components/Modal/Modal/Modal.types.tsx | 19 ++++ .../Modal/Modal/__tests__/Modal.test.tsx | 90 +++++++++++++++++++ 3 files changed, 132 insertions(+), 3 deletions(-) diff --git a/packages/core/src/components/Modal/Modal/Modal.tsx b/packages/core/src/components/Modal/Modal/Modal.tsx index 6dd14a878e..11e0e8d0ce 100644 --- a/packages/core/src/components/Modal/Modal/Modal.tsx +++ b/packages/core/src/components/Modal/Modal/Modal.tsx @@ -38,6 +38,7 @@ const Modal = forwardRef( closeButtonAriaLabel, onClose = () => {}, autoFocus = true, + onFocusAttempt, anchorElementRef, alertModal, container = document.body, @@ -79,9 +80,10 @@ const Modal = forwardRef( () => ({ modalId: id, setTitleId: setTitleIdCallback, - setDescriptionId: setDescriptionIdCallback + setDescriptionId: setDescriptionIdCallback, + autoFocus }), - [id, setTitleIdCallback, setDescriptionIdCallback] + [id, setTitleIdCallback, setDescriptionIdCallback, autoFocus] ); const onBackdropClick = useCallback>( @@ -109,13 +111,31 @@ const Modal = forwardRef( const zIndexStyle = zIndex ? ({ "--monday-modal-z-index": zIndex } as React.CSSProperties) : {}; + const handleFocusLockWhiteList = useCallback( + (nextFocusedElement?: HTMLElement) => { + if (!onFocusAttempt) return true; + + const outcome = onFocusAttempt(nextFocusedElement); + + if (outcome === true) return true; + + if (outcome instanceof HTMLElement) { + outcome.focus(); + return false; + } + + return false; + }, + [onFocusAttempt] + ); + return ( {show && ( {createPortal( - +
void; /** + * This is intended for advanced use-cases. + * It allows you to control the default focus behavior when the modal mounts. + * Make sure to use this prop only when you understand the implications. + * * Determines if focus should automatically move to the first focusable element when the component mounts. * When set to `false` - disables the automatic focus behavior. + * - Notice this might break keyboard and general accessibility and should be used with caution. */ autoFocus?: boolean; + /** + * This is intended for advanced use-cases. + * It allows you to control the focus behavior when moving between elements within the modal. + * Make sure to use this prop only when you understand the implications. + * + * Called whenever focus is about to move to a new element within the modal. + * Return: + * - `true` to allow normal flow focus. + * - `false` to block it (let the browser decide, usually moves to body). + * - Notice this might break keyboard accessibility and should be used with caution. + * - An HTMLElement to redirect focus to instead of normal flow. + * - Any other value (e.g., null, undefined) would act as `false`. + */ + onFocusAttempt?: (nextFocusedElement?: HTMLElement) => boolean | HTMLElement; /** * Additional action to render in the header area. */ diff --git a/packages/core/src/components/Modal/Modal/__tests__/Modal.test.tsx b/packages/core/src/components/Modal/Modal/__tests__/Modal.test.tsx index 249328e2d4..1947fe2cc0 100644 --- a/packages/core/src/components/Modal/Modal/__tests__/Modal.test.tsx +++ b/packages/core/src/components/Modal/Modal/__tests__/Modal.test.tsx @@ -241,6 +241,78 @@ describe("Modal", () => { expect(getByText("Focusable 1")).toHaveFocus(); }); + it("should allow focus by default if onFocusAttempt is not provided", () => { + const { getByText, getByLabelText } = render( + <> + + + + + + + ); + + expect(getByText("Focusable 1")).toHaveFocus(); + + userEvent.tab(); + expect(getByText("Focusable 2")).toHaveFocus(); + + userEvent.tab(); + expect(getByLabelText(closeButtonAriaLabel)).toHaveFocus(); + }); + + it("should block focus if onFocusAttempt returns HTMLElement", () => { + const modalRef = React.createRef(); + const onFocusAttemptMock = jest.fn(nextFocusedElement => { + return nextFocusedElement.textContent === "Focusable 2" ? modalRef.current : true; + }); + + const { getByText, getByRole } = render( + <> + + + + + + + ); + + expect(getByText("Focusable 1")).toHaveFocus(); + + userEvent.tab(); + expect(onFocusAttemptMock).toHaveBeenCalledWith(getByText("Focusable 2")); + // initial + focusable 1 + ignored focusable 2 + enforced modal + expect(onFocusAttemptMock).toHaveBeenCalledTimes(4); + expect(getByRole("dialog")).toHaveFocus(); + }); + + it("should not auto-focus any modal content when autoFocus is false", () => { + const outsideButtonRef = React.createRef(); + const { getByText } = render( + <> + + + + + + ); + + expect(document.body).toHaveFocus(); + userEvent.tab(); + expect(document.body).toHaveFocus(); + + userEvent.tab(); + expect(getByText("Focusable 1")).toHaveFocus(); + }); + describe("integrated with ModalContent", () => { it("should trap and moves focus to focusable element inside ModalContent and to cycle through full focus flow", () => { const { getByLabelText, getByText } = render( @@ -266,6 +338,24 @@ describe("Modal", () => { userEvent.tab(); expect(getByText("Focusable inside ModalContent")).toHaveFocus(); }); + + it("should pass autoFocus prop value to ModalContent's data-autofocus-inside attribute", () => { + const { getByTestId, rerender } = render( + + Some content + + ); + + expect(getByTestId("modal-content")).toHaveAttribute("data-autofocus-inside", "true"); + + rerender( + + Some content + + ); + + expect(getByTestId("modal-content")).toHaveAttribute("data-autofocus-inside", "false"); + }); }); describe("integrated with ModalHeader", () => { From a3fe2da2ee9fc6ae8205a06a8d270cf8a4178fe1 Mon Sep 17 00:00:00 2001 From: Yossi Saadi Date: Sun, 26 Jan 2025 13:28:50 +0200 Subject: [PATCH 4/7] trigger [prerelease] From 0f7699fb5cce722f74945cf90b4965565bf62c3d Mon Sep 17 00:00:00 2001 From: Yossi Saadi Date: Sun, 26 Jan 2025 18:37:31 +0200 Subject: [PATCH 5/7] feat(Modal): add responsiveness for modal --- .../components/Modal/Modal/Modal.module.scss | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/Modal/Modal/Modal.module.scss b/packages/core/src/components/Modal/Modal/Modal.module.scss index f390d2001a..15c36fb34c 100644 --- a/packages/core/src/components/Modal/Modal/Modal.module.scss +++ b/packages/core/src/components/Modal/Modal/Modal.module.scss @@ -61,10 +61,44 @@ $full-view-margin: 40px; inset: 0; height: calc(100% - $full-view-margin); - margin-inline: $full-view-margin; + margin-inline: var(--spacing-large); margin-block-start: $full-view-margin; border-end-start-radius: unset; border-end-end-radius: unset; } + + @media (min-width: 1280px) { + &.sizeSmall { + --modal-width: 480px; + } + + &.sizeMedium { + --modal-width: 580px; + } + + &.sizeLarge { + --modal-width: 840px; + } + } + + @media (min-width: 1440px) { + &.sizeSmall { + --modal-width: 520px; + } + + &.sizeMedium { + --modal-width: 620px; + } + + &.sizeLarge { + --modal-width: 900px; + } + } + + @media (min-width: 1720px) { + &.sizeFullView { + margin-inline: $full-view-margin; + } + } } From ae05b99a246462d975a8c73ea2062a8832ffbd7f Mon Sep 17 00:00:00 2001 From: Yossi Saadi Date: Sun, 26 Jan 2025 18:37:40 +0200 Subject: [PATCH 6/7] docs(Modal): add responsiveness for modal --- .../__stories__/Modal.stories.module.scss | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/core/src/components/Modal/Modal/__stories__/Modal.stories.module.scss b/packages/core/src/components/Modal/Modal/__stories__/Modal.stories.module.scss index 8805e69297..df487fb54a 100644 --- a/packages/core/src/components/Modal/Modal/__stories__/Modal.stories.module.scss +++ b/packages/core/src/components/Modal/Modal/__stories__/Modal.stories.module.scss @@ -63,5 +63,38 @@ --modal-max-height: 80%; --modal-width: 840px; } + + &[class*="sizeFullView"] { + --modal-max-height: 100%; + --modal-width: auto; + } + + @container (min-width: 1280px) { + &[class*="sizeSmall"] { + --modal-width: 480px; + } + + &[class*="sizeMedium"] { + --modal-width: 580px; + } + + &[class*="sizeLarge"] { + --modal-width: 840px; + } + } + + @container (min-width: 1440px) { + &[class*="sizeSmall"] { + --modal-width: 520px; + } + + &[class*="sizeMedium"] { + --modal-width: 620px; + } + + &[class*="sizeLarge"] { + --modal-width: 900px; + } + } } } From 47e842cb8789fa7263a1c49d20cb21cc94ed6869 Mon Sep 17 00:00:00 2001 From: Yossi Saadi Date: Sun, 26 Jan 2025 18:56:44 +0200 Subject: [PATCH 7/7] feat(Modal): fix initial values to fit design (0-1280px) --- packages/core/src/components/Modal/Modal/Modal.module.scss | 6 +++--- .../Modal/Modal/__stories__/Modal.stories.module.scss | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core/src/components/Modal/Modal/Modal.module.scss b/packages/core/src/components/Modal/Modal/Modal.module.scss index 15c36fb34c..69e99f2360 100644 --- a/packages/core/src/components/Modal/Modal/Modal.module.scss +++ b/packages/core/src/components/Modal/Modal/Modal.module.scss @@ -39,17 +39,17 @@ $full-view-margin: 40px; &.sizeSmall { --modal-max-height: 50%; - --modal-width: 480px; + --modal-width: 460px; } &.sizeMedium { --modal-max-height: 80%; - --modal-width: 580px; + --modal-width: 540px; } &.sizeLarge { --modal-max-height: 80%; - --modal-width: 840px; + --modal-width: 800px; } &.sizeFullView { diff --git a/packages/core/src/components/Modal/Modal/__stories__/Modal.stories.module.scss b/packages/core/src/components/Modal/Modal/__stories__/Modal.stories.module.scss index df487fb54a..391f4191ee 100644 --- a/packages/core/src/components/Modal/Modal/__stories__/Modal.stories.module.scss +++ b/packages/core/src/components/Modal/Modal/__stories__/Modal.stories.module.scss @@ -51,17 +51,17 @@ [aria-modal][role="dialog"] { &[class*="sizeSmall"] { --modal-max-height: 50%; - --modal-width: 480px; + --modal-width: 460px; } &[class*="sizeMedium"] { --modal-max-height: 80%; - --modal-width: 580px; + --modal-width: 540px; } &[class*="sizeLarge"] { --modal-max-height: 80%; - --modal-width: 840px; + --modal-width: 800px; } &[class*="sizeFullView"] {