Material-ui: [Table] onRowSelection Bug

Created on 16 Oct 2015  路  22Comments  路  Source: mui-org/material-ui

Evening guys,

Let's say i have the following table:

                        <Table
                            height='100%'
                            selectable={true}
                            multiSelectable={true}
                            onRowSelection={this._onRowSelection}>
                            >

If i do something like this, it'll work fine:

    _onRowSelection: function(selectedRows){
        console.log(selectedRows);
    },

but if i attach any kind of more complex event, it'll make impossible to 'check' ANY other rows.
here's some examples of events:

    _onRowSelection: function(selectedRows){
        this.context.flux.getActions('conversation').setSelectedRows(selectedRows);
    },

    _onRowSelection: function(selectedRows){
       this.setState({
        selected: selectedRows.slice(0);
      });
    },

Both of them (Flux and non-flux version) cause the same issue.

Just to clarify it just freeze all my rows as unchecked, and if i put an console.log with the setState, it's returning just the last selected row instead of all rows.

any idea guys?

bug 馃悰 Table

Most helpful comment

The table 'resets' on the render method being (re)called. You can see this if you wrap the setState in your _onRowSelection method in a timeout.

To get around it, you can do something like this:

// on row selection event handler like this
_onRowSelection(rows) {
  this.setState({selectedRows: rows});
},

// when you generate your table body, set the 'selected' attribute of each row based
// on the list you're keeping in state
<TableBody>
  {this.state.rowDataList.map((d, i) => {
    return <TableRow key={i}
                     selected={this.state.selectedRows.indexOf(i) !== -1}>
      <TableRowColumn>
        {d.attr}
      </TableRowColumn>
    </TableRow>
  })}
</TableBody>

All 22 comments

The table 'resets' on the render method being (re)called. You can see this if you wrap the setState in your _onRowSelection method in a timeout.

To get around it, you can do something like this:

// on row selection event handler like this
_onRowSelection(rows) {
  this.setState({selectedRows: rows});
},

// when you generate your table body, set the 'selected' attribute of each row based
// on the list you're keeping in state
<TableBody>
  {this.state.rowDataList.map((d, i) => {
    return <TableRow key={i}
                     selected={this.state.selectedRows.indexOf(i) !== -1}>
      <TableRowColumn>
        {d.attr}
      </TableRowColumn>
    </TableRow>
  })}
</TableBody>

@andrewhn,

Hi Andrew,
Thanks for your answer, however, i still get the same results as described before.

Any ideas?

@husek Does row selection work briefly when you change your _onRowSelection code to the following?

_onRowSelection: function(selectedRows){
   setTimeout(function() {
     this.setState({
      selected: selectedRows.slice(0);
     });
   }, 2000);
},

@andrewhn , Yes, it does work for a short time with this snippet.

@husek So you see that when render() is called, your table is 'resetting' to having no rows selected. To get around this, you need to set the selected attribute of the TableRow components yourself, as in my example above.

ahd strange thing, @andrewhn , it's that i set selected at TableRow, but still doesn't working after the render 'reset', here's an simplified version of my component:

    getInitialState: function(){
        return {
            offset: 0,
            selectedRows: []
        }
    },


    _onRowSelection: function(selectedRows){
        setTimeout(function() {
            this.setState({
                selectedRows: selectedRows.slice(0)
            });
        }, 2000);
    },




    render: function () {
        console.log(this.state.selectedRows);

        return (
            <div>
                <Table
                    height='100%'
                    selectable={true}
                    multiSelectable={true}
                    onRowSelection={this._onRowSelection}
                    >
                    <TableBody
                        ref='currentTable'
                        deselectOnClickaway={false}
                        multiSelectable={true}
                        preScanRows={false}
                        showRowHover={true}
                        stripedRows={false}
                        >

                        {this.props.conversations.results.get('list').map(function(item, i){
                            return (
                                <TableRow key={i}  selected={this.state.selectedRows.indexOf(i) !== -1}>
                                    <TableRowColumn colSpan="1"><LogoGenerator integration={item.int_key}/></TableRowColumn>
                                    <TableRowColumn colSpan="2">[COSTUMER_MARKUP]</TableRowColumn>
                                    <TableRowColumn colSpan="3" >
                                        <Link to='Chat' params={{ id : item.id || '' }}>
                                            <strong>{item.title}</strong><br/>
                                            <span>{item.latest_content_repr}</span>
                                        </Link>

                                    </TableRowColumn>
                                    <TableRowColumn colSpan="3">
                                        <Link to='editItem' params={{ id : linkGenerator(item, 'id') || '' }}>
                                            {linkGenerator(item, 'name')}
                                        </Link>
                                    </TableRowColumn>
                                    <TableRowColumn colSpan="2">{Moment(item.latest_message_time).format('DD/MM/YYYY [-] HH:MM:SS')}</TableRowColumn>
                                </TableRow>
                            )
                        }.bind(this))}

                    </TableBody>
                </Table>

            </div>
        );
    }
});

This code snippet will work for a single checked box, and it can be extended for multiple checked boxes by using an array as described above by @andrewhn


  _onRowSelection(selectedRows) {
    let rowNumber = selectedRows[0]
    this.setState({
      selectedRow: rowNumber,
    })
  },

  render() {
    let items = this.state.items.map((item, index) => {
      return (
        <TableRow key={item.id} selected={this.state.selectedRow === index}>
          <TableRowColumn>{item.name}</TableRowColumn>
        </TableRow>
      )
    })
  }

One possible fix for this issue would involve the library owner following a similar approach, persisting all checked rows and rebuilding them as checked rows when render() is called, and subsequently deleting the persisted checked rows in componentWillUnmount()

Hi ,
Any plan on fixing this issue?
I get the same problem...

Even if you set <TableRow key={item.id} selected={true}> you can remove selection by clicking on the row. Is seems there is no way to make a controlled table widget now.

This bug makes the Table Material UI element completely unusable for elements that want to use it dynamically, I am disappoint :( Specifically, I wanted to use a multi-select Table for adding many items in a collection (this.props.collection) with an "Add all selected" button, utilizing the onRowSelection callback of the Table component to alter local state. Unfortunately the table does to maintain state across renders-- which is totally okay, but there isn't a way for the TableRow components to re-compute their selected values on re-render. I propose that an optional isSelected func prop be added to TableRow so we can make this almost awesome Table work in our applications :D

Edit: The TableRow component should be changed, not the TableRowColumn component.

Threw together an example component with my proposed optimistic functionality, closely mimicing code I wrote before encountering this bug:

import React, { Component, PropTypes } from 'react';

import {
  Table,
  TableHeader,
  TableHeaderColumn,
  TableBody,
  TableRow,
  TableRowColumn
} from 'material-ui';
import ContentAddIcon from 'material-ui/svg-icons/content/add';

class MyComponent extends Component {
  constructor (props) {
    super(props);
    this.state = {
      selectedPeople: []
    };
  }

  handleRowSelection (indices) {
    if (indices === 'all') {
      this.setState({ selectedPeople: this.props.people });
    } else if (indices === 'none') {
      this.setState({ selectedPeople: [] });
    } else {
      let people = indices.map(index => {
        return this.props.people[index];
      })
      this.setState({ selectedPeople: people })
    }
  }

  isPersonSelected (subjectPerson) {
    this.state.selectedPeople.forEach(selectedPerson => {
      if (subjectPerson.id === selectedPerson.id)
        return true;
    });
    return false;
  }

  doSomethingWithAllSelectedPeople () {
    // Whatevs
  }

  render() {
    var doSomethingButton = null;
    if (this.state.selectedPeople.length > 0) {
      doSomethingButton = (
        <FloatingActionButton onMouseUp={this.doSomethingWithAllSelectedPeople.bind(this)}>
          <ContentAddIcon />
        </FloatingActionButton>
      );
    }

    return (
      <div className="my-component">
        <Table
          selectable={true}
          multiSelectable={true}
          onRowSelection={this.handleRowSelection.bind(this)}
        >
          <TableHeader>
            <TableRow>
              <TableHeaderColumn>ID</TableHeaderColumn>
              <TableHeaderColumn>Name</TableHeaderColumn>
            </TableRow>
          </TableHeader>
          <TableBody>
            {this.props.people.map((person, index) => {
              return (
                <TableRow key={index} isSelected={this.isPersonSelected.bind(this, person)}>
                  <TableRowColumn>{person.id}</TableRowColumn>
                  <TableRowColumn>{person.name}</TableRowColumn>
                </TableRow>
              )
            })}
          </TableBody>
        </Table>
        {doSomethingButton}
      </div>
    );
  }
};

MyComponent.propTypes = {
  people: PropTypes.arrayOf(PropTypes.shape({
    id: PropTypes.number,
    name: PropTypes.string
  }))
};

export default MyComponent;

Any plan on fixing this issue?

Well, i have opened this issue almost a year ago, and so far i've used two different approaches to fix this problem:

1) Forked the project and added this method into the /table/table-body.js:

  getValue: function getValue() {
    if (this.props.allRowsSelected) {
      return 'all'
    } else {
      return this.state.selectedRows
    }
  }

So i was able to do something like this.refs.theTable.getValue();

2) Not sure why, but this started to work fine after the 0.14 release:

<Table
                                    height='100%'
                                    selectable={true}
                                    multiSelectable={true}
                                    onRowSelection={this.handleRowSelect}
                                    >
 <TableHeader enableSelectAll={true}>[yadda yadda]</TableHeader>
                        <TableBody
                                        ref='currentTable'
                                        deselectOnClickaway={false}
                                        multiSelectable={true}
                                        showRowHover={true}
                                        stripedRows={false}
                                        >
{// Your content.map
 <TableRow key={i}  selected={this.state.selectedRows.indexOf(i) !== -1}> </TableRow>}
                                    </TableBody>

and

    handleRowSelect: function(newArray){
            var result = newArray.slice(0);
            if (result == 'none') {
                result = [];
            }
            this.setState({selectedRows: result})
        }

Huh, not sure what fever dream I was in when I posted because I just got it to work too. I think my selected={} function was just busted. D'oh!

@husek Maybe it's time to close this issue :)

@corytheboyd, indeed it's!

(But i still think that the getValue() method should be part of the lib :smile: )

I am using version 0.16.4 and still seeing this issue. I dynamically set the selected property to true before the table is rendered but the checkboxes do not get selected.

var rows = [];
        this.props.cashLineItems.forEach(function (cli) {
            if (cli) {
                var isSelected = false;
                this.state.userSelectedRows.forEach(function(row) {
                    if (row.cashLineItemId === cli.cashLineItemId)
                        isSelected = true;
                });
                rows.push(
                    <TableRow key={cli.cashLineItemId} selected={isSelected}>
                        <TableRowColumn>{moment(cli.accountingDate).format("MM/DD/YYYY")}</TableRowColumn>
                        <TableRowColumn>{cli.accountCode}</TableRowColumn>
                        <TableRowColumn>{cli.ledgerAccount}</TableRowColumn>
                        <TableRowColumn>{cli.description}</TableRowColumn>
                        <TableRowColumn/>
                    </TableRow>
                );
            }
        }, this); 

Any ideas? Thanks.

@rachel-singh : I found a way to hack this behaviour. Instead of managing the selected property of the rows, use a ref to your TableBody and modify its state. This updates the selected state of the rows and does not fire a onRowSelection event :

<TableBody ref={(tableBody) => { this.tableBody = tableBody; }}>

Then in your handler :

this.tableBody.setState({ selectedRows: selection });

It would be nice to have a documented and tested way to implement this though.

Anyone know why the above works?

I am still having the same problem :/

I used the solution of ugomeda. Important is that you set the state of the TableBody after you set your state. To achieve that, I set the state of the TableBody as callback of my own state. Following works for me:

    constructor() {
        super();
        this.state = {
            selectedRows: []
        };

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


    render() {
        return (
                <Table multiSelectable onRowSelection={this._onRowSelection}>
                    <TableHeader>
                        <TableRow o>
                            <TableHeaderColumn>c1</TableHeaderColumn>
                            <TableHeaderColumn>c2</TableHeaderColumn>
                            <TableHeaderColumn>c3</TableHeaderColumn>
                        </TableRow>
                    </TableHeader>
                    <TableBody showRowHover ref={(tableBody) => { this.tableBody = tableBody; }}>
                        {this._renderTableRow()}
                    </TableBody>
                </Table>
        );
    }

    _onRowSelection(rows: Array<number>) {
        this.setState({selectedRows: rows}, () => this.tableBody.setState({ selectedRows: rows }));
    }

Another late solution for arrow functional style: Using "pure" from _recompose_
https://github.com/acdlite/recompose

import { compose, pure, setDisplayName } from 'recompose';

const enhance = compose(
  setDisplayName('MailAttachments'),

  setPropTypes({
    onRowSelection: PropTypes.func.isRequired
  }),

  pure
)

export default enhance(  ({ onRowSelection }) => (
    <Table selectable onRowSelection={onRowSelection}>
         ...
   </Table>
)  )

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mattmiddlesworth picture mattmiddlesworth  路  3Comments

revskill10 picture revskill10  路  3Comments

mb-copart picture mb-copart  路  3Comments

ryanflorence picture ryanflorence  路  3Comments

rbozan picture rbozan  路  3Comments