From f3eddf6e08aef40a4d0fce569d93eb4a976dc9b4 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 5 Dec 2023 16:38:27 +0100 Subject: [PATCH 01/14] copy changes from #4018 --- src/Overlay/Overlay.tsx | 2 +- src/drafts/SelectPanel2/SelectPanel.tsx | 89 ++++++++++++++++--------- 2 files changed, 60 insertions(+), 31 deletions(-) diff --git a/src/Overlay/Overlay.tsx b/src/Overlay/Overlay.tsx index 817e80ec992..75221744541 100644 --- a/src/Overlay/Overlay.tsx +++ b/src/Overlay/Overlay.tsx @@ -54,7 +54,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..45035c35541 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,36 @@ 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) // 🚨 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 +146,43 @@ const Panel: React.FC = ({ [internalOpen], ) + /* Dialog */ + const dialogRef = React.useRef(null) + if (internalOpen) dialogRef.current?.showModal() + else dialogRef.current?.close() + + /* Anchored */ + const {position} = useAnchoredPosition( + { + anchorElementRef: anchorRef, + floatingElementRef: dialogRef, + side: 'outside-bottom', + align: 'start', + alignmentOffset: undefined, + anchorOffset: undefined, + allowOutOfBounds: undefined, + }, + [anchorRef.current, dialogRef.current], + ) + return ( <> - setInternalOpen(true)} - onClose={onInternalClose} + {Anchor} + + = ({ > = ({ {slots.footer} - + ) } From 262316c0838eb50d54a709abf655b3609547d084 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 7 Dec 2023 14:33:23 +0100 Subject: [PATCH 02/14] remove undefined values --- src/drafts/SelectPanel2/SelectPanel.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 45035c35541..88565ba455c 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -158,9 +158,6 @@ const Panel: React.FC = ({ floatingElementRef: dialogRef, side: 'outside-bottom', align: 'start', - alignmentOffset: undefined, - anchorOffset: undefined, - allowOutOfBounds: undefined, }, [anchorRef.current, dialogRef.current], ) From 907c1a04edeeabc93ec7614b031f86ce9b3d8676 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Fri, 8 Dec 2023 16:09:49 +0100 Subject: [PATCH 03/14] add autofocus --- src/drafts/SelectPanel2/SelectPanel.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 88565ba455c..a0215819af8 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -316,11 +316,15 @@ const SelectPanelSearchInput: React.FC = ({onChange: propsOnChan else setSearchQuery(event.target.value) } + /** + 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 yet, so we have to do it + */ + React.useEffect(() => inputRef.current?.focus(), [inputRef]) + return ( Date: Fri, 8 Dec 2023 16:10:47 +0100 Subject: [PATCH 04/14] sync esc with internalClose --- src/drafts/SelectPanel2/SelectPanel.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index a0215819af8..a34e2096fc0 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -151,6 +151,9 @@ const Panel: React.FC = ({ 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)) + /* Anchored */ const {position} = useAnchoredPosition( { @@ -305,6 +308,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) From 4f5e5bd7babab2b19d4c6204621fc4863d1c0f83 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Fri, 8 Dec 2023 16:47:09 +0100 Subject: [PATCH 05/14] move focus logic to only work once --- src/drafts/SelectPanel2/SelectPanel.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index a34e2096fc0..3aa52bcbde3 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -154,6 +154,13 @@ const Panel: React.FC = ({ // 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( { @@ -320,13 +327,6 @@ const SelectPanelSearchInput: React.FC = ({onChange: propsOnChan else setSearchQuery(event.target.value) } - /** - 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 yet, so we have to do it - */ - React.useEffect(() => inputRef.current?.focus(), [inputRef]) - return ( Date: Fri, 8 Dec 2023 16:50:08 +0100 Subject: [PATCH 06/14] change tooltip direction to stay within input --- src/drafts/SelectPanel2/SelectPanel.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 3aa52bcbde3..38d1e74eb47 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -337,6 +337,7 @@ const SelectPanelSearchInput: React.FC = ({onChange: propsOnChan { if (inputRef.current) inputRef.current.value = '' From ca8f8b6b23cee1c39cc777d6029d2c064f33ad10 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 13 Dec 2023 14:16:22 +0100 Subject: [PATCH 07/14] note for self --- src/drafts/SelectPanel2/SelectPanel.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 38d1e74eb47..ded25c486ea 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -86,6 +86,8 @@ const Panel: React.FC = ({ // 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 wire it up to Dialog // with additional props for accessibility @@ -215,6 +217,7 @@ const Panel: React.FC = ({ }} > {/* render default header as fallback */} + {slots.header ?? } Date: Mon, 18 Dec 2023 16:20:04 +0100 Subject: [PATCH 08/14] add temporary example for question --- .../stories/SelectPanel.examples.stories.tsx | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx index 81698bdb900..0c79468ce54 100644 --- a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx +++ b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx @@ -66,6 +66,61 @@ export const Minimal = () => { ) } +export const Temporary = () => { + const initialSelectedLabels = data.issue.labelIds // mock initial state: has selected labels + const [selectedLabelIds, setSelectedLabelIds] = React.useState(initialSelectedLabels) + + /* Selection */ + const onLabelSelect = (labelId: string) => { + if (!selectedLabelIds.includes(labelId)) setSelectedLabelIds([...selectedLabelIds, labelId]) + else setSelectedLabelIds(selectedLabelIds.filter(id => id !== labelId)) + } + + const onSubmit = () => { + data.issue.labelIds = selectedLabelIds // pretending to persist changes + + // eslint-disable-next-line no-console + console.log('form submitted') + } + + const sortingFn = (itemA: {id: string}, itemB: {id: string}) => { + const initialSelectedIds = data.issue.labelIds + if (initialSelectedIds.includes(itemA.id) && initialSelectedIds.includes(itemB.id)) return 1 + else if (initialSelectedIds.includes(itemA.id)) return -1 + else if (initialSelectedIds.includes(itemB.id)) return 1 + else return 1 + } + + const onClearSelection = () => setSelectedLabelIds([]) + + const itemsToShow = data.labels.sort(sortingFn) + + return ( + <> +

Temporary SelectPanel

+ + + Assign label + + + {itemsToShow.map(label => ( + onLabelSelect(label.id)} + selected={selectedLabelIds.includes(label.id)} + > + {getCircle(label.color)} + {label.name} + {label.description} + + ))} + + + + + ) +} + export const WithGroups = () => { /* Selection */ const initialAssigneeIds = data.issue.assigneeIds // mock initial state From 5de27294e9981a90e5dc6dbe0aaa1347758190a4 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 18 Dec 2023 16:33:22 +0100 Subject: [PATCH 09/14] Revert "add temporary example for question" This reverts commit 19bc49249ce474343fe99744c9b68de168874736. --- .../stories/SelectPanel.examples.stories.tsx | 55 ------------------- 1 file changed, 55 deletions(-) diff --git a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx index 0c79468ce54..81698bdb900 100644 --- a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx +++ b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx @@ -66,61 +66,6 @@ export const Minimal = () => { ) } -export const Temporary = () => { - const initialSelectedLabels = data.issue.labelIds // mock initial state: has selected labels - const [selectedLabelIds, setSelectedLabelIds] = React.useState(initialSelectedLabels) - - /* Selection */ - const onLabelSelect = (labelId: string) => { - if (!selectedLabelIds.includes(labelId)) setSelectedLabelIds([...selectedLabelIds, labelId]) - else setSelectedLabelIds(selectedLabelIds.filter(id => id !== labelId)) - } - - const onSubmit = () => { - data.issue.labelIds = selectedLabelIds // pretending to persist changes - - // eslint-disable-next-line no-console - console.log('form submitted') - } - - const sortingFn = (itemA: {id: string}, itemB: {id: string}) => { - const initialSelectedIds = data.issue.labelIds - if (initialSelectedIds.includes(itemA.id) && initialSelectedIds.includes(itemB.id)) return 1 - else if (initialSelectedIds.includes(itemA.id)) return -1 - else if (initialSelectedIds.includes(itemB.id)) return 1 - else return 1 - } - - const onClearSelection = () => setSelectedLabelIds([]) - - const itemsToShow = data.labels.sort(sortingFn) - - return ( - <> -

Temporary SelectPanel

- - - Assign label - - - {itemsToShow.map(label => ( - onLabelSelect(label.id)} - selected={selectedLabelIds.includes(label.id)} - > - {getCircle(label.color)} - {label.name} - {label.description} - - ))} - - - - - ) -} - export const WithGroups = () => { /* Selection */ const initialAssigneeIds = data.issue.assigneeIds // mock initial state From 2d68105e0e8e744e187aaa8ba0fe427775bea5e0 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 18 Dec 2023 17:09:15 +0100 Subject: [PATCH 10/14] move comment closer to code --- src/drafts/SelectPanel2/SelectPanel.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index ded25c486ea..df5ad6864b7 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -216,9 +216,8 @@ const Panel: React.FC = ({ height: '100%', }} > - {/* render default header as fallback */} + {slots.header ?? /* render default header as fallback */ } - {slots.header ?? } } From 529639aa37636eea2faa74154260a5fa9a4e31fb Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 19 Dec 2023 16:23:02 +0100 Subject: [PATCH 11/14] nudge user towards actions when clicking outside --- src/drafts/SelectPanel2/SelectPanel.tsx | 32 +++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index df5ad6864b7..bfc9330fbdd 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -18,9 +18,10 @@ import { } from '../../../src/index' import {ActionListContainerContext} from '../../../src/ActionList/ActionListContainerContext' import {useSlots} from '../../hooks/useSlots' -import {useProvidedRefOrCreate, useId, useAnchoredPosition} from '../../hooks' +import {useProvidedRefOrCreate, useId, useAnchoredPosition, useOnOutsideClick} from '../../hooks' import {useFocusZone} from '../../hooks/useFocusZone' import {StyledOverlay, OverlayProps} from '../../Overlay/Overlay' +import {P} from 'ts-toolbelt/out/Object/_api' const SelectPanelContext = React.createContext<{ title: string @@ -174,6 +175,19 @@ const Panel: React.FC = ({ [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), 500) + } + return ( <> {Anchor} @@ -192,6 +206,20 @@ const Panel: React.FC = ({ padding: 0, margin: 0, '::backdrop': {background: 'transparent'}, + + '& [data-selectpanel-primary-actions]': { + animation: footerAnimationEnabled ? 'selectpanel-gelatine 0.5s 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)'}, + }, + }} + onClick={event => { + if (event.target === event.currentTarget) onClickOutside() }} > { {props.children} {hidePrimaryActions ? null : ( - + From 635ad1a19979325260c74f6a5ff22734f03b7faf Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 19 Dec 2023 16:29:57 +0100 Subject: [PATCH 12/14] oops --- src/drafts/SelectPanel2/SelectPanel.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index bfc9330fbdd..cb269de55db 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -18,10 +18,9 @@ import { } from '../../../src/index' import {ActionListContainerContext} from '../../../src/ActionList/ActionListContainerContext' import {useSlots} from '../../hooks/useSlots' -import {useProvidedRefOrCreate, useId, useAnchoredPosition, useOnOutsideClick} from '../../hooks' +import {useProvidedRefOrCreate, useId, useAnchoredPosition} from '../../hooks' import {useFocusZone} from '../../hooks/useFocusZone' import {StyledOverlay, OverlayProps} from '../../Overlay/Overlay' -import {P} from 'ts-toolbelt/out/Object/_api' const SelectPanelContext = React.createContext<{ title: string From dfcb85ce5445bc9e6a4cb08c4618aa609165a777 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 19 Dec 2023 17:18:05 +0100 Subject: [PATCH 13/14] Create eleven-lizards-draw.md --- .changeset/eleven-lizards-draw.md | 5 +++++ 1 file changed, 5 insertions(+) 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 From a8430d4a0f7aafd62081345a684305364847d3fd Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 20 Dec 2023 16:19:02 +0100 Subject: [PATCH 14/14] change animation duration to 350ms --- src/drafts/SelectPanel2/SelectPanel.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index cb269de55db..8c5ed602d75 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -184,7 +184,7 @@ const Panel: React.FC = ({ const [footerAnimationEnabled, setFooterAnimationEnabled] = React.useState(false) const onClickOutside = () => { setFooterAnimationEnabled(true) - window.setTimeout(() => setFooterAnimationEnabled(false), 500) + window.setTimeout(() => setFooterAnimationEnabled(false), 350) } return ( @@ -207,7 +207,7 @@ const Panel: React.FC = ({ '::backdrop': {background: 'transparent'}, '& [data-selectpanel-primary-actions]': { - animation: footerAnimationEnabled ? 'selectpanel-gelatine 0.5s linear' : 'none', + animation: footerAnimationEnabled ? 'selectpanel-gelatine 350ms linear' : 'none', }, '@keyframes selectpanel-gelatine': { '0%': {transform: 'scale(1, 1)'},