diff --git a/.changeset/good-lizards-brush.md b/.changeset/good-lizards-brush.md new file mode 100644 index 00000000000..76ae9237abe --- /dev/null +++ b/.changeset/good-lizards-brush.md @@ -0,0 +1,8 @@ +--- +'@shopify/polaris': minor +--- + +- adds dirty and unselected states to `use-index-resource-state` +- adds stronger types to `use-index-resource-state` +- Keeps the same array interface of the API but uses `Set` under the hood for performance optimization. + - Set was used since it offers O(1) direct access to its values. In some cases we removed O(N) traversal times since we cared about access lookup. diff --git a/polaris-react/src/utilities/tests/use-index-resource-state.test.tsx b/polaris-react/src/utilities/tests/use-index-resource-state.test.tsx index fd25d7847c1..0dced3f4522 100644 --- a/polaris-react/src/utilities/tests/use-index-resource-state.test.tsx +++ b/polaris-react/src/utilities/tests/use-index-resource-state.test.tsx @@ -6,18 +6,7 @@ import { SelectionType, } from '../use-index-resource-state'; -interface TypedChildProps { - onClick: ReturnType['handleSelectionChange']; - allResourcesSelected: ReturnType< - typeof useIndexResourceState - >['allResourcesSelected']; - selectedResources: ReturnType< - typeof useIndexResourceState - >['selectedResources']; - removeSelectedResources: ReturnType< - typeof useIndexResourceState - >['removeSelectedResources']; -} +interface TypedChildProps extends ReturnType {} describe('useIndexResourceState', () => { function TypedChild(_: TypedChildProps) { @@ -28,7 +17,7 @@ describe('useIndexResourceState', () => { resources = [], options, }: { - resources?: T[]; + resources?: readonly T[]; options?: Parameters[1]; }) { const { @@ -36,38 +25,20 @@ describe('useIndexResourceState', () => { allResourcesSelected, handleSelectionChange, removeSelectedResources, - } = useIndexResourceState(resources, options); - - return ( - - ); - } - - function MockClearComponent({ - resources = [], - options, - }: { - resources?: T[]; - options?: Parameters[1]; - }) { - const { - selectedResources, - allResourcesSelected, + dirty, + unselectedResources, clearSelection, - removeSelectedResources, } = useIndexResourceState(resources, options); return ( ); } @@ -122,7 +93,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Page, true); + .trigger('handleSelectionChange', SelectionType.Page, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [selectedID], @@ -139,7 +110,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Page, true); + .trigger('handleSelectionChange', SelectionType.Page, true); } expect(throwResourceSelectionError).toThrow( @@ -161,7 +132,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Page, true); + .trigger('handleSelectionChange', SelectionType.Page, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [selectedID], @@ -177,7 +148,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Page, true); + .trigger('handleSelectionChange', SelectionType.Page, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [selectedID], @@ -199,7 +170,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Page, true); + .trigger('handleSelectionChange', SelectionType.Page, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [selectedID], @@ -214,7 +185,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.All, true); + .trigger('handleSelectionChange', SelectionType.All, true); expect(mockComponent).toContainReactComponent(TypedChild, { allResourcesSelected: true, @@ -228,7 +199,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.All, false); + .trigger('handleSelectionChange', SelectionType.All, false); expect(mockComponent).toContainReactComponent(TypedChild, { allResourcesSelected: false, @@ -242,7 +213,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Single, false, '1'); + .trigger('handleSelectionChange', SelectionType.Single, false, '1'); expect(mockComponent).toContainReactComponent(TypedChild, { allResourcesSelected: false, @@ -260,7 +231,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Single, true, id); + .trigger('handleSelectionChange', SelectionType.Single, true, id); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [id], @@ -279,7 +250,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Single, false, id); + .trigger('handleSelectionChange', SelectionType.Single, false, id); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [], @@ -305,7 +276,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.All, true); + .trigger('handleSelectionChange', SelectionType.All, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idOne], @@ -323,7 +294,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.All, true); + .trigger('handleSelectionChange', SelectionType.All, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idOne, idTwo], @@ -343,7 +314,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.All, false); + .trigger('handleSelectionChange', SelectionType.All, false); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [], @@ -369,7 +340,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.All, true); + .trigger('handleSelectionChange', SelectionType.All, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idOne], @@ -387,7 +358,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Page, true); + .trigger('handleSelectionChange', SelectionType.Page, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idOne, idTwo], @@ -407,7 +378,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Page, false); + .trigger('handleSelectionChange', SelectionType.Page, false); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [], @@ -434,7 +405,12 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Multi, true, [0, 1]); + .trigger( + 'handleSelectionChange', + SelectionType.Multi, + true, + [0, 1], + ); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idOne], @@ -450,7 +426,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Multi, true); + .trigger('handleSelectionChange', SelectionType.Multi, true); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources, @@ -468,7 +444,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Multi, true, [0, 1]); + .trigger('handleSelectionChange', SelectionType.Multi, true, [0, 1]); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idOne, idTwo], @@ -489,7 +465,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Multi, false, [0, 1]); + .trigger('handleSelectionChange', SelectionType.Multi, false, [0, 1]); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idThree], @@ -516,7 +492,12 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Range, true, [0, 2]); + .trigger( + 'handleSelectionChange', + SelectionType.Range, + true, + [0, 2], + ); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idTwo, idThree], @@ -535,7 +516,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Range, true, [0, 2]); + .trigger('handleSelectionChange', SelectionType.Range, true, [0, 2]); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idOne, idTwo, idThree], @@ -554,7 +535,7 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Range, true, [0, 2]); + .trigger('handleSelectionChange', SelectionType.Range, true, [0, 2]); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [idOne, idTwo, idThree], @@ -575,13 +556,27 @@ describe('useIndexResourceState', () => { mockComponent .find(TypedChild)! - .trigger('onClick', SelectionType.Range, false, [0, 2]); + .trigger('handleSelectionChange', SelectionType.Range, false, [0, 2]); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [], }); }); }); + + describe('dirty', () => { + it('sets dirty to true when a selection is made', () => { + const mockComponent = mountWithApp(); + + mockComponent + .find(TypedChild)! + .trigger('handleSelectionChange', SelectionType.Single, true, '1'); + + expect(mockComponent).toContainReactComponent(TypedChild, { + dirty: true, + }); + }); + }); }); describe('clearSelection', () => { @@ -591,16 +586,41 @@ describe('useIndexResourceState', () => { const idThree = '3'; const resources = [{id: idOne}, {id: idTwo}, {id: idThree}]; const mockComponent = mountWithApp( - , ); - mockComponent.find(TypedChild)!.trigger('onClick'); + mockComponent + .find(TypedChild)! + .trigger('handleSelectionChange', SelectionType.Single, false, '1'); + + expect(mockComponent).toContainReactComponent(TypedChild, { + selectedResources: [idThree], + allResourcesSelected: false, + dirty: true, + unselectedResources: [idOne], + }); + + mockComponent + .find(TypedChild)! + .trigger('handleSelectionChange', SelectionType.All, true); + + expect(mockComponent).toContainReactComponent(TypedChild, { + selectedResources: [idOne, idTwo, idThree], + allResourcesSelected: true, + dirty: true, + unselectedResources: [], + }); + + mockComponent.find(TypedChild)!.trigger('clearSelection'); expect(mockComponent).toContainReactComponent(TypedChild, { selectedResources: [], + allResourcesSelected: false, + dirty: true, + unselectedResources: [], }); }); }); @@ -612,7 +632,7 @@ describe('useIndexResourceState', () => { const idThree = '3'; const resources = [{id: idOne}, {id: idTwo}, {id: idThree}]; const mockComponent = mountWithApp( - , diff --git a/polaris-react/src/utilities/use-index-resource-state.ts b/polaris-react/src/utilities/use-index-resource-state.ts index 5baf70235f3..7f9288141bf 100644 --- a/polaris-react/src/utilities/use-index-resource-state.ts +++ b/polaris-react/src/utilities/use-index-resource-state.ts @@ -1,4 +1,5 @@ -import {useState, useCallback} from 'react'; +import {useState, useCallback, useEffect, useRef} from 'react'; +import isEqual from 'react-fast-compare'; export enum SelectionType { All = 'all', @@ -23,14 +24,14 @@ function defaultResourceIDResolver(resource: {[key: string]: any}): string { } export function useIndexResourceState( - resources: T[], + resources: readonly T[], { - selectedResources: initSelectedResources = [], + selectedResources: preCheckedResources = [], allResourcesSelected: initAllResourcesSelected = false, resourceIDResolver = defaultResourceIDResolver, resourceFilter = undefined, }: { - selectedResources?: string[]; + selectedResources?: readonly string[]; allResourcesSelected?: boolean; resourceIDResolver?: ResourceIDResolver; resourceFilter?: (value: T, index: number) => boolean; @@ -41,12 +42,49 @@ export function useIndexResourceState( resourceFilter: undefined, }, ) { - const [selectedResources, setSelectedResources] = useState( - initSelectedResources, - ); + const [selectedResources, setSelectedResources] = useState< + ReadonlySet + >(new Set(preCheckedResources)); const [allResourcesSelected, setAllResourcesSelected] = useState( initAllResourcesSelected, ); + const [unselectedResources, setUnselectedResources] = useState< + ReadonlySet + >(new Set()); + const [dirty, setDirty] = useState(false); + + const prevPreCheckedResourcesRef = useRef(preCheckedResources); + const initialSelectedResources = useRef(selectedResources); + const initialUnselectedResources = useRef(unselectedResources); + + useEffect(() => { + if (!isEqual(prevPreCheckedResourcesRef.current, preCheckedResources)) { + const filteredResources = dirty + ? preCheckedResources.filter( + (resource) => !unselectedResources.has(resource), + ) + : preCheckedResources; + + setSelectedResources(new Set(filteredResources)); + prevPreCheckedResourcesRef.current = preCheckedResources; + } + }, [dirty, preCheckedResources, unselectedResources]); + + useEffect(() => { + if ( + dirty && + isEqual(initialSelectedResources.current, selectedResources) && + isEqual(initialUnselectedResources.current, unselectedResources) + ) { + setDirty(false); + } + }, [dirty, selectedResources, unselectedResources]); + + useEffect(() => { + return () => { + setDirty(false); + }; + }, []); const handleSelectionChange = useCallback( ( @@ -56,6 +94,10 @@ export function useIndexResourceState( // This is not used in the function, but needed to keep the type compatible with IndexProviderProps onSelectionChange _position?: number, ) => { + if (!dirty) { + setDirty(true); + } + if (selectionType === SelectionType.All) { setAllResourcesSelected(isSelecting); } else if (allResourcesSelected) { @@ -64,55 +106,86 @@ export function useIndexResourceState( switch (selectionType) { case SelectionType.Single: - setSelectedResources((newSelectedResources) => - isSelecting - ? [...newSelectedResources, selection as string] - : newSelectedResources.filter((id) => id !== selection), - ); + if (typeof selection !== 'string') break; + + setSelectedResources((currentSelectedResources) => { + const newSelectedResources = new Set(currentSelectedResources); + const newUnselectedResources = new Set(unselectedResources); + const [resourcesToAddTo, resourcesToRemoveFrom] = isSelecting + ? [newSelectedResources, newUnselectedResources] + : [newUnselectedResources, newSelectedResources]; + + resourcesToAddTo.add(selection); + resourcesToRemoveFrom.delete(selection); + + setUnselectedResources(newUnselectedResources); + return newSelectedResources; + }); break; + case SelectionType.All: - case SelectionType.Page: - if (resourceFilter) { - const filteredResources = resources.filter(resourceFilter); - setSelectedResources( - isSelecting && selectedResources.length < filteredResources.length - ? filteredResources.map(resourceIDResolver) - : [], - ); - } else { - setSelectedResources( - isSelecting ? resources.map(resourceIDResolver) : [], - ); - } + case SelectionType.Page: { + const resourceList = resourceFilter + ? resources.filter(resourceFilter) + : resources; + const mappedResources = new Set(resourceList.map(resourceIDResolver)); + + const hasRoomForMoreSelection = + isSelecting && selectedResources.size < resourceList.length; + setSelectedResources( + hasRoomForMoreSelection || isSelecting + ? mappedResources + : new Set(), + ); + setUnselectedResources( + hasRoomForMoreSelection || isSelecting + ? new Set() + : mappedResources, + ); break; + } case SelectionType.Multi: if (!selection) break; + setSelectedResources((currentSelectedResources) => { - const ids: string[] = []; - const filteredResources = resourceFilter - ? resources.filter(resourceFilter) - : resources; + const ids: Set = new Set(); + const filteredResourcesSet = new Set( + resourceFilter ? resources.filter(resourceFilter) : resources, + ); + for ( let i = selection[0] as number; i <= (selection[1] as number); i++ ) { - if (filteredResources.includes(resources[i])) { + if (filteredResourcesSet.has(resources[i])) { const id = resourceIDResolver(resources[i]); if ( - (isSelecting && !currentSelectedResources.includes(id)) || - (!isSelecting && currentSelectedResources.includes(id)) + (isSelecting && !currentSelectedResources.has(id)) || + (!isSelecting && currentSelectedResources.has(id)) ) { - ids.push(id); + ids.add(id); } } } - return isSelecting - ? [...currentSelectedResources, ...ids] - : currentSelectedResources.filter((id) => !ids.includes(id)); + const newSelectedResources = new Set(currentSelectedResources); + const newUnselectedResources = new Set(unselectedResources); + + ids.forEach((id) => { + if (isSelecting) { + newSelectedResources.add(id); + newUnselectedResources.delete(id); + } else { + newSelectedResources.delete(id); + newUnselectedResources.add(id); + } + }); + + setUnselectedResources(newUnselectedResources); + return newSelectedResources; }); break; @@ -131,56 +204,62 @@ export function useIndexResourceState( Number(selection[1]) + 1, ); - const isIndeterminate = selectedIds.some((id) => { - return selectedResources.includes(id); - }); + const isIndeterminate = selectedIds.some((id) => + selectedResources.has(id), + ); - const isChecked = selectedIds.every((id) => { - return selectedResources.includes(id); - }); + const isChecked = selectedIds.every((id) => + selectedResources.has(id), + ); const isSelectingAllInRange = !isChecked && (isSelecting || isIndeterminate); - const nextSelectedResources = isSelectingAllInRange - ? [ - ...new Set([ - ...currentSelectedResources, - ...selectedIds, - ]).values(), - ] - : currentSelectedResources.filter( - (id) => !selectedIds.includes(id), - ); - - return nextSelectedResources; + const newSelectedResources = new Set(currentSelectedResources); + const newUnselectedResources = new Set(unselectedResources); + + selectedIds.forEach((id) => { + if (isSelectingAllInRange) { + newSelectedResources.add(id); + newUnselectedResources.delete(id); + } else { + newSelectedResources.delete(id); + newUnselectedResources.add(id); + } + }); + + setUnselectedResources(newUnselectedResources); + return newSelectedResources; }); break; } }, [ + dirty, allResourcesSelected, resourceFilter, - selectedResources, + unselectedResources, resources, + selectedResources, resourceIDResolver, ], ); const clearSelection = useCallback(() => { - setSelectedResources([]); + setSelectedResources(new Set()); + setUnselectedResources(new Set()); setAllResourcesSelected(false); }, []); const removeSelectedResources = useCallback( - (removeResources: string[]) => { - const selectedResourcesCopy = [...selectedResources]; + (removeResources: readonly string[]) => { + const removeResourcesSet = new Set(removeResources); - const newSelectedResources = selectedResourcesCopy.filter( - (resource) => !removeResources.includes(resource), + const newSelectedResources = [...selectedResources].filter( + (resource) => !removeResourcesSet.has(resource), ); - setSelectedResources(newSelectedResources); + setSelectedResources(new Set(newSelectedResources)); if (newSelectedResources.length === 0) { setAllResourcesSelected(false); @@ -190,10 +269,12 @@ export function useIndexResourceState( ); return { - selectedResources, + unselectedResources: [...unselectedResources], + selectedResources: [...selectedResources], allResourcesSelected, + dirty, handleSelectionChange, clearSelection, removeSelectedResources, - }; + } as const; }