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

Deprecate usage of JSON in favor of composability. #2456

Closed
4 tasks done
alitaheri opened this issue Dec 9, 2015 · 4 comments
Closed
4 tasks done

Deprecate usage of JSON in favor of composability. #2456

alitaheri opened this issue Dec 9, 2015 · 4 comments
Labels
umbrella For grouping multiple issues to provide a holistic view

Comments

@alitaheri
Copy link
Member

Many components (Dialog, SelectField, DropDownMenu, LeftNav) still have props like menuItems, items, actions, etc... that a a JSON object. these should either take an array (or single) component or be removed in favor of children.

TODO:

@oliviertassinari and @shaurya947 please give me your feedback on this.

@alitaheri alitaheri added the Core label Dec 9, 2015
@alitaheri alitaheri self-assigned this Dec 9, 2015
@alitaheri alitaheri added this to the 0.14.0 Release milestone Dec 9, 2015
@mbrookes
Copy link
Member

mbrookes commented Dec 9, 2015

The one concern I would have is that, particularly for components such as selectField, TypeAhead etc, the data is often generated programmatically (from database, API etc), so having to compose with children would require extra code in every app that is currently handled by the components.

The reason for requiring objects is that the items often require other attributes than just a label (some kind of reference for example).

I would propose:

  • Supporting both prop and children as alternatives
  • Standardising the API naming for that prop across components
  • Where possible, limit and standardise the attributes supported through the array / object

(In fact, standardising API naming in general across components would be a good long-term objective to add to the roadmap. )

@alitaheri
Copy link
Member Author

@mbrookes The extra code would only be a Array.prototype.map call, but the outcome would be greater customizability. What if you want (based on the object) the select items to look different? or each handle some extra interactions, like dragging? I think the trade-off is worth it. Besides there are worse issues with these. did you know if displayMember and valueMember are different that value and payload the object is mutated to contain those properies? That means you HAVE to clone your data in some way.

but isn't this prettier:

<SelectField value={...} onChange={(value, payload) => {/**/}}>
  {data.map(d => 
    <SelectItem key={d.id} value={d.id} payload={d.name /*or d itself*/} label={d.fullname} style={...} />
  )}
</SelectField>

You can even nest children inside the SelectItem, or make it editable later, etc...

If we also support JSON objects then how will they be ordered? how will they look like?

With typeahead I haven't come up with a solution yet. That might better work with templates or something. But that component is still new, it's better to leave it as is and have the community field test it and provide feed back. then we'll decide :D

@mbrookes
Copy link
Member

Okay, I"m sold. :D

@alitaheri alitaheri added the umbrella For grouping multiple issues to provide a holistic view label Dec 22, 2015
@alitaheri alitaheri modified the milestones: 0.14.0 Release, 0.14.1 Dec 22, 2015
@mbrookes
Copy link
Member

Dupe of / resolves #27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
umbrella For grouping multiple issues to provide a holistic view
Projects
None yet
Development

No branches or pull requests

3 participants