[Select] Create-able Select components (a different approach)

[cmslewis]

ℹ️ A second attempt at the work started in #3306. Requesting feedback.

Fixes #1710

• Includes tests
• Update documentation

Changes proposed in this pull request:

I forked #3306 to see if I could avoid breaking the API without bloating the props interface. I think I've found a workable approach! There's a non-zero chance that this could break someone, but the probability is effectively zero. Read on for an explanation.

API

// Changed in this PR:
// ===================

/**
 * The currently focused item for keyboard interactions, or `null` to
 * indicate that no item is active. If omitted or `undefined`, this prop will be
 * uncontrolled (managed by the component's state). Use `onActiveItemChange`
 * to listen for updates.
 */
activeItem?: T | ICreateNewItem | null;

/**
 * Invoked when user interaction should change the active item: arrow keys
 * move it up/down in the list, selecting an item makes it active, and
 * changing the query may reset it to the first item in the list if it no
 * longer matches the filter.
 *
 * If the "Create Item" option is displayed and currently active, then
 * `isCreateNewItem` will be `true` and `activeItem` will be `null`. In this
 * case, you should provide a valid "Create Item" object to the `activeItem`
 * _prop_ in order for the "Create Item" option to appear as active.
 *
 * __Note:__ You can instantiate a "Create Item" object using the
 * `getCreateNewItem()` utility exported from this package.
 */
// I added a second parameter here:
onActiveItemChange?: (activeItem: T | null, isCreateNewItem: boolean) => void;

// Unchanged from #3306:
// =====================

/**
 * If provided, allows new items to be created using the current query
 * string. This is invoked when user interaction causes a new item to be
 * created, either by pressing the `Enter` key or by clicking on the "Create
 * Item" option. It transforms a query string into an item type.
 */
createNewItemFromQuery?: (query: string) => T;

/**
 * Custom renderer to transform the current query string into a selectable
 * "Create Item" option. If this function is provided, a "Create Item"
 * option will be rendered at the end of the list of items. If this function
 * is not provided, a "Create Item" option will not be displayed.
 */
createNewItemRenderer?: (
    query: string,
    active: boolean,
    handleClick: React.MouseEventHandler<HTMLElement>,
) => JSX.Element | undefined;

Here's the shape of ICreateNewItem. It's designed to be as unlikely as possible to conflict with an actual custom type <T>:

interface ICreateNewItem {
    __blueprintCreateNewItemBrand: "blueprint-create-new-item";
}

Technically, if a custom type T === ICreateNewItem, there could be a type conflict, but that seems really, really unlikely. The only nuance is around controlling the active item when a "Create Item" option is present; see my docs changes for more information on that.

Overall, note that no existing tests had to change to support this. 👍

Old description (still valid)

• Select components, via IListItemsProps, now takes two additional props:
  • createNewItemFromQuery?: (query: string) => T which converts a query to a new item; if this prop is present, the select component is considered "create-able";
  • createNewItemRenderer?: (query: string, handleClick: React.MouseEventHandler<HTMLElement>) => JSX.Element | undefined which could be used to render a custom MenuItem at the end of the list, that represents "create a new element from query".
• If these props are provided, when the component has a non-empty input, a "create new item" entry is rendered at the end of the filtered item list using the given createNewItemRenderer, and the user can activate it to create a new item with the current query.
• If the component is create-able, when a query returns no results, pressing "enter" key will create a new item with the current query.

[cmslewis]

Hmm, lint was failing in an unrelated file, resources.tsx. Went ahead and fixed it. 🤷‍♂️

[blueprint-bot]

Fix lint in unrelated files?

Previews: documentation | landing | table

[cmslewis]

📚 Here's a direct link to the updated docs from the most recent build.

[adidahiya]

the popover opens again briefly after creating an item, is there anything we can do about that?

[adidahiya]

didn't look through all the new code but this approach looks reasonable

[adidahiya]

I think you should use the actual type here, ICreateNewItem, instead of "Create Item"

[adidahiya]

same here

[adidahiya]

is (activeItem: T | null) => void (the old typedef) assignable to this new type? if not, it's a break, and we can mitigate that by making the second param optional.

[cmslewis]

@adidahiya Yes, it's assignable.

You could assign an incorrect second-parameter type, and TS would complain:

But simply omitting the parameter is acceptable:

[cmslewis]

Another example with component usage. Works great. 👍

[cmslewis]

(Force pushed the last commit to fix your GHE handle in the commit message.)

[blueprint-bot]

Respond to @adidahiya CR

Previews: documentation | landing | table

[cmslewis]

@adidahiya Looks like the menu flashing is actually not a new thing; it already happens whenever the query is not empty:

Haven't determined exactly why yet, but it's related to this block, which invokes setState again after it's already invoked a few lines above:

blueprint/packages/select/src/components/query-list/queryList.tsx

Lines 231 to 233 in f7d08a9

	if (hasQueryChanged && shouldUpdateActiveItem) {
	this.setActiveItem(getFirstEnabledItem(filteredItems, props.itemDisabled));
	}

TL;DR: This PR shouldn't block on the flickering.

[adidahiya]

code looks good, but there's a small UX problem in the docs... you need to update the list of items after one has been created. see here where I've "created" Moonlight but it doesn't show up in the list when I search for it again:

[blueprint-bot]

Write tests

Previews: documentation | landing | table

[cmslewis]

@adidahiya I fixed two issues that we discussed offline:

1. Fix the <MultiSelect> example. Keep track of which items have already been created, and render created items as selected items in the list.

2. Hide the "Create item" option if the query exactly matches an item. In tandem with the fix above, this prevents two previous annoyances: (1) you can't redundantly create a new item that would exactly match an existing one, and likewise (2) if you render a created item in the list, the "Create item" option won't appear when you input that value again (it'll just show the already created item).

[adidahiya]

Your latest commits look good but unfortunately with the way we've set up the examples we'd have to replicate this new logic in all the places where we allow creating new items... which includes the Select, Omnibar, and Suggest examples.

Here's Select now:

maybe we should remove "Create new item" from those examples?

[adidahiya]

if this ends up being a problem, we can debounce it ourselves or offer some API which lets users debounce the function call, in a future PR

[shuyangli]

This reconciliation logic now falls on the users—is it possible to keep this inside the multiselect component? i.e. wrap the passed-in onItemSelect and move the logic for maintaining the union of {passed-in items, newly-created items}

[cmslewis]

@shuyangli It's a good thought, but it seems like the component should remain as data-agnostic as possible. Tracking created items would violate that. @adidahiya what do you think?

[cmslewis]

In particular, a major blocker for doing that is that if a user passes in a totally different set of items, wiping away the ones from before, then our components won't have any way to know if they should clear their internally tracked list of createdItems (which would be stored in this.state, presumably). The mutually dependent props.items and state.createdItems could easily get out of sync and cause more problems than they solve.

[adidahiya]

yeah I agree with @cmslewis here, I would prefer to do less data handling in the component and keep it more presentation-focused. I know it's pretty verbose for consumers this way, but we can always add additional conveniences on top of this approach later on after we see concrete use cases of this feature

[cmslewis]

@adidahiya I went ahead and kept the create-item functionality in all the examples, because it's helpful to play with it in different contexts. I added some new functions in films.tsx to handle the updating of the items and createdItems arrays; all examples are now using these utilities and behaving properly. (Note that the Omnibar example doesn't track the selected item anywhere in the component, so I didn't update that example.)

I think we're good to merge here.

Select example

Suggest example

Multiselect example

[blueprint-bot]

Small optimization: compare created item to filteredItems, not items

Previews: documentation | landing | table