From e4baa51c59903b59a3ccb841db1e55b35009193c Mon Sep 17 00:00:00 2001 From: Dan Branley <61979454+dbranley@users.noreply.github.com> Date: Tue, 3 Sep 2024 07:29:42 -0400 Subject: [PATCH] fix: Make table filter select all work in all scenarios. #2386 (#2387) Co-authored-by: Martin Turoci --- ui/src/table.test.tsx | 50 +++++++++++++++++++++++++++++++++++-------- ui/src/table.tsx | 2 +- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/ui/src/table.test.tsx b/ui/src/table.test.tsx index c806655bdd..2709c059d6 100644 --- a/ui/src/table.test.tsx +++ b/ui/src/table.test.tsx @@ -16,6 +16,7 @@ import { fireEvent, render } from '@testing-library/react' import React from 'react' import { Table, XTable } from './table' import { wave } from './ui' +import { KeyCodes } from '@fluentui/react' const name = 'table', @@ -1012,9 +1013,40 @@ describe('Table.tsx', () => { expect(getAllByRole('row')).toHaveLength(2 + headerRow) }) + it('Select All on 2nd filter selects all filter checkboxes', () => { + tableProps = { + ...tableProps, + columns: [ + { name: 'colname1', label: 'col1', searchable: true }, + { name: 'colname2', label: 'col2', filterable: true }, + { name: 'colname3', label: 'col3', filterable: true }, + ], + rows: [ + { name: 'rowname1', cells: [cell11, 'col2-val2', 'On'] }, + { name: 'rowname2', cells: [cell21, 'col2-val1', 'Off'] }, + { name: 'rowname3', cells: [cell31, 'col2-val3', 'On'] }, + ] + } + const { container, getByLabelText, getAllByRole, getByText, queryByText } = render() + + fireEvent.click(container.querySelectorAll(filterSelectorName)[0]!) + fireEvent.click(getByLabelText('col2-val3')) + expect(getAllByRole('checkbox', { checked: true })).toHaveLength(1) + + // FluentUI uses a deprecated 'which' property instead of the 'key' prop. + // Close the menu with escape (does not close when clicking other menu in test). + fireEvent.keyDown(window, { which: KeyCodes.escape }) + expect(queryByText('Show only')).not.toBeInTheDocument() + + fireEvent.click(container.querySelectorAll(filterSelectorName)[1]!) + fireEvent.click(getByText('Select All')) + expect(queryByText('Show only')).toBeInTheDocument() + expect(getAllByRole('checkbox', { checked: true })).toHaveLength(getAllByRole('checkbox').length) + }) + it('Fires event when pagination enabled', () => { const { container, getAllByText } = render() - + fireEvent.click(container.querySelector(filterSelectorName) as HTMLDivElement) fireEvent.click(getAllByText('1')[3].parentElement as HTMLDivElement) @@ -1054,7 +1086,7 @@ describe('Table.tsx', () => { fireEvent.click(getByLabelText('2')) expect(getAllByRole('row')).toHaveLength(2 + headerRow) expect(queryByTestId('filter-count')).toHaveTextContent('2') - }) + }) it('Filter counts - manual deselect', () => { const { container, getAllByRole, getByLabelText, queryByTestId } = render() @@ -1076,25 +1108,25 @@ describe('Table.tsx', () => { fireEvent.click(getByLabelText('2')) expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) expect(queryByTestId('filter-count')).toBeNull - }) + }) it('Filter counts - select all', () => { const { container, getAllByRole, queryByTestId, getByText } = render() expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) fireEvent.click(container.querySelector(filterSelectorName)!) - + fireEvent.click(getByText('Select All')) expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) expect(queryByTestId('filter-count')).toHaveTextContent(tableProps.rows!.length.toString()) - }) + }) it('Filter counts - deselect all', () => { const { container, getAllByRole, queryByTestId, getByText } = render() expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) fireEvent.click(container.querySelector(filterSelectorName)!) - + fireEvent.click(getByText('Select All')) expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) expect(queryByTestId('filter-count')).toHaveTextContent(tableProps.rows!.length.toString()) @@ -1126,7 +1158,7 @@ describe('Table.tsx', () => { expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) fireEvent.click(container.querySelector(filterSelectorName)!) - + fireEvent.click(getByText('Select All')) expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) expect(queryByTestId('filter-count')).toHaveTextContent('9+') @@ -1134,7 +1166,7 @@ describe('Table.tsx', () => { fireEvent.click(getByText('Deselect All')) expect(getAllByRole('row')).toHaveLength(tableProps.rows!.length + headerRow) expect(queryByTestId('filter-count')).toBeNull - }) + }) it('Filter counts - clear selection with reset', () => { const { container, getAllByRole, getByLabelText, queryByTestId, getByText } = render() @@ -1300,7 +1332,7 @@ describe('Table.tsx', () => { }) }) - + describe('Group by', () => { beforeEach(() => { tableProps = { diff --git a/ui/src/table.tsx b/ui/src/table.tsx index 434275049a..118cc2b3ed 100644 --- a/ui/src/table.tsx +++ b/ui/src/table.tsx @@ -294,7 +294,7 @@ const }, ContextualMenu = ({ onFilterChange, col, listProps, selectedFilters, setFiltersInBulk }: ContextualMenuProps) => { const - isFilterChecked = (data: S, key: S) => !!selectedFilters && selectedFilters[data]?.includes(key), + isFilterChecked = (data: S, key: S) => !!selectedFilters && !!selectedFilters[data]?.includes(key), [menuFilters, setMenuFilters] = React.useState(col.cellType?.tag ? Array.from(listProps.items.reduce((_filters, { key, text, data }) => { key.split(',').forEach(key => _filters.set(key, { key, text, data, checked: isFilterChecked(data, key) }))