[select] itemListRenderer #2252
Conversation
menuRenderer => itemListRenderer
add filteredItems to renderer props. itemListRenderer is now the only one with access to renderItem/items/itemsParentRef. the default itemListRenderer produces a Menu using filteredItems (to preserve arrow keys order).
|
@reiv, your input on this change would be greatly appreciated! |
Merge branch 'develop' of github.com:palantir/blueprint into gg/item-list-rendererPreview: documentation | landing | table |
update docsPreview: documentation | landing | table |
| * | ||
| * The default implementation invokes `itemRenderer` for each item that passes the predicate | ||
| * and wraps them all in a `Menu` element. If the query is empty then `initialContent` is returned, | ||
| * and if all items are filtered away then `noResults` is returned. |
There was a problem hiding this comment.
Nit: "if there are no items that match the predicate" (items could be empty, for all we know.)
| * This is required for the `QueryList` to scroll the active item into view automatically. | ||
| */ | ||
| itemsParentRef: (ref: HTMLElement | null) => void; | ||
| /** Rendered elements returned from `menuRenderer` prop. */ |
There was a problem hiding this comment.
menuRenderer -> itemListRenderer
| @@ -231,6 +269,28 @@ export class QueryList<T> extends React.Component<IQueryListProps<T>, IQueryList | |||
| } | |||
| } | |||
|
|
|||
| private defaultMenuRenderer = (listProps: IItemListRendererProps<T>) => { | |||
There was a problem hiding this comment.
Would have suggested defaultItemListRenderer just for consistency, but that's a mouthful isn't it?
|
|
||
| private renderItem = (item: T, index?: number) => { | ||
| const { activeItem, query } = this.props; | ||
| const matchesPredicate = this.state.filteredItems.indexOf(item) >= 0; |
There was a problem hiding this comment.
I know this isn't new, but wouldn't it have been better in hindsight to cache the filtered items as an ES6 Set (or anything that's not O(N), really) to speed up this lookup?
There was a problem hiding this comment.
yes, definitely a pain point in my head. we should do such a refactor.
| * Helper method for rendering `props.filteredItems`, with optional support for `noResults` | ||
| * (when filtered items is empty) and `initialContent` (when query is empty). | ||
| */ | ||
| public static renderFilteredItems( |
There was a problem hiding this comment.
Can we broaden the scope of this helper? How about instead of taking IItemListRendererProps, we take items: T[]; then an itemListRenderer can call it with either filteredItems or items, depending on which is more appropriate. Also, since this is meant to be called first and foremost by an itemListRenderer, it's a bit awkward that it's defined on QueryList, which we usually don't import directly when consuming Select and friends. Can we maybe make it a top-level export of the select package?
There was a problem hiding this comment.
cool yes def down for the root export.
taking the props as a first argument was a quick way to get off the ground, cuz it receives both items and renderItem in one go. the name indicates which set of items it'll use. my goal here was to streamline the 99% case; if you want to render items then you're on your own.
There was a problem hiding this comment.
Yep, fair enough on the streamlining part.
add common/index.ts
Merge branch 'develop' of github.com:palantir/blueprint into gg/item-list-rendererPreview: documentation | landing | table |
renderFilteredItems tests filePreview: documentation | landing | table |
following up from @reiv's excellent work in #2080, this PR moves
menuRendererprop toQueryListand makes its behavior available to allselectcomponents. it also separate therendererprops fromitemListRendererprops, such thatiLRis the only one who knows aboutrenderItemand the ref handler.renderernow receives the rendereditemListelement, which simplifies its logic greatly.menuRenderer⇒itemListRendererto match other prop names (tho this prop hasn't shipped yet, so not breaking)renderFilteredItemsstatic function is generally useful for ...rendering filtered items.initialContent,noResults,itemListRendererup toQueryListprops.QueryListrendererreceives rendereditemListReactNode.filteredItemsto renderer props (per Add filteredItems to queryList #2213).itemListRendereris now the only one with access torenderItem/items/itemsParentRef. the defaultitemListRendererproduces aMenuusingfilteredItems(to preserve arrow keys order).listProps.itemList