Revert List component to old API #62
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DrewML
reviewed
Jun 20, 2018
|
|
||
| if (prop != null && !isIterable(prop)) { | ||
| return new Error( | ||
| `Invalid prop \`${propName}\` supplied to \`${componentName}\`. Validation failed.` |
There was a problem hiding this comment.
Can we make this message more specific? Doesn't seem to call out that we're expecting an iterable
|
Thanks for doing this! Good that it now works seamlessly with our payloads, but can be useable by others with differently-shaped data. |
DrewML
previously approved these changes
Jun 21, 2018
jimbo
changed the title
jimbo
force-pushed
the
jimbo/list-api-change
branch
from
31b5af6 to
b8481a5
Compare
|
No problem, it's just a different approach to the same solution. Fixes #41. |
Instead of accepting a Map-compatible iterable for items, accept an Array-compatible iterable and a keymaking function `getItemKey`.
jimbo
force-pushed
the
jimbo/list-api-change
branch
from
b8481a5 to
6e040ba
Compare
Pull Request Test Coverage Report for Build 132
💛 - Coveralls |
DrewML
approved these changes
Jun 21, 2018
This was referenced Jun 21, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a:
[ ] New feature
[ ] Enhancement/Optimization
[x] Refactor
[ ] Bugfix
[ ] Test for existing code
[ ] Documentation
Summary
When this pull request is merged, it will...
Change the
Listcomponent's API, such that it only expectsitemsto be an iterable, instead of a stricter,Map-compatible, two-dimensional iterable. This change will make it more expedient to pass collections of data straight from the API toListwithout having to transform them first.Additional information
Since we can't safely mandate that all collections consist of objects with an
idproperty,Listmust also accept a key-generation function,getItemKey. This function will default to({ id }) => id, which plucks theidprop from a plain object, covering the most common case. For flat collections of strings, such as tags, users can supply an identity function; for more unusual collections where the primary key is something other thanid, they can supply a custom function.This PR is marked [WIP] until this API change is approved. Once approved, instances of
Listin this repo have to be updated.