From fd6d753122ca02b4479bedf816e4d6882d40f53f Mon Sep 17 00:00:00 2001 From: Greg Baroni Date: Wed, 18 Oct 2023 13:44:24 -0700 Subject: [PATCH] feat: update uncontrolled table behavior to only update page if user is out of bounds - Rather than having a prop for if the page should be reset when the rows change, the default behavior will be to only update the page if the user is out of bounds when the rows change - This also fixes a bug with the resetPageOnRowChange prop where, if this was false and the user was on the last page, then it would be possible for the user to be outside of the page bounds if rows were removed --- src/components/Table/UncontrolledTable.js | 11 +-- .../Table/UncontrolledTable.spec.js | 72 ++++++++++++------- 2 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/components/Table/UncontrolledTable.js b/src/components/Table/UncontrolledTable.js index 6d76b4052..54c1afbe4 100644 --- a/src/components/Table/UncontrolledTable.js +++ b/src/components/Table/UncontrolledTable.js @@ -14,7 +14,6 @@ export default class UncontrolledTable extends React.Component { onPageChange: PropTypes.func, page: PropTypes.number, pageSize: PropTypes.number, - resetPageOnRowChange: PropTypes.bool, selected: PropTypes.oneOfType([PropTypes.array, PropTypes.object]), sort: PropTypes.shape({ column: PropTypes.string, @@ -32,7 +31,6 @@ export default class UncontrolledTable extends React.Component { onPageChange: () => {}, page: 0, pageSize: 10, - resetPageOnRowChange: true, selected: [], sort: { ascending: true, @@ -177,8 +175,13 @@ export default class UncontrolledTable extends React.Component { this.setState({ expanded: [] }); } - if (nextProps.resetPageOnRowChange && rowsChanged) { - this.setState({ page: 0 }); + if (rowsChanged) { + const numPages = Math.ceil(nextProps.rows.length / nextProps.pageSize); + if (!numPages) { + this.setState({ page: 0 }); + } else if (this.state.page + 1 > numPages) { + this.setState({ page: numPages - 1 }); + } } if (selectedChanged) { diff --git a/src/components/Table/UncontrolledTable.spec.js b/src/components/Table/UncontrolledTable.spec.js index 00c32e6ab..7659fa7ce 100644 --- a/src/components/Table/UncontrolledTable.spec.js +++ b/src/components/Table/UncontrolledTable.spec.js @@ -628,6 +628,7 @@ describe('', () => { rows = [ { name: 'Alpha', key: '1' }, { name: 'Bravo', key: '2' }, + { name: 'Charlie', key: '3' }, ]; expanded = [{ name: 'Alpha', key: '1' }]; selected = [{ name: 'Bravo', key: '2' }]; @@ -643,14 +644,12 @@ describe('', () => { ); }); - it('should not reset state for expanded, page, and selected if a rows key attributes have not changed', () => { + it('should not reset state for expanded or selected if a rows key attributes have not changed', () => { assert(wrapper.props().expanded.length > 0, 'the expanded props should be non empty'); assert(wrapper.props().selected.length > 0, 'the selected props should be non empty'); - assert.strictEqual(wrapper.props().page, 1, 'the page prop should be 1'); const expectedStateExpanded = wrapper.state().expanded; const expectedStateSelected = wrapper.state().selected; - const expectedStatePage = wrapper.state().page; const newProps = JSON.parse(JSON.stringify(wrapper.props())); newProps.rows[0].name = 'Charlie'; @@ -666,17 +665,11 @@ describe('', () => { expectedStateSelected.map((r) => r.key), 'the selected state should not have changed' ); - assert.strictEqual( - wrapper.state().page, - expectedStatePage, - 'the page state should not have been changed' - ); }); - it('should reset state for expanded, and page if a rows key attributes have changed', () => { + it('should reset state for expanded if a rows key attributes have changed', () => { assert(wrapper.props().expanded.length > 0, 'the expanded props should be non empty'); assert(wrapper.props().selected.length > 0, 'the selected props should be non empty'); - assert.strictEqual(wrapper.props().page, 1, 'the page prop should be 1'); const prevSelectedLength = wrapper.props().selected.length; const newProps = JSON.parse(JSON.stringify(wrapper.props())); @@ -693,40 +686,69 @@ describe('', () => { prevSelectedLength, 'the selected state should not change' ); - assert.strictEqual(wrapper.state().page, 0, 'the page state should have been set to 0'); }); - it('should reset state for expanded, but not page if a rows key attributes have changed and resetPageOnRowChange is false', () => { + it('should not change the page if rows change and user is still within the page bounds', () => { wrapper = mount( ); - assert(wrapper.props().expanded.length > 0, 'the expanded props should be non empty'); - assert(wrapper.props().selected.length > 0, 'the selected props should be non empty'); assert.strictEqual(wrapper.props().page, 1, 'the page prop should be 1'); - const prevSelectedLength = wrapper.props().selected.length; const newProps = JSON.parse(JSON.stringify(wrapper.props())); - newProps.rows[0].key = '3'; + newProps.rows.pop(); wrapper.instance().UNSAFE_componentWillReceiveProps(newProps); - assert.strictEqual( - wrapper.state().expanded.length, - 0, - 'the expanded state should be reset to empty' + assert.strictEqual(wrapper.state().page, 1, 'the page state should still be 1'); + }); + + it('should set page to the last page if rows change and user is now out of page bounds', () => { + wrapper = mount( + ); - assert.strictEqual( - wrapper.state().selected.length, - prevSelectedLength, - 'the selected state should not change' + assert.strictEqual(wrapper.props().page, 2, 'the page prop should be 2'); + + const newProps = JSON.parse(JSON.stringify(wrapper.props())); + newProps.rows.pop(); + wrapper.instance().UNSAFE_componentWillReceiveProps(newProps); + + assert.strictEqual(wrapper.state().page, 1, 'the page state should now be 1'); + }); + + it('should set page to the first page if rows change and there are no rows', () => { + wrapper = mount( + ); - assert.strictEqual(wrapper.state().page, 1, 'the page state should still be 1'); + assert.strictEqual(wrapper.props().page, 2, 'the page prop should be 2'); + + const newProps = JSON.parse(JSON.stringify(wrapper.props())); + newProps.rows = []; + wrapper.instance().UNSAFE_componentWillReceiveProps(newProps); + + assert.strictEqual(wrapper.state().page, 1, 'the page state should now be 0'); }); it('should reset state for selected if a change is detected in the selected key attributes have changed', () => {