diff --git a/.changeset/bright-terms-listen.md b/.changeset/bright-terms-listen.md new file mode 100644 index 00000000000..63982b7dd0c --- /dev/null +++ b/.changeset/bright-terms-listen.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Update useOpenAndCloseFocus hook to apply return focus when preventFocusOnOpen prop is given diff --git a/packages/react/src/Overlay/Overlay.dev.stories.tsx b/packages/react/src/Overlay/Overlay.dev.stories.tsx index 1f848d4d6c2..abcb86d6992 100644 --- a/packages/react/src/Overlay/Overlay.dev.stories.tsx +++ b/packages/react/src/Overlay/Overlay.dev.stories.tsx @@ -89,3 +89,145 @@ export const SxProps = (args: Args) => { ) } + +export const PreventFocusOnOpen = (args: Args) => { + const [isOpen, setIsOpen] = useState(false) + const openButtonRef = useRef(null) + const confirmButtonRef = useRef(null) + const anchorRef = useRef(null) + const closeOverlay = () => setIsOpen(false) + const containerRef = useRef(null) + + return ( + + + {isOpen || args.open ? ( + + + + + + Are you sure? + + + + + + + + + ) : null} + + ) +} +PreventFocusOnOpen.args = { + width: 'auto', + height: 'auto', + side: 'outside-bottom', + preventOverflow: 'false', + role: 'dialog', + visibility: 'visible', + open: false, + preventFocusOnOpen: false, +} +PreventFocusOnOpen.argTypes = { + width: { + type: { + name: 'enum', + value: ['small', 'medium', 'large', 'xlarge', 'xxlarge', 'auto'], + }, + }, + height: { + type: { + name: 'enum', + value: ['xsmall', 'small', 'medium', 'large', 'xlarge', 'auto', 'initial'], + }, + }, + side: { + type: { + name: 'enum', + value: [ + 'inside-top', + 'inside-bottom', + 'inside-left', + 'inside-right', + 'inside-center', + 'outside-top', + 'outside-bottom', + 'outside-left', + 'outside-right', + ], + }, + }, + open: { + control: false, + visible: false, + }, + portalContainerName: { + control: false, + }, + style: { + control: false, + }, + preventOverflow: { + type: 'boolean', + }, + role: { + type: 'string', + }, + visibility: { + type: { + name: 'enum', + value: ['visible', 'hidden'], + }, + }, + preventFocusOnOpen: { + type: 'boolean', + }, +} diff --git a/packages/react/src/Overlay/Overlay.test.tsx b/packages/react/src/Overlay/Overlay.test.tsx index 60d3d71b918..f52ebaba446 100644 --- a/packages/react/src/Overlay/Overlay.test.tsx +++ b/packages/react/src/Overlay/Overlay.test.tsx @@ -13,10 +13,16 @@ type TestComponentSettings = { initialFocus?: 'button' width?: 'small' | 'medium' | 'large' | 'auto' | 'xlarge' | 'xxlarge' callback?: () => void + preventFocusOnOpen?: boolean } -const TestComponent = ({initialFocus, width = 'small', callback}: TestComponentSettings) => { +const TestComponent = ({ + initialFocus, + width = 'small', + preventFocusOnOpen = undefined, + callback, +}: TestComponentSettings) => { const [isOpen, setIsOpen] = useState(false) - const buttonRef = useRef(null) + const openButtonRef = useRef(null) const confirmButtonRef = useRef(null) const anchorRef = useRef(null) const closeOverlay = () => { @@ -29,18 +35,20 @@ const TestComponent = ({initialFocus, width = 'small', callback}: TestComponentS - {isOpen ? ( Are you sure? @@ -66,7 +74,7 @@ describe('Overlay', () => { expect(results).toHaveNoViolations() }) - it('should focus element passed into function', async () => { + it('should focus initialFocusRef element passed into function on open', async () => { const user = userEvent.setup() const {getByRole} = render() await user.click(getByRole('button', {name: 'open overlay'})) @@ -75,7 +83,7 @@ describe('Overlay', () => { expect(document.activeElement).toEqual(confirmButton) }) - it('should focus first element when nothing is passed', async () => { + it('should focus first element on open when no initialFocusRef is passed', async () => { const user = userEvent.setup() const {getByRole} = render() await user.click(getByRole('button', {name: 'open overlay'})) @@ -84,6 +92,53 @@ describe('Overlay', () => { expect(document.activeElement).toEqual(cancelButton) }) + it('should not focus any element within the overlay on open when preventFocusOnOpen prop is true', async () => { + const user = userEvent.setup() + const {getByRole} = render() + await user.click(getByRole('button', {name: 'open overlay'})) + + const overlayContainer = getByRole('dialog') + const focusedElement = document.activeElement! as HTMLElement + expect(focusedElement).toBeInTheDocument() + expect(overlayContainer).not.toContainElement(focusedElement) + }) + + it('should focus returnFocusRef element on close', async () => { + const user = userEvent.setup() + const {getByRole} = render() + + // Open overlay + await waitFor(() => getByRole('button', {name: 'open overlay'})) + const openButton = getByRole('button', {name: 'open overlay'}) + await user.click(openButton) + + // Close overlay + await waitFor(() => getByRole('button', {name: 'Cancel'})) + const cancelButton = getByRole('button', {name: 'Cancel'}) + await user.click(cancelButton) + + // Focus should return to button that was originally clicked to open overlay + expect(document.activeElement).toEqual(openButton) + }) + + it('should focus returnFocusRef element on close when preventFocusOnOpen prop is true', async () => { + const user = userEvent.setup() + const {getByRole} = render() + + // Open overlay + await waitFor(() => getByRole('button', {name: 'open overlay'})) + const openButton = getByRole('button', {name: 'open overlay'}) + await user.click(openButton) + + // Close overlay + await waitFor(() => getByRole('button', {name: 'Cancel'})) + const cancelButton = getByRole('button', {name: 'Cancel'}) + await user.click(cancelButton) + + // Focus should return to button that was originally clicked to open overlay + expect(document.activeElement).toEqual(openButton) + }) + it('should call function when user clicks outside container', async () => { const user = userEvent.setup() const mockFunction = jest.fn() @@ -296,7 +351,7 @@ describe('Overlay', () => { await user.click(getByRole('button', {name: 'open overlay'})) - const container = getByRole('none') + const container = getByRole('dialog') expect(container).not.toHaveAttribute('data-reflow-container') }) @@ -310,7 +365,7 @@ describe('Overlay', () => { await user.click(getByRole('button', {name: 'open overlay'})) - const container = getByRole('none') + const container = getByRole('dialog') expect(container).toHaveAttribute('data-reflow-container') }) }) diff --git a/packages/react/src/Overlay/Overlay.tsx b/packages/react/src/Overlay/Overlay.tsx index 6d111c5a8aa..7aad4b3a3d5 100644 --- a/packages/react/src/Overlay/Overlay.tsx +++ b/packages/react/src/Overlay/Overlay.tsx @@ -232,6 +232,7 @@ type internalOverlayProps = Merge * @param onEscape Required. Function to call when user presses `Escape`. Typically this function removes the Overlay. * @param portalContainerName Optional. The name of the portal container to render the Overlay into. * @param preventOverflow Optional. The Overlay width will be adjusted responsively if there is not enough space to display the Overlay. If `preventOverflow` is `true`, the width of the `Overlay` will not be adjusted. + * @param preventFocusOnOpen Optional. If 'true', focus will not be applied when the component is first mounted, even if initialFocusRef prop is given. * @param returnFocusRef Required. Ref for the element to focus when the `Overlay` is closed. * @param right Optional. Horizontal right position of the overlay, relative to its closest positioned ancestor (often its `Portal`). * @param width Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `480px`, `xlarge` corresponds to `640px`, `xxlarge` corresponds to `960px`. diff --git a/packages/react/src/__tests__/hooks/useOpenAndCloseFocus.test.tsx b/packages/react/src/__tests__/hooks/useOpenAndCloseFocus.test.tsx index a05421af1c5..bc80623196a 100644 --- a/packages/react/src/__tests__/hooks/useOpenAndCloseFocus.test.tsx +++ b/packages/react/src/__tests__/hooks/useOpenAndCloseFocus.test.tsx @@ -1,5 +1,6 @@ -import React, {useRef} from 'react' +import React, {useRef, useState} from 'react' import {render, waitFor} from '@testing-library/react' +import userEvent from '@testing-library/user-event' import {useOpenAndCloseFocus} from '../../hooks/useOpenAndCloseFocus' const Component = () => { @@ -39,16 +40,59 @@ const ComponentTwo = () => { ) } -it('should focus element passed into function', async () => { +const ComponentThree = () => { + const [isOpen, setIsOpen] = useState(true) + + const containerRef = useRef(null) + const returnFocusRef = useRef(null) + const noButtonRef = useRef(null) + useOpenAndCloseFocus({containerRef, initialFocusRef: noButtonRef, returnFocusRef, preventFocusOnOpen: true}) + + return ( + <> + + {isOpen && ( +
+ + +
+ )} + + ) +} + +it('should focus initialFocusRef element passed into function', async () => { const {getByText} = render() await waitFor(() => getByText('no')) const noButton = getByText('no') expect(document.activeElement).toEqual(noButton) }) -it('should focus first element when nothing is passed', async () => { +it('should focus first element when no initialFocusRef prop is passed', async () => { const {getByText} = render() await waitFor(() => getByText('yes')) const yesButton = getByText('yes') expect(document.activeElement).toEqual(yesButton) }) + +it('should not focus any element if preventFocusOnOpen prop is passed', async () => { + render() + expect(document.activeElement).toEqual(document.body) +}) + +it('should focus returnFocusRef element when rendered', async () => { + const user = userEvent.setup() + const {getByText} = render() + + await waitFor(() => getByText('toggle')) + const toggleButton = getByText('toggle') + + // Close container, so containerRef and initialFocusRef elements are no longer rendered + await user.click(toggleButton) + + expect(document.activeElement).toEqual(toggleButton) +}) diff --git a/packages/react/src/hooks/useOpenAndCloseFocus.ts b/packages/react/src/hooks/useOpenAndCloseFocus.ts index ca5d15d68c9..06cccfe0f2f 100644 --- a/packages/react/src/hooks/useOpenAndCloseFocus.ts +++ b/packages/react/src/hooks/useOpenAndCloseFocus.ts @@ -16,18 +16,21 @@ export function useOpenAndCloseFocus({ preventFocusOnOpen, }: UseOpenAndCloseFocusSettings): void { useEffect(() => { - if (preventFocusOnOpen) { - return - } - const returnRef = returnFocusRef.current - if (initialFocusRef && initialFocusRef.current) { - initialFocusRef.current.focus() - } else if (containerRef.current) { - const firstItem = iterateFocusableElements(containerRef.current).next().value - firstItem?.focus() + // If focus should be applied on open, apply focus to correct element, + // either the initialFocusRef if given, otherwise the first focusable element + if (!preventFocusOnOpen) { + if (initialFocusRef && initialFocusRef.current) { + initialFocusRef.current.focus() + } else if (containerRef.current) { + const firstItem = iterateFocusableElements(containerRef.current).next().value + firstItem?.focus() + } } + + // If returnFocusRef element is rendered, apply focus + const returnFocusRefCurrent = returnFocusRef.current return function () { - returnRef?.focus() + returnFocusRefCurrent?.focus() } }, [initialFocusRef, returnFocusRef, containerRef, preventFocusOnOpen]) }