From 6067c8747e5d8a0bf6bcdf7a71cb5f482e5deee4 Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Thu, 7 Sep 2023 09:29:04 +0000 Subject: [PATCH 1/2] fix(modal): handle events and focus --- .changeset/four-lizards-do.md | 5 ++ packages/ui/src/components/Modal/Dialog.tsx | 76 ++++++++++++++++++- .../ui/src/components/Modal/Disclosure.tsx | 6 ++ .../components/Modal/__tests__/index.test.tsx | 5 -- 4 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 .changeset/four-lizards-do.md diff --git a/.changeset/four-lizards-do.md b/.changeset/four-lizards-do.md new file mode 100644 index 0000000000..e8a8459773 --- /dev/null +++ b/.changeset/four-lizards-do.md @@ -0,0 +1,5 @@ +--- +'@ultraviolet/ui': patch +--- + +fix(modal): handle events and focus diff --git a/packages/ui/src/components/Modal/Dialog.tsx b/packages/ui/src/components/Modal/Dialog.tsx index 547c05d3fa..f47865deef 100644 --- a/packages/ui/src/components/Modal/Dialog.tsx +++ b/packages/ui/src/components/Modal/Dialog.tsx @@ -1,5 +1,10 @@ import styled from '@emotion/styled' -import type { KeyboardEventHandler, MouseEventHandler } from 'react' +import type { + FocusEventHandler, + KeyboardEventHandler, + MouseEventHandler, + ReactEventHandler, +} from 'react' import { useCallback, useEffect, useRef } from 'react' import { createPortal } from 'react-dom' import { MODAL_PLACEMENT, MODAL_WIDTH } from './constants' @@ -75,6 +80,7 @@ export const Dialog = ({ backdropCss, }: DialogProps) => { const containerRef = useRef(document.createElement('div')) + const dialogRef = useRef(document.createElement('dialog')) const onCloseRef = useRef(onClose) // Portal to put the modal in @@ -82,6 +88,7 @@ export const Dialog = ({ const element = containerRef.current if (open) { document.body.appendChild(element) + dialogRef.current.focus() } return () => { @@ -100,15 +107,25 @@ export const Dialog = ({ useEffect(() => { const handleEscPress = (event: KeyboardEvent) => { if (event.key === 'Escape' && hideOnEsc) { + event.preventDefault() + event.stopPropagation() onCloseRef.current() } } if (open) { - document.addEventListener('keyup', handleEscPress) + document.body.addEventListener('keyup', handleEscPress, { capture: true }) + document.body.addEventListener('keydown', handleEscPress, { + capture: true, + }) } return () => { - document.removeEventListener('keyup', handleEscPress) + document.body.removeEventListener('keyup', handleEscPress, { + capture: true, + }) + document.body.removeEventListener('keydown', handleEscPress, { + capture: true, + }) } }, [open, onCloseRef, hideOnEsc]) @@ -121,6 +138,11 @@ export const Dialog = ({ } }, [preventBodyScroll, open]) + // Stop focus to prevent unexpected body loose focus + const stopFocus: FocusEventHandler = useCallback(event => { + event.stopPropagation() + }, []) + // Stop click to prevent unexpected dialog close const stopClick: MouseEventHandler = useCallback(event => { event.stopPropagation() @@ -131,6 +153,49 @@ export const Dialog = ({ event.stopPropagation() }, []) + // Enable focus trap inside the modal + const handleFocusTrap: KeyboardEventHandler = useCallback(event => { + event.stopPropagation() + if (event.key === 'Escape') { + event.preventDefault() + + return + } + const isTabPressed = event.key === 'Tab' + + if (!isTabPressed) { + return + } + + const focusableEls = dialogRef.current.querySelectorAll( + 'a[href]:not([disabled]), button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled])', + ) + + // Handle case when no interactive element are within the modal (including close icon) + if (focusableEls.length === 0) { + event.preventDefault() + } + + const firstFocusableEl = focusableEls[0] as HTMLElement + const lastFocusableEl = focusableEls[focusableEls.length - 1] as HTMLElement + + if (event.shiftKey) { + if (document.activeElement === firstFocusableEl) { + lastFocusableEl.focus() + event.preventDefault() + } + } else if (document.activeElement === lastFocusableEl) { + firstFocusableEl.focus() + event.preventDefault() + } + }, []) + + // Prevent default behaviour on Escape + const stopCancel: ReactEventHandler = event => { + event.preventDefault() + event.stopPropagation() + } + return createPortal( {open ? children : null} diff --git a/packages/ui/src/components/Modal/Disclosure.tsx b/packages/ui/src/components/Modal/Disclosure.tsx index ed7139b992..29dce88f76 100644 --- a/packages/ui/src/components/Modal/Disclosure.tsx +++ b/packages/ui/src/components/Modal/Disclosure.tsx @@ -21,6 +21,12 @@ export const Disclosure = ({ } }, [handleOpen, disclosureRef]) + useEffect(() => { + if (!visible) { + disclosureRef.current?.focus() + } + }, [visible, disclosureRef]) + if (typeof disclosure === 'function') { return disclosure({ visible, diff --git a/packages/ui/src/components/Modal/__tests__/index.test.tsx b/packages/ui/src/components/Modal/__tests__/index.test.tsx index 7fa6c1ac59..77b90e33a2 100644 --- a/packages/ui/src/components/Modal/__tests__/index.test.tsx +++ b/packages/ui/src/components/Modal/__tests__/index.test.tsx @@ -14,7 +14,6 @@ import { renderWithTheme, shouldMatchEmotionSnapshotWithPortal, } from '../../../../.jest/helpers' -import { TextInput } from '../../TextInput' const customDialogBackdropStyles = css` background-color: aliceblue; @@ -279,12 +278,8 @@ describe('Modal', () => { opened >
test
- , ) - await userEvent.type(screen.getByRole('textbox'), 'test{Escape}') - await userEvent.keyboard('{Escape}') - expect(mockOnClose).toBeCalledTimes(0) await userEvent.click(screen.getByRole('dialog')) await userEvent.keyboard('{Escape}') From e19dd0627fc7608b61a09bf04e77d4bde1a8c2fd Mon Sep 17 00:00:00 2001 From: Dorian Maliszewski Date: Thu, 7 Sep 2023 12:12:28 +0000 Subject: [PATCH 2/2] fix: feedback --- packages/ui/src/components/Modal/Dialog.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/ui/src/components/Modal/Dialog.tsx b/packages/ui/src/components/Modal/Dialog.tsx index f47865deef..a7bdf0f22c 100644 --- a/packages/ui/src/components/Modal/Dialog.tsx +++ b/packages/ui/src/components/Modal/Dialog.tsx @@ -80,7 +80,7 @@ export const Dialog = ({ backdropCss, }: DialogProps) => { const containerRef = useRef(document.createElement('div')) - const dialogRef = useRef(document.createElement('dialog')) + const dialogRef = useRef(null) const onCloseRef = useRef(onClose) // Portal to put the modal in @@ -88,7 +88,7 @@ export const Dialog = ({ const element = containerRef.current if (open) { document.body.appendChild(element) - dialogRef.current.focus() + dialogRef.current?.focus() } return () => { @@ -167,9 +167,10 @@ export const Dialog = ({ return } - const focusableEls = dialogRef.current.querySelectorAll( - 'a[href]:not([disabled]), button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled])', - ) + const focusableEls = + dialogRef.current?.querySelectorAll( + 'a[href]:not([disabled]), button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled])', + ) ?? [] // Handle case when no interactive element are within the modal (including close icon) if (focusableEls.length === 0) {