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

Make MenuItem key in ComboBox to use unique id first #4969

Closed
1 of 2 tasks
fred104 opened this issue Jan 8, 2020 · 4 comments · Fixed by #4999
Closed
1 of 2 tasks

Make MenuItem key in ComboBox to use unique id first #4969

fred104 opened this issue Jan 8, 2020 · 4 comments · Fixed by #4999

Comments

@fred104
Copy link

fred104 commented Jan 8, 2020

What package(s) are you using?

  • carbon-components
  • carbon-components-react

Detailed description

Describe in detail the issue you're having. Is this a feature request (new
component, new icon), a bug, or a general issue?

Is this issue related to a specific component?
Yes, ComboBox

What did you expect to happen? What happened instead? What would you like to
see changed?
There are exceptions from React since there are some duplicated key errors. Tracing back to ComboBox. We are using itemToString to be MenuItem key and the function is also using in menu option. So, if there are options with same text, the error occurs. It's better to use unique id to be MenuItem key.

What browser are you working in?
Chrome

What version of the Carbon Design System are you using?
7.4.0

What offering/product do you work on? Any pressing ship or release dates we
should be aware of?
IBM Resilient. We are currently reset the input field after selection.

Steps to reproduce the issue

Giving two items with save text but with different id can reproduce it.

https://codesandbox.io/s/wizardly-pond-q3uh0?fontsize=14&hidenavigation=1&theme=dark

Additional information

image

Add labels

Please choose the appropriate label(s) from our existing label list to ensure
that your issue is properly categorized. This will help us to better understand
and address your issue.

@asudoh
Copy link
Contributor

asudoh commented Jan 8, 2020

Hi 👋 would you want to create a reduced case based on https://codesandbox.io/s/github/carbon-design-system/carbon/tree/master/packages/react/examples/codesandbox? Thanks!

@fred104
Copy link
Author

fred104 commented Jan 8, 2020

@asudoh
Copy link
Contributor

asudoh commented Jan 8, 2020

Seems that the problem does not reproduce in the Sandbox. Downloading the content to the local machine yielded the same result. Also I couldn't open the dropdown in the Sandbox. Probably more work is needed in the Sandbox...?

@fred104
Copy link
Author

fred104 commented Jan 9, 2020

image
@asudoh Did you see the same screen above?

Here is the sample code.

import React from 'react';
import { render } from 'react-dom';
import { ComboBox } from 'carbon-components-react';

const App = () => (
  <div>
    <ComboBox
    ariaLabel="Choose an item"
    disabled={false}
    helperText="Optional helper text here"
    id="carbon-combobox-example"
    invalidText="A valid value is required"
    itemToString={(item) => {return item ? item.text : '' }}
    items={[
      {
        id: 'option-0',
        text: 'Option'
      },
      {
        id: 'option-1',
        text: 'Option'
      },
    ]}
    light={false}
    onChange={function noRefCheck(){}}
    placeholder="Filter..."
    size={undefined}
    titleText="Combobox title"
    type="default"
  />
  </div>
);

render(<App />, document.getElementById('root'));

asudoh added a commit to asudoh/carbon-components that referenced this issue Jan 10, 2020
This change swiches React `key`s of the items of `<Dropdown>` as well
as the one of `<ComboBox>` from their text content to provided keys in
data.

This is applying the corresponding logic of `<MultiSelect>`.
In this way duplicate item text won't yeild duplicate React keys.

Fixes carbon-design-system#4969.
asudoh added a commit that referenced this issue Jan 11, 2020
This change swiches React `key`s of the items of `<Dropdown>` as well
as the one of `<ComboBox>` from their text content to provided keys in
data.

This is applying the corresponding logic of `<MultiSelect>`.
In this way duplicate item text won't yeild duplicate React keys.

Fixes #4969.
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

Successfully merging a pull request may close this issue.

2 participants