[TableBody] checkboxes do not reflect new selected state of TableRows (v0.11)

[rscheuermann]

I am testing the latest version of Table on the v0.11 branch, which introduces composable Tables (nicely done @jkruder).

Steps To Reproduce the bug:

1. Build a Table w/ TableBody and Table Rows that have preselected "selected" props

        let tableRows = [];

        sortedSuppliers.forEach(function (row) {
            tableRows.push(
                <TableRow key={'a-supplier-'+row.id} selected={this.props.selectedSupplierIds.indexOf(row.id) !== -1}>
                    <TableRowColumn>{row.name}</TableRowColumn>
                </TableRow>
            );
        }, this);

        return (
            <Table
                height="400px"
                selectable={true}
                multiSelectable={true}
                onRowSelection={this.onRowSelection}>
                <TableHeader enableSelectAll={false}
                             adjustForCheckbox={true}
                             displaySelectAll={false}>
                    <TableRow>
                        <TableHeaderColumn>Name</TableHeaderColumn>
                    </TableRow>
                </TableHeader>
                <TableBody
                    displayRowCheckbox={true}
                    deselectOnClickaway={false}
                    showRowHover={false}
                    stripedRows={true}>
                    {tableRows}
                </TableBody>
            </Table>
        );

1. Select a row by clicking the Checkbox (which adds a selectedId)
2. Now, remove the row from the selectedIds using an external action, completely outside the state of the Table. The Table and the TableRows will re-render.

Expected Behavior: The TableRow for the removed id becomes unchecked.
Action Behavior: The TableRow remains checked.

I've debugged this, and the TableRow's are rendered with selected={false}, accurately. BUT the TableBody is what controls the state of the checkboxes for each row. And the TableBody does not updated its state of selectedRows when receiving new properties. TableBody doesn't look at the new state of it's children TableRows to determine the new state of selectedRows.

My suggestion is to move the rendering of the checkbox into TableRow itself where it knows the state of the "selected" prop.

Any of this making sense? It's a complicated bug, I know.

[jkruder]

@rscheuermann Thanks for the details! Part of the problem is reporting the selected rows so some thing needs to know how many rows are selected. We could modify this so that we regenerate the list every time properties change. The changes in #1325 should address this issue or programmatically changing the selected rows.

[shaurya947]

@rscheuermann #1325 was merged so I am closing this. Please re-test and if you still experience this issue, let me know -- i'll re-open

[That-David-Guy]

I'm getting this issue in v0.13.4 of material-ui.

I have the following table

<Table
  fixedHeader={true}
  fixedFooter={true}
  selectable={true}
  multiSelectable={true}
  onRowSelection={
      (rows) => actions.selectedScheduledInstances(
           (rows === 'all')
              ? map( (o) => o.instanceId, instances )
              : (rows === 'none')
                  ? []
                  : map( (i) => instances[i].instanceId, rows )
              )}
    >
 <TableHeader enableSelectAll={true}>
    <TableRow>
       <TableHeaderColumn style={style.idCol}>ID</TableHeaderColumn>
       <TableHeaderColumn style={style.nameCol}>Name</TableHeaderColumn>
       <TableHeaderColumn>Backup Schedule</TableHeaderColumn>
       <TableHeaderColumn>History</TableHeaderColumn>
     </TableRow>
</TableHeader>
<TableBody
  deselectOnClickaway={false}
  showRowHover={true}
  stripedRows={false}>
    {instances.map(instance =>
       <TableRow
           key={instance.instanceId}
           selected={contains(instance.instanceId, selectedScheduledInstances)}>
           <TableRowColumn style={style.idCol}>{instance.instanceId}</TableRowColumn>
           <TableRowColumn style={style.nameCol}>{instance.name}</TableRowColumn>
           <TableRowColumn>{instance.backupSchedule}</TableRowColumn>
           <TableRowColumn>
              <History 
                 isScheduled={ instance.backupSchedule != null || instance.backupSchedule != undefined}
               /> 
             </TableRowColumn> 
      </TableRow>)
    }
</TableBody>
</Table>

selectedScheduledInstances is the list of selected elements, I keep this in a store because it is used else were in the site.

When I individual select and deselect the rows everything works fine.
If I use the select all everything runs fine
When I deselect all, everything BUT the last row gets deselected. Even though selectedScheduledInstances is an empty array and selected={contains(instance.instanceId, selectedScheduledInstances)} returns false for every row (including the last one).

(On a side note, it would be nice if instead of 'all' and 'none' if it just returned the array like all the other times)

[Inggo]

I'm getting the same problem as @That-David-Guy. Here's a gist of my Component:

const tableData = [
  {
    id: 1,
    name: 'John Smith',
    status: 'Employed',
  },
  {
    id: 2,
    name: 'Randal White',
    status: 'Unemployed',
  },
  {
    id: 3,
    name: 'Stephanie Sanders',
    status: 'Employed',
  },
  {
    id: 4,
    name: 'Steve Brown',
    status: 'Employed',
  },
  {
    id: 5,
    name: 'Joyce Whitten',
    status: 'Employed',
  },
  {
    id: 6,
    name: 'Samuel Roberts',
    status: 'Employed',
  },
  {
    id: 7,
    name: 'Adam Moore',
    status: 'Employed',
  },
];

export default class TestIndex extends React.Component {

  constructor(props) {
    super(props);

    this.state = {
      fixedHeader: true,
      stripedRows: true,
      showRowHover: true,
      selectable: true,
      multiSelectable: true,
      displaySelectAll: true,
      enableSelectAll: true,
      deselectOnClickaway: false,
      showMassActions: false,
      selectedRows: [],
    };

    this.handleRowSelection = this.handleRowSelection.bind(this);
  }

  handleRowSelection(selectedRows) {
    if (selectedRows === 'all') {
      selectedRows = Array.apply(null, {length: tableData.length}).map(Number.call, Number);
    }

    if (selectedRows === 'none') {
      selectedRows = [];
    }

    console.log(selectedRows);

    this.setState({
      showMassActions: selectedRows.length > 0,
      selectedRows: selectedRows,
    });
  }

  _showRowStyleAsNumber(i) {
    return {
      test: i
    };
  }

  render() {
    return (
      <div>
        <Table
          fixedHeader={this.state.fixedHeader}
          selectable={this.state.selectable}
          multiSelectable={this.state.multiSelectable}
          displaySelectAll={this.state.displaySelectAll}
          enableSelectAll={this.state.enableSelectAll}
          onRowSelection={this.handleRowSelection}
        >
          <TableHeader enableSelectAll={this.state.enableSelectAll}>
            <TableRow>
              <TableHeaderColumn>ID</TableHeaderColumn>
              <TableHeaderColumn>Name</TableHeaderColumn>
              <TableHeaderColumn>Status</TableHeaderColumn>
              <TableHeaderColumn>Actions</TableHeaderColumn>
            </TableRow>
          </TableHeader>
          <TableBody
            deselectOnClickaway={this.state.deselectOnClickaway}
            showRowHover={this.state.showRowHover}
            stripedRows={this.state.stripedRows}
          >
            {tableData.map( (row, i) => (
              <TableRow key={row.id} selected={this.state.selectedRows.indexOf(i) !== -1}>
                <TableRowColumn>{row.id}</TableRowColumn>
                <TableRowColumn>{row.name}</TableRowColumn>
                <TableRowColumn>{row.status}</TableRowColumn>
                <TableRowColumn><span>{this.state.selectedRows.indexOf(i)}</span></TableRowColumn>
              </TableRow>
            ))}
          </TableBody>
        </Table>
      </div>
    );
  }
}

Select All works fine, and the selectedRows store the correct array properly. But the last item does not get deselected when trying to deselect everything.

[cody-lettau]

Is there any update on this bug? I'm using v0.15.0 and I still am seeing the issue with the last row not being unchecked when you use the unselect all button. This pretty much renders the select/unselect all feature unusable until it is fixed.

[dcworldwide]

I have the same issue. Using 16.6.

1. Selected all using Select All cb
2. De-selected all as above
3. Last row still selected, even though the TableRow selected prop is definitely being set to false.

[dchambers]

This may be fixed #5884?

[rsolomon]

This is still a reproducible issue in 16.7. It's also apparent when changing the rows. Ex:

1. Render 7x Table rows with the row at index [4] set to selected
2. Re-render 6 rows: The 7 rows minus the original row at index [4]. No rows should be set to selected
3. Observe that the row now at index [4] renders as selected.

[Ajaay]

I'm not entirely sure if this is the same bug, but 16.7 introduced a problem for me. I have two tables that I move data between by clicking on them and using the row key to add/remove array data feeding the tables. This is done through state.

As of 16.7, the first click provides the key value to my onRowSelection method, but any following that I receive no value.

addQuestion(key) {
	console.log(key)
	let selectedQuestion = this.state.questionBank[key];
	let questionBank = this.state.questionBank.slice();
	let selectedQuestions = this.state.selectedQuestions.slice();
        
        //Adds question to second table
        selectedQuestions.push(selectedQuestion);
        
        //Removes question from first table
        questionBank.splice(key, 1);
	
        this.setState({questionBank: questionBank, selectedQuestions: selectedQuestions})
}

Presumably this is due to the change in #5884 but I can't figure out why...

[Cleanshooter]

The way the component is coded currently the description should read...

Indicates that a particular row is selected. This property can be used to programmatically select rows on first load.

And it's name should be preSelected
not selected... the current label is misleading...

Changing this value programmatically will not change the state of the tables "selectedRows". The Table component itself would need to provide access to the selectedRows state in order for selection to change.

I too need the ability to programmatically change the state of the "selectedRows", but I'm not sure how open to change the owners are on this control. The problem is that you can't change the state of the row itself... you have to change the state of the entire table which means it will need to expensively re-render the entire component... rather than just the row... This is all due to the fact the selectedRows state is stored by the table component itself.

I think the smallest footprint solution would be to create a setSelectedRows prop which would allow the developer to pass an array similar to the one stored in the state and simply change the state in the componentWillRecieveProps function which would cause a re-render....

some time passes... writes some code... installs the linter... runs the linter... wonders why eslint hints aren't showing up in sublime anymore... barrels on anyway and DONE.

Check out idea here:
master...Cleanshooter:table-rows-not-reflecting-selected-value-prop-value

The optimal solution is to let the row handle "selected" in its' own state instead of the table handling it but.... that would require re-writting the entire table component... and I don't have that kind of time atm 😢 . I'm going to test this spike out in the morning but I'm open to feedback and any ideas... I won't do a pull until one of the maintainers asks... to many dead pull requests experiences in other projects and heartache...

[mbrookes]

that would require re-writting the entire table component... and I don't have that kind of time

Good news - it's already been done! https://github.com/callemall/material-ui/tree/next/src/Table

[apendua]

@mbrookes Is there a way I can already use those updates? Has this been released or is it still work in progress?

[Cleanshooter]

@mbrookes Well that's great news but... the last time I tried to use the next branch I ran into a several issues and figured it wasn't stable enough for use yet. I really hope that it's release ready for my next project but for now I'm planning to stick with this release.

Since we still need a fix for the current version I had a chance to test out my spike this morning and the trouble was that the state can be both a string or an array... which the proptype wasn't happy about. So i junked the proptype (which is bad form I know , I know) but it works and at the end of the day thats all I really care about.

Dirty working version: master...Cleanshooter:table-rows-not-reflecting-selected-value-prop-value

If anyone is interested (or in a pinch like I am) and needs the fix ASAP I have a build of it here: https://github.com/Cleanshooter/material-ui-test

used basically in the same way as core but you can pass something like this in:

import {TableBody} from 'material-ui-test/Table';
<TableBody
	showRowHover
	stripedRows
	deselectOnClickaway={false}
	setSelectedRows={this.state.selected}
>

this.state.selected is updated by the "Table" onRowSelection prop/function and in other places manually when I need to pragmatically change which items are selected.. I think the rest is a bit self explanatory on how its used... 😉

[mbrookes]

@apendua you can npm install --save material-ui@next.It is WIP though.

@Cleanshooter thanks for sharing your fix.

[hayesmaker]

i tried installing material-ui@next and I get all these errors now...

Error in ./src/App.js
[2] Module not found: 'material-ui/styles/getMuiTheme' in /Users/hayesmaker/Workspace/portfolio/portfolio-admin/client/src
[2]
[2]  @ ./src/App.js 16:19-60
[2]
[2] Error in ./src/App.js
[2] Module not found: 'material-ui/styles/baseThemes/darkBaseTheme' in /Users/hayesmaker/Workspace/portfolio/portfolio-admin/client/src
[2]
[2]  @ ./src/App.js 20:21-75
[2]
[2] Error in ./src/Articles.js
[2] Module not found: 'material-ui/FloatingActionButton' in /Users/hayesmaker/Workspace/portfolio/portfolio-admin/client/src
[2]
[2]  @ ./src/Articles.js 17:28-71
[2]
[2] Error in ./src/Articles.js
[2] Module not found: 'material-ui/svg-icons/content/add' in /Users/hayesmaker/Workspace/portfolio/portfolio-admin/client/src
[2]
[2]  @ ./src/Articles.js 21:11-55
[2]
[2] Error in ./src/AddArticleModal.js
[2] Module not found: 'material-ui/FlatButton' in /Users/hayesmaker/Workspace/portfolio/portfolio-admin/client/src
[2]
[2]  @ ./src/AddArticleModal.js 18:18-51

[sreerajkksd]

Is it still in WIP?

[oliviertassinari]

We have been porting the component on the v1-beta branch. We reimplemented it from the ground-up. While we haven't tested it, I think that the issue is most likely fixed on that branch. Hence, I'm closing it.
Still, we will accept PR fixes until v1-beta takes over the master branch.