Skip to content
This repository has been archived by the owner on Apr 1, 2021. It is now read-only.

Carousel: Update children if different (needed for the Hub #71

Merged
merged 2 commits into from
Dec 21, 2016

Conversation

MorganeLecurieux
Copy link
Contributor

capture d ecran 2016-12-19 a 16 53 43

In the case the second Carousel need to display different children according to the first Carousel, it won't work with the actual version.

This is a fix to make this work.

@codecov-io
Copy link

codecov-io commented Dec 19, 2016

Current coverage is 93.47% (diff: 100%)

Merging #71 into v2.0.0 will not change coverage

@@             v2.0.0        #71   diff @@
==========================================
  Files            29         29          
  Lines          2436       2436          
  Methods         416        414     -2   
  Messages          0          0          
  Branches        207        209     +2   
==========================================
  Hits           2277       2277          
  Misses          159        159          
  Partials          0          0          

Powered by Codecov. Last update f1c1b80...00718ef

Copy link
Contributor

@FlorentD FlorentD left a comment

Choose a reason for hiding this comment

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

Big love for the PR !

@@ -91,7 +92,7 @@ class Carousel extends Component {
}

componentWillUpdate({ children }) {
if (this.props.children.length === 0) {
if (!isEqual(children, this.props.children)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using hasDiff.js function ?
We don't want to create a new dependency to lodash.

@FlorentD FlorentD merged commit 5072911 into v2.0.0 Dec 21, 2016
@FlorentD FlorentD deleted the Carousel-reload-children branch December 21, 2016 15:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants