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

Auto-detect props.items changes #36

Closed
Hendrik opened this issue Feb 3, 2016 · 2 comments
Closed

Auto-detect props.items changes #36

Hendrik opened this issue Feb 3, 2016 · 2 comments

Comments

@Hendrik
Copy link
Contributor

Hendrik commented Feb 3, 2016

At the moment, changing props.items won't reset state.currentIndex automatically, so that props.items changes can result in a broken image. The current workaround is to use slideToIndex to reset the currentIndex back to the correct start position on props.items changes.

It's possible to auto-detect props.items changes by comparing the current and next props.items arrays. However, the solution I'm using on my fork adds 2 more dependencies to the project (immutable and immutablediff), so that there's quite a bit of extra code just to have the ability to compare the property changes. That's also the reason I didn't submit a pull request for this, as I don't know whether you would want to add such extra dependencies to the original code base.

I mentioned about sharing my solution in one of the previous commit messages, so here are the relevant changes from my build/image-gallery.js file:

  var diff = require('immutablediff');
  var Immutable = require('immutable');

  ...  

  componentWillReceiveProps: function componentWillReceiveProps(nextProps) {
    var propsDiff = diff(Immutable.fromJS(this.props.items), Immutable.fromJS(nextProps.items));
    if (propsDiff.size !== 0) {
      var startIndex = nextProps.startIndex || 0;
      this.setState({ currentIndex: startIndex });
    }
  }
@xiaolin
Copy link
Owner

xiaolin commented Feb 7, 2016

@Hendrik Thanks for sharing. I did some thinking, and realize this function might fix one issue, but break another.

For example, If you decides to change other properties in items that are not image src related, e.g. description, originalClass or thumbnailClass, the index reset would be a bad side effect.

I think for this specific issue using slideToIndex after you manipulated the items is the right way to go.

@Hendrik
Copy link
Contributor Author

Hendrik commented Feb 9, 2016

No problem.
I'll close this issue then.

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

2 participants