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

Dropdown: when options contain custom elements, setState in onChange leads to mismatch of 'active' and 'selected' option #2296

Open
jjjkkklll opened this issue Nov 6, 2017 · 14 comments

Comments

@jjjkkklll
Copy link

Steps

I want to setState() within onChange of the Dropdown and have custom elements within the options like so:

const options = [ { value: "1",
                    content: <span>a</span> 
                              }, ... ];

Expected Result

  1. The selected option should be active and selected

  2. The state should be set and the dropdown should change its value. The content of the option <span>a</span> should appear in the dropdown.

Actual Result

  1. The state is set as expected. The selected option is set to active, the previously selected option is kept as selected.
    => Selected and active do not match the same option anymore

  2. The content does not appear in the dropdown

If text is used instead of content

  1. The selected option is set to active, the previously selected option is kept as selected.
    => Selected and active do not match the same option anymore

  2. The content does appear in the dropdown

Questions:
What should I use, text or content for this case?
How to solve the mismatch?

Version

0.76.0

Showcase
https://codesandbox.io/s/pp0w7wrn7q

Partially solved here:
#2252

@layershifter
Copy link
Member

Valid issue, help is much appreciated.

@oskarkook
Copy link

Here's another one which is bugged: https://codesandbox.io/s/xr4xz2x9n4

In the first dropdown, select "3", then "1" and then open the second dropdown. You will see the mismatch.

@levithomason
Copy link
Member

levithomason commented Nov 16, 2017

There is a bug here, however, the examples are also using the Dropdown options incorrectly. Object keys in Dropdown options are as follows:

  • text Displayed in the Dropdown Input after the item is made active, should be a string.
  • content Displayed in the Dropdown Menu, can be any element.
  • value Used as the component value.

text

A Dropdown Item with text that is an element is not valid. A Dropdown Item without text is also not valid. All Dropdown Items should have a text string.

content

The example notes only "a" appears in the DD. 'content' is ignored. This, however, is the expected behavior. Dropdown Item content only appears in the Menu, not in the Input once made active.

key

All SUIR shorthand arrays (e.g. options) require a key in each object. React requires a constant and unique key when creating arrays of elements. This is precisely what SUIR shorthand props do, they create arrays of elements for you. None of the examples provided were using keys.


Again, there is still a bug here but here is an example showing the bug with correct Dropdown usage:

https://codesandbox.io/s/nk4zz5l780

@jjjkkklll
Copy link
Author

jjjkkklll commented Nov 19, 2017

@levithomason Thank you for the example, but now I do not understand how to get something like this:

If the text has to be a string, I can't use text for it and content won't display it...

Edit: regarding the element in the text tag. I took it from this post, which gave this example

@levithomason
Copy link
Member

@jjjkkklll This is a longer standing issue that we'd love a PR for. We do not currently support an icon in the displayed active item, see #1147.

I believe we changed the API since the links you've provided. You can still work around this by using the trigger prop and updating it on change.

https://react.semantic-ui.com/modules/dropdown#dropdown-example-trigger

@hudovisk
Copy link

Had a similar problem, fixed with trigger prop. Thanks @levithomason!

<Form.Dropdown
                    trigger={<Label color={data.color}/>}
                    label="Color"
                    name="color"
                    onChange={this.onChange}
                    value={data.color}
                    selection
                    width={2}
                    compact
                    options={COLORS.map(color => ({
                        label: { color },
                        key: color,
                        value: color,
                        text: "",
                    }))}
                />

@top1st
Copy link

top1st commented Dec 12, 2017

This problem solved when opened dropdown It select automatically other index value :( ??!!

@stale
Copy link

stale bot commented May 15, 2018

There has been no activity in this thread for 90 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 90 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label May 15, 2018
@ahashim
Copy link

ahashim commented Jul 22, 2018

Any updates on this? The Dropdown component still seem to make all choices active (and bold) when choosing a single one. You can see this currently in action on the examples in the docs: https://react.semantic-ui.com/modules/dropdown#usage-trigger-image

If you trigger the dropdown by clicking on the avatar, and then choose an option - the next time you trigger the dropdown: all the options are set to active.

Also not to nitpick, but I can see an inverted triangle for the pointer below where it's rendered as seen here: https://i.imgur.com/iqcV7gk.png (note: this only occurs before you select an active choice).

Any help would be much appreciated on this. Thank you!

@stale stale bot removed the stale label Jul 22, 2018
@hsch
Copy link

hsch commented Aug 14, 2018

I believe I've run into yet another variation of this issue (mismatch of "selected" and "active" attributes) that appears in even much simpler scenarios, i.e. not involving avatars or other non-text components in the options.

Steps

  1. Open example: https://codesandbox.io/s/2pzlnrxx40
  2. Wait for options to "load" and value to be set
  3. Open dropdown

Expected result

"Charlie" should be both active and selected.

Actual result

"Charlie" is active (correct), but "Alpha" is selected (not correct).

This is a problem, because, when you open and then again blur the dropdown (without making an intentional change to the selection), it will still switch to to the selected one.

This seems to happen after any modification of the options after the initial creation of the dropdown, e.g. initialising with non-empty (but different) options doesn't help.

Version

0.82.2

Showcase

https://codesandbox.io/s/2pzlnrxx40

@mfbieber
Copy link

mfbieber commented Aug 15, 2018

Hey @hsch, try triggering another render() by changing your componentDidMount() method, by calling this.setState() one more time to set the active value of your dropdown (as you probably know, each setState() triggers render()...for more information, read for example here: https://reactjs.org/docs/react-component.html#componentdidmount):

componentDidMount() {
    setTimeout(() => {
      this.setState({
        options: [
          { key: "A", value: "a", text: "Alpha" },
          { key: "B", value: "b", text: "Beta" },
          { key: "C", value: "c", text: "Charlie" }
        ]
      });
      this.setState({ value: "c" });
    }, 1000);
  }

Here is your adapted code example, which seems to be working for me:
https://codesandbox.io/s/6z9p0mzpmk

@theodesp
Copy link

theodesp commented Nov 7, 2018

Had a similar problem, fixed with trigger prop. Thanks @levithomason!

<Form.Dropdown
                    trigger={<Label color={data.color}/>}
                    label="Color"
                    name="color"
                    onChange={this.onChange}
                    value={data.color}
                    selection
                    width={2}
                    compact
                    options={COLORS.map(color => ({
                        label: { color },
                        key: color,
                        value: color,
                        text: "",
                    }))}
                />

This works

@matthew-williamson-he
Copy link

Hey @hsch, try triggering another render() by changing your componentDidMount() method, by calling this.setState() one more time to set the active value of your dropdown (as you probably know, each setState() triggers render()...for more information, read for example here: https://reactjs.org/docs/react-component.html#componentdidmount):

componentDidMount() {
    setTimeout(() => {
      this.setState({
        options: [
          { key: "A", value: "a", text: "Alpha" },
          { key: "B", value: "b", text: "Beta" },
          { key: "C", value: "c", text: "Charlie" }
        ]
      });
      this.setState({ value: "c" });
    }, 1000);
  }

Here is your adapted code example, which seems to be working for me:
https://codesandbox.io/s/6z9p0mzpmk

This works for me, but is this just because componentWillReceiveProps seems to only take into account one prop at a time?

if (!shallowEqual(nextProps.value, this.props.value)) {
  debug('value changed, setting', nextProps.value)
  this.setValue(nextProps.value)
  this.setSelectedIndex(nextProps.value)
}

if (
  !_.isEqual(this.getKeyAndValues(nextProps.options), this.getKeyAndValues(this.props.options))
) {
  this.setSelectedIndex(undefined, nextProps.options)
}

This should check the new value prop against the new options, and vice versa?

@stale
Copy link

stale bot commented Sep 22, 2019

There has been no activity in this thread for 180 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 180 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label Sep 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests