From 080351c269bac222c7be7cdd33f9d9d967b9945b Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 20 Sep 2023 17:17:50 +0530 Subject: [PATCH 01/35] remove sort + divider --- .../SelectPanel2/SelectPanel.stories.tsx | 75 +++++-------------- 1 file changed, 17 insertions(+), 58 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 6e6702b2fcc..96089572199 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -11,8 +11,8 @@ const getCircle = (color: string) => ( export const AControlled = () => { const [filteredLabels, setFilteredLabels] = React.useState(data.labels) - const initialSelectedLabels: string[] = [] // initial state: no labels - // const initialSelectedLabels = data.issue.labelIds // initial state: has labels + // const initialSelectedLabels: string[] = [] // initial state: no labels + const initialSelectedLabels = data.issue.labelIds // initial state: has labels const [selectedLabelIds, setSelectedLabelIds] = React.useState(initialSelectedLabels) @@ -58,25 +58,14 @@ export const AControlled = () => { console.log('form submitted') } - const sortingFn = (labelA: {id: string}, labelB: {id: string}) => { - /* Important! This sorting is only for initial selected ids, not for subsequent changes! - deterministic sorting for better UX: don't change positions with other selected items. - - TODO: should this sorting be baked-in OR we only validate + warn OR do nothing - need to either own or accept the selection state to make that automatic - OR provide a API for sorting in ActionList like sort by key or sort fn - */ - if (selectedLabelIds.includes(labelA.id) && selectedLabelIds.includes(labelB.id)) return 1 - else if (selectedLabelIds.includes(labelA.id)) return -1 - else if (selectedLabelIds.includes(labelB.id)) return 1 - else return 1 - } + const labelsToShow = query ? filteredLabels : data.labels return ( <>

Controlled SelectPanel

{ - {/* slightly different view for search results view and list view */} - {query ? ( - filteredLabels.length > 1 ? ( - filteredLabels.map(label => ( - onLabelSelect(label.id)} - selected={selectedLabelIds.includes(label.id)} - > - {getCircle(label.color)} - {label.name} - {label.description} - - )) - ) : ( - No labels found for "{query}" - ) + {labelsToShow.length > 1 ? ( + labelsToShow.map(label => ( + onLabelSelect(label.id)} + selected={selectedLabelIds.includes(label.id)} + > + {getCircle(label.color)} + {label.name} + {label.description} + + )) ) : ( - <> - {data.labels.sort(sortingFn).map((label, index) => { - /* - we want to render a divider between the group of selected and unselected items. - kinda hack: if this is the last item that is selected, render an divider after it - TODO: can this be cleaner? - */ - const nextLabel = data.labels.sort(sortingFn)[index + 1] - const showDivider = selectedLabelIds.includes(label.id) && !selectedLabelIds.includes(nextLabel?.id) - - return ( - <> - onLabelSelect(label.id)} - selected={selectedLabelIds.includes(label.id)} - > - {getCircle(label.color)} - {label.name} - {label.description} - - {showDivider ? : null} - - ) - })} - + No labels found for "{query}" )} From 445e150c3633de6a405a0ff3db2e64b3cf4f4f62 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 20 Sep 2023 17:18:28 +0530 Subject: [PATCH 02/35] reorder state to be more cohesive --- .../SelectPanel2/SelectPanel.stories.tsx | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 96089572199..a5af323563e 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -9,13 +9,30 @@ const getCircle = (color: string) => ( ) export const AControlled = () => { - const [filteredLabels, setFilteredLabels] = React.useState(data.labels) + const initialSelectedLabels = data.issue.labelIds // mock initial state: has selected labels + const [selectedLabelIds, setSelectedLabelIds] = React.useState(initialSelectedLabels) - // const initialSelectedLabels: string[] = [] // initial state: no labels - const initialSelectedLabels = data.issue.labelIds // initial state: has labels + /* Selection */ + const onLabelSelect = (labelId: string) => { + if (!selectedLabelIds.includes(labelId)) setSelectedLabelIds([...selectedLabelIds, labelId]) + else setSelectedLabelIds(selectedLabelIds.filter(id => id !== labelId)) + } - const [selectedLabelIds, setSelectedLabelIds] = React.useState(initialSelectedLabels) + const onClearSelection = () => { + // soft set, does not save until submit + setSelectedLabelIds([]) + } + const onSubmit = (event: {preventDefault: () => void}) => { + event.preventDefault() // coz form submit, innit + data.issue.labelIds = selectedLabelIds // pretending to persist changes + + // eslint-disable-next-line no-console + console.log('form submitted') + } + + /* Filtering */ + const [filteredLabels, setFilteredLabels] = React.useState(data.labels) const [query, setQuery] = React.useState('') // TODO: should this be baked-in @@ -40,25 +57,7 @@ export const AControlled = () => { } } - const onLabelSelect = (labelId: string) => { - if (!selectedLabelIds.includes(labelId)) setSelectedLabelIds([...selectedLabelIds, labelId]) - else setSelectedLabelIds(selectedLabelIds.filter(id => id !== labelId)) - } - - const onClearSelection = () => { - // soft set, does not save until submit - setSelectedLabelIds([]) - } - - const onSubmit = (event: {preventDefault: () => void}) => { - event.preventDefault() // coz form submit, innit - data.issue.labelIds = selectedLabelIds // pretending to persist changes - - // eslint-disable-next-line no-console - console.log('form submitted') - } - - const labelsToShow = query ? filteredLabels : data.labels + const itemsToShow = query ? filteredLabels : data.labels return ( <> @@ -95,8 +94,10 @@ export const AControlled = () => { - {labelsToShow.length > 1 ? ( - labelsToShow.map(label => ( + {itemsToShow.length === 0 ? ( + No labels found for "{query}" + ) : ( + itemsToShow.map(label => ( onLabelSelect(label.id)} @@ -107,8 +108,6 @@ export const AControlled = () => { {label.description} )) - ) : ( - No labels found for "{query}" )} From 3b36ae080068c8d7768e7e4a781238185bbf26c4 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 20 Sep 2023 17:21:35 +0530 Subject: [PATCH 03/35] clean up suspended story --- .../SelectPanel2/SelectPanel.stories.tsx | 40 ++++--------------- 1 file changed, 8 insertions(+), 32 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index a5af323563e..18cb55ed261 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -153,6 +153,7 @@ export const BWithSuspendedList = () => { const SuspendedActionList: React.FC<{query: string}> = ({query}) => { const fetchedData: typeof data = use(getData({key: 'suspended-action-list'})) + /* Selection */ const initialSelectedLabels: string[] = fetchedData.issue.labelIds const [selectedLabelIds, setSelectedLabelIds] = React.useState(initialSelectedLabels) @@ -161,13 +162,7 @@ const SuspendedActionList: React.FC<{query: string}> = ({query}) => { else setSelectedLabelIds(selectedLabelIds.filter(id => id !== labelId)) } - const sortingFn = (labelA: {id: string}, labelB: {id: string}) => { - if (selectedLabelIds.includes(labelA.id) && selectedLabelIds.includes(labelB.id)) return 1 - else if (selectedLabelIds.includes(labelA.id)) return -1 - else if (selectedLabelIds.includes(labelB.id)) return 1 - else return 1 - } - + /* Filtering */ const filteredLabels = fetchedData.labels .map(label => { if (label.name.toLowerCase().startsWith(query)) return {priority: 1, label} @@ -178,11 +173,14 @@ const SuspendedActionList: React.FC<{query: string}> = ({query}) => { .filter(result => result.priority > 0) .map(result => result.label) + const itemsToShow = query ? filteredLabels : data.labels + return ( - {/* slightly different view for search results view and list view */} - {query ? ( - filteredLabels.map(label => ( + {itemsToShow.length === 0 ? ( + No labels found for "{query}" + ) : ( + itemsToShow.map(label => ( onLabelSelect(label.id)} @@ -193,28 +191,6 @@ const SuspendedActionList: React.FC<{query: string}> = ({query}) => { {label.description} )) - ) : ( - <> - {fetchedData.labels.sort(sortingFn).map((label, index) => { - const nextLabel = fetchedData.labels.sort(sortingFn)[index + 1] - const showDivider = selectedLabelIds.includes(label.id) && !selectedLabelIds.includes(nextLabel?.id) - - return ( - <> - onLabelSelect(label.id)} - selected={selectedLabelIds.includes(label.id)} - > - {getCircle(label.color)} - {label.name} - {label.description} - - {showDivider ? : null} - - ) - })} - )} ) From f14d727a0ea32f33f44e3aa54b0e07a7c82def22 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 20 Sep 2023 17:26:53 +0530 Subject: [PATCH 04/35] clean up C and D --- .../SelectPanel2/SelectPanel.stories.tsx | 73 ++++++------------- 1 file changed, 23 insertions(+), 50 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 18cb55ed261..4657a8f6832 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -231,35 +231,33 @@ export const CAsyncSearchWithSuspenseKey = () => { ) } +/* + `data` is already pre-fetched with the issue + `users` are fetched async on search +*/ const SearchableUserList: React.FC<{query: string; showLoading?: boolean}> = ({query, showLoading = false}) => { - // issue `data` is already pre-fetched const repository = {collaborators: data.collaborators} - // `users` are fetched async on search - const filteredUsers: typeof data.users = query ? use(queryUsers({query})) : [] + /* Selection */ const initialSelectedUsers: string[] = data.issue.assigneeIds const [selectedUserIds, setSelectedUserIds] = React.useState(initialSelectedUsers) - const onUserSelect = (userId: string) => { if (!selectedUserIds.includes(userId)) setSelectedUserIds([...selectedUserIds, userId]) else setSelectedUserIds(selectedUserIds.filter(id => id !== userId)) } - const sortingFn = (userA: {id: string}, userB: {id: string}) => { - if (selectedUserIds.includes(userA.id) && selectedUserIds.includes(userB.id)) return 1 - else if (selectedUserIds.includes(userA.id)) return -1 - else if (selectedUserIds.includes(userB.id)) return 1 - else return 1 - } + /* Filtering */ + const filteredUsers: typeof data.users = query ? use(queryUsers({query})) : [] - // only used with useTransition example if (showLoading) return Search for users... + const itemsToShow = query ? filteredUsers : repository.collaborators - /* slightly different view for search results view and list view */ - if (query) { - return ( - - {filteredUsers.map(user => ( + return ( + + {itemsToShow.length === 0 ? ( + No users found for "{query}" + ) : ( + itemsToShow.map(user => ( onUserSelect(user.id)} @@ -271,42 +269,17 @@ const SearchableUserList: React.FC<{query: string; showLoading?: boolean}> = ({q {user.login} {user.name} - ))} - - ) - } else { - return ( - - {repository.collaborators.sort(sortingFn).map((user, index) => { - // tiny bit of additional logic to show divider - const nextUser = repository.collaborators.sort(sortingFn)[index + 1] - const showDivider = selectedUserIds.includes(user.id) && !selectedUserIds.includes(nextUser?.id) - return ( - <> - onUserSelect(user.id)} - selected={selectedUserIds.includes(user.id)} - > - - - - {user.login} - {user.name} - - {showDivider ? : null} - - ) - })} - - ) - } + )) + )} + + ) } +/* + `data` is already pre-fetched with the issue + `users` are fetched async on search +*/ export const DAsyncSearchWithUseTransition = () => { - // issue `data` is already pre-fetched - // `users` are fetched async on search - const [isPending, startTransition] = React.useTransition() const [query, setQuery] = React.useState('') @@ -340,7 +313,7 @@ export const DAsyncSearchWithUseTransition = () => { export const TODO1Uncontrolled = () => { /* features to implement: 1. search - 2. sort + divider + 2. sort 3. selection 4. clear selection 5. different results view From c7ed2d7e3e55f4de3f582dde349c197d99b74e3a Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 20 Sep 2023 17:35:24 +0530 Subject: [PATCH 05/35] A: sort items to show initially selected ones first --- src/drafts/SelectPanel2/SelectPanel.stories.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 4657a8f6832..22ed1987a9b 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -57,7 +57,15 @@ export const AControlled = () => { } } - const itemsToShow = query ? filteredLabels : data.labels + 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) return ( <> From 6296f32b59e474c849ba9ad6bed8ee1a2f041643 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 Sep 2023 17:24:09 +0530 Subject: [PATCH 06/35] add event.preventDefault by default --- 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 f193c9b43d7..0bb5356af99 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -58,6 +58,7 @@ const SelectPanel = props => { } // @ts-ignore todo const onInternalSubmit = event => { + event.preventDefault() setOpen(false) if (typeof props.onSubmit === 'function') props.onSubmit(event) } From 7350510b50d579cafb0a22921522991a0bbcde45 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 Sep 2023 17:24:22 +0530 Subject: [PATCH 07/35] add initial sorting function to stories --- .../SelectPanel2/SelectPanel.stories.tsx | 69 +++++++++++++------ src/drafts/SelectPanel2/work-log.md | 14 ++-- 2 files changed, 58 insertions(+), 25 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 22ed1987a9b..f06fded26c3 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -23,8 +23,7 @@ export const AControlled = () => { setSelectedLabelIds([]) } - const onSubmit = (event: {preventDefault: () => void}) => { - event.preventDefault() // coz form submit, innit + const onSubmit = () => { data.issue.labelIds = selectedLabelIds // pretending to persist changes // eslint-disable-next-line no-console @@ -181,7 +180,15 @@ const SuspendedActionList: React.FC<{query: string}> = ({query}) => { .filter(result => result.priority > 0) .map(result => result.label) - const itemsToShow = query ? filteredLabels : data.labels + 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) return ( @@ -214,12 +221,27 @@ export const CAsyncSearchWithSuspenseKey = () => { setQuery(query) } + /* Selection */ + const initialAssigneeIds: string[] = data.issue.assigneeIds + const [selectedUserIds, setSelectedUserIds] = React.useState(initialAssigneeIds) + const onUserSelect = (userId: string) => { + if (!selectedUserIds.includes(userId)) setSelectedUserIds([...selectedUserIds, userId]) + else setSelectedUserIds(selectedUserIds.filter(id => id !== userId)) + } + + const onSubmit = () => { + data.issue.assigneeIds = selectedUserIds // pretending to persist changes + + // eslint-disable-next-line no-console + console.log('form submitted') + } + return ( <>

Async search with useTransition

Fetching items on every keystroke search (like github users)

- + {/* @ts-ignore todo */} Select assignees @@ -231,7 +253,12 @@ export const CAsyncSearchWithSuspenseKey = () => { Docs reference: https://react.dev/reference/react/Suspense#resetting-suspense-boundaries-on-navigation */} Fetching users...}> - + @@ -243,22 +270,27 @@ export const CAsyncSearchWithSuspenseKey = () => { `data` is already pre-fetched with the issue `users` are fetched async on search */ -const SearchableUserList: React.FC<{query: string; showLoading?: boolean}> = ({query, showLoading = false}) => { +const SearchableUserList: React.FC<{ + query: string + showLoading?: boolean + initialAssigneeIds: string[] + selectedUserIds: string[] + onUserSelect: (id: string) => void +}> = ({query, showLoading = false, initialAssigneeIds, selectedUserIds, onUserSelect}) => { const repository = {collaborators: data.collaborators} - /* Selection */ - const initialSelectedUsers: string[] = data.issue.assigneeIds - const [selectedUserIds, setSelectedUserIds] = React.useState(initialSelectedUsers) - const onUserSelect = (userId: string) => { - if (!selectedUserIds.includes(userId)) setSelectedUserIds([...selectedUserIds, userId]) - else setSelectedUserIds(selectedUserIds.filter(id => id !== userId)) - } - /* Filtering */ const filteredUsers: typeof data.users = query ? use(queryUsers({query})) : [] if (showLoading) return Search for users... - const itemsToShow = query ? filteredUsers : repository.collaborators + + const sortingFn = (itemA: {id: string}, itemB: {id: string}) => { + if (initialAssigneeIds.includes(itemA.id) && initialAssigneeIds.includes(itemB.id)) return 1 + else if (initialAssigneeIds.includes(itemA.id)) return -1 + else if (initialAssigneeIds.includes(itemB.id)) return 1 + else return 1 + } + const itemsToShow = query ? filteredUsers : repository.collaborators.sort(sortingFn) return ( @@ -330,9 +362,7 @@ export const TODO1Uncontrolled = () => { 9. empty state */ - const onSubmit = (event: {preventDefault: () => void}) => { - event.preventDefault() // coz form submit, innit - + const onSubmit = () => { // TODO: where does saved data come from? // data.issue.labelIds = selectedLabelIds // pretending to persist changes @@ -432,8 +462,7 @@ export const TODO4WithFilterButtons = () => { else setSelectedLabelIds(selectedLabelIds.filter(id => id !== labelId)) } - const onSubmit = (event: {preventDefault: () => void}) => { - event.preventDefault() // coz form submit, innit + const onSubmit = () => { data.issue.labelIds = selectedLabelIds // pretending to persist changes // eslint-disable-next-line no-console diff --git a/src/drafts/SelectPanel2/work-log.md b/src/drafts/SelectPanel2/work-log.md index 3c75cef1efb..39884c13b35 100644 --- a/src/drafts/SelectPanel2/work-log.md +++ b/src/drafts/SelectPanel2/work-log.md @@ -37,12 +37,16 @@ ## Implementation notes -1. [Next for Sid] Improve divider logic in stories (we don't need 2 branches, instead a ConditionalDivider component or a showDividerAtIndex variable) (we don't even need it anymore, + also leave a comment in the code to the issue where we decided not to do it) 1. Is there a way to absorb divider logic, right now it's the application's responsibility -1. Add controlled state for `open` (use cases: 1. fetch data when opened, 2. nested menus, 3. keep panel open till it's saved: https://github.com/github/primer/issues/2403) +1. Add controlled state for `open` (use cases: 1. fetch data when opened, 2. nested menus +1. keep panel open till it's saved: https://github.com/github/primer/issues/2403) 1. We probably (need to check) should not even render Overlay contents until it's opened -1. Add SelectPanel.EmptyMessage to all stories -1. SelectPanel.Overlay -1. The flicker in story with useTransition is unfortunate, is there already a way to add a minimum time to avoid this (debounce)? and is it possible/ergonomic to bake that in the component or should it be delegated to the application +1. SelectPanel.Overlay API 1. I think it's nice that there is a `` because you can wrap it in suspense along with the search results 1. Need to make Save and Cancel optional (selectionVariant="instant"?) + +### Stories + +1. [Next for Sid] Improve divider logic in stories (we don't need 2 branches, instead a ConditionalDivider component or a showDividerAtIndex variable) (we don't even need it anymore, + also leave a comment in the code to the issue where we decided not to do it) +1. Add SelectPanel.EmptyMessage to all stories +1. The flicker in story with useTransition is unfortunate, is there already a way to add a minimum time to avoid this (debounce)? and is it possible/ergonomic to bake that in the component or should it be delegated to the application From a5dbe92b17c4eb750532acbae3294dbab18c268d Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 Sep 2023 17:27:49 +0530 Subject: [PATCH 08/35] add missing props for D with use transition --- .../SelectPanel2/SelectPanel.stories.tsx | 24 +++++++++++++++++-- src/drafts/SelectPanel2/mock-data.ts | 2 +- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index f06fded26c3..3e818596702 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -328,12 +328,26 @@ export const DAsyncSearchWithUseTransition = () => { startTransition(() => setQuery(query)) } + /* Selection */ + const initialAssigneeIds: string[] = data.issue.assigneeIds + const [selectedUserIds, setSelectedUserIds] = React.useState(initialAssigneeIds) + const onUserSelect = (userId: string) => { + if (!selectedUserIds.includes(userId)) setSelectedUserIds([...selectedUserIds, userId]) + else setSelectedUserIds(selectedUserIds.filter(id => id !== userId)) + } + + const onSubmit = () => { + data.issue.assigneeIds = selectedUserIds // pretending to persist changes + // eslint-disable-next-line no-console + console.log('form submitted') + } + return ( <>

Async search with useTransition

Fetching items on every keystroke search (like github users)

- + {/* @ts-ignore todo */} Select assignees @@ -342,7 +356,13 @@ export const DAsyncSearchWithUseTransition = () => { Fetching users...}> - + diff --git a/src/drafts/SelectPanel2/mock-data.ts b/src/drafts/SelectPanel2/mock-data.ts index 6d42267963f..ff510a72a19 100644 --- a/src/drafts/SelectPanel2/mock-data.ts +++ b/src/drafts/SelectPanel2/mock-data.ts @@ -1,7 +1,7 @@ const data = { issue: { labelIds: ['MDU6TGFiZWw4Mzk2MzgxMTU=', 'MDU6TGFiZWw4Mzk2MzgxMjE='], - assigneeIds: [], + assigneeIds: ['MDQ6VXNlcjQxNzI2OA=='], }, collaborators: [ { From 2f12c354ce9c999328321eafdf8bf09a6ca400ad Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Fri, 22 Sep 2023 14:23:13 +0530 Subject: [PATCH 09/35] Replace SelectPanel.Heading with title --- .../SelectPanel2/SelectPanel.stories.tsx | 19 +++++--------- src/drafts/SelectPanel2/SelectPanel.tsx | 25 ++++++++----------- 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 3e818596702..f35a7abbf94 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -71,6 +71,7 @@ export const AControlled = () => {

Controlled SelectPanel

{ Assign label {/* TODO: header and heading is confusing. maybe skip header completely. */} - {/* TODO: Heading is not optional, but what if you don't give it - Should we throw a big error or should we make that impossible in the API? - */} - Select labels @@ -137,12 +134,11 @@ export const BWithSuspendedList = () => { <>

Suspended list

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

- + {/* @ts-ignore todo */} Assign label - Select labels @@ -241,11 +237,10 @@ export const CAsyncSearchWithSuspenseKey = () => {

Async search with useTransition

Fetching items on every keystroke search (like github users)

- + {/* @ts-ignore todo */} Select assignees - Select collaborators @@ -347,11 +342,10 @@ export const DAsyncSearchWithUseTransition = () => {

Async search with useTransition

Fetching items on every keystroke search (like github users)

- + {/* @ts-ignore todo */} Select assignees - Select collaborators @@ -399,12 +393,11 @@ export const TODO1Uncontrolled = () => { <>

Does not work yet: Uncontrolled SelectPanel

- + {/* @ts-ignore todo */} Assign label - Select labels @@ -504,6 +497,7 @@ export const TODO4WithFilterButtons = () => {

With Filter Buttons

{ @@ -518,7 +512,6 @@ export const TODO4WithFilterButtons = () => { - Switch branches/tags diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 0bb5356af99..a2d46d07798 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -20,11 +20,13 @@ import {useSlots} from '../../hooks/useSlots' import {ClearIcon} from './tmp-ClearIcon' const SelectPanelContext = React.createContext<{ + title: string onCancel: () => void onClearSelection: undefined | (() => void) searchQuery: string setSearchQuery: () => void }>({ + title: '', onCancel: () => {}, onClearSelection: undefined, searchQuery: '', @@ -87,6 +89,7 @@ const SelectPanel = props => { */} = ({children, ...props}) => { const [slots, childrenWithoutSlots] = useSlots(children, { - heading: SelectPanelHeading, searchInput: SelectPanelSearchInput, }) - const {onCancel, onClearSelection} = React.useContext(SelectPanelContext) + const {title, onCancel, onClearSelection} = React.useContext(SelectPanelContext) return ( - {slots.heading} + {/* heading element is intentionally hardcoded to h1, it is not customisable + see https://github.com/github/primer/issues/2578 for context + */} + + + {title} + {/* Will not need tooltip after https://github.com/primer/react/issues/2008 */} {onClearSelection ? ( @@ -140,17 +148,6 @@ const SelectPanelHeader: React.FC = ({children, ...prop } SelectPanel.Header = SelectPanelHeader -const SelectPanelHeading: React.FC> = ({children, ...props}) => { - // heading element is intentionally hardcoded to h1, it is not customisable - // see https://github.com/github/primer/issues/2578 for context - return ( - - {children} - - ) -} -SelectPanel.Heading = SelectPanelHeading - // @ts-ignore todo const SelectPanelSearchInput = props => { const inputRef = React.createRef() From 8413f74cc55cba18978fc47c552bbcacfdb27a69 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Fri, 22 Sep 2023 16:07:12 +0530 Subject: [PATCH 10/35] add default footer --- src/drafts/SelectPanel2/SelectPanel.tsx | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index a2d46d07798..046969635aa 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -72,6 +72,8 @@ const SelectPanel = props => { /* Search/Filter */ const [searchQuery, setSearchQuery] = React.useState('') + const [slots, childrenInBody] = useSlots(contents, {header: SelectPanelHeader, footer: SelectPanelFooter}) + return ( <> { }} > - {contents} + {/* render default header as fallback */} + {slots.header || } + {childrenInBody} + {/* render default footer as fallback */} + {slots.footer || } @@ -120,8 +126,18 @@ const SelectPanelHeader: React.FC = ({children, ...prop const {title, onCancel, onClearSelection} = React.useContext(SelectPanelContext) return ( - - + + {/* heading element is intentionally hardcoded to h1, it is not customisable see https://github.com/github/primer/issues/2578 for context */} @@ -209,7 +225,7 @@ const SelectPanelActionList: React.FC> return ( <> - + {props.children} @@ -222,7 +238,6 @@ const SelectPanelFooter = ({...props}) => { return ( Date: Fri, 22 Sep 2023 16:07:17 +0530 Subject: [PATCH 11/35] add minimal story --- .../SelectPanel2/SelectPanel.stories.tsx | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index f35a7abbf94..b3cc8bbaf4c 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -512,7 +512,7 @@ export const TODO4WithFilterButtons = () => { - + + + { + setOpen(false) // close on submit + onSubmit() + }} + onCancel={() => setOpen(false)} // close on cancel + > + + {itemsToShow.map(label => ( + onLabelSelect(label.id)} + selected={selectedLabelIds.includes(label.id)} + > + {getCircle(label.color)} + {label.name} + {label.description} + + ))} + + + + ) +} + +export const GOpenFromMenu = () => { + 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 itemsToShow = data.labels.sort(sortingFn) + + const [selectPanelOpen, setSelectPanelOpen] = React.useState(false) + const buttonRef = React.useRef(null) + + return ( + <> +

Open from ActionMenu

+

+ To open SelectPanel from a menu, you would need to use an external anchor and pass `anchorRef` to `SelectPanel`. + You would also need to control the `open` state with `onSubmit` and `onCancel`. +
+
+ Important: Pass the same `anchorRef` to both ActionMenu and SelectPanel, otherwise the SelectPanel would not be + visible. +

+ + + + Unwatch + + + + + Participating and @mentions + + Only receive notifications from this repository when participating or @mentioned. + + + + All activity + + Notified of all notifications on this repository. + + + + Ignore + Never be notified. + + setSelectPanelOpen(true)}> + Custom + + + + + Select events you want to be notified of in addition to participating and @mentions. + + + + + + + { + setSelectPanelOpen(false) + onSubmit() + }} + onCancel={() => setSelectPanelOpen(false)} + > + + {itemsToShow.map(label => ( + onLabelSelect(label.id)} + selected={selectedLabelIds.includes(label.id)} + > + {getCircle(label.color)} + {label.name} + {label.description} + + ))} + + + + ) +} + // ----- Suspense implementation details ---- const cache = new Map() diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 046969635aa..2183535537b 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -18,6 +18,7 @@ import { } from '../../../src/index' import {useSlots} from '../../hooks/useSlots' import {ClearIcon} from './tmp-ClearIcon' +import {useProvidedRefOrCreate} from '../../hooks' const SelectPanelContext = React.createContext<{ title: string @@ -35,7 +36,7 @@ const SelectPanelContext = React.createContext<{ // @ts-ignore todo const SelectPanel = props => { - const anchorRef = React.useRef(null) + const anchorRef = useProvidedRefOrCreate(props.anchorRef) // 🚨 Hack for good API! // we strip out Anchor from children and pass it to AnchoredOverlay to render @@ -49,19 +50,18 @@ const SelectPanel = props => { return child }) - // TODO: defaultOpen is not for debugging, I don't intend to make - // it part of the API - const [open, setOpen] = React.useState(props.defaultOpen) + const [internalOpen, setInternalOpen] = React.useState(props.defaultOpen) + // sync open state + React.useEffect(() => setInternalOpen(props.open), [props.open]) const onInternalClose = () => { - setOpen(false) - + if (props.open === 'undefined') setInternalOpen(false) if (typeof props.onCancel === 'function') props.onCancel() } // @ts-ignore todo const onInternalSubmit = event => { event.preventDefault() - setOpen(false) + if (props.open === 'undefined') setInternalOpen(false) if (typeof props.onSubmit === 'function') props.onSubmit(event) } @@ -77,10 +77,11 @@ const SelectPanel = props => { return ( <> setOpen(true)} + open={internalOpen} + onOpen={() => setInternalOpen(true)} onClose={onInternalClose} width="medium" height="large" From 6fc2508b473f9b6114d7be65254a31adfbeffd98 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 13:21:41 +0530 Subject: [PATCH 15/35] improve custom events story --- .../SelectPanel2/SelectPanel.stories.tsx | 39 +++++++------------ src/drafts/SelectPanel2/SelectPanel.tsx | 4 +- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 71659b4ad4e..584a68493bd 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -660,31 +660,23 @@ export const FExternalAnchor = () => { } export const GOpenFromMenu = () => { - 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 [selectedEvents, setSelectedEvents] = React.useState([]) + + const onEventSelect = (event: string) => { + if (!selectedEvents.includes(event)) setSelectedEvents([...selectedEvents, event]) + else setSelectedEvents(selectedEvents.filter(name => name !== event)) } const onSubmit = () => { - data.issue.labelIds = selectedLabelIds // pretending to persist changes + data.issue.labelIds = selectedEvents // 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 itemsToShow = data.labels.sort(sortingFn) + const itemsToShow = ['Issues', 'Pull requests', 'Releases', 'Discussions', 'Security alerts'] const [selectPanelOpen, setSelectPanelOpen] = React.useState(false) const buttonRef = React.useRef(null) @@ -737,7 +729,7 @@ export const GOpenFromMenu = () => { { @@ -745,17 +737,12 @@ export const GOpenFromMenu = () => { onSubmit() }} onCancel={() => setSelectPanelOpen(false)} + height="medium" > - {itemsToShow.map(label => ( - onLabelSelect(label.id)} - selected={selectedLabelIds.includes(label.id)} - > - {getCircle(label.color)} - {label.name} - {label.description} + {itemsToShow.map(item => ( + onEventSelect(item)} selected={selectedEvents.includes(item)}> + {item} ))} diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 2183535537b..7492ae288be 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -83,8 +83,8 @@ const SelectPanel = props => { open={internalOpen} onOpen={() => setInternalOpen(true)} onClose={onInternalClose} - width="medium" - height="large" + width={props.width || 'medium'} + height={props.height || 'large'} focusZoneSettings={{bindKeys: FocusKeys.Tab}} > {/* TODO: Keyboard navigation of actionlist should be arrow keys From dd17537f336f4f5759ff6940bbc09237c54b6382 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 13:38:56 +0530 Subject: [PATCH 16/35] story: bring back filter for Filter story --- .../SelectPanel2/SelectPanel.stories.tsx | 58 +++++++++---------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index a7e176c885a..91b83ad0025 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -421,14 +421,15 @@ export const TODO1Uncontrolled = () => { export const TODO2SingleSelection = () =>

TODO

-export const TODO4WithFilterButtons = () => { +export const FWithFilterButtons = () => { const [selectedFilter, setSelectedFilter] = React.useState<'branches' | 'tags'>('branches') /* Selection */ - const initialSelectedRef = data.ref // 'main' - const [selectedRef, setSelectedRef] = React.useState(initialSelectedRef) + const [savedInitialRef, setSavedInitialRef] = React.useState(data.ref) + const [selectedRef, setSelectedRef] = React.useState(savedInitialRef) const onSubmit = () => { + setSavedInitialRef(selectedRef) data.ref = selectedRef // pretending to persist changes // eslint-disable-next-line no-console @@ -436,7 +437,6 @@ export const TODO4WithFilterButtons = () => { } /* Filter */ - const [query, setQuery] = React.useState('') const onSearchInputChange = (event: React.KeyboardEvent) => { const query = event.currentTarget.value @@ -444,33 +444,31 @@ export const TODO4WithFilterButtons = () => { } const [filteredRefs, setFilteredRefs] = React.useState(data.branches) - // const setSearchResults = (query: string, selectedFilter: 'branches' | 'tags') => { - // if (query === '') setFilteredRefs(data[selectedFilter]) - // else { - // // TODO: should probably add a highlight for matching text - // // TODO: This should be a joined array, not seperate, only separated at the render level - // setFilteredRefs( - // data[selectedFilter] - // .map(item => { - // if (item.name.toLowerCase().startsWith(query)) return {priority: 1, item} - // else if (item.name.toLowerCase().includes(query)) return {priority: 2, item} - // else return {priority: -1, item} - // }) - // .filter(result => result.priority > 0) - // .map(result => result.item), - // ) - // } - // } - - // React.useEffect( - // function updateSearchResults() { - // setSearchResults(query, selectedFilter) - // }, - // [query, selectedFilter], - // ) + const setSearchResults = (query: string, selectedFilter: 'branches' | 'tags') => { + if (query === '') setFilteredRefs(data[selectedFilter]) + else { + setFilteredRefs( + data[selectedFilter] + .map(item => { + if (item.name.toLowerCase().startsWith(query)) return {priority: 1, item} + else if (item.name.toLowerCase().includes(query)) return {priority: 2, item} + else return {priority: -1, item} + }) + .filter(result => result.priority > 0) + .map(result => result.item), + ) + } + } + + React.useEffect( + function updateSearchResults() { + setSearchResults(query, selectedFilter) + }, + [query, selectedFilter], + ) const sortingFn = (ref: {id: string}) => { - if (ref.id === initialSelectedRef) return -1 + if (ref.id === savedInitialRef) return -1 else return 1 } @@ -484,7 +482,7 @@ export const TODO4WithFilterButtons = () => { {/* TODO: the ref types don't match here, use useProvidedRefOrCreate */} {/* @ts-ignore todo */} - {selectedRef} + {savedInitialRef} From 5dd28531fe75da16ddb969cfb92e13c0e9cb2d07 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 13:41:03 +0530 Subject: [PATCH 17/35] rename story --- src/drafts/SelectPanel2/SelectPanel.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index e882ec1d689..05a067db233 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -421,7 +421,7 @@ export const TODO1Uncontrolled = () => { export const TODO2SingleSelection = () =>

TODO

-export const FWithFilterButtons = () => { +export const HWithFilterButtons = () => { const [selectedFilter, setSelectedFilter] = React.useState<'branches' | 'tags'>('branches') /* Selection */ From 211d5ba776c4d00d9c3e69050d402d0898f93cda Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 16:37:14 +0530 Subject: [PATCH 18/35] remove the need for SelectPanel.ActionList --- src/ActionList/ActionListContainerContext.tsx | 2 +- src/ActionList/List.tsx | 1 + .../SelectPanel2/SelectPanel.stories.tsx | 32 +++++++-------- src/drafts/SelectPanel2/SelectPanel.tsx | 41 +++++++++---------- 4 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/ActionList/ActionListContainerContext.tsx b/src/ActionList/ActionListContainerContext.tsx index bc57c3a06de..bf5c797bff0 100644 --- a/src/ActionList/ActionListContainerContext.tsx +++ b/src/ActionList/ActionListContainerContext.tsx @@ -6,7 +6,7 @@ import {AriaRole} from '../utils/types' type ContextProps = { container?: string listRole?: AriaRole - selectionVariant?: 'single' | 'multiple' // TODO: Remove after DropdownMenu2 deprecation + selectionVariant?: 'single' | 'multiple' selectionAttribute?: 'aria-selected' | 'aria-checked' listLabelledBy?: string // This can be any function, we don't know anything about the arguments diff --git a/src/ActionList/List.tsx b/src/ActionList/List.tsx index 29bc2f54953..67be23d5a83 100644 --- a/src/ActionList/List.tsx +++ b/src/ActionList/List.tsx @@ -54,6 +54,7 @@ export const List = React.forwardRef( sx={merge(styles, sxProp as SxProp)} role={role || listRole} aria-labelledby={listLabelledBy} + data-component="ActionList" {...props} ref={forwardedRef} > diff --git a/src/drafts/SelectPanel2/SelectPanel.stories.tsx b/src/drafts/SelectPanel2/SelectPanel.stories.tsx index 05a067db233..e83f7ad5e29 100644 --- a/src/drafts/SelectPanel2/SelectPanel.stories.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.stories.tsx @@ -97,7 +97,7 @@ export const AControlled = () => { - + {itemsToShow.length === 0 ? ( No labels found for "{query}" ) : ( @@ -113,7 +113,7 @@ export const AControlled = () => { )) )} - + Edit labels @@ -187,7 +187,7 @@ const SuspendedActionList: React.FC<{query: string}> = ({query}) => { const itemsToShow = query ? filteredLabels : data.labels.sort(sortingFn) return ( - + {itemsToShow.length === 0 ? ( No labels found for "{query}" ) : ( @@ -203,7 +203,7 @@ const SuspendedActionList: React.FC<{query: string}> = ({query}) => { )) )} - + ) } @@ -288,7 +288,7 @@ const SearchableUserList: React.FC<{ const itemsToShow = query ? filteredUsers : repository.collaborators.sort(sortingFn) return ( - + {itemsToShow.length === 0 ? ( No users found for "{query}" ) : ( @@ -306,7 +306,7 @@ const SearchableUserList: React.FC<{ )) )} - + ) } @@ -401,7 +401,7 @@ export const TODO1Uncontrolled = () => { - + {data.labels.map(label => ( {getCircle(label.color)} @@ -409,7 +409,7 @@ export const TODO1Uncontrolled = () => { {label.description} ))} - + Edit labels @@ -506,7 +506,7 @@ export const HWithFilterButtons = () => { - + {itemsToShow.length === 0 ? ( No labels found for "{'query'}" ) : ( @@ -521,7 +521,7 @@ export const HWithFilterButtons = () => { )) )} - + @@ -569,7 +569,7 @@ export const EMinimal = () => { {/* @ts-ignore todo */} Assign label - + {itemsToShow.map(label => ( { {label.description} ))} - + ) @@ -639,7 +639,7 @@ export const FExternalAnchor = () => { }} onCancel={() => setOpen(false)} // close on cancel > - + {itemsToShow.map(label => ( { {label.description} ))} - + ) @@ -737,13 +737,13 @@ export const GOpenFromMenu = () => { onCancel={() => setSelectPanelOpen(false)} height="medium" > - + {itemsToShow.map(item => ( onEventSelect(item)} selected={selectedEvents.includes(item)}> {item} ))} - + ) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 7492ae288be..70949320b4f 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -16,6 +16,7 @@ import { Spinner, Text, } from '../../../src/index' +import {ActionListContainerContext} from '../../../src/ActionList/ActionListContainerContext' import {useSlots} from '../../hooks/useSlots' import {ClearIcon} from './tmp-ClearIcon' import {useProvidedRefOrCreate} from '../../hooks' @@ -60,7 +61,7 @@ const SelectPanel = props => { } // @ts-ignore todo const onInternalSubmit = event => { - event.preventDefault() + event?.preventDefault() if (props.open === 'undefined') setInternalOpen(false) if (typeof props.onSubmit === 'function') props.onSubmit(event) } @@ -100,10 +101,25 @@ const SelectPanel = props => { setSearchQuery, }} > - + {/* render default header as fallback */} {slots.header || } - {childrenInBody} + + {childrenInBody} + {/* render default footer as fallback */} {slots.footer || } @@ -215,25 +231,6 @@ const SelectPanelSearchInput = props => { } SelectPanel.SearchInput = SelectPanelSearchInput -const SelectPanelActionList: React.FC> = props => { - /* features to implement for uncontrolled: - 1. select - 2. sort - 3. divider - 4. search - 5. different results view - */ - - return ( - <> - - {props.children} - - - ) -} -SelectPanel.ActionList = SelectPanelActionList - const SelectPanelFooter = ({...props}) => { const {onCancel} = React.useContext(SelectPanelContext) From 47206532fd4fbf668f68eff2f5dad0afb45b3896 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 16:50:18 +0530 Subject: [PATCH 19/35] add selection variable by default --- src/drafts/SelectPanel2/SelectPanel.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index 70949320b4f..cc6d8898cf4 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -117,7 +117,12 @@ const SelectPanel = props => { > {/* render default header as fallback */} {slots.header || } - + {childrenInBody} {/* render default footer as fallback */} From ba0736458efa17f75707cd7f3c003d894dd8383e Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 16:51:39 +0530 Subject: [PATCH 20/35] add list and item role automatically --- src/ActionList/Item.tsx | 10 +++++++++- src/drafts/SelectPanel2/SelectPanel.tsx | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index cbe721b9cfa..44135c6506e 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -38,7 +38,12 @@ export const Item = React.forwardRef( trailingVisual: TrailingVisual, description: Description, }) - const {variant: listVariant, showDividers, selectionVariant: listSelectionVariant} = React.useContext(ListContext) + const { + variant: listVariant, + role: listRole, + showDividers, + selectionVariant: listSelectionVariant, + } = React.useContext(ListContext) const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext) const {container, afterSelect, selectionAttribute} = React.useContext(ActionListContainerContext) @@ -65,6 +70,9 @@ export const Item = React.forwardRef( if (selectionVariant === 'single') itemRole = 'menuitemradio' else if (selectionVariant === 'multiple') itemRole = 'menuitemcheckbox' else itemRole = 'menuitem' + } else if (listRole === 'listbox') { + if (selectionVariant !== undefined) itemRole = 'option' + else itemRole = 'listitem' } const {theme} = useTheme() diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index cc6d8898cf4..b7f75b35fef 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -120,6 +120,8 @@ const SelectPanel = props => { From a4197ad802bff4c9c4a1440e424e4a650356653f Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 17:10:20 +0530 Subject: [PATCH 21/35] oops, unused imports --- src/drafts/SelectPanel2/SelectPanel.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/drafts/SelectPanel2/SelectPanel.tsx b/src/drafts/SelectPanel2/SelectPanel.tsx index b7f75b35fef..48584c02634 100644 --- a/src/drafts/SelectPanel2/SelectPanel.tsx +++ b/src/drafts/SelectPanel2/SelectPanel.tsx @@ -11,8 +11,6 @@ import { Tooltip, TextInput, AnchoredOverlayProps, - ActionList, - ActionListProps, Spinner, Text, } from '../../../src/index' From 1420edc74f9939ae890a2e8a90f43dc4a7502b9b Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 17:59:53 +0530 Subject: [PATCH 22/35] reduce scope --- src/ActionList/Item.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 44135c6506e..95266c130e9 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -70,9 +70,8 @@ export const Item = React.forwardRef( if (selectionVariant === 'single') itemRole = 'menuitemradio' else if (selectionVariant === 'multiple') itemRole = 'menuitemcheckbox' else itemRole = 'menuitem' - } else if (listRole === 'listbox') { + } else if (container === 'SelectPanel' && listRole === 'listbox') { if (selectionVariant !== undefined) itemRole = 'option' - else itemRole = 'listitem' } const {theme} = useTheme() From be443807ab6a2f5681bb771d65ddf78acf7a99d6 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 25 Sep 2023 18:09:25 +0530 Subject: [PATCH 23/35] update ActionList snapshot --- src/__tests__/__snapshots__/ActionList.test.tsx.snap | 1 + 1 file changed, 1 insertion(+) diff --git a/src/__tests__/__snapshots__/ActionList.test.tsx.snap b/src/__tests__/__snapshots__/ActionList.test.tsx.snap index 5bbbad35875..d9f4948a795 100644 --- a/src/__tests__/__snapshots__/ActionList.test.tsx.snap +++ b/src/__tests__/__snapshots__/ActionList.test.tsx.snap @@ -10,5 +10,6 @@ exports[`ActionList renders consistently 1`] = `