Blueprint: Question: [Omnibar]: How do I get the keyboard controls to work when initialContent is displayed?

Created on 16 Nov 2018  ·  10Comments  ·  Source: palantir/blueprint

Environment

  • __Package version(s)__: @blueprintjs/core - v3.7.0 & @blueprintjs/select v3.2.0
  • __Browser and OS versions__: Chrome v70 / macOS Sierra

Question

I'm using Omnibar in https://pasteapp.com.

My setup is as follows;

  1. I have a React Component Foo
  2. The component Foo is stateful
  3. The state is composed of items and filtered items
  4. The component renders Omnibar like this;
<Omnibar
  isOpen={this.props.isOpen}
  itemRenderer={this.renderItem}
  onQueryChange={this.handleQueryChange}
  items={this.state.filteredItems}
  onItemSelect={this.props.onItemSelect}
  onClose={this.props.onClose}
  inputProps={{
    leftIcon: null
  }}
/>

It works just fine, I can use keyboard controls (arrow keys) to go up/down in the results, however, as soon as I introduce initialContent like this;

<Omnibar
  ...
  initialContent={this.state.items.slice(0, 10).map(this.renderItem)}
/>

...the items are shown, but I can't use the keyboard controls anymore. No exceptions were thrown, or errors were logged.

What am I doing wrong here?

select enhancement help wanted

All 10 comments

@manpreetssethi i think this is a bug. i'll take a look now.

ok so i think one part of this was fixed in #3130 which i am about to release.

@manpreetssethi but the other part about rendering items into initialContent definitely won't work properly because you don't have access to itemProps when rendering items yourself, so you can't properly attach handleClick, etc.

@giladgray Indeed, i've tried a bunch of things, for instance, passing items by default so it shows up, however, that didn't work out either. Would you suggest an alternative?

My goal is to give users a few items which were recently edited, so they don't have to search and just select one from the set.

@manpreetssethi this is simply impossible right now. we'll need to add initialItems (or allow initialContent to take a T[]) and then run those items through the internal item renderer logic. happy to review if you push up the code!

@giladgray I'll see what I can come up with, hey, thanks for the quick response. 🙏

@giladgray I read the code and tried dry running it, after the fix was released, I was able to solve my issue by simply providing a custom itemListRenderer. Here's how it looks like now;

<Omnibar
  ...
  itemRenderer={this.renderItem}
  itemListRenderer={this.renderItemList}
/>

...and the method's definition being;

renderItemList(props) {
  return <Menu ulRef={props.itemsParentRef}>
    {props.items.map(props.renderItem)}
  </Menu>
}

However here's where it gets interesting, I added the following test in the file https://github.com/palantir/blueprint/blob/develop/packages/select/test/queryListTests.tsx

describe("rendering", () => {
    it("items are rendered by default if there is no query", () => {
        const itemRenderer = sinon.spy(() => <li className="foo">foo</li>);
        const wrapper = shallow(
            <FilmQueryList {...testProps} itemRenderer={itemRenderer} />
        );
        assert.lengthOf(wrapper.find("li.foo"), 20, "should find elements");
        assert.equal(itemRenderer.callCount, 20);
    });
});
  • In the method renderItemList, renderFilteredItems is invoked, like this
  • The test above passes just fine because both query and initialContent are falsy. (Referring to the logic here) and thus the items are rendered.
  • Where Omnibar fails to render the default items, as per my understanding so far, is because of initialContent is null by default here

What do you think of this PR?

I ran into the same issue and just used @manpreetssethi's itemListRenderer workaround.

private renderItemList = (props: any) => {
    if (props.filteredItems.length === 0) {
        return <Menu ulRef={props.itemsParentRef}>
            <MenuItem disabled={true} text="No results." />
        </Menu>
    }
    return <Menu ulRef={props.itemsParentRef}>
        {props.items.map(props.renderItem)}
    </Menu>
}

@giladgray — What if we added something like a displayAllItemsInitially: boolean prop (that defaults to false) as to not conflict with the existing API for QueryList?

Is there any plan to enable omnibar to show all items when query is empty? Looks like the pull request was closed, but the bug is still present.

What about the displayAllItemsInitially prop proposed above?

I call this a bug, because it is explicitly stated in documentation that omitting initialContent should display all items in omnibar, but initialContent is set to null when ommited, which causes no items to be shown.

Bumping this since I came across this issue when trying to implement a two-step behaviour for the Omnibar where you select an item and are able to select a "sub-item" associated with it similar to some VSCode commands in the command palette. I could find a nice solution to initially render all items, even with the predicate returning true during my second step.

I was able to get tab navigation to work in the Omnibar by hardcoding the tabindex:

const itemRenderer: ItemRenderer<any> = ({
  id,
  label
}) => {
  return (
    <MenuItem
      key={id}
      tabIndex={
        // hack to make tab navigation work
        0
      }
      label={label}
    />
  );
};
Was this page helpful?
0 / 5 - 0 ratings

Related issues

vinz243 picture vinz243  ·  3Comments

havesomeleeway picture havesomeleeway  ·  3Comments

adidahiya picture adidahiya  ·  3Comments

havesomeleeway picture havesomeleeway  ·  3Comments

shahzeb1 picture shahzeb1  ·  3Comments