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

[EuiComboBox] Allow any objects as options #1456

Closed
lukasolson opened this issue Jan 18, 2019 · 6 comments
Closed

[EuiComboBox] Allow any objects as options #1456

lukasolson opened this issue Jan 18, 2019 · 6 comments

Comments

@lukasolson
Copy link
Member

lukasolson commented Jan 18, 2019

EuiComboBox currently expects a list of options in the following format:

{
  color: "blue",
  label: "my label"
}

When the onChange event fires, it gives you back the selectedOptions in the same format.

As a result, any place that the EuiComboBox is used with a list of some complex object, there needs to be a wrapping/unwrapping that takes place. First, the objects (and selected objects) need to be mapped to the format above. Then, in the onChange, they need to be mapped back to the original object.

For example, say I have a list of index patterns for which I'd like to display a combo box. In order to create the options, I need to map over the list and create an object like { label: indexPattern.title }. Then, in the onChange, I need to map over the selectedOptions and retrieve the original indexPattern by matching the title to the option label.

I would propose, instead of expecting the options in this format, we could pass the options in as the original objects. Then, we'd add a getOptionLabel and getOptionColor, etc. that would be invoked to get the actual label or color to display in the component. Then, in the onChange, you could pass the selected options back in their original format. This would eliminate a lot of the boilerplate code associated with the EuiComboBox.

Thoughts?

@chandlerprall
Copy link
Contributor

The request totally makes sense, but I'd be hesitant to make that change in the existing component itself - either it's a breaking change, or we look for some specific object key's presence and switch behaviour based on that, adding quite a bit of complexity. As an easier way around, I'd recommend during the mapping of Object -> Option to create a Map() from the Option back to Object - this simplifies the lookup behaviour in onChange (and makes it faster), and possibly this could be a util in EUI itself.

@cjcenizal
Copy link
Contributor

I really like this idea! Could we make this change BWC by assuming that options is a list of objects in the existing format (and therefore use existing behavior) if getOptionLabel isn't defined? The downside is that this introduces a "secret mode" of behavior in the component, but I think with documentation we can reduce confusion and surprises.

@lukasolson
Copy link
Member Author

lukasolson commented Jan 18, 2019

@cjcenizal That's kind of what I was thinking, we could just default the getOptionLabel to option => option.label, etc. I'm sure it's more complicated than that to actually make the change in the code but we could preserve BWC that way.

@cchaos cchaos changed the title EuiComboBox: Allow any objects as options [EuiComboBox] Allow any objects as options Sep 19, 2020
@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@cchaos
Copy link
Contributor

cchaos commented Mar 19, 2021

I'd like to add this as part of the #2841 issue.

@cee-chen
Copy link
Member

cee-chen commented Apr 3, 2023

Due to the age of the request and lack of other requests, we're closing this issue as not planned by the EUI team. That being said, we'll welcome a PR if anyone feels strongly about contributing or having this functionality in EUI.

@cee-chen cee-chen closed this as not planned Won't fix, can't repro, duplicate, stale Apr 3, 2023
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

5 participants