Skip to content

Commit

Permalink
feat: update uncontrolled table behavior to only update page if user …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
gregbaroni committed Oct 18, 2023
1 parent 6ff8b99 commit fd6d753
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 29 deletions.
11 changes: 7 additions & 4 deletions src/components/Table/UncontrolledTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -32,7 +31,6 @@ export default class UncontrolledTable extends React.Component {
onPageChange: () => {},
page: 0,
pageSize: 10,
resetPageOnRowChange: true,
selected: [],
sort: {
ascending: true,
Expand Down Expand Up @@ -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) {
Expand Down
72 changes: 47 additions & 25 deletions src/components/Table/UncontrolledTable.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,7 @@ describe('<UncontrolledTable />', () => {
rows = [
{ name: 'Alpha', key: '1' },
{ name: 'Bravo', key: '2' },
{ name: 'Charlie', key: '3' },
];
expanded = [{ name: 'Alpha', key: '1' }];
selected = [{ name: 'Bravo', key: '2' }];
Expand All @@ -643,14 +644,12 @@ describe('<UncontrolledTable />', () => {
);
});

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';
Expand All @@ -666,17 +665,11 @@ describe('<UncontrolledTable />', () => {
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()));
Expand All @@ -693,40 +686,69 @@ describe('<UncontrolledTable />', () => {
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(
<UncontrolledTable
columns={columns}
rows={rows}
expanded={expanded}
page={1}
pageSize={1}
selected={selected}
resetPageOnRowChange={false}
/>
);
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(
<UncontrolledTable
columns={columns}
rows={rows}
expanded={expanded}
page={2}
pageSize={1}
selected={selected}
resetPageOnRowChange={false}
/>
);
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(
<UncontrolledTable
columns={columns}
rows={rows}
expanded={expanded}
page={2}
pageSize={1}
selected={selected}
resetPageOnRowChange={false}
/>
);
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', () => {
Expand Down

0 comments on commit fd6d753

Please sign in to comment.