-
Notifications
You must be signed in to change notification settings - Fork 386
fix(List): render children list only when required #1472
Conversation
Deploy preview for react-instantsearch ready! Built with commit d455524 https://deploy-preview-1472--react-instantsearch.netlify.com |
.map(child => this.renderItem(child, item))} | ||
</ul> | ||
); | ||
const isItemHasChildren = item.items && Boolean(item.items.length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasChildren
should be clear enough too right? This name now is pretty unclear :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. But I prefer to be explicit with something like itemHasChildren
. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
itemHasChildren also would be fine :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
8e90e7b
to
d455524
Compare
Summary
This PR is a followup of #1459. It adds tests to the
List
component (only for the changes not the all component). Now we render the children list only when theitem
contains an non empty array ofitems
. The same condition is applied on the classitem--parent
.Before
After
Note: The arrow is not present on the latter since we removed the class.