Skip to content

Commit

Permalink
[TableBody] Fix row selection re-render bug
Browse files Browse the repository at this point in the history
Non-selection related table re-rendering previously caused the user's
selection to be lost because the state was being reset unnecessarily.

I took the opportunity to remove the prescient `TODO` comment that did
correctly predict that this might be a problem.
  • Loading branch information
dchamb committed Jan 6, 2017
1 parent 78a28b2 commit 8bdc527
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 9 deletions.
19 changes: 10 additions & 9 deletions src/Table/TableBody.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,16 @@ class TableBody extends Component {
}

componentWillReceiveProps(nextProps) {
if (this.props.allRowsSelected && !nextProps.allRowsSelected) {
this.setState({
selectedRows: [],
});
// TODO: should else be conditional, not run any time props other than allRowsSelected change?
} else {
this.setState({
selectedRows: this.calculatePreselectedRows(nextProps),
});
if (this.props.allRowsSelected !== nextProps.allRowsSelected) {
if (!nextProps.allRowsSelected) {
this.setState({
selectedRows: [],
});
} else {
this.setState({
selectedRows: this.calculatePreselectedRows(nextProps),
});
}
}
}

Expand Down
55 changes: 55 additions & 0 deletions test/integration/Table/SelectionObservingTable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React from 'react';
import {
Table,
TableBody,
TableHeader,
TableHeaderColumn,
TableRow,
TableRowColumn,
} from 'src/Table';

const tableData = [
{name: 'John Smith'},
{name: 'Randal White'},
{name: 'Olivier'},
];

class SelectionObservingTable extends React.Component {
constructor(props) {
super(props);
this.state = {
selectionChangeCount: 0,
};
}

render() {
return (
<div>
Selection Change Count: {this.state.selectionChangeCount}
<Table
selectable={true}
onRowSelection={() => this.setState({selectionChangeCount: this.state.selectionChangeCount + 1})}
>
<TableHeader>
<TableRow>
<TableHeaderColumn>
Name
</TableHeaderColumn>
</TableRow>
</TableHeader>
<TableBody displayRowCheckbox={true}>
{tableData.map( (row, index) => (
<TableRow key={index}>
<TableRowColumn>
{row.name}
</TableRowColumn>
</TableRow>
))}
</TableBody>
</Table>
</div>
);
}
}

export default SelectionObservingTable;
62 changes: 62 additions & 0 deletions test/integration/Table/SelectionObservingTable.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/* eslint-env mocha */
import React from 'react';
import {mount} from 'enzyme';
import {assert} from 'chai';
import getMuiTheme from 'src/styles/getMuiTheme';
import SelectionObservingTable from './SelectionObservingTable';

describe('<SelectionObservingTable />', () => {
it('preserves selected change', () => {
const muiTheme = getMuiTheme();
const mountWithContext = (node) => mount(node, {
context: {muiTheme},
childContextTypes: {muiTheme: React.PropTypes.object},
});

const wrapper = mountWithContext(
<SelectionObservingTable />
);

assert.deepEqual(
wrapper.find('Checkbox').map((checkbox) => checkbox.props().checked),
[
false,
false,
false,
false,
],
'should use the selected property for the initial value'
);

let input;
input = wrapper.find('Checkbox').at(1).find('input');
input.node.checked = !input.node.checked;
input.simulate('change');

assert.deepEqual(
wrapper.find('Checkbox').map((checkbox) => checkbox.props().checked),
[
false,
true,
false,
false,
],
'should preserve initial selection'
);

input = wrapper.find('Checkbox').at(2).find('input');
input.node.checked = !input.node.checked;
input.simulate('change');

assert.deepEqual(
wrapper.find('Checkbox').map((checkbox) => checkbox.props().checked),
[
false,
false,
true,
false,
],
'should preserve updated selection'
);
});
});

0 comments on commit 8bdc527

Please sign in to comment.