initialContent now shows up for Omnibars; arrow keys work on initial items #2633
Conversation
don't try to render initialContent when it's nullPreview: documentation | landing | table |
|
I did notice that even with this fix, |
jkillian
changed the title
Omnibars; arrow keys work on initial items
jkillian
changed the title
Omnibars; arrow keys work on initial items
|
Pushed an update with an additional fix; is there anything else this needs @giladgray? |
make arrow keys work on initial renderPreview: documentation | landing | table |
|
why no arrow keys? |
|
When the omnibar shows its list of items when first opened (which it failed to do until the However, if you provided |
|
@jkillian i don't think this is a bug. omnibar is supposed to be empty on first open. see Alfred or Mac OS Spotlight. see this line in the docs: /**
* React content to render when query is empty.
* If omitted, all items will be rendered (or result of `itemListPredicate` with empty query).
==> * If explicit `null`, nothing will be rendered when query is empty. // <== expected behavior
*
* This prop is ignored if a custom `itemListRenderer` is supplied.
*/
initialContent?: React.ReactNode | null; |
|
@giladgray, the line above the one that you mentioned is the behavior that should happen:
Only if a user specifically says
|
|
In a lot of use cases, I believe users will want to customize |
| const handlers = isOpen && !this.isQueryEmpty() ? { onKeyDown: handleKeyDown, onKeyUp: handleKeyUp } : {}; | ||
| // handle arrow key presses once we have a query or w/ no query if we're not displaying custom initial content | ||
| const shouldHandleKeys = isOpen && (initialContent == null || !this.isQueryEmpty()); | ||
| const handlers = shouldHandleKeys ? { onKeyDown: handleKeyDown, onKeyUp: handleKeyUp } : {}; |
There was a problem hiding this comment.
i think this was fixed in #3130
| @@ -57,7 +57,7 @@ export function renderFilteredItems( | |||
| noResults?: React.ReactNode, | |||
| initialContent?: React.ReactNode | null, | |||
| ): React.ReactNode { | |||
| if (props.query.length === 0 && initialContent !== undefined) { | |||
| if (props.query.length === 0 && initialContent != null) { | |||
There was a problem hiding this comment.
this change feels wrong -- the explicit undefined check is supported by the docs.
Changes proposed in this pull request:
When
initialContentwasnull, it was still getting improperly returned byrenderFilteredItems. This led to bugs in some of the list components and omnibar where the regular list of items wouldn't be displayed when first opened.Also fixed a related bug where the arrow keys wouldn't work in an omnibar for the initial content.