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

Fixing problem with externally controlling expanded rows. #1198

Merged
merged 1 commit into from
Apr 7, 2017

Conversation

dangerbell
Copy link
Contributor

Changing the "expanding" option was not changing which rows were selected. Setting an initial list of expanded rows worked, but modifying that list through the props to BootstrapTable did not work.

The value being passed down to TableBody is this.state.expanding, but that is only set from props as part of the constructor, so changes to props.options.expanding of an already mounted BootstrapTable weren't being taken into account.

I've added some handling in componentWillReceiveProps to deal with change to options.expanding. I've copied the pattern used for the pagination options.

Changing the "expanding" option was not changing which rows were selected. Setting an initial list of expanded rows worked, but modifying that list through the props to BootstrapTable did not work.

The value being passed down to TableBody is `this.state.expanding`, but that is only set from props as part of the constructor, so changes to props.options.expanding of an already mounted BootstrapTable weren't being taken into account.

I've added some handling in componentWillReceiveProps to deal with change to `options.expanding`. I've copied the pattern used for the pagination options.
@dangerbell
Copy link
Contributor Author

Looks like this build is failing because AllenFang/react-toastr doesn't exist. I assume that's a problem with where I branched off of. Can you tell me what's needed to fix that?

Copy link
Owner

@AllenFang AllenFang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AllenFang
Copy link
Owner

Thanks your contribution, I'll test it again 👍

@dangerbell
Copy link
Contributor Author

Thanks! I didn't run gulp prod on it, should I have done that?

@AllenFang
Copy link
Owner

Thanks! I didn't run gulp prod on it, should I have done that?

ah, that's my job :) it's for building production before release :) Thanks 👍

@AllenFang AllenFang merged commit 6f0b5fb into AllenFang:master Apr 7, 2017
@AllenFang
Copy link
Owner

Released on v3.1.7 and thanks your contributions :)

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

Successfully merging this pull request may close these issues.

2 participants