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

[CarouselIndicators] Warning: Encountered two children with the same key #1310

Closed
LouisJS opened this issue Nov 28, 2018 · 4 comments · Fixed by #1311 or #1342
Closed

[CarouselIndicators] Warning: Encountered two children with the same key #1310

LouisJS opened this issue Nov 28, 2018 · 4 comments · Fixed by #1311 or #1342

Comments

@LouisJS
Copy link

LouisJS commented Nov 28, 2018

  • components: CarouselIndicators
  • reactstrap version ^6.1.0
  • react version ^16.4.1
  • bootstrap version ^4.1.0

What is happening?

When you pass your data to

<CarouselIndicators items={items} activeIndex={activeIndex} onClickHandler={this.goToIndex} />

It provoke

screenshot 2018-11-28 at 15 46 26

What should be happening?

CarouselIndicators shouldn't rely on dataset implementation (which is not using src)

Error message in console

screenshot 2018-11-28 at 15 46 26

Code

<CarouselIndicators items={items} activeIndex={activeIndex} onClickHandler={this.goToIndex} />
@walker-tx
Copy link
Contributor

walker-tx commented Nov 29, 2018

@LouisJS

CarouselIndicator keys are generated by concatenating the item values (belonging to keys src, caption, and altText) shown to be passed to the component in the docs through the items props... but there is another...

See here: CarouselIndicators.js

Check out the line 16 - it's the one that builds the key prop for each indicator. If a key field is included in each item object of your items array, then that will prevent you from having duplicate keys. So your object array should look something like this:

const items = [
  {
    'key': 0,
    'foo': 'bar',
  },
  {
    'key': 1,
    'foo': 'bahr',
  }
]

The resulting keys look something like 0undefinedundefined, but it works...

@LouisJS
Copy link
Author

LouisJS commented Nov 29, 2018

It's the same approach as src. Instead, i'll have to add a key properties to my data instead.
That doesn't solve the problem.

What if we could pass a chosen key to look for as props to CarouselIndicator. And then it will look for it instead of relying on the fact that the user will have a dataset which will match the one CarouselIndicator is waiting for.

Such as

const CarouselIndicators = (props) => {
  const { items, activeIndex, cssModule, onClickHandler, className, theKeyIdLikeToUse } = props;

 ...

 <li
     key={theKeyIdLikeToUse.length != 0 ? `${item[theKeyIdLikeToUse]` : ``${item.key || item.src}${item.caption}${item.altText}``}
     onClick={(e) => {
        e.preventDefault();
        onClickHandler(idx);
     }}
     className={indicatorClasses}
/>);

So the implementation would look like

<CarouselIndicators items={items} activeIndex={activeIndex} onClickHandler={this.goToIndex} theKeyIdLikeToUse={'id'} />

@LouisJS
Copy link
Author

LouisJS commented Nov 29, 2018

Something like this. Making it optional by default

@walker-tx
Copy link
Contributor

I think just concatenating whatever values are passed in the item objects would be less painful. No change needed to the docs, no logic required. Only stuff passed to the items prop that's unique.

Hopefully the PR is accepted, or at least we find a way to fix the issue if my solution isn't the way to go.

TheSharpieOne pushed a commit that referenced this issue Dec 1, 2018
…1311)

Generation of key items for `li` components in `CarouselIndicators` is a
concatenation of specific, optional values (`src`, `caption`, and
`altText`) in each item of the `items` object array. Lacking these
key/value pairs will cause each `li` element to have a key along the
lines of `undefinedundefinedundefined`, resulting two or more of the
`li`'s to be identical and causing an error to log in the console.
While either including any one of the specified keys with a unique
value, or adding a unique `key` field in each of the items objects will
remedy this issue, that solution is not well documented. The proposed
solution is a transparent fix: preventing the aforementioned error while
requiring no change of code for the end-user. The new `key` prop value
for each `li` will be a concatenation of all the values of its
respective item object.

Closes #1310
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants