From 050759e301dc14c5a21c16aeee0d271ff59969ee Mon Sep 17 00:00:00 2001 From: Chancellor Clark Date: Mon, 26 Sep 2022 15:13:12 -0500 Subject: [PATCH] fix(component): persist selection across TableNext pages --- packages/big-design/jest.config.js | 4 +- .../components/TableNext/Actions/Actions.tsx | 1 + .../TableNext/SelectAll/SelectAll.tsx | 16 +- .../components/TableNext/SelectAll/helpers.ts | 95 ++++--- .../TableNext/SelectAll/useSelectAllState.ts | 49 +--- .../src/components/TableNext/TableNext.tsx | 7 +- .../TableNext/__snapshots__/spec.tsx.snap | 2 +- .../src/components/TableNext/helpers.ts | 7 + .../src/components/TableNext/spec.tsx | 236 +++++++++++++++--- 9 files changed, 295 insertions(+), 122 deletions(-) create mode 100644 packages/big-design/src/components/TableNext/helpers.ts diff --git a/packages/big-design/jest.config.js b/packages/big-design/jest.config.js index 08de62502..555fdd66f 100644 --- a/packages/big-design/jest.config.js +++ b/packages/big-design/jest.config.js @@ -9,10 +9,10 @@ module.exports = { setupFilesAfterEnv: ['/setupTests.ts'], coverageThreshold: { global: { - statements: 95, + statements: 96, branches: 87, functions: 97, - lines: 95, + lines: 96, }, }, }; diff --git a/packages/big-design/src/components/TableNext/Actions/Actions.tsx b/packages/big-design/src/components/TableNext/Actions/Actions.tsx index 665f66ccb..64c04a140 100644 --- a/packages/big-design/src/components/TableNext/Actions/Actions.tsx +++ b/packages/big-design/src/components/TableNext/Actions/Actions.tsx @@ -74,6 +74,7 @@ const InternalActions = ({ isExpandable={isExpandable} items={items} onChange={onSelectionChange} + pagination={pagination} selectedItems={selectedItems} totalItems={totalItems} /> diff --git a/packages/big-design/src/components/TableNext/SelectAll/SelectAll.tsx b/packages/big-design/src/components/TableNext/SelectAll/SelectAll.tsx index 07ae0f787..7de9587fe 100644 --- a/packages/big-design/src/components/TableNext/SelectAll/SelectAll.tsx +++ b/packages/big-design/src/components/TableNext/SelectAll/SelectAll.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { Checkbox } from '../../Checkbox'; import { Flex, FlexItem } from '../../Flex'; import { Text } from '../../Typography'; -import { TableExpandable, TableItem, TableSelectable } from '../types'; +import { TableExpandable, TableItem, TablePaginationProps, TableSelectable } from '../types'; import { useSelectAllState } from './useSelectAllState'; @@ -14,18 +14,14 @@ export interface SelectAllProps { onChange?: TableSelectable['onSelectionChange']; selectedItems: TableSelectable['selectedItems']; totalItems: number; + pagination?: TablePaginationProps; } -export const SelectAll = ({ - expandedRowSelector, - isExpandable, - items = [], - onChange, - selectedItems, - totalItems, -}: SelectAllProps) => { +export const SelectAll = (props: SelectAllProps) => { const { allInPageSelected, handleSelectAll, label, someInPageSelected, totalSelectedItems } = - useSelectAllState({ expandedRowSelector, isExpandable, items, selectedItems, onChange }); + useSelectAllState(props); + + const { totalItems } = props; return ( diff --git a/packages/big-design/src/components/TableNext/SelectAll/helpers.ts b/packages/big-design/src/components/TableNext/SelectAll/helpers.ts index 0e6395556..485d3e02f 100644 --- a/packages/big-design/src/components/TableNext/SelectAll/helpers.ts +++ b/packages/big-design/src/components/TableNext/SelectAll/helpers.ts @@ -1,23 +1,12 @@ -import { TableExpandable, TableSelectable } from '../types'; +import { getPagedIndex } from '../helpers'; +import { TableExpandable, TablePaginationProps, TableSelectable } from '../types'; -interface SelectAllRowsArg { - isExpandable: boolean; - items: T[]; - selectedItems: TableSelectable['selectedItems']; - expandedRowSelector?: TableExpandable['expandedRowSelector']; -} +import { SelectAllProps } from './SelectAll'; -export function getTotalSelectedItems( - items: T[], - selectedItems: TableSelectable['selectedItems'], -) { - return items.reduce((acc, _parentRow, parentRowIndex) => { - if (selectedItems[parentRowIndex] !== undefined) { - return acc + 1; - } +type SelectAllRowsArg = Omit, 'onChange'>; - return acc; - }, 0); +export function getTotalSelectedItems(selectedItems: TableSelectable['selectedItems']) { + return Object.keys(selectedItems).length; } export function getChildrenRows( @@ -38,20 +27,22 @@ export function areAllInPageSelected({ items, selectedItems, expandedRowSelector, + pagination, }: SelectAllRowsArg) { if (items.length <= 0) { return false; } return items.every((parentRow, parentRowIndex) => { + const pagedIndex = getPagedIndex(parentRowIndex, pagination); const childrenRows: T[] = getChildrenRows(parentRow, expandedRowSelector); // Not need to check childrens since expandable mode is not used. if (!isExpandable || childrenRows.length === 0) { - return selectedItems[parentRowIndex] !== undefined; + return selectedItems[pagedIndex] !== undefined; } - return areAllParentsAndChildrenSelected(childrenRows, selectedItems, parentRowIndex); + return areAllParentsAndChildrenSelected(childrenRows, selectedItems, pagedIndex, pagination); }); } @@ -60,20 +51,22 @@ export function areSomeInPageSelected({ items, selectedItems, expandedRowSelector, + pagination, }: SelectAllRowsArg): boolean { if (items.length <= 0) { return false; } return items.some((parentRow, parentRowIndex) => { + const pagedIndex = getPagedIndex(parentRowIndex, pagination); const childrenRows: T[] = getChildrenRows(parentRow, expandedRowSelector); // Not need to check childrens since expandable mode is not used. if (!isExpandable || childrenRows.length === 0) { - return selectedItems[parentRowIndex] !== undefined; + return selectedItems[pagedIndex] !== undefined; } - return areSomeParentsAndChildrenSelected(childrenRows, selectedItems, parentRowIndex); + return areSomeParentsAndChildrenSelected(childrenRows, selectedItems, pagedIndex); }); } @@ -81,44 +74,78 @@ function areAllParentsAndChildrenSelected( childrenRows: T[], selectedItems: TableSelectable['selectedItems'], parentRowIndex: number, + pagination?: TablePaginationProps, ) { + const pagedIndex = getPagedIndex(parentRowIndex, pagination); + const allChildrenRowsSelected = childrenRows.every((_childRow, childRowIndex) => { - return selectedItems[`${parentRowIndex}.${childRowIndex}`] !== undefined; + return selectedItems[`${pagedIndex}.${childRowIndex}`] !== undefined; }); - return selectedItems[parentRowIndex] !== undefined && allChildrenRowsSelected; + return selectedItems[pagedIndex] !== undefined && allChildrenRowsSelected; } function areSomeParentsAndChildrenSelected( childrenRows: T[], selectedItems: TableSelectable['selectedItems'], parentRowIndex: number, + pagination?: TablePaginationProps, ) { + const pagedIndex = getPagedIndex(parentRowIndex, pagination); + const someChildrenRowsInPageSelected = childrenRows.some((_childRow, childRowIndex) => { - return selectedItems[`${parentRowIndex}.${childRowIndex}`] !== undefined; + return selectedItems[`${pagedIndex}.${childRowIndex}`] !== undefined; }); - return selectedItems[parentRowIndex] !== undefined && someChildrenRowsInPageSelected; + return selectedItems[pagedIndex] !== undefined && someChildrenRowsInPageSelected; } -export function selectAll({ - expandedRowSelector, - isExpandable, - items, -}: Omit, 'selectedItems'>): TableSelectable['selectedItems'] { +function deselectAllOnCurrentPage(params: SelectAllRowsArg) { + const { items, selectedItems, pagination } = params; + + const filteredSelectedItems = Object.keys(selectedItems) + .filter((selectedKey) => { + const [parentIndex] = selectedKey.split('.').map((key) => parseInt(key, 10)); + const item = items.find((_, index) => getPagedIndex(index, pagination) === parentIndex); + + if (item) { + return false; + } + + return true; + }) + .map<[string, true]>((key) => [key, true]); + + return Object.fromEntries(filteredSelectedItems); +} + +function selectAllOnCurrentPage(params: SelectAllRowsArg) { + const { isExpandable, items, selectedItems, expandedRowSelector, pagination } = params; + const newSelectedItems = items.map((parentRow, parentRowIndex) => { + const pagedIndex = getPagedIndex(parentRowIndex, pagination); const childrenRows: T[] = getChildrenRows(parentRow, expandedRowSelector); if (isExpandable) { const newSelectedChildrenRows = childrenRows.map<[string, true]>((_child, childRowIndex) => { - return [`${parentRowIndex}.${childRowIndex}`, true]; + return [`${pagedIndex}.${childRowIndex}`, true]; }); - return [[`${parentRowIndex}`, true], ...newSelectedChildrenRows]; + return [[`${pagedIndex}`, true], ...newSelectedChildrenRows]; } - return [[`${parentRowIndex}`, true]]; + return [[`${pagedIndex}`, true]]; }); - return Object.fromEntries(newSelectedItems.flat()); + return { ...selectedItems, ...Object.fromEntries(newSelectedItems.flat()) }; +} + +export function getSelectAllState( + params: SelectAllRowsArg, +): TableSelectable['selectedItems'] { + if (areAllInPageSelected(params)) { + return deselectAllOnCurrentPage(params); + } + + return selectAllOnCurrentPage(params); } diff --git a/packages/big-design/src/components/TableNext/SelectAll/useSelectAllState.ts b/packages/big-design/src/components/TableNext/SelectAll/useSelectAllState.ts index b34705aa7..0483776f0 100644 --- a/packages/big-design/src/components/TableNext/SelectAll/useSelectAllState.ts +++ b/packages/big-design/src/components/TableNext/SelectAll/useSelectAllState.ts @@ -1,42 +1,17 @@ -import { TableExpandable, TableSelectable } from '../types'; - import { areAllInPageSelected, areSomeInPageSelected, + getSelectAllState, getTotalSelectedItems, - selectAll, } from './helpers'; +import { SelectAllProps } from './SelectAll'; -interface useSelectAllStateProps { - isExpandable: boolean; - items: T[]; - selectedItems: TableSelectable['selectedItems']; - expandedRowSelector?: TableExpandable['expandedRowSelector']; - onChange?: TableSelectable['onSelectionChange']; -} - -export const useSelectAllState = ({ - expandedRowSelector, - isExpandable, - items, - selectedItems, - onChange, -}: useSelectAllStateProps) => { - const allInPageSelected = areAllInPageSelected({ - expandedRowSelector, - isExpandable, - items, - selectedItems, - }); - - const someInPageSelected = areSomeInPageSelected({ - expandedRowSelector, - isExpandable, - items, - selectedItems, - }); +export const useSelectAllState = (props: SelectAllProps) => { + const { selectedItems, onChange } = props; - const totalSelectedItems = getTotalSelectedItems(items, selectedItems); + const allInPageSelected = areAllInPageSelected(props); + const someInPageSelected = areSomeInPageSelected(props); + const totalSelectedItems = getTotalSelectedItems(selectedItems); const label = allInPageSelected ? 'Deselect All' : 'Select All'; const handleSelectAll = () => { @@ -44,15 +19,7 @@ export const useSelectAllState = ({ return; } - if (allInPageSelected) { - return onChange({}); - } - - const newSelectedItems = selectAll({ - expandedRowSelector, - isExpandable, - items, - }); + const newSelectedItems = getSelectAllState(props); return onChange(newSelectedItems); }; diff --git a/packages/big-design/src/components/TableNext/TableNext.tsx b/packages/big-design/src/components/TableNext/TableNext.tsx index 408322b76..351b501f1 100644 --- a/packages/big-design/src/components/TableNext/TableNext.tsx +++ b/packages/big-design/src/components/TableNext/TableNext.tsx @@ -14,6 +14,7 @@ import { ExpandableHeaderCell, HeaderCheckboxCell, } from './HeaderCell/HeaderCell'; +import { getPagedIndex } from './helpers'; import { useExpandable, useSelectable } from './hooks'; import { RowContainer } from './RowContainer'; import { StyledTable, StyledTableFigure } from './styled'; @@ -166,6 +167,7 @@ const InternalTableNext = ( {items.map((item: T, index) => { const key = getItemKey(item, index); + const pagedIndex = getPagedIndex(index, pagination); return ( @@ -186,7 +188,7 @@ const InternalTableNext = ( key={key} onExpandedRow={onExpandedRow} onItemSelect={onItemSelect} - parentRowIndex={index} + parentRowIndex={pagedIndex} ref={provided.innerRef} selectedItems={selectedItems} showDragIcon={true} @@ -208,6 +210,7 @@ const InternalTableNext = ( {items.map((item: T, index) => { const key = getItemKey(item, index); + const pagedIndex = getPagedIndex(index, pagination); return ( ( key={key} onExpandedRow={onExpandedRow} onItemSelect={onItemSelect} - parentRowIndex={index} + parentRowIndex={pagedIndex} selectedItems={selectedItems} /> ); diff --git a/packages/big-design/src/components/TableNext/__snapshots__/spec.tsx.snap b/packages/big-design/src/components/TableNext/__snapshots__/spec.tsx.snap index 9fbda54b3..ccc096f70 100644 --- a/packages/big-design/src/components/TableNext/__snapshots__/spec.tsx.snap +++ b/packages/big-design/src/components/TableNext/__snapshots__/spec.tsx.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`renders a pagination component 1`] = ` +exports[`pagination renders a pagination component 1`] = ` .c10 { vertical-align: middle; height: 2rem; diff --git a/packages/big-design/src/components/TableNext/helpers.ts b/packages/big-design/src/components/TableNext/helpers.ts new file mode 100644 index 000000000..f533df685 --- /dev/null +++ b/packages/big-design/src/components/TableNext/helpers.ts @@ -0,0 +1,7 @@ +import { TablePaginationProps } from './types'; + +export function getPagedIndex(index: number, pagination?: TablePaginationProps) { + const { currentPage, itemsPerPage } = pagination ?? { currentPage: 1, itemsPerPage: 0 }; + + return index + (currentPage - 1) * itemsPerPage; +} diff --git a/packages/big-design/src/components/TableNext/spec.tsx b/packages/big-design/src/components/TableNext/spec.tsx index 362e7df5a..aa02f1940 100644 --- a/packages/big-design/src/components/TableNext/spec.tsx +++ b/packages/big-design/src/components/TableNext/spec.tsx @@ -211,42 +211,214 @@ test('renders the total number of items + item name', () => { expect(itemNameNode).toBeInTheDocument(); }); -test('renders a pagination component', async () => { - const onItemsPerPageChange = jest.fn(); - const onPageChange = jest.fn(); +describe('pagination', () => { + test('renders a pagination component', async () => { + const onItemsPerPageChange = jest.fn(); + const onPageChange = jest.fn(); - const { container, findByRole, getByTitle } = render( - sku }, - { header: 'Name', hash: 'name', render: ({ name }) => name }, - { header: 'Stock', hash: 'stock', render: ({ stock }) => stock }, - ]} - items={[ - { sku: 'SM13', name: '[Sample] Smith Journal 13', stock: 25 }, - { sku: 'DPB', name: '[Sample] Dustpan & Brush', stock: 34 }, - { sku: 'OFSUC', name: '[Sample] Utility Caddy', stock: 45 }, - { sku: 'CLC', name: '[Sample] Canvas Laundry Cart', stock: 2 }, - { sku: 'CGLD', name: '[Sample] Laundry Detergent', stock: 29 }, - ]} - keyField="sku" - pagination={{ - currentPage: 1, - itemsPerPage: 3, - totalItems: 5, - itemsPerPageOptions: [3, 5, 10], - onItemsPerPageChange, - onPageChange, - }} - />, - ); + const { container, findByRole, getByTitle } = render( + sku }, + { header: 'Name', hash: 'name', render: ({ name }) => name }, + { header: 'Stock', hash: 'stock', render: ({ stock }) => stock }, + ]} + items={[ + { sku: 'SM13', name: '[Sample] Smith Journal 13', stock: 25 }, + { sku: 'DPB', name: '[Sample] Dustpan & Brush', stock: 34 }, + { sku: 'OFSUC', name: '[Sample] Utility Caddy', stock: 45 }, + { sku: 'CLC', name: '[Sample] Canvas Laundry Cart', stock: 2 }, + { sku: 'CGLD', name: '[Sample] Laundry Detergent', stock: 29 }, + ]} + keyField="sku" + pagination={{ + currentPage: 1, + itemsPerPage: 3, + totalItems: 5, + itemsPerPageOptions: [3, 5, 10], + onItemsPerPageChange, + onPageChange, + }} + />, + ); - fireEvent.click(getByTitle('Next page')); + fireEvent.click(getByTitle('Next page')); - await findByRole('table'); + await findByRole('table'); - expect(onPageChange).toHaveBeenCalledWith(2); - expect(container.firstChild).toMatchSnapshot(); + expect(onPageChange).toHaveBeenCalledWith(2); + expect(container.firstChild).toMatchSnapshot(); + }); + + test('selection persists indexes on other pages', async () => { + const onSelectionChange = jest.fn(); + + const items = [ + { sku: 'SM13', name: '[Sample] Smith Journal 13', stock: 25 }, + { sku: 'DPB', name: '[Sample] Dustpan & Brush', stock: 34 }, + { sku: 'OFSUC', name: '[Sample] Utility Caddy', stock: 45 }, + { sku: 'CLC', name: '[Sample] Canvas Laundry Cart', stock: 2 }, + { sku: 'CGLD', name: '[Sample] Laundry Detergent', stock: 29 }, + ]; + + render( + sku }, + { header: 'Name', hash: 'name', render: ({ name }) => name }, + { header: 'Stock', hash: 'stock', render: ({ stock }) => stock }, + ]} + items={items.filter((_, index) => index >= 3 && index < 6)} + keyField="sku" + pagination={{ + currentPage: 2, + itemsPerPage: 3, + totalItems: items.length, + itemsPerPageOptions: [3, 5, 10], + onItemsPerPageChange: jest.fn(), + onPageChange: jest.fn(), + }} + selectable={{ + selectedItems: { 0: true }, + onSelectionChange, + }} + />, + ); + + const [, firstRow] = await screen.findAllByRole('row'); + const firstRowCheckbox = within(firstRow).getByRole('checkbox'); + + await userEvent.click(firstRowCheckbox); + + expect(onSelectionChange).toHaveBeenCalledWith({ 0: true, 3: true }); + }); + + test('deselection persists indexes on other pages', async () => { + const onSelectionChange = jest.fn(); + + const items = [ + { sku: 'SM13', name: '[Sample] Smith Journal 13', stock: 25 }, + { sku: 'DPB', name: '[Sample] Dustpan & Brush', stock: 34 }, + { sku: 'OFSUC', name: '[Sample] Utility Caddy', stock: 45 }, + { sku: 'CLC', name: '[Sample] Canvas Laundry Cart', stock: 2 }, + { sku: 'CGLD', name: '[Sample] Laundry Detergent', stock: 29 }, + ]; + + render( + sku }, + { header: 'Name', hash: 'name', render: ({ name }) => name }, + { header: 'Stock', hash: 'stock', render: ({ stock }) => stock }, + ]} + items={items.filter((_, index) => index >= 3 && index < 6)} + keyField="sku" + pagination={{ + currentPage: 2, + itemsPerPage: 3, + totalItems: items.length, + itemsPerPageOptions: [3, 5, 10], + onItemsPerPageChange: jest.fn(), + onPageChange: jest.fn(), + }} + selectable={{ + selectedItems: { 0: true, 3: true }, + onSelectionChange, + }} + />, + ); + + const [, firstRow] = await screen.findAllByRole('row'); + const firstRowCheckbox = within(firstRow).getByRole('checkbox'); + + await userEvent.click(firstRowCheckbox); + + expect(onSelectionChange).toHaveBeenCalledWith({ 0: true }); + }); + + test('select all persists indexes on other pages', async () => { + const onSelectionChange = jest.fn(); + + const items = [ + { sku: 'SM13', name: '[Sample] Smith Journal 13', stock: 25 }, + { sku: 'DPB', name: '[Sample] Dustpan & Brush', stock: 34 }, + { sku: 'OFSUC', name: '[Sample] Utility Caddy', stock: 45 }, + { sku: 'CLC', name: '[Sample] Canvas Laundry Cart', stock: 2 }, + { sku: 'CGLD', name: '[Sample] Laundry Detergent', stock: 29 }, + ]; + + render( + sku }, + { header: 'Name', hash: 'name', render: ({ name }) => name }, + { header: 'Stock', hash: 'stock', render: ({ stock }) => stock }, + ]} + items={items.filter((_, index) => index >= 3 && index < 6)} + keyField="sku" + pagination={{ + currentPage: 2, + itemsPerPage: 3, + totalItems: items.length, + itemsPerPageOptions: [3, 5, 10], + onItemsPerPageChange: jest.fn(), + onPageChange: jest.fn(), + }} + selectable={{ + selectedItems: { 0: true, 3: true }, + onSelectionChange, + }} + />, + ); + + const selectAll = await screen.findByRole('checkbox', { name: /select all/i }); + + await userEvent.click(selectAll); + + expect(onSelectionChange).toHaveBeenCalledWith({ 0: true, 3: true, 4: true }); + }); + + test('deselect all persists indexes on other pages', async () => { + const onSelectionChange = jest.fn(); + + const items = [ + { sku: 'SM13', name: '[Sample] Smith Journal 13', stock: 25 }, + { sku: 'DPB', name: '[Sample] Dustpan & Brush', stock: 34 }, + { sku: 'OFSUC', name: '[Sample] Utility Caddy', stock: 45 }, + { sku: 'CLC', name: '[Sample] Canvas Laundry Cart', stock: 2 }, + { sku: 'CGLD', name: '[Sample] Laundry Detergent', stock: 29 }, + ]; + + render( + sku }, + { header: 'Name', hash: 'name', render: ({ name }) => name }, + { header: 'Stock', hash: 'stock', render: ({ stock }) => stock }, + ]} + items={items.filter((_, index) => index >= 3 && index < 6)} + keyField="sku" + pagination={{ + currentPage: 2, + itemsPerPage: 3, + totalItems: items.length, + itemsPerPageOptions: [3, 5, 10], + onItemsPerPageChange: jest.fn(), + onPageChange: jest.fn(), + }} + selectable={{ + selectedItems: { 0: true, 3: true, 4: true }, + onSelectionChange, + }} + />, + ); + + const deselectAll = await screen.findByRole('checkbox', { name: /deselect all/i }); + + await userEvent.click(deselectAll); + + expect(onSelectionChange).toHaveBeenCalledWith({ 0: true }); + }); }); describe('selectable', () => {