From c2a53a003fb392b97cd33aa5eea0329e4f726874 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 20 Dec 2023 16:27:43 +0100 Subject: [PATCH] SelectPanel2: Use html dialog (#4020) * copy changes from #4018 * remove undefined values * add autofocus * sync esc with internalClose * move focus logic to only work once * change tooltip direction to stay within input * note for self * add temporary example for question * Revert "add temporary example for question" This reverts commit 19bc49249ce474343fe99744c9b68de168874736. * move comment closer to code * nudge user towards actions when clicking outside * oops * Create eleven-lizards-draw.md * change animation duration to 350ms --- .changeset/eleven-lizards-draw.md | 5 + src/Overlay/Overlay.tsx | 2 +- src/drafts/SelectPanel2/SelectPanel.tsx | 134 +++++++++++++++++------- 3 files changed, 105 insertions(+), 36 deletions(-) create mode 100644 .changeset/eleven-lizards-draw.md diff --git a/.changeset/eleven-lizards-draw.md b/.changeset/eleven-lizards-draw.md new file mode 100644 index 00000000000..824702e7da2 --- /dev/null +++ b/.changeset/eleven-lizards-draw.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +experimental/SelectPanel2: Use `` element diff --git a/src/Overlay/Overlay.tsx b/src/Overlay/Overlay.tsx index 3b1c867fe5d..fb3998392b1 100644 --- a/src/Overlay/Overlay.tsx +++ b/src/Overlay/Overlay.tsx @@ -55,7 +55,7 @@ function getSlideAnimationStartingVector(anchorSide?: AnchorSide): {x: number; y return {x: 0, y: 0} } -const StyledOverlay = styled.div` +export const StyledOverlay = styled.div` background-color: ${get('colors.canvas.overlay')}; box-shadow: ${get('shadows.overlay.shadow')}; position: absolute; diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 1d4db055b46..8c5ed602d75 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -8,8 +8,6 @@ import { IconButton, Heading, Box, - AnchoredOverlay, - AnchoredOverlayProps, Tooltip, TextInput, TextInputProps, @@ -20,8 +18,9 @@ import { } from '../../../src/index' import {ActionListContainerContext} from '../../../src/ActionList/ActionListContainerContext' import {useSlots} from '../../hooks/useSlots' -import {useProvidedRefOrCreate, useId} from '../../hooks' +import {useProvidedRefOrCreate, useId, useAnchoredPosition} from '../../hooks' import {useFocusZone} from '../../hooks/useFocusZone' +import {StyledOverlay, OverlayProps} from '../../Overlay/Overlay' const SelectPanelContext = React.createContext<{ title: string @@ -58,8 +57,8 @@ export type SelectPanelProps = { onSubmit?: (event?: React.FormEvent) => void // TODO: move these to SelectPanel.Overlay or overlayProps - width?: AnchoredOverlayProps['width'] - height?: AnchoredOverlayProps['height'] + width?: OverlayProps['width'] + height?: OverlayProps['height'] children: React.ReactNode } @@ -82,24 +81,38 @@ const Panel: React.FC = ({ height = 'large', ...props }) => { - const anchorRef = useProvidedRefOrCreate(providedAnchorRef) + const [internalOpen, setInternalOpen] = React.useState(defaultOpen) + + // sync open state with props + if (propsOpen !== undefined && internalOpen !== propsOpen) setInternalOpen(propsOpen) + + // TODO: replace this hack with clone element? // 🚨 Hack for good API! - // we strip out Anchor from children and pass it to AnchoredOverlay to render + // we strip out Anchor from children and wire it up to Dialog // with additional props for accessibility - let renderAnchor: AnchoredOverlayProps['renderAnchor'] = null + let Anchor: React.ReactElement | undefined + const anchorRef = useProvidedRefOrCreate(providedAnchorRef) + + const onAnchorClick = () => { + if (!internalOpen) setInternalOpen(true) + else onInternalClose() + } + const contents = React.Children.map(props.children, child => { if (React.isValidElement(child) && child.type === SelectPanelButton) { - renderAnchor = anchorProps => React.cloneElement(child, anchorProps) + Anchor = React.cloneElement(child, { + // @ts-ignore TODO + ref: anchorRef, + onClick: onAnchorClick, + 'aria-haspopup': true, + 'aria-expanded': internalOpen, + }) return null } return child }) - const [internalOpen, setInternalOpen] = React.useState(defaultOpen) - // sync open state - if (propsOpen !== undefined && internalOpen !== propsOpen) setInternalOpen(propsOpen) - const onInternalClose = () => { if (propsOpen === undefined) setInternalOpen(false) if (typeof propsOnCancel === 'function') propsOnCancel() @@ -135,26 +148,77 @@ const Panel: React.FC = ({ [internalOpen], ) + /* Dialog */ + const dialogRef = React.useRef(null) + if (internalOpen) dialogRef.current?.showModal() + else dialogRef.current?.close() + + // dialog handles Esc automatically, so we have to sync internal state + React.useEffect(() => dialogRef.current?.addEventListener('close', onInternalClose)) + + // React doesn't support autoFocus for dialog: https://github.com/facebook/react/issues/23301 + // tl;dr: react takes over autofocus instead of letting the browser handle it, + // but not for dialogs, so we have to do it + React.useEffect(() => { + if (internalOpen) document.querySelector('input')?.focus() + }, [internalOpen]) + + /* Anchored */ + const {position} = useAnchoredPosition( + { + anchorElementRef: anchorRef, + floatingElementRef: dialogRef, + side: 'outside-bottom', + align: 'start', + }, + [anchorRef.current, dialogRef.current], + ) + + /* + We don't close the panel when clicking outside. + For many years, we used to save changes and closed the dialog (for label picker) + which isn't accessible, clicking outside should discard changes and close the dialog + Fixing this a11y bug would confuse users, so as a middle ground, + we don't close the menu and nudge the user towards the footer actions + */ + const [footerAnimationEnabled, setFooterAnimationEnabled] = React.useState(false) + const onClickOutside = () => { + setFooterAnimationEnabled(true) + window.setTimeout(() => setFooterAnimationEnabled(false), 350) + } + return ( <> - setInternalOpen(true)} - onClose={onInternalClose} + {Anchor} + + { + if (event.target === event.currentTarget) onClickOutside() }} > = ({ > = ({ height: '100%', }} > - {/* render default header as fallback */} - {slots.header ?? } + {slots.header ?? /* render default header as fallback */ } + } @@ -209,7 +274,7 @@ const Panel: React.FC = ({ {slots.footer} - + ) } @@ -279,6 +344,7 @@ const SelectPanelHeader: React.FC = ({children, ...prop } const SelectPanelSearchInput: React.FC = ({onChange: propsOnChange, ...props}) => { + // TODO: use forwardedRef const inputRef = React.createRef() const {setSearchQuery} = React.useContext(SelectPanelContext) @@ -292,9 +358,6 @@ const SelectPanelSearchInput: React.FC = ({onChange: propsOnChan return ( = ({onChange: propsOnChan { if (inputRef.current) inputRef.current.value = '' @@ -349,7 +413,7 @@ const SelectPanelFooter = ({...props}) => { {props.children} {hidePrimaryActions ? null : ( - +