From 75a425e3eb88cab4f9ff70bc5dedc85059b4e7ee Mon Sep 17 00:00:00 2001 From: Keith Cirkel Date: Wed, 18 Oct 2023 12:23:21 +0100 Subject: [PATCH 01/12] refactor Dialog to use internally --- src/ConfirmationDialog/ConfirmationDialog.tsx | 1 + src/Dialog/Dialog.tsx | 173 ++++++++---------- src/utils/test-helpers.tsx | 9 + 3 files changed, 86 insertions(+), 97 deletions(-) diff --git a/src/ConfirmationDialog/ConfirmationDialog.tsx b/src/ConfirmationDialog/ConfirmationDialog.tsx index fa951fd6962..e2fa9f4c9c1 100644 --- a/src/ConfirmationDialog/ConfirmationDialog.tsx +++ b/src/ConfirmationDialog/ConfirmationDialog.tsx @@ -156,6 +156,7 @@ async function confirm(themeProps: ThemeProviderProps, options: ConfirmOptions): const root = createRoot(hostElement) const onClose: ConfirmationDialogProps['onClose'] = gesture => { root.unmount() + if (gesture === 'confirm') { resolve(true) } else { diff --git a/src/Dialog/Dialog.tsx b/src/Dialog/Dialog.tsx index aa00fed479f..ff98377696d 100644 --- a/src/Dialog/Dialog.tsx +++ b/src/Dialog/Dialog.tsx @@ -1,19 +1,18 @@ -import React, {useCallback, useEffect, useRef, useState} from 'react' +import React, {useCallback, useRef} from 'react' import styled from 'styled-components' import {Button, ButtonProps} from '../Button' import Box from '../Box' import {get} from '../constants' -import {useOnEscapePress, useProvidedRefOrCreate} from '../hooks' -import {useFocusTrap} from '../hooks/useFocusTrap' import sx, {SxProp} from '../sx' import Octicon from '../Octicon' import {XIcon} from '@primer/octicons-react' import {useFocusZone} from '../hooks/useFocusZone' import {FocusKeys} from '@primer/behaviors' -import Portal from '../Portal' -import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' +import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' import {useId} from '../hooks/useId' import {ScrollableRegion} from '../internal/components/ScrollableRegion' +import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate' +import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' /* Dialog Version 2 */ @@ -96,13 +95,23 @@ export interface DialogProps extends SxProp { footerButtons?: DialogButtonProps[] /** - * This method is invoked when a gesture to close the dialog is used (either - * an Escape key press or clicking the "X" in the top-right corner). The - * gesture argument indicates the gesture that was used to close the dialog - * (either 'close-button' or 'escape'). + * This method is invoked when the dialog has been closed. This could + * be from the Dialog. + * @param gesture - Deprecated: The gesture argument was used to + * indicate if the gesture was from the close-button or escape button. + * It will always be `'close-button'`, to check for _cancelations_ of + * the dialog using the Esc key, use onCancel. */ onClose: (gesture: 'close-button' | 'escape') => void + /** + * This method is invoked when the user instructs the browser they wish to + * dismiss the dialog. Typically this means they have pressed the `Esc` key. + * + * @see https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/cancel_event + */ + onCancel?: () => void + /** * Default: "dialog". The ARIA role to assign to this dialog. * @see https://www.w3.org/TR/wai-aria-practices-1.1/#dialog_modal @@ -126,6 +135,11 @@ export interface DialogProps extends SxProp { * auto: variable based on contents */ height?: DialogHeight + + /** + * Whether or not the Dialog is rendered initially open + */ + initiallyOpen?: boolean } /** @@ -145,32 +159,11 @@ export interface DialogHeaderProps extends DialogProps { dialogDescriptionId: string } -const Backdrop = styled('div')` - position: fixed; - top: 0; - left: 0; - bottom: 0; - right: 0; - display: flex; - align-items: center; - justify-content: center; - background-color: ${get('colors.primer.canvas.backdrop')}; - animation: dialog-backdrop-appear ${ANIMATION_DURATION} ${get('animation.easeOutCubic')}; - - @keyframes dialog-backdrop-appear { - 0% { - opacity: 0; - } - 100% { - opacity: 1; - } - } -` - const heightMap = { small: '480px', large: '640px', auto: 'auto', + 'min-content': 'min-content', } as const const widthMap = { @@ -188,19 +181,40 @@ type StyledDialogProps = { height?: DialogHeight } & SxProp -const StyledDialog = styled.div` - display: flex; +const StyledDialog = styled.dialog` flex-direction: column; background-color: ${get('colors.canvas.overlay')}; + color: ${get('colors.fg.default')}; box-shadow: ${get('shadows.overlay.shadow')}; min-width: 296px; max-width: calc(100vw - 64px); max-height: calc(100vh - 64px); width: ${props => widthMap[props.width ?? ('xlarge' as const)]}; height: ${props => heightMap[props.height ?? ('auto' as const)]}; + border: 0; border-radius: 12px; + padding: 0; opacity: 1; animation: overlay--dialog-appear ${ANIMATION_DURATION} ${get('animation.easeOutCubic')}; + overflow: initial; + + &[open] { + display: flex; + } + + &::backdrop { + background-color: ${get('colors.primer.canvas.backdrop')}; + animation: dialog-backdrop-appear ${ANIMATION_DURATION} ${get('animation.easeOutCubic')}; + } + + @keyframes dialog-backdrop-appear { + 0% { + opacity: 0; + } + 100% { + opacity: 1; + } + } @keyframes overlay--dialog-appear { 0% { @@ -253,7 +267,7 @@ const DefaultFooter: React.FC> = ({footerBu ) : null } -const _Dialog = React.forwardRef>((props, forwardedRef) => { +const _Dialog = React.forwardRef>((props, forwardedRef) => { const { title = 'Dialog', subtitle = '', @@ -261,78 +275,50 @@ const _Dialog = React.forwardRef(null) - for (const footerButton of footerButtons) { - if (footerButton.autoFocus) { - footerButton.ref = autoFocusedFooterButtonRef - } - } const defaultedProps = {...props, title, subtitle, role, dialogLabelId, dialogDescriptionId} - const dialogRef = useRef(null) + const dialogRef = useRef(null) useRefObjectAsForwardedRef(forwardedRef, dialogRef) - const backdropRef = useRef(null) - useFocusTrap({containerRef: dialogRef, restoreFocusOnCleanUp: true, initialFocusRef: autoFocusedFooterButtonRef}) - - useOnEscapePress( - (event: KeyboardEvent) => { - onClose('escape') - event.preventDefault() - }, - [onClose], - ) - React.useEffect(() => { - const bodyOverflowStyle = document.body.style.overflow || '' - // If the body is already set to overflow: hidden, it likely means - // that there is already a modal open. In that case, we should bail - // so we don't re-enable scroll after the second dialog is closed. - if (bodyOverflowStyle === 'hidden') { - return + useIsomorphicLayoutEffect(() => { + if (initiallyOpen && !dialogRef.current?.open && dialogRef.current?.isConnected) { + dialogRef.current.showModal() } + }, [initiallyOpen, dialogRef]) - document.body.style.overflow = 'hidden' - - return () => { - document.body.style.overflow = bodyOverflowStyle - } - }, []) + const onCloseHandler = useCallback(() => onClose('close-button'), [onClose]) const header = (renderHeader ?? DefaultHeader)(defaultedProps) const body = (renderBody ?? DefaultBody)(defaultedProps) const footer = (renderFooter ?? DefaultFooter)(defaultedProps) return ( - <> - - - - {header} - - {body} - - {footer} - - - - + + {header} + + {body} + + {footer} + ) }) _Dialog.displayName = 'Dialog' @@ -384,16 +370,9 @@ const Footer = styled.div` const Buttons: React.FC> = ({buttons}) => { const autoFocusRef = useProvidedRefOrCreate(buttons.find(button => button.autoFocus)?.ref) let autoFocusCount = 0 - const [hasRendered, setHasRendered] = useState(0) - useEffect(() => { - // hack to work around dialogs originating from other focus traps. - if (hasRendered === 1) { - autoFocusRef.current?.focus() - } else { - setHasRendered(hasRendered + 1) - } - }, [autoFocusRef, hasRendered]) - + useIsomorphicLayoutEffect(() => { + autoFocusRef.current?.setAttribute('autofocus', '') + }, [autoFocusRef]) return ( <> {buttons.map((dialogButtonProps, index) => { diff --git a/src/utils/test-helpers.tsx b/src/utils/test-helpers.tsx index bcca7a6d1cc..b73c29a259a 100644 --- a/src/utils/test-helpers.tsx +++ b/src/utils/test-helpers.tsx @@ -18,3 +18,12 @@ global.CSS = { } global.TextEncoder = TextEncoder + +// Dialog showModal isn't implemented in JSDOM https://github.com/jsdom/jsdom/issues/3294 +global.HTMLDialogElement.prototype.showModal = jest.fn(function mock(this: HTMLDialogElement) { + // eslint-disable-next-line no-invalid-this + this.open = true + // eslint-disable-next-line no-invalid-this + const element: HTMLElement | null = this.querySelector('[autofocus]') + element?.focus() +}) From 51c8194c8df40a04851c0183a0bb5a630f4e48fa Mon Sep 17 00:00:00 2001 From: Keith Cirkel Date: Thu, 7 Dec 2023 12:16:52 +0000 Subject: [PATCH 02/12] changeset --- .changeset/little-dryers-sort.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/little-dryers-sort.md diff --git a/.changeset/little-dryers-sort.md b/.changeset/little-dryers-sort.md new file mode 100644 index 00000000000..b8192f2fecf --- /dev/null +++ b/.changeset/little-dryers-sort.md @@ -0,0 +1,7 @@ +--- +'@primer/react': patch +--- + +Dialog now uses + + From 42fc1a14673f7b87b0ed7565df7dca693f133ae4 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 01:04:24 +0530 Subject: [PATCH 03/12] simplify mock data --- src/drafts/SelectPanel2/stories/mock-data.ts | 69 -------------------- 1 file changed, 69 deletions(-) diff --git a/src/drafts/SelectPanel2/stories/mock-data.ts b/src/drafts/SelectPanel2/stories/mock-data.ts index 75f86fd3c77..e10ad1f868d 100644 --- a/src/drafts/SelectPanel2/stories/mock-data.ts +++ b/src/drafts/SelectPanel2/stories/mock-data.ts @@ -50,315 +50,246 @@ const data = { id: 'MDU6TGFiZWw4Mzk2MzgxMTU=', name: 'bug', description: "Something isn't working", - createdAt: '2018-02-17T00:09:05Z', }, { color: 'cfd3d7', id: 'MDU6TGFiZWw4Mzk2MzgxMTY=', name: 'duplicate', description: 'This issue or pull request already exists', - createdAt: '2018-02-17T00:09:05Z', }, { color: 'a2eeef', id: 'MDU6TGFiZWw4Mzk2MzgxMTc=', name: 'enhancement', description: 'New feature or request', - createdAt: '2018-02-17T00:09:05Z', }, { color: 'd876e3', id: 'MDU6TGFiZWw4Mzk2MzgxMjE=', name: 'futher info needed', description: 'Further information is requested', - createdAt: '2018-02-17T00:09:05Z', }, { color: '7057ff', id: 'MDU6TGFiZWw4Mzk2MzgxMTk=', name: 'good first issue', description: 'Good for newcomers', - createdAt: '2018-02-17T00:09:05Z', }, { color: '008672', id: 'MDU6TGFiZWw4Mzk2MzgxMTg=', name: 'up for grabs', description: "anyone can take on this work, it's ready to go", - createdAt: '2018-02-17T00:09:05Z', }, { color: 'ffffff', id: 'MDU6TGFiZWw4Mzk2MzgxMjI=', name: 'wontfix', description: 'This will not be worked on', - createdAt: '2018-02-17T00:09:05Z', }, { color: 'FFD2ED', id: 'MDU6TGFiZWw4ODkxNjUwNzU=', name: '💓collab', description: 'a vibrant hub of collaboration', - createdAt: '2018-04-03T21:03:53Z', }, { color: '9eea90', id: 'MDU6TGFiZWw5NDAyMzcyOTU=', name: 'status: wip', - description: '', - createdAt: '2018-05-21T19:11:15Z', }, { color: 'eae658', id: 'MDU6TGFiZWw5NDAyMzc1NzY=', name: 'status: review needed', - description: '', - createdAt: '2018-05-21T19:11:36Z', }, { color: 'a9abe8', id: 'MDU6TGFiZWw5NDA1MDc5NzQ=', name: 'type: discussion', - description: '', - createdAt: '2018-05-22T02:06:49Z', }, { color: '86181d', id: 'MDU6TGFiZWw5NDg2NDM1MzI=', name: '🚧 blocked', description: 'Someone or something is preventing this from moving forward', - createdAt: '2018-05-29T22:02:54Z', }, { color: '1d76db', id: 'MDU6TGFiZWwxMDI2NDE1MTYw', name: 'docs', description: 'Documentation', - createdAt: '2018-08-16T16:35:43Z', }, { color: 'fcc0dc', id: 'MDU6TGFiZWwxMDU3MjcwNzA4', name: 'patch release', description: 'bug fixes, docs, housekeeping', - createdAt: '2018-09-14T20:27:34Z', }, { color: 'ff73b4', id: 'MDU6TGFiZWwxMDU3MjcxMDUz', name: 'minor release', description: 'new features', - createdAt: '2018-09-14T20:28:01Z', }, { color: 'e3006a', id: 'MDU6TGFiZWwxMDU3MjcxNDE5', name: 'major release', description: 'breaking changes', - createdAt: '2018-09-14T20:28:30Z', }, { color: '107d93', id: 'MDU6TGFiZWwxMDU3MjczMjkz', name: 'deployment', - description: '', - createdAt: '2018-09-14T20:31:10Z', }, { color: 'd4c5f9', id: 'MDU6TGFiZWwxMzgyNTYyNjQ0', name: 'accessibility', - description: '', - createdAt: '2019-05-29T19:56:35Z', }, { color: '0366d6', id: 'MDU6TGFiZWwxNjE2MTQ5MDE2', name: 'dependencies', description: 'Pull requests that update a dependency file', - createdAt: '2019-10-14T18:19:49Z', }, { color: 'e1e4e8', id: 'MDU6TGFiZWwxNjU3MjI0NTcw', name: 'fr-skip', description: 'Remove this from the Design Systems first responder list', - createdAt: '2019-11-04T18:18:03Z', }, { color: 'fcba03', id: 'MDU6TGFiZWwxNzkwMDY4Mzk5', name: 'developer experience', - description: '', - createdAt: '2020-01-15T22:30:35Z', }, { color: '68f9cc', id: 'MDU6TGFiZWwxNzkwMDY4NDg1', name: 'contributor experience', - description: '', - createdAt: '2020-01-15T22:30:41Z', }, { color: 'a1b220', id: 'MDU6TGFiZWwxNzkwMDcyODY5', name: 'API', - description: '', - createdAt: '2020-01-15T22:34:09Z', }, { color: '6494f4', id: 'MDU6TGFiZWwxNzkxNjM5MzM1', name: 'new component', - description: '', - createdAt: '2020-01-16T16:13:35Z', }, { color: 'f48b96', id: 'MDU6TGFiZWwxOTg5MDAwNjk0', name: 'experimental', - description: '', - createdAt: '2020-04-15T20:52:13Z', }, { color: 'db9360', id: 'MDU6TGFiZWwxOTkzNTcwNzUx', name: 'design', - description: '', - createdAt: '2020-04-17T15:08:45Z', }, { color: 'F9D0C4', id: 'MDU6TGFiZWwyNjkzNTE0OTQw', name: 'coverage', - description: '', - createdAt: '2021-01-27T22:25:23Z', }, { color: '077CA4', id: 'MDU6TGFiZWwyNjkzNTE5NTc1', name: 'epic', - description: '', - createdAt: '2021-01-27T22:27:57Z', }, { color: 'F9D0C4', id: 'MDU6TGFiZWwyNjkzNTMwODUy', name: 'behaviors', - description: '', - createdAt: '2021-01-27T22:34:49Z', }, { color: '1d76db', id: 'MDU6TGFiZWwyNzkzMjYwMTgz', name: 'typescript', - description: '', - createdAt: '2021-03-04T20:17:59Z', }, { color: 'DBEDFF', id: 'MDU6TGFiZWwzMzA5MjY0Nzcz', name: 'size: sand', - description: '', - createdAt: '2021-08-31T02:08:34Z', }, { color: 'DBEDFF', id: 'MDU6TGFiZWwzMzA5MjY1MDM2', name: 'size: pebble', - description: '', - createdAt: '2021-08-31T02:08:42Z', }, { color: 'DBEDFF', id: 'MDU6TGFiZWwzMzA5MjY1MTc4', name: 'size: rock', - description: '', - createdAt: '2021-08-31T02:08:49Z', }, { color: 'DBEDFF', id: 'MDU6TGFiZWwzMzA5MjY1NTg3', name: 'size: boulder', - description: '', - createdAt: '2021-08-31T02:09:00Z', }, { color: 'ededed', id: 'MDU6TGFiZWwzMzE4NjA1ODgy', name: 'Stale', description: null, - createdAt: '2021-09-02T22:04:15Z', }, { color: '0052CC', id: 'LA_kwDOB0K8ws7Oq_eD', name: 'react', - description: '', - createdAt: '2021-10-19T16:48:08Z', }, { color: 'eeeeee', id: 'LA_kwDOB0K8ws7O4h4u', name: 'skip changeset', - description: '', - createdAt: '2021-10-20T16:15:21Z', }, { color: 'D93F0B', id: 'LA_kwDOB0K8ws7WuYLd', name: 'do not merge', - description: '', - createdAt: '2021-12-01T19:02:44Z', }, { color: 'e5534b', id: 'LA_kwDOB0K8ws73bq-W', name: 'needs triage', - description: '', - createdAt: '2022-05-20T13:41:47Z', }, { color: 'B60205', id: 'LA_kwDOB0K8ws8AAAABA3IfBw', name: 'a11y-eng-secondary', - description: '', - createdAt: '2022-07-22T10:04:29Z', }, { color: 'ffffff', id: 'LA_kwDOB0K8ws8AAAABBfG7EQ', name: 'support', description: 'Tasks where the team is supporting and helping other teams', - createdAt: '2022-08-04T15:33:56Z', }, { color: 'DDF4FF', id: 'LA_kwDOB0K8ws8AAAABG14Q1Q', name: 'component: tree view', description: 'Issues related to the TreeView component', - createdAt: '2022-11-02T18:49:40Z', }, { color: 'DDF4FF', id: 'LA_kwDOB0K8ws8AAAABHLXdxw', name: 'component: SplitPageLayout', description: 'Issues related to the PageLayout and SplitPageLayout components', - createdAt: '2022-11-08T18:57:47Z', }, { color: 'DDF4FF', id: 'LA_kwDOB0K8ws8AAAABHLXucw', name: 'component: nav list', description: 'Issues related to the NavList component', - createdAt: '2022-11-08T18:58:49Z', }, { color: 'DDF4FF', id: 'LA_kwDOB0K8ws8AAAABHLYCfQ', name: 'component: button', description: 'Issues related to the Button component', - createdAt: '2022-11-08T19:00:13Z', }, ], users: [ From 5ad8740dacb8bc1b32ae6565c3e0340819d29732 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 01:05:16 +0530 Subject: [PATCH 04/12] dont shrink footer --- 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 82e122a4181..6e81fa5b02a 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -429,6 +429,7 @@ const SelectPanelFooter = ({...props}) => { Date: Thu, 25 Jan 2024 01:05:27 +0530 Subject: [PATCH 05/12] prepare empty message for button --- src/drafts/SelectPanel2/SelectPanel.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 6e81fa5b02a..a13d0ade603 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -557,7 +557,7 @@ const SelectPanelMessage: React.FC = ({ flexGrow: 1, height: '100%', gap: 1, - paddingX: 4, + paddingX: 3, textAlign: 'center', a: {color: 'inherit', textDecoration: 'underline'}, }} @@ -566,7 +566,11 @@ const SelectPanelMessage: React.FC = ({ ) : null} {title} - {children} + + {children} + ) } else { From c519449eb46417bcf00bc5bcc83768815d8ca8d5 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 01:05:53 +0530 Subject: [PATCH 06/12] possible mock for create new label --- .../stories/SelectPanel.examples.stories.tsx | 137 +++++++++++++++++- 1 file changed, 136 insertions(+), 1 deletion(-) diff --git a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx index 3eaf75e9060..90c330ef7e1 100644 --- a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx +++ b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx @@ -1,7 +1,14 @@ import React from 'react' import {SelectPanel} from '../SelectPanel' import {ActionList, ActionMenu, Avatar, Box, Button, Text} from '../../../index' -import {ArrowRightIcon, EyeIcon, GitBranchIcon, TriangleDownIcon, GearIcon} from '@primer/octicons-react' +import { + ArrowRightIcon, + EyeIcon, + GitBranchIcon, + TriangleDownIcon, + GearIcon, + PlusCircleIcon, +} from '@primer/octicons-react' import data from './mock-data' export default { @@ -640,6 +647,134 @@ export const ShortSelectPanel = () => { ) } +export const CreateNewRow = () => { + 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 onClearSelection = () => { + setSelectedLabelIds([]) + } + + const onSubmit = () => { + data.issue.labelIds = selectedLabelIds // pretending to persist changes + } + + /* Filtering */ + const [filteredLabels, setFilteredLabels] = React.useState(data.labels) + const [query, setQuery] = React.useState('') + + const onSearchInputChange: React.ChangeEventHandler = event => { + const query = event.currentTarget.value + setQuery(query) + + if (query === '') setFilteredLabels(data.labels) + else { + setFilteredLabels( + data.labels + .map(label => { + if (label.name.toLowerCase().startsWith(query)) return {priority: 1, label} + else if (label.name.toLowerCase().includes(query)) return {priority: 2, label} + else if (label.description?.toLowerCase().includes(query)) return {priority: 3, label} + else return {priority: -1, label} + }) + .filter(result => result.priority > 0) + .map(result => result.label), + ) + } + } + + 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 itemsToShow = query ? filteredLabels : data.labels.sort(sortingFn) + + const createNewLabel = (name: string) => { + const id = `new-${name}` + + // pretending to persist changes + data.labels.unshift({id, name, color: 'fcba03'}) + + setQuery('') // clear search input + onLabelSelect(id) // select newly created label + } + + return ( + <> + { + /* optional callback, for example: for multi-step overlay or to fire sync actions */ + // eslint-disable-next-line no-console + console.log('panel was closed') + }} + onClearSelection={onClearSelection} + > + Assign label + + + + + + {itemsToShow.length === 0 ? ( + + Select the button below to create this label + + + ) : ( + <> + + {itemsToShow.map(label => ( + onLabelSelect(label.id)} + selected={selectedLabelIds.includes(label.id)} + > + + + + {label.name} + {label.description} + + ))} + + {query && ( + + + + )} + + )} + + + Edit labels + + + + ) +} + // ----- Suspense implementation details ---- const cache = new Map() From 14ff8658fa719617f6911d06fc2b5873737bad1c Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 01:32:56 +0530 Subject: [PATCH 07/12] include button in focus zone --- src/drafts/SelectPanel2/SelectPanel.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index a13d0ade603..96fe3688ea3 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -156,7 +156,7 @@ const Panel: React.FC = ({ const {containerRef: listContainerRef} = useFocusZone( { bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown, - focusableElementFilter: element => element.tagName === 'LI', + focusableElementFilter: element => element.tagName === 'LI' || element.tagName === 'BUTTON', }, [internalOpen], ) From acf99576e493588bdd9620a22f40c0354300c7dc Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 03:17:29 +0530 Subject: [PATCH 08/12] use controlled state for Panel --- src/drafts/SelectPanel2/SelectPanel.tsx | 2 +- .../stories/SelectPanel.examples.stories.tsx | 85 ++++++++++++++++--- 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 96fe3688ea3..7fb2fd42974 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -112,7 +112,7 @@ const Panel: React.FC = ({ Anchor = React.cloneElement(child, { // @ts-ignore TODO ref: anchorRef, - onClick: onAnchorClick, + onClick: child.props.onClick || onAnchorClick, 'aria-haspopup': true, 'aria-expanded': internalOpen, }) diff --git a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx index 90c330ef7e1..ea301a31d9e 100644 --- a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx +++ b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx @@ -1,6 +1,7 @@ -import React from 'react' +import React, {FormEvent} from 'react' import {SelectPanel} from '../SelectPanel' -import {ActionList, ActionMenu, Avatar, Box, Button, Text} from '../../../index' +import {ActionList, ActionMenu, Avatar, Box, Button, Flash, FormControl, Text, TextInput} from '../../../index' +import {Dialog} from '../../../drafts' import { ArrowRightIcon, EyeIcon, @@ -698,29 +699,53 @@ export const CreateNewRow = () => { const itemsToShow = query ? filteredLabels : data.labels.sort(sortingFn) - const createNewLabel = (name: string) => { - const id = `new-${name}` + /* + Controlled state + Create new label Dialog + We only have to do this until https://github.com/primer/react/pull/3840 is merged + */ + const [panelOpen, setPanelOpen] = React.useState(false) + const [dialogOpen, setDialogOpen] = React.useState(false) + + const openCreateLabelDialog = () => { + setPanelOpen(false) + setDialogOpen(true) + } + + const onDialogSubmit = (event: FormEvent) => { + event.preventDefault() + + const formData = new FormData(event.target as HTMLFormElement) + const {name, color, description} = Object.fromEntries(formData) as Record // pretending to persist changes - data.labels.unshift({id, name, color: 'fcba03'}) + const id = `new-${name}` + data.labels.unshift({id, name, color, description}) + + setDialogOpen(false) + setPanelOpen(true) setQuery('') // clear search input onLabelSelect(id) // select newly created label } + const formSubmitRef = React.useRef(null) + return ( <> +

Create new item from panel

+ + Note this example is not yet ready, do not copy it. It is blocked by{' '} + primer/react/pull/3840 + + { - /* optional callback, for example: for multi-step overlay or to fire sync actions */ - // eslint-disable-next-line no-console - console.log('panel was closed') - }} + onCancel={() => setPanelOpen(false)} onClearSelection={onClearSelection} > - Assign label + setPanelOpen(true)}>Assign label @@ -729,7 +754,7 @@ export const CreateNewRow = () => { {itemsToShow.length === 0 ? ( Select the button below to create this label - + ) : ( <> @@ -757,8 +782,9 @@ export const CreateNewRow = () => { variant="invisible" leadingVisual={PlusCircleIcon} block - sx={{'[data-component="buttonContent"]': {justifyContent: 'start', fontWeight: 'normal'}}} - onClick={() => createNewLabel(query)} + alignContent="start" + sx={{'[data-component=text]': {fontWeight: 'normal'}}} + onClick={openCreateLabelDialog} > Create new label "{query}"... @@ -771,6 +797,37 @@ export const CreateNewRow = () => { Edit labels + + {dialogOpen && ( + setDialogOpen(false)} + width="medium" + footerButtons={[ + {buttonType: 'default', content: 'Cancel', onClick: () => setDialogOpen(false)}, + {type: 'submit', buttonType: 'primary', content: 'Save', onClick: () => formSubmitRef.current?.click()}, + ]} + > + + Note this Dialog is not accessible. Do not copy this. + +
+ + Name + + + + Color + + + + Description + + + +
+
+ )} ) } From be7bd6fe9515e100b331d942e17b2efeaba93932 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 03:26:57 +0530 Subject: [PATCH 09/12] move new label dialog into its own component --- .../stories/SelectPanel.examples.stories.tsx | 112 +++++++++++------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx index ea301a31d9e..53839c34081 100644 --- a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx +++ b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx @@ -704,31 +704,21 @@ export const CreateNewRow = () => { We only have to do this until https://github.com/primer/react/pull/3840 is merged */ const [panelOpen, setPanelOpen] = React.useState(false) - const [dialogOpen, setDialogOpen] = React.useState(false) + const [newLabelDialogOpen, setNewLabelDialogOpen] = React.useState(false) const openCreateLabelDialog = () => { setPanelOpen(false) - setDialogOpen(true) + setNewLabelDialogOpen(true) } - const onDialogSubmit = (event: FormEvent) => { - event.preventDefault() - - const formData = new FormData(event.target as HTMLFormElement) - const {name, color, description} = Object.fromEntries(formData) as Record - - // pretending to persist changes - const id = `new-${name}` - data.labels.unshift({id, name, color, description}) - - setDialogOpen(false) - setPanelOpen(true) + const onNewLabelDialogSave = (id: string) => { + setNewLabelDialogOpen(false) setQuery('') // clear search input onLabelSelect(id) // select newly created label - } - const formSubmitRef = React.useRef(null) + setPanelOpen(true) + } return ( <> @@ -798,40 +788,72 @@ export const CreateNewRow = () => { - {dialogOpen && ( - setDialogOpen(false)} - width="medium" - footerButtons={[ - {buttonType: 'default', content: 'Cancel', onClick: () => setDialogOpen(false)}, - {type: 'submit', buttonType: 'primary', content: 'Save', onClick: () => formSubmitRef.current?.click()}, - ]} - > - - Note this Dialog is not accessible. Do not copy this. - -
- - Name - - - - Color - - - - Description - - - -
-
+ {newLabelDialogOpen && ( + setNewLabelDialogOpen(false)} + /> )} ) } +const CreateNewLabelDialog = ({ + initialValue, + onSave, + onCancel, +}: { + initialValue: string + onSave: (id: string) => void + onCancel: () => void +}) => { + const formSubmitRef = React.useRef(null) + + const onSubmit = (event: FormEvent) => { + event.preventDefault() + + const formData = new FormData(event.target as HTMLFormElement) + const {name, color, description} = Object.fromEntries(formData) as Record + + // pretending to persist changes + const id = `new-${name}` + data.labels.unshift({id, name, color, description}) + onSave(id) + } + + return ( + setDialogOpen(false)}, + {type: 'submit', buttonType: 'primary', content: 'Save', onClick: () => formSubmitRef.current?.click()}, + ]} + > + + Note this Dialog is not accessible. Do not copy this. + +
+ + Name + + + + Color + + + + Description + + + +
+
+ ) +} + // ----- Suspense implementation details ---- const cache = new Map() From fc976732a27a83741ceea4932133e1a53162c6d4 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 04:03:17 +0530 Subject: [PATCH 10/12] generate id to allow spaces --- .../SelectPanel2/stories/SelectPanel.examples.stories.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx index 53839c34081..1f622b2b11b 100644 --- a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx +++ b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx @@ -817,7 +817,7 @@ const CreateNewLabelDialog = ({ const {name, color, description} = Object.fromEntries(formData) as Record // pretending to persist changes - const id = `new-${name}` + const id = Math.random().toString(26).slice(6) data.labels.unshift({id, name, color, description}) onSave(id) } @@ -828,7 +828,7 @@ const CreateNewLabelDialog = ({ onClose={onCancel} width="medium" footerButtons={[ - {buttonType: 'default', content: 'Cancel', onClick: () => setDialogOpen(false)}, + {buttonType: 'default', content: 'Cancel', onClick: onCancel}, {type: 'submit', buttonType: 'primary', content: 'Save', onClick: () => formSubmitRef.current?.click()}, ]} > From 1e4decfbc8cdb0787ef24f40cd9a3f5ce193284f Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 04:06:09 +0530 Subject: [PATCH 11/12] remove controlled state --- .../stories/SelectPanel.examples.stories.tsx | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx index 1f622b2b11b..c18a1e80154 100644 --- a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx +++ b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx @@ -1,6 +1,6 @@ import React, {FormEvent} from 'react' import {SelectPanel} from '../SelectPanel' -import {ActionList, ActionMenu, Avatar, Box, Button, Flash, FormControl, Text, TextInput} from '../../../index' +import {ActionList, ActionMenu, Avatar, Box, Button, FormControl, Text, TextInput} from '../../../index' import {Dialog} from '../../../drafts' import { ArrowRightIcon, @@ -699,43 +699,27 @@ export const CreateNewRow = () => { const itemsToShow = query ? filteredLabels : data.labels.sort(sortingFn) - /* - Controlled state + Create new label Dialog - We only have to do this until https://github.com/primer/react/pull/3840 is merged - */ - const [panelOpen, setPanelOpen] = React.useState(false) + /* Create new label Dialog */ const [newLabelDialogOpen, setNewLabelDialogOpen] = React.useState(false) - const openCreateLabelDialog = () => { - setPanelOpen(false) - setNewLabelDialogOpen(true) - } - + const openCreateLabelDialog = () => setNewLabelDialogOpen(true) const onNewLabelDialogSave = (id: string) => { - setNewLabelDialogOpen(false) - setQuery('') // clear search input onLabelSelect(id) // select newly created label - setPanelOpen(true) + // focus newly created label once it renders + window.requestAnimationFrame(() => { + const newLabelElement = document.querySelector(`[data-id=${id}]`) as HTMLLIElement + newLabelElement.focus() + }) } return ( <>

Create new item from panel

- - Note this example is not yet ready, do not copy it. It is blocked by{' '} - primer/react/pull/3840 - - setPanelOpen(false)} - onClearSelection={onClearSelection} - > - setPanelOpen(true)}>Assign label + + Assign label @@ -754,6 +738,7 @@ export const CreateNewRow = () => { key={label.id} onSelect={() => onLabelSelect(label.id)} selected={selectedLabelIds.includes(label.id)} + data-id={label.id} > { {newLabelDialogOpen && ( { + setNewLabelDialogOpen(false) + onNewLabelDialogSave(id) + }} onCancel={() => setNewLabelDialogOpen(false)} /> )} @@ -829,13 +817,17 @@ const CreateNewLabelDialog = ({ width="medium" footerButtons={[ {buttonType: 'default', content: 'Cancel', onClick: onCancel}, - {type: 'submit', buttonType: 'primary', content: 'Save', onClick: () => formSubmitRef.current?.click()}, + { + type: 'submit', + buttonType: 'primary', + content: 'Save', + onClick: () => { + formSubmitRef.current?.click() // footer buttons are rendered outside the form :( + }, + }, ]} > - - Note this Dialog is not accessible. Do not copy this. - -
+ Name From 5ee75d682cc8aa9578797cbfa3cd784b2f1f39d8 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 25 Jan 2024 04:18:09 +0530 Subject: [PATCH 12/12] wrap id in quotes --- .../SelectPanel2/stories/SelectPanel.examples.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx index c18a1e80154..533b018f31b 100644 --- a/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx +++ b/src/drafts/SelectPanel2/stories/SelectPanel.examples.stories.tsx @@ -709,7 +709,7 @@ export const CreateNewRow = () => { // focus newly created label once it renders window.requestAnimationFrame(() => { - const newLabelElement = document.querySelector(`[data-id=${id}]`) as HTMLLIElement + const newLabelElement = document.querySelector(`[data-id="${id}"]`) as HTMLLIElement newLabelElement.focus() }) }