Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

selected rows are lost when the component re-render #151

Closed
bjrmatos opened this issue May 4, 2015 · 6 comments
Closed

selected rows are lost when the component re-render #151

bjrmatos opened this issue May 4, 2015 · 6 comments

Comments

@bjrmatos
Copy link

bjrmatos commented May 4, 2015

Hi, i've found another bug in #145, i've got this:

var Griddle = require('griddle-react');

var data = [{
  id: 1,
  name: 'Boris'
}, {
  id: 2,
  name: 'Junior'
}];

var App = React.createClass({
  _onSomeEvent: function() {
     console.log(this.refs.griddleTable.getSelectedRowIds()); // selected ids
   },

  render: function() {
    return (
      <button onClick={this._onSomeEvent}>Click Me</button>
      <Griddle ref="griddleTable" isMultipleSelection={true} results={someData} />
    );
  }
});

the only way to get access to the selected row ids is via a ref in the griddle component (i think this is problematic, because makes harder to sync the state of App component with the selected rows, maybe we could find a nice way to keep in sync App state), the problem that i have found is if my App component re-render for what-ever reason (a call to setState in another part) the selected rows are lost.

I've found that the problem is this logic in componentWillReciveProps:

componentWillReceiveProps: function(nextProps) {
        this.setMaxPage(nextProps.results);

        if(nextProps.columns !== this.columnSettings.filteredColumns){
            this.columnSettings.filteredColumns = nextProps.columns;
        }

        // NEW CODE IN #145. THIS CAUSES AN OUT-OF-SYNC BETWEEN STATE MANAGER (App component) AND THE TABLE (Griddle) 
        // BECAUSE WHEN APP RE-RENDER THERE IS NO `selectedRowIds` PROPERTY, THIS CAUSES THAT THE PREVIOUS ROWS ARE RESETTED
        if(nextProps.selectedRowIds) {
             var visibleRows = this.getDataForRender(this.getCurrentResults(), this.columnSettings.getColumns(), true);

             this.setState({
                 isSelectAllChecked: this._getAreAllRowsChecked(nextProps.selectedRowIds, _.pluck(visibleRows, this.props.uniqueIdentifier)),
                 selectedRowIds: nextProps.selectedRowIds
             });
         }
    },

any ideas how to improve this feature?

@cosminnicula
Copy link
Contributor

@bjrmatos I'll take a look one of these days and fix it. stay tuned.

@bjrmatos
Copy link
Author

bjrmatos commented May 7, 2015

thnks @cosminnicula, i think we should discuss this in #153

@aphillipo
Copy link

Hmmm, I like the look of #153, but no multiple selection support with re-rendering is very problematic for me.

@aphillipo
Copy link

Ah, setting a prop as follows seems to fix things.

selectedRowIds={this.state.selected}

onRowClick should be paralleled by another function called onSelectionChange that happens after things are interacted with.

@dw3e9
Copy link

dw3e9 commented Nov 9, 2015

can you clarify what you meant by this, I'm trying to get checkboxes to persist after rerendering, but couln't seem to get it to work @aphillipo Thanks!

@ryanlanciaux
Copy link
Member

I realize that this issue is a bit older but we have released a new version of Griddle. We've defaulted to providing less features out of the box (like row selection) but added additional customization options and the ability for user/community plugins.

In addition to this, the state management is a bit more flexible and shouldn't lose part of the state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants