From 2d36acfd7bd923d48468982480425bab150046fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arma=C4=9Fan?= Date: Thu, 21 Dec 2023 13:36:10 +1000 Subject: [PATCH] Revert "SelectPanel2: Use html dialog (#4020)" This reverts commit c2a53a003fb392b97cd33aa5eea0329e4f726874. --- .changeset/eleven-lizards-draw.md | 5 - src/Overlay/Overlay.tsx | 2 +- src/drafts/SelectPanel2/SelectPanel.tsx | 134 +++++++----------------- 3 files changed, 36 insertions(+), 105 deletions(-) delete mode 100644 .changeset/eleven-lizards-draw.md diff --git a/.changeset/eleven-lizards-draw.md b/.changeset/eleven-lizards-draw.md deleted file mode 100644 index 824702e7da2..00000000000 --- a/.changeset/eleven-lizards-draw.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@primer/react": patch ---- - -experimental/SelectPanel2: Use `` element diff --git a/src/Overlay/Overlay.tsx b/src/Overlay/Overlay.tsx index fb3998392b1..3b1c867fe5d 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} } -export const StyledOverlay = styled.div` +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 8c5ed602d75..1d4db055b46 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -8,6 +8,8 @@ import { IconButton, Heading, Box, + AnchoredOverlay, + AnchoredOverlayProps, Tooltip, TextInput, TextInputProps, @@ -18,9 +20,8 @@ import { } from '../../../src/index' import {ActionListContainerContext} from '../../../src/ActionList/ActionListContainerContext' import {useSlots} from '../../hooks/useSlots' -import {useProvidedRefOrCreate, useId, useAnchoredPosition} from '../../hooks' +import {useProvidedRefOrCreate, useId} from '../../hooks' import {useFocusZone} from '../../hooks/useFocusZone' -import {StyledOverlay, OverlayProps} from '../../Overlay/Overlay' const SelectPanelContext = React.createContext<{ title: string @@ -57,8 +58,8 @@ export type SelectPanelProps = { onSubmit?: (event?: React.FormEvent) => void // TODO: move these to SelectPanel.Overlay or overlayProps - width?: OverlayProps['width'] - height?: OverlayProps['height'] + width?: AnchoredOverlayProps['width'] + height?: AnchoredOverlayProps['height'] children: React.ReactNode } @@ -81,38 +82,24 @@ const Panel: React.FC = ({ height = 'large', ...props }) => { - 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? + const anchorRef = useProvidedRefOrCreate(providedAnchorRef) // 🚨 Hack for good API! - // we strip out Anchor from children and wire it up to Dialog + // we strip out Anchor from children and pass it to AnchoredOverlay to render // with additional props for accessibility - let Anchor: React.ReactElement | undefined - const anchorRef = useProvidedRefOrCreate(providedAnchorRef) - - const onAnchorClick = () => { - if (!internalOpen) setInternalOpen(true) - else onInternalClose() - } - + let renderAnchor: AnchoredOverlayProps['renderAnchor'] = null const contents = React.Children.map(props.children, child => { if (React.isValidElement(child) && child.type === SelectPanelButton) { - Anchor = React.cloneElement(child, { - // @ts-ignore TODO - ref: anchorRef, - onClick: onAnchorClick, - 'aria-haspopup': true, - 'aria-expanded': internalOpen, - }) + renderAnchor = anchorProps => React.cloneElement(child, anchorProps) 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() @@ -148,77 +135,26 @@ 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 ( <> - {Anchor} - - setInternalOpen(true)} + onClose={onInternalClose} width={width} height={height} - sx={{ - ...position, - // reset dialog default styles - border: 'none', - padding: 0, - margin: 0, - '::backdrop': {background: 'transparent'}, - - '& [data-selectpanel-primary-actions]': { - animation: footerAnimationEnabled ? 'selectpanel-gelatine 350ms linear' : 'none', - }, - '@keyframes selectpanel-gelatine': { - '0%': {transform: 'scale(1, 1)'}, - '25%': {transform: 'scale(0.9, 1.1)'}, - '50%': {transform: 'scale(1.1, 0.9)'}, - '75%': {transform: 'scale(0.95, 1.05)'}, - '100%': {transform: 'scale(1, 1)'}, - }, + focusZoneSettings={{ + // we only want focus trap from the overlay, + // we don't want focus zone on the whole overlay because + // we have a focus zone on the list + disabled: true, }} - onClick={event => { - if (event.target === event.currentTarget) onClickOutside() + overlayProps={{ + role: 'dialog', + 'aria-labelledby': `${panelId}--title`, + 'aria-describedby': description ? `${panelId}--description` : undefined, }} > = ({ > = ({ height: '100%', }} > - {slots.header ?? /* render default header as fallback */ } - + {/* render default header as fallback */} + {slots.header ?? } } @@ -274,7 +209,7 @@ const Panel: React.FC = ({ {slots.footer} - + ) } @@ -344,7 +279,6 @@ 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) @@ -358,6 +292,9 @@ const SelectPanelSearchInput: React.FC = ({onChange: propsOnChan return ( = ({onChange: propsOnChan { if (inputRef.current) inputRef.current.value = '' @@ -413,7 +349,7 @@ const SelectPanelFooter = ({...props}) => { {props.children} {hidePrimaryActions ? null : ( - +