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

[SelectField] Composeable Items. #2380

Closed
alitaheri opened this issue Dec 5, 2015 · 8 comments
Closed

[SelectField] Composeable Items. #2380

alitaheri opened this issue Dec 5, 2015 · 8 comments
Labels
new feature New feature or request

Comments

@alitaheri
Copy link
Member

I think all combo boxes especially the SelectField component should be composeable instead of requiring an object to generate their items from.
don't you think this is better:

<SelectField selectedValue={1 /* optional */} onChange={(value, payload) => {/*...*/}}>
  <SelectItem value={1} payload={{foo:1}}/>
  <SelectItem style={...} value={2} payload={{foo:'something'}}/>
  <SelectItem value={3} payload={{foo:2}}/>

  <SelectGroup title='Barz'>
    <SelectItem value={7} payload={{bar:1}}/>
  </SelectGroup>

  <SelectItem value={8} payload={{foo:5}}/>
</SelectField>

This will provide a huge flexibility (value can be anything comparable) and payload can be anything. Besides many other customizations will also be possible.
Thoughts? @oliviertassinari @shaurya947

@alitaheri alitaheri added this to the Composable Components milestone Dec 5, 2015
@oliviertassinari
Copy link
Member

I'm wondering if this couldn't be applied to the Autocomplete too.
I think that it's a good idea. My only concern would be regarding performances.
Could we still implement an efficient shouldComponentUpdate?
I'm wondering because the children of the SelectField will never be equals between two render, but an object property can.

@alitaheri
Copy link
Member Author

I think if we use react-list we wouldn't need to worry about performance here. You can have at most 10 items in view.

@oliviertassinari
Copy link
Member

Well, react-list accept a itemRenderer() property and a lenght property.
Their API seems close to what SelectField is now and far from what we want to move to.

I'm assuming that we can find a way to make them work together with this new SelectField API.
However handling an update will be tricky, react-list need an property to rely on to know if it has to rerender or not. What are we going to use?

@alitaheri
Copy link
Member Author

Hmmmm, I guess this needs more thought and community feedback then.

@shaurya947
Copy link
Contributor

@subjectix @oliviertassinari I definitely think that this would be a better way of using SelectField.

@subjectix can you elaborate more on SelectGroup vs SelectItem?

@oliviertassinari regarding performance, is it really that bad? If we implement a shouldComponentUpdate for the outer SelectField component that compares the value, style and payload prop of each child, is that any better than what react would do under the hood?

@alitaheri
Copy link
Member Author

I definitely think that this would be a better way of using SelectField.

I'll try to work out the semantics for this.

can you elaborate more on SelectGroup vs SelectItem?

@shaurya947 That was just for show, wanted to express the capabilities it can provide. Something like categorization within the SelectField's Menu.

@alitaheri alitaheri self-assigned this Dec 7, 2015
@alitaheri alitaheri added new feature New feature or request and removed Enhancement labels Dec 8, 2015
@alitaheri alitaheri modified the milestones: 0.14.0 Release, Composable Components Dec 9, 2015
@oliviertassinari
Copy link
Member

I really like this idea. I also noticed that with the new popover teleportation feature. It's really hard to test a SelectField.

@alitaheri
Copy link
Member Author

@oliviertassinari Yeah >.> it is. that's why we have to do something about it all. I very much like the idea @newoga proposed in #2416.

@alitaheri alitaheri modified the milestones: 0.14.0 Release, 0.14.1 Dec 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants