Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 32 commits into from
Feb 28, 2019
Merged
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
6668d07
Add option to create new item
Jan 21, 2019
3fcd343
Properly clear input on item click
Jan 21, 2019
b42418f
Move props to queryList
Jan 21, 2019
2548a01
Lint and test
Jan 21, 2019
bccb5e2
Lint and docs
Jan 21, 2019
684919c
Properly handle keyboard events with create item
shuyangli Feb 6, 2019
ff20ae6
Merge branch 'develop' into sl/1710-create-able-multiselect
shuyangli Feb 6, 2019
6964bbb
Minor cleanup
shuyangli Feb 6, 2019
4596f78
Factor out getActiveItem
shuyangli Feb 6, 2019
01a1538
createItem => createNewItem
Feb 20, 2019
d99d570
Fix existing tests
Feb 20, 2019
6b676d5
Use real item list in filtering test
Feb 20, 2019
2aa4b54
More type-safe active item getter
Feb 20, 2019
321cd03
Add tests for interacting with create item
Feb 20, 2019
4c47f03
Add docs for creating items
Feb 20, 2019
59a86aa
Merge branch 'develop' into sl/1710-create-able-multiselect
Feb 20, 2019
f54513f
Try a non-breaking API that adds a new generic type C
cmslewis Feb 26, 2019
cde4086
Actually, just create a reserved type with a __brand, and incorporate…
cmslewis Feb 26, 2019
4ea4706
Update examples
cmslewis Feb 26, 2019
f86db48
Update tests
cmslewis Feb 26, 2019
dfbcedd
Update function comments
cmslewis Feb 26, 2019
16b9fe5
Avoid breaking the API, fix tests
cmslewis Feb 26, 2019
d1fd4fe
Update docs
cmslewis Feb 26, 2019
2925fec
Make the brand even more specific, and add a deep-comparison check
cmslewis Feb 26, 2019
3e6d62f
Fix unintentional code changes
cmslewis Feb 26, 2019
015e06f
Fix lint in unrelated files?
cmslewis Feb 27, 2019
6ecdfa0
Respond to @adidahiya CR
cmslewis Feb 27, 2019
631b86e
Fix examples: Add created item to list of items (to enable deselection)
cmslewis Feb 28, 2019
3e020ed
Hide 'Create item' option if query already appears in items
cmslewis Feb 28, 2019
f126a9d
Write tests
cmslewis Feb 28, 2019
1f7b3bb
Share created-item logic in Select and Suggest components too
cmslewis Feb 28, 2019
b9e363f
Small optimization: compare created item to filteredItems, not items
cmslewis Feb 28, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 42 additions & 8 deletions packages/select/src/components/query-list/queryList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ export interface IQueryListProps<T> extends IListItemsProps<T> {
* An object describing how to render a `QueryList`.
* A `QueryList` `renderer` receives this object as its sole argument.
*/
export interface IQueryListRendererProps<T> extends IQueryListState<T>, IProps {
export interface IQueryListRendererProps<T> // Omit `createNewItem`, because it's used strictly for internal tracking.
extends Pick<IQueryListState<T>, "activeItem" | "filteredItems" | "query">,
IProps {
/**
* Selection handler that should be invoked when a new item has been chosen,
* perhaps because the user clicked it.
Expand Down Expand Up @@ -78,6 +80,14 @@ export interface IQueryListState<T> {
/** The currently focused item (for keyboard interactions). */
activeItem: T | ICreateNewItem | null;

/**
* The item returned from `createNewItemFromQuery(this.state.query)`, cached
* to avoid continuous reinstantions within `isCreateItemRendered`, where
* this element will be used to hide the "Create Item" option if its value
* matches the current `query`.
*/
createNewItem: T | undefined;

/** The original `items` array filtered by `itemListPredicate` or `itemPredicate`. */
filteredItems: T[];

Expand Down Expand Up @@ -117,29 +127,34 @@ export class QueryList<T> extends React.Component<IQueryListProps<T>, IQueryList

public constructor(props: IQueryListProps<T>, context?: any) {
super(props, context);
const { query = "" } = this.props;
const filteredItems = getFilteredItems(query, this.props);

const { query = "" } = props;
const createNewItem = Utils.safeInvoke(props.createNewItemFromQuery, query);
const filteredItems = getFilteredItems(query, props);

this.state = {
activeItem:
this.props.activeItem !== undefined
? this.props.activeItem
: getFirstEnabledItem(filteredItems, this.props.itemDisabled),
: getFirstEnabledItem(filteredItems, props.itemDisabled),
createNewItem,
filteredItems,
query,
};
}

public render() {
const { className, items, renderer, itemListRenderer = this.renderItemList } = this.props;
const { createNewItem, ...spreadableState } = this.state;
return renderer({
...this.state,
...spreadableState,
className,
handleItemSelect: this.handleItemSelect,
handleKeyDown: this.handleKeyDown,
handleKeyUp: this.handleKeyUp,
handleQueryChange: this.handleQueryChange,
itemList: itemListRenderer({
...this.state,
...spreadableState,
items,
itemsParentRef: this.refHandlers.itemsParent,
renderItem: this.renderItem,
Expand Down Expand Up @@ -214,14 +229,18 @@ export class QueryList<T> extends React.Component<IQueryListProps<T>, IQueryList
}

public setQuery(query: string, resetActiveItem = this.props.resetOnQuery, props = this.props) {
const { createNewItemFromQuery } = props;

this.shouldCheckActiveItemInViewport = true;
const hasQueryChanged = query !== this.state.query;
if (hasQueryChanged) {
Utils.safeInvoke(props.onQueryChange, query);
}

const filteredItems = getFilteredItems(query, props);
this.setState({ filteredItems, query });
const createNewItem =
createNewItemFromQuery != null && query !== "" ? createNewItemFromQuery(query) : undefined;
this.setState({ createNewItem, filteredItems, query });

// always reset active item if it's now filtered or disabled
const activeIndex = this.getActiveIndex(filteredItems);
Expand Down Expand Up @@ -314,6 +333,8 @@ export class QueryList<T> extends React.Component<IQueryListProps<T>, IQueryList
}

private handleItemCreate = (query: string, evt?: React.SyntheticEvent<HTMLElement>) => {
// we keep a cached createNewItem in state, but might as well recompute
// the result just to be sure it's perfectly in sync with the query.
const item = Utils.safeInvoke(this.props.createNewItemFromQuery, query);
if (item != null) {
Utils.safeInvoke(this.props.onItemSelect, item, evt);
Expand Down Expand Up @@ -397,7 +418,20 @@ export class QueryList<T> extends React.Component<IQueryListProps<T>, IQueryList
}

private isCreateItemRendered(): boolean {
return !!(this.props.createNewItemFromQuery && this.props.createNewItemRenderer && this.state.query !== "");
const { createNewItemFromQuery } = this.props;
return (
createNewItemFromQuery != null &&
this.props.createNewItemRenderer != null &&
this.state.query !== "" &&
// this check is unfortunately O(N) on the number of items, but
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// alas, hiding the "Create Item" option when it exactly matches an
// existing item is much clearer.
!this.wouldCreatedItemMatchSomeExistingItem()
);
}

private wouldCreatedItemMatchSomeExistingItem() {
return this.props.items.some(item => executeItemsEqual(this.props.itemsEqual, item, this.state.createNewItem));
}
}

Expand Down