From ffc0eba16a6d9a716c52d79f3b0154cd04280527 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 5 Dec 2023 14:37:50 +0100 Subject: [PATCH] SelectPanel2: Types! (#3929) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * breaking changes + cleanup * add SelectPanel.Warning * add link styles * remove todo story :) * typo * I not H * change example from labels to assignees * 1. make it work.... * move implementation details from story → component * absorb EmptyMessage into SelectPanel.Message * absorb WarningMessage into SelectPanel.Message * absorb ErrorMessage into SelectPanel.Message * stricter types for empty * clean up lint errors * tiny cleanup for bug * clean up message types real good! * child can be null * types: first round * types: round 2 - cleanup * types: round 3 more cleanup * types: 4 clean up stories ignores * types: 5 clean up stories * SelectPanel.Loading no longer exists * Revert "SelectPanel.Loading no longer exists" This reverts commit 0afa9278a210a9621230a324ece559a77ac39992. * use TextInputProps for SelectPanel.SearchInput * sync internal and props.open with state * use ?? instead of || Co-authored-by: Josh Black --------- Co-authored-by: Josh Black --- .../SelectPanel2/SelectPanel.stories.tsx | 59 ++----- src/drafts/SelectPanel2/SelectPanel.tsx | 156 ++++++++++-------- 2 files changed, 99 insertions(+), 116 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 9c799aa24e5..b4dd23b130f 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -42,14 +42,13 @@ export const AControlled = () => { const [filteredLabels, setFilteredLabels] = React.useState(data.labels) const [query, setQuery] = React.useState('') - // TODO: should this be baked-in - const onSearchInputChange = (event: React.KeyboardEvent) => { + const onSearchInputChange: React.ChangeEventHandler = event => { const query = event.currentTarget.value setQuery(query) if (query === '') setFilteredLabels(data.labels) else { - // TODO: should probably add a highlight for matching text + // Note: in the future, we should probably add a highlight for matching text setFilteredLabels( data.labels .map(label => { @@ -90,18 +89,11 @@ export const AControlled = () => { // eslint-disable-next-line no-console console.log('panel was closed') }} - // TODO: onClearSelection feels even more odd on the parent, instead of on the header. - // @ts-ignore todo - onClearSelection={event => { - // not optional, we don't control the selection, so we just pass this through - // @ts-ignore todo - onClearSelection(event) - }} + // API TODO: onClearSelection feels even more odd on the parent, instead of on the header. + onClearSelection={onClearSelection} > - {/* TODO: the ref types don't match here, use useProvidedRefOrCreate */} - {/* @ts-ignore todo */} Assign label - {/* TODO: header and heading is confusing. maybe skip header completely. */} + {/* API TODO: header and heading is confusing. maybe skip header completely. */} @@ -137,7 +129,7 @@ export const AControlled = () => { export const BWithSuspendedList = () => { const [query, setQuery] = React.useState('') - const onSearchInputChange = (event: React.KeyboardEvent) => { + const onSearchInputChange: React.ChangeEventHandler = event => { const query = event.currentTarget.value setQuery(query) } @@ -147,7 +139,6 @@ export const BWithSuspendedList = () => {

Suspended list

Fetching items once when the panel is opened (like repo labels)

- {/* @ts-ignore todo */} Assign label @@ -224,7 +215,7 @@ export const CAsyncSearchWithSuspenseKey = () => { // `users` are fetched async on search const [query, setQuery] = React.useState('') - const onSearchInputChange = (event: React.KeyboardEvent) => { + const onSearchInputChange: React.ChangeEventHandler = event => { const query = event.currentTarget.value setQuery(query) } @@ -250,7 +241,6 @@ export const CAsyncSearchWithSuspenseKey = () => {

Fetching items on every keystroke search (like github users)

- {/* @ts-ignore todo */} Select assignees @@ -330,7 +320,7 @@ export const DAsyncSearchWithUseTransition = () => { const [isPending, startTransition] = React.useTransition() const [query, setQuery] = React.useState('') - const onSearchInputChange = (event: React.KeyboardEvent) => { + const onSearchInputChange: React.ChangeEventHandler = event => { const query = event.currentTarget.value startTransition(() => setQuery(query)) } @@ -355,7 +345,6 @@ export const DAsyncSearchWithUseTransition = () => {

Fetching items on every keystroke search (like github users)

- {/* @ts-ignore todo */} Select assignees @@ -395,7 +384,7 @@ export const HWithFilterButtons = () => { /* Filter */ const [query, setQuery] = React.useState('') - const onSearchInputChange = (event: React.KeyboardEvent) => { + const onSearchInputChange: React.ChangeEventHandler = event => { const query = event.currentTarget.value setQuery(query) } @@ -436,8 +425,6 @@ export const HWithFilterButtons = () => {

With Filter Buttons

- {/* TODO: the ref types don't match here, use useProvidedRefOrCreate */} - {/* @ts-ignore todo */} {savedInitialRef} @@ -485,6 +472,7 @@ export const HWithFilterButtons = () => { )} + {/* @ts-ignore TODO as prop is not identified by button? */} View all {selectedFilter} @@ -526,8 +514,6 @@ export const EMinimal = () => {

Minimal SelectPanel

- {/* TODO: the ref types don't match here, use useProvidedRefOrCreate */} - {/* @ts-ignore todo */} Assign label @@ -759,7 +745,6 @@ export const FInstantSelectionVariant = () => {

Instant selection variant

- {/* @ts-ignore todo */} {selectedTag || 'Choose a tag'} @@ -798,13 +783,12 @@ export const IWithWarning = () => { const [filteredUsers, setFilteredUsers] = React.useState(data.collaborators) const [query, setQuery] = React.useState('') - const onSearchInputChange = (event: React.KeyboardEvent) => { + const onSearchInputChange: React.ChangeEventHandler = event => { const query = event.currentTarget.value setQuery(query) if (query === '') setFilteredUsers(data.collaborators) else { - // TODO: should probably add a highlight for matching text setFilteredUsers( data.collaborators .map(collaborator => { @@ -839,14 +823,8 @@ export const IWithWarning = () => { description={`Select up to ${MAX_LIMIT} people`} defaultOpen onSubmit={onSubmit} - // @ts-ignore todo - onClearSelection={event => { - // @ts-ignore todo - onClearSelection(event) - }} + onClearSelection={onClearSelection} > - {/* TODO: the ref types don't match here, use useProvidedRefOrCreate */} - {/* @ts-ignore todo */} { > Assignees - {/* TODO: header and heading is confusing. maybe skip header completely. */} @@ -920,7 +897,7 @@ export const JWithErrors = () => { const [query, setQuery] = React.useState('') - const onSearchInputChange = (event: React.KeyboardEvent) => { + const onSearchInputChange: React.ChangeEventHandler = event => { const query = event.currentTarget.value setQuery(query) @@ -993,15 +970,7 @@ export const JWithErrors = () => { /> - - {/* TODO: the ref types don't match here, use useProvidedRefOrCreate */} - {/* @ts-ignore todo */} + void onClearSelection: undefined | (() => void) searchQuery: string - setSearchQuery: () => void + setSearchQuery: React.Dispatch> selectionVariant: ActionListProps['selectionVariant'] | 'instant' }>({ title: '', - description: '', + description: undefined, panelId: '', onCancel: () => {}, onClearSelection: undefined, @@ -41,50 +43,87 @@ const SelectPanelContext = React.createContext<{ selectionVariant: 'multiple', }) -// @ts-ignore todo -const SelectPanel = props => { - const anchorRef = useProvidedRefOrCreate(props.anchorRef) +export type SelectPanelProps = { + title: string + description?: string + selectionVariant?: ActionListProps['selectionVariant'] | 'instant' + id?: string + + defaultOpen?: boolean + open?: boolean + anchorRef?: React.RefObject + + onCancel?: () => void + onClearSelection?: undefined | (() => void) + onSubmit?: (event?: React.FormEvent) => void + + // TODO: move these to SelectPanel.Overlay or overlayProps + width?: AnchoredOverlayProps['width'] + height?: AnchoredOverlayProps['height'] + + children: React.ReactNode +} + +const Panel: React.FC = ({ + title, + description, + selectionVariant = 'multiple', + id, + + defaultOpen = false, + open: propsOpen, + anchorRef: providedAnchorRef, + + onCancel: propsOnCancel, + onClearSelection: propsOnClearSelection, + onSubmit: propsOnSubmit, + + width = 'medium', + height = 'large', + ...props +}) => { + const anchorRef = useProvidedRefOrCreate(providedAnchorRef) // 🚨 Hack for good API! // we strip out Anchor from children and pass it to AnchoredOverlay to render // with additional props for accessibility let renderAnchor: AnchoredOverlayProps['renderAnchor'] = null const contents = React.Children.map(props.children, child => { - if (child?.type === SelectPanelButton) { + if (React.isValidElement(child) && child.type === SelectPanelButton) { renderAnchor = anchorProps => React.cloneElement(child, anchorProps) return null } return child }) - const [internalOpen, setInternalOpen] = React.useState(props.defaultOpen) + const [internalOpen, setInternalOpen] = React.useState(defaultOpen) // sync open state - React.useEffect(() => setInternalOpen(props.open), [props.open]) + if (propsOpen !== undefined && internalOpen !== propsOpen) setInternalOpen(propsOpen) const onInternalClose = () => { - if (props.open === undefined) setInternalOpen(false) - if (typeof props.onCancel === 'function') props.onCancel() + if (propsOpen === undefined) setInternalOpen(false) + if (typeof propsOnCancel === 'function') propsOnCancel() } - const onInternalSubmit = (event?: React.SyntheticEvent) => { + const onInternalSubmit = (event?: React.FormEvent) => { event?.preventDefault() // there is no event with selectionVariant=instant - if (props.open === undefined) setInternalOpen(false) - if (typeof props.onSubmit === 'function') props.onSubmit(event) + if (propsOpen === undefined) setInternalOpen(false) + if (typeof propsOnSubmit === 'function') propsOnSubmit(event) } const onInternalClearSelection = () => { - if (typeof props.onSubmit === 'function') props.onClearSelection() + if (typeof propsOnClearSelection === 'function') propsOnClearSelection() } const internalAfterSelect = () => { - if (props.selectionVariant === 'instant') onInternalSubmit() + if (selectionVariant === 'instant') onInternalSubmit() } /* Search/Filter */ - const [searchQuery, setSearchQuery] = React.useState('') + const [searchQuery, setSearchQuery] = React.useState('') /* Panel plumbing */ - const panelId = useId(props.id) + const panelId = useId(id) const [slots, childrenInBody] = useSlots(contents, {header: SelectPanelHeader, footer: SelectPanelFooter}) /* Arrow keys navigation for list items */ @@ -99,14 +138,13 @@ const SelectPanel = props => { return ( <> setInternalOpen(true)} onClose={onInternalClose} - width={props.width || 'medium'} - height={props.height || 'large'} + width={width} + height={height} focusZoneSettings={{ // we only want focus trap from the overlay, // we don't want focus zone on the whole overlay because @@ -116,20 +154,19 @@ const SelectPanel = props => { overlayProps={{ role: 'dialog', 'aria-labelledby': `${panelId}--title`, - 'aria-describedby': props.description ? `${panelId}--description` : undefined, + 'aria-describedby': description ? `${panelId}--description` : undefined, }} > { }} > {/* render default header as fallback */} - {slots.header || } + {slots.header ?? } } @@ -162,8 +199,7 @@ const SelectPanel = props => { container: 'SelectPanel', listRole: 'listbox', selectionAttribute: 'aria-selected', - selectionVariant: - props.selectionVariant === 'instant' ? 'single' : props.selectionVariant || 'multiple', + selectionVariant: selectionVariant === 'instant' ? 'single' : selectionVariant, afterSelect: internalAfterSelect, }} > @@ -178,11 +214,9 @@ const SelectPanel = props => { ) } -const SelectPanelButton = React.forwardRef((props, anchorRef) => { - // @ts-ignore todo +const SelectPanelButton = React.forwardRef((props, anchorRef) => { return