From 0521c13c6e53644e8bb5d6777054231cd06e7d55 Mon Sep 17 00:00:00 2001 From: hshockley <30321326+hshockley@users.noreply.github.com> Date: Fri, 25 Oct 2019 11:59:06 -0400 Subject: [PATCH] fix(data-table): select all behavior (#4430) * fix(data-table): select all behavior * fix(data-table): minor changes * test(data-table): test cases * test(data-table): update snapshot --- .../src/components/DataTable/DataTable.js | 42 ++++- .../DataTable/__tests__/DataTable-test.js | 175 ++++++++++++++++++ .../__snapshots__/DataTable-test.js.snap | 36 ++-- 3 files changed, 225 insertions(+), 28 deletions(-) diff --git a/packages/react/src/components/DataTable/DataTable.js b/packages/react/src/components/DataTable/DataTable.js index 69b3dc30370b..31f02feefe85 100644 --- a/packages/react/src/components/DataTable/DataTable.js +++ b/packages/react/src/components/DataTable/DataTable.js @@ -310,7 +310,6 @@ export default class DataTable extends React.Component { const checked = rowCount > 0 && selectedRowCount === rowCount; const indeterminate = rowCount > 0 && selectedRowCount > 0 && selectedRowCount !== rowCount; - const translationKey = checked ? translationKeys.unselectAll : translationKeys.selectAll; @@ -378,6 +377,26 @@ export default class DataTable extends React.Component { return row.isSelected; }); + /** + * Helper utility to get all of the available rows after applying the filter + * @returns {Array} the array of rowIds that are currently included through the filter + * */ + getFilteredRowIds = () => { + const filteredRowIds = + typeof this.state.filterInputValue === 'string' + ? this.props.filterRows({ + rowIds: this.state.rowIds, + headers: this.props.headers, + cellsById: this.state.cellsById, + inputValue: this.state.filterInputValue, + }) + : this.state.rowIds; + if (filteredRowIds.length == 0) { + return this.state.rowIds; + } + return filteredRowIds; + }; + /** * Helper for getting the table prefix for elements that require an * `id` attribute that is unique. @@ -392,7 +411,7 @@ export default class DataTable extends React.Component { * @param {object} initialState * @returns {object} object to put into this.setState (use spread operator) */ - setAllSelectedState = (initialState, isSelected) => { + setAllSelectedState = (initialState, isSelected, filteredRowIds) => { const { rowIds } = initialState; return { rowsById: rowIds.reduce( @@ -400,7 +419,10 @@ export default class DataTable extends React.Component { ...acc, [id]: { ...initialState.rowsById[id], - isSelected: initialState.rowsById[id].disabled ? false : isSelected, + isSelected: + !initialState.rowsById[id].disabled && + filteredRowIds.includes(id) && + isSelected, }, }), {} @@ -416,7 +438,7 @@ export default class DataTable extends React.Component { this.setState(state => { return { shouldShowBatchActions: false, - ...this.setAllSelectedState(state, false), + ...this.setAllSelectedState(state, false, this.getFilteredRowIds()), }; }); }; @@ -426,14 +448,14 @@ export default class DataTable extends React.Component { */ handleSelectAll = () => { this.setState(state => { - const { rowIds, rowsById } = state; - const selectableRows = rowIds.reduce((acc, rowId) => { - return (acc += rowsById[rowId].disabled ? 0 : 1); - }, 0); - const isSelected = this.getSelectedRows().length !== selectableRows; + const filteredRowIds = this.getFilteredRowIds(); + const { rowsById } = state; + const isSelected = !( + Object.values(rowsById).filter(row => row.isSelected == true).length > 0 + ); return { shouldShowBatchActions: isSelected, - ...this.setAllSelectedState(state, isSelected), + ...this.setAllSelectedState(state, isSelected, filteredRowIds), }; }); }; diff --git a/packages/react/src/components/DataTable/__tests__/DataTable-test.js b/packages/react/src/components/DataTable/__tests__/DataTable-test.js index 3b4355be2294..e042bc7f96d2 100644 --- a/packages/react/src/components/DataTable/__tests__/DataTable-test.js +++ b/packages/react/src/components/DataTable/__tests__/DataTable-test.js @@ -340,6 +340,181 @@ describe('DataTable', () => { }); }); + describe('selection with filtering', () => { + let mockProps; + + beforeEach(() => { + mockProps = { + rows: [ + { + id: 'b', + fieldA: 'Field 2:A', + fieldB: 'Field 2:B', + }, + { + id: 'a', + fieldA: 'Field 1:A', + fieldB: 'Field 1:B', + }, + { + id: 'c', + fieldA: 'Field 3:A', + fieldB: 'Field 3:B', + }, + ], + headers: [ + { + key: 'fieldA', + header: 'Field A', + }, + { + key: 'fieldB', + header: 'Field B', + }, + ], + locale: 'en', + render: jest.fn( + ({ + rows, + headers, + getHeaderProps, + getSelectionProps, + onInputChange, + }) => ( + + + + + + + Action 1 + + + Action 2 + + + Action 3 + + + + + + + + + + {headers.map(header => ( + + {header.header} + + ))} + + + + {rows.map(row => ( + + + {row.cells.map(cell => ( + {cell.value} + ))} + + ))} + +
+ + ) + ), + }; + }); + + it('should only select all from filtered items', () => { + const wrapper = mount(); + + expect(getSelectAll(wrapper).prop('checked')).toBe(false); + + const filterInput = getFilterInput(wrapper); + + filterInput.getDOMNode().value = 'Field 1'; + filterInput.simulate('change'); + + getInputAtIndex({ wrapper, index: 0, inputType: 'checkbox' }).simulate( + 'click' + ); + + filterInput.getDOMNode().value = ''; + filterInput.simulate('change'); + + expect(wrapper.find('TableSelectAll').prop('indeterminate')).toBe(true); + + let { selectedRows } = getLastCallFor(mockProps.render)[0]; + expect(selectedRows.length).toBe(1); + + getSelectAll(wrapper).simulate('click'); + + selectedRows = getLastCallFor(mockProps.render)[0].selectedRows; + expect(selectedRows.length).toBe(0); + }); + + it('should only select rows that are not disabled even when filtered', () => { + const wrapper = mount(); + + const nextRows = [ + ...mockProps.rows.map(row => ({ ...row })), + { + id: 'd', + fieldA: 'Field 3:A', + fieldB: 'Field 3:B', + disabled: true, + }, + ]; + + wrapper.setProps({ rows: nextRows }); + + const filterInput = getFilterInput(wrapper); + + filterInput.getDOMNode().value = 'Field 3'; + filterInput.simulate('change'); + + getSelectAll(wrapper).simulate('click'); + + const { selectedRows } = getLastCallFor(mockProps.render)[0]; + expect(selectedRows.length).toBe(1); + + expect(wrapper.find('TableSelectAll').prop('indeterminate')).toBe(true); + }); + + it('does not select a row if they are all disabled', () => { + const wrapper = mount(); + + const nextRows = [ + ...mockProps.rows.map(row => ({ ...row, disabled: true })), + ]; + + wrapper.setProps({ rows: nextRows }); + + getSelectAll(wrapper).simulate('click'); + + expect(wrapper.find('TableSelectAll').prop('indeterminate')).toBe(false); + expect(wrapper.find('TableSelectAll').prop('checked')).toBe(false); + + const filterInput = getFilterInput(wrapper); + + filterInput.getDOMNode().value = 'Field 3'; + filterInput.simulate('change'); + + getSelectAll(wrapper).simulate('click'); + + const { selectedRows } = getLastCallFor(mockProps.render)[0]; + expect(selectedRows.length).toBe(0); + }); + }); + describe('selection -- radio buttons', () => { let mockProps; diff --git a/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap b/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap index 5c64ea4d6500..ad9a27b78d23 100644 --- a/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap +++ b/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap @@ -364,7 +364,7 @@ exports[`DataTable selection -- radio buttons should render 1`] = ` ariaLabel="Select row" checked={false} disabled={false} - id="data-table-11__select-row-b" + id="data-table-14__select-row-b" name="select-row-b" onSelect={[Function]} radio={true} @@ -381,7 +381,7 @@ exports[`DataTable selection -- radio buttons should render 1`] = ` ariaLabel="Select row" checked={false} disabled={false} - id="data-table-11__select-row-a" + id="data-table-14__select-row-a" name="select-row-a" onSelect={[Function]} radio={true} @@ -398,7 +398,7 @@ exports[`DataTable selection -- radio buttons should render 1`] = ` ariaLabel="Select row" checked={false} disabled={false} - id="data-table-11__select-row-c" + id="data-table-14__select-row-c" name="select-row-c" onSelect={[Function]} radio={true} @@ -523,7 +523,7 @@ exports[`DataTable selection -- radio buttons should render 1`] = ` ariaLabel="Select row" checked={false} disabled={false} - id="data-table-11__select-row-b" + id="data-table-14__select-row-b" name="select-row-b" onSelect={[Function]} radio={true} @@ -535,7 +535,7 @@ exports[`DataTable selection -- radio buttons should render 1`] = ` checked={false} disabled={false} hideLabel={true} - id="data-table-11__select-row-b" + id="data-table-14__select-row-b" labelText="Select row" name="select-row-b" onClick={[Function]} @@ -544,7 +544,7 @@ exports[`DataTable selection -- radio buttons should render 1`] = ` checked={false} disabled={false} hideLabel={true} - id="data-table-11__select-row-b" + id="data-table-14__select-row-b" innerRef={null} labelPosition="right" labelText="Select row" @@ -560,7 +560,7 @@ exports[`DataTable selection -- radio buttons should render 1`] = ` checked={false} className="bx--radio-button" disabled={false} - id="data-table-11__select-row-b" + id="data-table-14__select-row-b" name="select-row-b" onChange={[Function]} onClick={[Function]} @@ -570,7 +570,7 @@ exports[`DataTable selection -- radio buttons should render 1`] = `