Conversation
… of Artworks paginated, and grouped by a category of his choice, currently supporting movieId and languageCode. Clicking in one of the Artworks in the list will allow the user to see the Artwork details containing: Full size image. Movie Id. Movie Name. Image Type. Language Code.
vinodkl
left a comment
vinodkl
left a comment
There was a problem hiding this comment.
Great work. couple of comments added. Thanks.
| const groupedArtworks = { ...state }; | ||
| switch (action.type) { | ||
| case types.LOAD_ARTWORKS_SUCCESS: | ||
| groupByArray.forEach((group) => { |
There was a problem hiding this comment.
on loading the data set from server, any reason why do we need to do this bucketing for all the groups at once? what if the there are 100's of grouping points? what if the user is interested in only one group?
There was a problem hiding this comment.
I really liked the data structure, but I agree with you that this could be a 2 step transformation.
1 - Just transform for the default group option.
2 - Lazely do it when the user select a new one.
This way we would not do any transformation work without needing it.
| * @return list of ArtworkListItem separated by Subheader (sections). | ||
| **/ | ||
| function generateList(groupedArtworks, groupBy, size, onExpand) { | ||
| const itemList = []; |
There was a problem hiding this comment.
do you think the current setup would help in handling 1000's of artworks on the page scroll? what would you do differently to handle the scroll performance.
There was a problem hiding this comment.
The current setup is paginated, but the user can choose to show all Items by pressing the "Show All" button, that would render all 1000's of Artworks. A way of solving it is using something like react-virtualized (https://github.com/bvaughn/react-virtualized) that would just render what is being shown. If using the library is not possible, this could be implemented by listening to the scroll events and rendering new items when needed.
|
Thanks for taking the time to review the code. |
2 participants
Hi @vinodkl, please review this PR and give me your thoughts.
Description:
This PR implements Netflix Artworks App allows the user to see a list of Artworks paginated, and grouped by a category of his choice, currently supporting movieId and languageCode.
Clicking in one of the Artworks in the list will allow the user to see the Artwork details containing:
Full size image.
Movie Id.
Movie Name.
Image Type.
Language Code.