From cfbf8d503851f3dcb5d96a0b6f478585e270dc92 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 5 Jun 2024 13:47:01 -0700 Subject: [PATCH 1/2] [EuiBasicTable] Support indeterminate checkboxes in the header selection checkbox - we have the technology!! I figured, why not? - make clicking an indeterminate checkbox more like gmail's behavior (i.e., it deselects) clarify deselect vs select behavior with more titles/aria labels --- packages/eui/changelogs/upcoming/7817.md | 1 + .../__snapshots__/basic_table.test.tsx.snap | 1 + .../in_memory_table.test.tsx.snap | 1 + .../basic_table/basic_table.test.tsx | 67 +++++++++++++++++++ .../components/basic_table/basic_table.tsx | 19 ++++-- 5 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 packages/eui/changelogs/upcoming/7817.md diff --git a/packages/eui/changelogs/upcoming/7817.md b/packages/eui/changelogs/upcoming/7817.md new file mode 100644 index 00000000000..ba972530cb4 --- /dev/null +++ b/packages/eui/changelogs/upcoming/7817.md @@ -0,0 +1 @@ +- Updated `EuiBasicTable` and `EuiInMemoryTable`s with `selection` - the header row checkbox will now render an indeterminate state if some (but not all) rows are selected diff --git a/packages/eui/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap b/packages/eui/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap index 37c0b232424..9be600ab7b8 100644 --- a/packages/eui/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap +++ b/packages/eui/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap @@ -152,6 +152,7 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin class="euiCheckbox__input" data-test-subj="checkboxSelectAll" id="_selection_column-checkbox_generated-id_desktop" + title="Select all rows" type="checkbox" />
{ expect(onSelectionChange).toHaveBeenCalledWith([]); expect(container.querySelectorAll('[checked]')).toHaveLength(0); }); + + describe('header checkbox', () => { + it('selects all rows', () => { + const props: EuiBasicTableProps = { + items: basicItems, + columns: basicColumns, + itemId: 'id', + selection: { + onSelectionChange: () => {}, + initialSelected: [], + }, + }; + const { getByTestSubject } = render(); + expect(getByTestSubject('checkboxSelectAll')).not.toBeChecked(); + + fireEvent.click(getByTestSubject('checkboxSelectAll')); + + expect(getByTestSubject('checkboxSelectAll')).toBeChecked(); + expect(getCheckboxAt(1)).toBeChecked(); + expect(getCheckboxAt(2)).toBeChecked(); + expect(getCheckboxAt(3)).toBeChecked(); + }); + + it('deselects all rows', () => { + const props: EuiBasicTableProps = { + items: basicItems, + columns: basicColumns, + itemId: 'id', + selection: { + onSelectionChange: () => {}, + initialSelected: basicItems, + }, + }; + const { getByTestSubject } = render(); + expect(getByTestSubject('checkboxSelectAll')).toBeChecked(); + + fireEvent.click(getByTestSubject('checkboxSelectAll')); + + expect(getByTestSubject('checkboxSelectAll')).not.toBeChecked(); + expect(getCheckboxAt(1)).not.toBeChecked(); + expect(getCheckboxAt(2)).not.toBeChecked(); + expect(getCheckboxAt(3)).not.toBeChecked(); + }); + + it('renders an indeterminate header checkbox if some but not all rows are selected', () => { + const props: EuiBasicTableProps = { + items: basicItems, + columns: basicColumns, + itemId: 'id', + selection: { + onSelectionChange: () => {}, + initialSelected: [], + }, + }; + const { getByTestSubject } = render(); + expect(getByTestSubject('checkboxSelectAll')).not.toBeChecked(); + + fireEvent.click(getCheckboxAt(1)); + expect(getCheckboxAt(1)).toBeChecked(); + expect(getByTestSubject('checkboxSelectAll')).toBePartiallyChecked(); + + // Should deselect all rows on indeterminate click + fireEvent.click(getByTestSubject('checkboxSelectAll')); + expect(getCheckboxAt(1)).not.toBeChecked(); + }); + }); }); test('footers', () => { diff --git a/packages/eui/src/components/basic_table/basic_table.tsx b/packages/eui/src/components/basic_table/basic_table.tsx index 3409ce01053..97ffc82579e 100644 --- a/packages/eui/src/components/basic_table/basic_table.tsx +++ b/packages/eui/src/components/basic_table/basic_table.tsx @@ -688,10 +688,16 @@ export class EuiBasicTable extends Component< selectableItems.length > 0 && this.state.selection.length === selectableItems.length; + const indeterminate = + !checked && + this.state.selection && + selectableItems.length > 0 && + this.state.selection.length > 0; + const disabled = selectableItems.length === 0; const onChange = (event: React.ChangeEvent) => { - if (event.target.checked) { + if (event.target.checked && !indeterminate) { this.changeSelection(selectableItems); } else { this.changeSelection([]); @@ -699,16 +705,21 @@ export class EuiBasicTable extends Component< }; return ( - - {(selectAllRows: string) => ( + + {([selectAllRows, deselectRows]: string[]) => ( )} From 39e6cb8d0531eaa1a82b993c26bf049bf7843aea Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 5 Jun 2024 14:06:51 -0700 Subject: [PATCH 2/2] [EuiBasicTable][tech debt] Clean up now-unnecessary conditional mobile select all checkbox logic - Since the mobile header no longer renders or non-mobile views, we don't need this conditional logic anymore wooo --- .../basic_table/__snapshots__/basic_table.test.tsx.snap | 2 +- .../basic_table/__snapshots__/in_memory_table.test.tsx.snap | 5 +++-- packages/eui/src/components/basic_table/basic_table.tsx | 5 ++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/eui/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap b/packages/eui/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap index 9be600ab7b8..2334b0aee41 100644 --- a/packages/eui/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap +++ b/packages/eui/src/components/basic_table/__snapshots__/basic_table.test.tsx.snap @@ -151,7 +151,7 @@ exports[`EuiBasicTable renders (kitchen sink) with pagination, selection, sortin aria-label="Select all rows" class="euiCheckbox__input" data-test-subj="checkboxSelectAll" - id="_selection_column-checkbox_generated-id_desktop" + id="_selection_column-checkbox_generated-id" title="Select all rows" type="checkbox" /> diff --git a/packages/eui/src/components/basic_table/__snapshots__/in_memory_table.test.tsx.snap b/packages/eui/src/components/basic_table/__snapshots__/in_memory_table.test.tsx.snap index 169da77c88a..c22cc2cbc85 100644 --- a/packages/eui/src/components/basic_table/__snapshots__/in_memory_table.test.tsx.snap +++ b/packages/eui/src/components/basic_table/__snapshots__/in_memory_table.test.tsx.snap @@ -10,7 +10,8 @@ exports[`EuiInMemoryTable behavior mobile header 1`] = ` @@ -19,7 +20,7 @@ exports[`EuiInMemoryTable behavior mobile header 1`] = ` /> diff --git a/packages/eui/src/components/basic_table/basic_table.tsx b/packages/eui/src/components/basic_table/basic_table.tsx index 97ffc82579e..0f09e5e16aa 100644 --- a/packages/eui/src/components/basic_table/basic_table.tsx +++ b/packages/eui/src/components/basic_table/basic_table.tsx @@ -711,13 +711,12 @@ export class EuiBasicTable extends Component< > {([selectAllRows, deselectRows]: string[]) => (