From 28c82073b9412b03b13a9aae3f9e9bae89456924 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Mon, 18 Mar 2019 10:13:31 -0700 Subject: [PATCH 01/12] Add TagInputAddMethod --- .../src/components/tag-input/tagInput.tsx | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/core/src/components/tag-input/tagInput.tsx b/packages/core/src/components/tag-input/tagInput.tsx index 6bba2a64a2..1ad0c8b1c3 100644 --- a/packages/core/src/components/tag-input/tagInput.tsx +++ b/packages/core/src/components/tag-input/tagInput.tsx @@ -15,6 +15,26 @@ import * as Utils from "../../common/utils"; import { Icon, IconName } from "../icon/icon"; import { ITagProps, Tag } from "../tag/tag"; +/** + * An enumeration of methods in which a `TagInput` value might have been added. + */ +export enum TagInputAddMethod { + /** Indicates that a value was added in the default fasion, via manual selection. */ + DEFAULT = "default", + + /** + * Indicates that a value was added when the `TagInput` lost focus. This is + * only possible when `addOnBlur=true`. + */ + BLUR = "blur", + + /** + * Indicates that a value was added via paste. This is only possible when + * `addOnPaste=true`. + */ + PASTE = "paste", +} + export interface ITagInputProps extends IIntentProps, IProps { /** * If true, `onAdd` will be invoked when the input loses focus. @@ -74,7 +94,7 @@ export interface ITagInputProps extends IIntentProps, IProps { * returns `false`. This is useful if the provided `value` is somehow invalid and should * not be added as a tag. */ - onAdd?: (values: string[]) => boolean | void; + onAdd?: (values: string[], method: TagInputAddMethod) => boolean | void; /** * Callback invoked when new tags are added or removed. Receives the updated list of `values`: @@ -260,10 +280,10 @@ export class TagInput extends AbstractPureComponent { + private addTags = (value: string, method: TagInputAddMethod = TagInputAddMethod.DEFAULT) => { const { inputValue, onAdd, onChange, values } = this.props; const newValues = this.getValues(value); - let shouldClearInput = Utils.safeInvoke(onAdd, newValues) !== false && inputValue === undefined; + let shouldClearInput = Utils.safeInvoke(onAdd, newValues, method) !== false && inputValue === undefined; // avoid a potentially expensive computation if this prop is omitted if (Utils.isFunction(onChange)) { shouldClearInput = onChange([...values, ...newValues]) !== false && shouldClearInput; @@ -342,7 +362,7 @@ export class TagInput extends AbstractPureComponent 0) { - this.addTags(this.state.inputValue); + this.addTags(this.state.inputValue, TagInputAddMethod.BLUR); } this.setState({ activeIndex: NONE, isInputFocused: false }); } @@ -367,7 +387,7 @@ export class TagInput extends AbstractPureComponent 0) { - this.addTags(value); + this.addTags(value, TagInputAddMethod.DEFAULT); } else if (selectionEnd === 0 && this.props.values.length > 0) { // cursor at beginning of input allows interaction with tags. // use selectionEnd to verify cursor position and no text selection. @@ -405,7 +425,7 @@ export class TagInput extends AbstractPureComponent) => { From 09874bbb03e74e906b9f0dbde21f55ccc437b507 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Mon, 18 Mar 2019 16:21:43 -0700 Subject: [PATCH 02/12] Implement pasting in Multiselect --- .../src/examples/select-examples/films.tsx | 4 ++ .../select-examples/multiSelectExample.tsx | 36 +++++++--- packages/select/src/common/listItemsProps.ts | 12 ++++ .../src/components/query-list/queryList.tsx | 72 ++++++++++++++++--- .../src/components/select/multiSelect.tsx | 15 +++- 5 files changed, 121 insertions(+), 18 deletions(-) diff --git a/packages/docs-app/src/examples/select-examples/films.tsx b/packages/docs-app/src/examples/select-examples/films.tsx index 04723365f2..00ab12aa35 100644 --- a/packages/docs-app/src/examples/select-examples/films.tsx +++ b/packages/docs-app/src/examples/select-examples/films.tsx @@ -210,6 +210,10 @@ export function areFilmsEqual(filmA: IFilm, filmB: IFilm) { return filmA.title.toLowerCase() === filmB.title.toLowerCase(); } +export function doesFilmEqualQuery(film: IFilm, query: string) { + return film.title.toLowerCase() === query.toLowerCase(); +} + export function arrayContainsFilm(films: IFilm[], filmToFind: IFilm): boolean { return films.some((film: IFilm) => film.title === filmToFind.title); } diff --git a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx index 8f138d3cb7..cec93ac960 100644 --- a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx +++ b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx @@ -13,6 +13,7 @@ import { areFilmsEqual, arrayContainsFilm, createFilm, + doesFilmEqualQuery, filmSelectProps, IFilm, maybeAddCreatedFilmToArrays, @@ -88,11 +89,13 @@ export class MultiSelectExample extends React.PureComponent} onItemSelect={this.handleFilmSelect} + onItemsPaste={this.handleFilmsPaste} popoverProps={{ minimal: popoverMinimal }} tagRenderer={this.renderTag} tagInputProps={{ tagProps: getTagProps, onRemove: this.handleTagRemove, rightElement: clearButton }} @@ -180,20 +183,29 @@ export class MultiSelectExample extends React.PureComponent { + const results = maybeAddCreatedFilmToArrays(nextItems, nextCreatedItems, film); + nextItems = results.items; + nextCreatedItems = results.createdItems; // Avoid re-creating an item that is already selected (the "Create // Item" option will be shown even if it matches an already selected // item). - films: !arrayContainsFilm(films, film) ? [...films, film] : films, + nextFilms = !arrayContainsFilm(nextFilms, film) ? [...nextFilms, film] : nextFilms; + }); + + this.setState({ + createdItems: nextCreatedItems, + films: nextFilms, items: nextItems, }); } @@ -224,6 +236,12 @@ export class MultiSelectExample extends React.PureComponent { + // On paste, don't bother with deselecting already selected values, just + // add the new ones. + this.selectFilms(films); + }; + private handleSwitchChange(prop: keyof IMultiSelectExampleState) { return (event: React.FormEvent) => { const checked = event.currentTarget.checked; diff --git a/packages/select/src/common/listItemsProps.ts b/packages/select/src/common/listItemsProps.ts index 555ebe7580..722d74c38e 100644 --- a/packages/select/src/common/listItemsProps.ts +++ b/packages/select/src/common/listItemsProps.ts @@ -34,6 +34,13 @@ export interface IListItemsProps extends IProps { /** Array of items in the list. */ items: T[]; + /** + * Whether the provided item fully matches the provided value. This is used + * to match pasted values to existing items, so that items can be emitted + * via `onItemsPaste`. + */ + itemEqualsQuery?: (item: T, query: string) => boolean; + /** * Specifies how to test if two items are equal. By default, simple strict * equality (`===`) is used to compare two items. @@ -130,6 +137,11 @@ export interface IListItemsProps extends IProps { */ onItemSelect: (item: T, event?: React.SyntheticEvent) => void; + /** + * Callback invoked when multiple items are selected at once via pasting. + */ + onItemsPaste?: (items: T[]) => void; + /** * Callback invoked when the query string changes. */ diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index cf4397b615..9fb8bf28c0 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -54,6 +54,24 @@ export interface IQueryListRendererProps // Omit `createNewItem`, because it */ handleItemSelect: (item: T, event?: React.SyntheticEvent) => void; + /** + * Handler that should be invoked when the user pastes one or more values. + * + * This callback will use `itemEqualsQuery` to find a subset of `items` + * exactly matching the pasted `values` provided, then it will invoke + * `onItemsPaste` with those found items. Each pasted value that does not + * exactly match an item will be ignored. + * + * If creating items is enabled (by providing both `createNewItemFromQuery` + * and `createNewItemRenderer`), then pasted values that do not exactly + * match an existing item will emit a new item as created via + * `createNewItemFromQuery`. + * + * If `itemEqualsQuery` returns multiple matching items for a particular + * `value`, then only the first matching item will be emitted. + */ + handlePaste: (values: string[]) => void; + /** * Keyboard handler for up/down arrow keys to shift the active item. * Attach this handler to any element that should support this interaction. @@ -152,6 +170,7 @@ export class QueryList extends React.Component, IQueryList handleItemSelect: this.handleItemSelect, handleKeyDown: this.handleKeyDown, handleKeyUp: this.handleKeyUp, + handlePaste: this.handlePaste, handleQueryChange: this.handleQueryChange, itemList: itemListRenderer({ ...spreadableState, @@ -350,6 +369,43 @@ export class QueryList extends React.Component, IQueryList } }; + private handlePaste = (values: string[]) => { + const { createNewItemFromQuery, items, itemEqualsQuery, onItemsPaste } = this.props; + + let nextActiveItem: T | undefined; + + // Find an exising item that exactly matches each pasted value, or + // create a new item if possible. Ignore unmatched values if creating + // items is disabled. + const pastedItemsToEmit = values.reduce((agg, value) => { + // Use .find() instead of .filter() so the operation short-circuits + // as soon as it finds an equal value. + const equalItem = + itemEqualsQuery === undefined ? undefined : items.find(item => itemEqualsQuery(item, value)); + + if (equalItem !== undefined) { + nextActiveItem = equalItem; + agg.push(equalItem); + } else if (this.canCreateItems()) { + const newItem = Utils.safeInvoke(createNewItemFromQuery, value); + if (newItem !== undefined) { + agg.push(newItem); + } + } + + return agg; + }, []); + + // UX nicety: update the active item if we matched with at least one + // existing item. + if (nextActiveItem !== undefined) { + this.setActiveItem(nextActiveItem); + } + + // No need to update the active item. + Utils.safeInvoke(onItemsPaste, pastedItemsToEmit); + }; + private handleKeyDown = (event: React.KeyboardEvent) => { const { keyCode } = event; if (keyCode === Keys.ARROW_UP || keyCode === Keys.ARROW_DOWN) { @@ -418,24 +474,24 @@ export class QueryList extends React.Component, IQueryList } private isCreateItemRendered(): boolean { - const { createNewItemFromQuery } = this.props; return ( - createNewItemFromQuery != null && - this.props.createNewItemRenderer != null && + this.canCreateItems() && this.state.query !== "" && // this check is unfortunately O(N) on the number of items, but // alas, hiding the "Create Item" option when it exactly matches an // existing item is much clearer. - !this.wouldCreatedItemMatchSomeExistingItem() + !this.doesItemMatchSomeExistingItem(this.state.createNewItem) ); } - private wouldCreatedItemMatchSomeExistingItem() { + private canCreateItems(): boolean { + return this.props.createNewItemFromQuery != null && this.props.createNewItemRenderer != null; + } + + private doesItemMatchSomeExistingItem(item: T | undefined, validItems: T[] = this.state.filteredItems) { // search only the filtered items, not the full items list, because we // only need to check items that match the current query. - return this.state.filteredItems.some(item => - executeItemsEqual(this.props.itemsEqual, item, this.state.createNewItem), - ); + return validItems.some(filteredItem => executeItemsEqual(this.props.itemsEqual, filteredItem, item)); } } diff --git a/packages/select/src/components/select/multiSelect.tsx b/packages/select/src/components/select/multiSelect.tsx index c04ed6eb12..a398ffbe7f 100644 --- a/packages/select/src/components/select/multiSelect.tsx +++ b/packages/select/src/components/select/multiSelect.tsx @@ -14,6 +14,7 @@ import { Popover, Position, TagInput, + TagInputAddMethod, Utils, } from "@blueprintjs/core"; import { Classes, IListItemsProps } from "../../common"; @@ -83,6 +84,7 @@ export class MultiSelect extends React.PureComponent, IM extends React.PureComponent, IM private renderQueryList = (listProps: IQueryListRendererProps) => { const { tagInputProps = {}, popoverProps = {}, selectedItems = [], placeholder } = this.props; - const { handleKeyDown, handleKeyUp } = listProps; + const { handlePaste, handleKeyDown, handleKeyUp } = listProps; + + const handleTagInputAdd = (values: any[], method: TagInputAddMethod) => { + if (method === TagInputAddMethod.PASTE) { + handlePaste(values); + } + }; return ( extends React.PureComponent, IM className={classNames(Classes.MULTISELECT, tagInputProps.className)} inputRef={this.refHandlers.input} inputValue={listProps.query} + onAdd={handleTagInputAdd} onInputChange={listProps.handleQueryChange} values={selectedItems.map(this.props.tagRenderer)} /> @@ -135,6 +144,10 @@ export class MultiSelect extends React.PureComponent, IM Utils.safeInvoke(this.props.onItemSelect, item, evt); }; + private handleItemsPaste = (items: T[]) => { + Utils.safeInvoke(this.props.onItemsPaste, items); + }; + private handleQueryChange = (query: string, evt?: React.ChangeEvent) => { this.setState({ isOpen: query.length > 0 || !this.props.openOnKeyDown }); Utils.safeInvoke(this.props.onQueryChange, query, evt); From 0fe9cb505ad6e4598483d81b65f92ba04ce6f992 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Mon, 18 Mar 2019 17:11:39 -0700 Subject: [PATCH 03/12] Undo unnecessary code change --- packages/select/src/components/query-list/queryList.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index 9fb8bf28c0..8fd6e391a8 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -480,7 +480,7 @@ export class QueryList extends React.Component, IQueryList // this check is unfortunately O(N) on the number of items, but // alas, hiding the "Create Item" option when it exactly matches an // existing item is much clearer. - !this.doesItemMatchSomeExistingItem(this.state.createNewItem) + !this.wouldCreatedItemMatchSomeExistingItem(this.state.createNewItem) ); } @@ -488,10 +488,12 @@ export class QueryList extends React.Component, IQueryList return this.props.createNewItemFromQuery != null && this.props.createNewItemRenderer != null; } - private doesItemMatchSomeExistingItem(item: T | undefined, validItems: T[] = this.state.filteredItems) { + private wouldCreatedItemMatchSomeExistingItem(item: T | undefined) { // search only the filtered items, not the full items list, because we // only need to check items that match the current query. - return validItems.some(filteredItem => executeItemsEqual(this.props.itemsEqual, filteredItem, item)); + return this.state.filteredItems.some(filteredItem => + executeItemsEqual(this.props.itemsEqual, filteredItem, item), + ); } } From b92350dad97fb0d2d58f346803947f19d0edf161 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Mon, 18 Mar 2019 17:16:27 -0700 Subject: [PATCH 04/12] Undo unnecessary code change --- packages/select/src/components/query-list/queryList.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index 8fd6e391a8..c1431b0f06 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -480,7 +480,7 @@ export class QueryList extends React.Component, IQueryList // this check is unfortunately O(N) on the number of items, but // alas, hiding the "Create Item" option when it exactly matches an // existing item is much clearer. - !this.wouldCreatedItemMatchSomeExistingItem(this.state.createNewItem) + !this.wouldCreatedItemMatchSomeExistingItem() ); } @@ -488,11 +488,11 @@ export class QueryList extends React.Component, IQueryList return this.props.createNewItemFromQuery != null && this.props.createNewItemRenderer != null; } - private wouldCreatedItemMatchSomeExistingItem(item: T | undefined) { + private wouldCreatedItemMatchSomeExistingItem() { // search only the filtered items, not the full items list, because we // only need to check items that match the current query. - return this.state.filteredItems.some(filteredItem => - executeItemsEqual(this.props.itemsEqual, filteredItem, item), + return this.state.filteredItems.some(item => + executeItemsEqual(this.props.itemsEqual, item, this.state.createNewItem), ); } } From 57a40ece1b0b0795a900c222bcab1da6da678202 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Mon, 18 Mar 2019 17:26:30 -0700 Subject: [PATCH 05/12] Fix compile errors (don't use .find()) --- .../src/components/query-list/queryList.tsx | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index c1431b0f06..089a151aee 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -370,7 +370,7 @@ export class QueryList extends React.Component, IQueryList }; private handlePaste = (values: string[]) => { - const { createNewItemFromQuery, items, itemEqualsQuery, onItemsPaste } = this.props; + const { createNewItemFromQuery, onItemsPaste } = this.props; let nextActiveItem: T | undefined; @@ -378,10 +378,7 @@ export class QueryList extends React.Component, IQueryList // create a new item if possible. Ignore unmatched values if creating // items is disabled. const pastedItemsToEmit = values.reduce((agg, value) => { - // Use .find() instead of .filter() so the operation short-circuits - // as soon as it finds an equal value. - const equalItem = - itemEqualsQuery === undefined ? undefined : items.find(item => itemEqualsQuery(item, value)); + const equalItem = getMatchingItem(value, this.props); if (equalItem !== undefined) { nextActiveItem = equalItem; @@ -501,6 +498,19 @@ function pxToNumber(value: string | null) { return value == null ? 0 : parseInt(value.slice(0, -2), 10); } +function getMatchingItem(query: string, { items, itemEqualsQuery }: IQueryListProps): T | undefined { + if (Utils.isFunction(itemEqualsQuery)) { + // .find() doesn't exist in ES5. Alternative: use a for loop instead of + // .filter() so that we can return as soon as we find the first match. + for (const item of items) { + if (itemEqualsQuery(item, query)) { + return item; + } + } + } + return undefined; +} + function getFilteredItems(query: string, { items, itemPredicate, itemListPredicate }: IQueryListProps) { if (Utils.isFunction(itemListPredicate)) { // note that implementations can reorder the items here From 9fb0d71b9543037c153e83c03063484dc1313ec4 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Tue, 19 Mar 2019 12:14:52 -0700 Subject: [PATCH 06/12] Remove enum, use union type instead --- .../src/components/tag-input/tagInput.tsx | 33 +++++++------------ .../src/components/select/multiSelect.tsx | 2 +- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/packages/core/src/components/tag-input/tagInput.tsx b/packages/core/src/components/tag-input/tagInput.tsx index 1ad0c8b1c3..d61602e221 100644 --- a/packages/core/src/components/tag-input/tagInput.tsx +++ b/packages/core/src/components/tag-input/tagInput.tsx @@ -16,24 +16,15 @@ import { Icon, IconName } from "../icon/icon"; import { ITagProps, Tag } from "../tag/tag"; /** - * An enumeration of methods in which a `TagInput` value might have been added. + * A type reflecting the manner in which a `TagInput` value was added. + * - `"default"` - indicates that a value was added in the default fasion, via + * manual selection. + * - `"blur"` - indicates that a value was added when the `TagInput` lost focus. + * This is only possible when `addOnBlur=true`. + * - `"paste"` - indicates that a value was added via paste. This is only + * possible when `addOnPaste=true`. */ -export enum TagInputAddMethod { - /** Indicates that a value was added in the default fasion, via manual selection. */ - DEFAULT = "default", - - /** - * Indicates that a value was added when the `TagInput` lost focus. This is - * only possible when `addOnBlur=true`. - */ - BLUR = "blur", - - /** - * Indicates that a value was added via paste. This is only possible when - * `addOnPaste=true`. - */ - PASTE = "paste", -} +export type TagInputAddMethod = "default" | "blur" | "paste"; export interface ITagInputProps extends IIntentProps, IProps { /** @@ -280,7 +271,7 @@ export class TagInput extends AbstractPureComponent { + private addTags = (value: string, method: TagInputAddMethod = "default") => { const { inputValue, onAdd, onChange, values } = this.props; const newValues = this.getValues(value); let shouldClearInput = Utils.safeInvoke(onAdd, newValues, method) !== false && inputValue === undefined; @@ -362,7 +353,7 @@ export class TagInput extends AbstractPureComponent 0) { - this.addTags(this.state.inputValue, TagInputAddMethod.BLUR); + this.addTags(this.state.inputValue, "blur"); } this.setState({ activeIndex: NONE, isInputFocused: false }); } @@ -387,7 +378,7 @@ export class TagInput extends AbstractPureComponent 0) { - this.addTags(value, TagInputAddMethod.DEFAULT); + this.addTags(value, "default"); } else if (selectionEnd === 0 && this.props.values.length > 0) { // cursor at beginning of input allows interaction with tags. // use selectionEnd to verify cursor position and no text selection. @@ -425,7 +416,7 @@ export class TagInput extends AbstractPureComponent) => { diff --git a/packages/select/src/components/select/multiSelect.tsx b/packages/select/src/components/select/multiSelect.tsx index a398ffbe7f..08a37398db 100644 --- a/packages/select/src/components/select/multiSelect.tsx +++ b/packages/select/src/components/select/multiSelect.tsx @@ -97,7 +97,7 @@ export class MultiSelect extends React.PureComponent, IM const { handlePaste, handleKeyDown, handleKeyUp } = listProps; const handleTagInputAdd = (values: any[], method: TagInputAddMethod) => { - if (method === TagInputAddMethod.PASTE) { + if (method === "paste") { handlePaste(values); } }; From 079c903c16758e6fed800bd98c158ae23554c4d7 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 22 Mar 2019 08:57:47 -0700 Subject: [PATCH 07/12] Try folding itemEqualsQuery into itemPredicate --- .../src/examples/select-examples/films.tsx | 11 ++++++-- .../select-examples/multiSelectExample.tsx | 1 - packages/select/src/common/listItemsProps.ts | 25 +++++++++++-------- packages/select/src/common/predicate.ts | 20 +++++++++++---- .../src/components/query-list/queryList.tsx | 19 +++++++------- .../src/components/select/multiSelect.tsx | 5 ---- 6 files changed, 48 insertions(+), 33 deletions(-) diff --git a/packages/docs-app/src/examples/select-examples/films.tsx b/packages/docs-app/src/examples/select-examples/films.tsx index 00ab12aa35..59bac3a415 100644 --- a/packages/docs-app/src/examples/select-examples/films.tsx +++ b/packages/docs-app/src/examples/select-examples/films.tsx @@ -152,8 +152,15 @@ export const renderCreateFilmOption = ( /> ); -export const filterFilm: ItemPredicate = (query, film) => { - return `${film.rank}. ${film.title.toLowerCase()} ${film.year}`.indexOf(query.toLowerCase()) >= 0; +export const filterFilm: ItemPredicate = (query, film, _index, exactMatch) => { + const normalizedTitle = film.title.toLowerCase(); + const normalizedQuery = query.toLowerCase(); + + if (exactMatch) { + return normalizedTitle === normalizedQuery; + } else { + return `${film.rank}. ${normalizedTitle} ${film.year}`.indexOf(normalizedQuery) >= 0; + } }; function highlightText(text: string, query: string) { diff --git a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx index cec93ac960..c346de9511 100644 --- a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx +++ b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx @@ -89,7 +89,6 @@ export class MultiSelectExample extends React.PureComponent extends IProps { /** Array of items in the list. */ items: T[]; - /** - * Whether the provided item fully matches the provided value. This is used - * to match pasted values to existing items, so that items can be emitted - * via `onItemsPaste`. - */ - itemEqualsQuery?: (item: T, query: string) => boolean; - /** * Specifies how to test if two items are equal. By default, simple strict * equality (`===`) is used to compare two items. @@ -73,11 +66,21 @@ export interface IListItemsProps extends IProps { itemListPredicate?: ItemListPredicate; /** - * Customize querying of individual items. Return `true` to keep the item, `false` to hide. - * This method will be invoked once for each item, so it should be performant. For more complex - * queries, use `itemListPredicate` to operate once on the entire array. + * Customize querying of individual items. + * + * __Filtering a list of items.__ This function is invoked to filter the + * list of items as a query is typed. Return `true` to keep the item, or + * `false` to hide. This method is invoked once for each item, so it should + * be performant. For more complex queries, use `itemListPredicate` to + * operate once on the entire array. For the purposes of filtering the list, + * this prop is ignored if `itemListPredicate` is also defined. * - * This prop is ignored if `itemListPredicate` is also defined. + * __Matching a pasted value to an item.__ This function is also invoked to + * match a pasted value to an existing item if possible. In this case, the + * function will receive `exactMatch=true`, and the function should return + * true only if the item _exactly_ matches the query. For the purposes of + * matching pasted values, this prop will be invoked even if + * `itemListPredicate` is defined. */ itemPredicate?: ItemPredicate; diff --git a/packages/select/src/common/predicate.ts b/packages/select/src/common/predicate.ts index 6fd90d79bc..4f7e2d7fe8 100644 --- a/packages/select/src/common/predicate.ts +++ b/packages/select/src/common/predicate.ts @@ -14,10 +14,20 @@ export type ItemListPredicate = (query: string, items: T[]) => T[]; /** - * Customize querying of individual items. Return `true` to keep the item, `false` to hide. - * This method will be invoked once for each item, so it should be performant. For more complex - * queries, use `itemListPredicate` to operate once on the entire array. + * Customize querying of individual items. * - * If defined with `itemListPredicate`, this prop will be ignored. + * __Filtering a list of items.__ This function is invoked to filter the list of + * items as a query is typed. Return `true` to keep the item, or `false` to + * hide. This method is invoked once for each item, so it should be performant. + * For more complex queries, use `itemListPredicate` to operate once on the + * entire array. For the purposes of filtering the list, this prop is ignored if + * `itemListPredicate` is also defined. + * + * __Matching a pasted value to an item.__ This function is also invoked to + * match a pasted value to an existing item if possible. In this case, the + * function will receive `exactMatch=true`, and the function should return true + * only if the item _exactly_ matches the query. For the purposes of matching + * pasted values, this prop will be invoked even if `itemListPredicate` is + * defined. */ -export type ItemPredicate = (query: string, item: T, index?: number) => boolean; +export type ItemPredicate = (query: string, item: T, index?: number, exactMatch?: boolean) => boolean; diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index 089a151aee..016c9cf809 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -57,17 +57,17 @@ export interface IQueryListRendererProps // Omit `createNewItem`, because it /** * Handler that should be invoked when the user pastes one or more values. * - * This callback will use `itemEqualsQuery` to find a subset of `items` - * exactly matching the pasted `values` provided, then it will invoke - * `onItemsPaste` with those found items. Each pasted value that does not - * exactly match an item will be ignored. + * This callback will use `itemPredicate` with `exactMatch=true` to find a + * subset of `items` exactly matching the pasted `values` provided, then it + * will invoke `onItemsPaste` with those found items. Each pasted value that + * does not exactly match an item will be ignored. * * If creating items is enabled (by providing both `createNewItemFromQuery` * and `createNewItemRenderer`), then pasted values that do not exactly * match an existing item will emit a new item as created via * `createNewItemFromQuery`. * - * If `itemEqualsQuery` returns multiple matching items for a particular + * If `itemPredicate` returns multiple matching items for a particular * `value`, then only the first matching item will be emitted. */ handlePaste: (values: string[]) => void; @@ -498,12 +498,13 @@ function pxToNumber(value: string | null) { return value == null ? 0 : parseInt(value.slice(0, -2), 10); } -function getMatchingItem(query: string, { items, itemEqualsQuery }: IQueryListProps): T | undefined { - if (Utils.isFunction(itemEqualsQuery)) { +function getMatchingItem(query: string, { items, itemPredicate }: IQueryListProps): T | undefined { + if (Utils.isFunction(itemPredicate)) { // .find() doesn't exist in ES5. Alternative: use a for loop instead of // .filter() so that we can return as soon as we find the first match. - for (const item of items) { - if (itemEqualsQuery(item, query)) { + for (let i = 0; i < items.length; i++) { + const item = items[i]; + if (itemPredicate(query, item, i, true)) { return item; } } diff --git a/packages/select/src/components/select/multiSelect.tsx b/packages/select/src/components/select/multiSelect.tsx index 08a37398db..15452b676f 100644 --- a/packages/select/src/components/select/multiSelect.tsx +++ b/packages/select/src/components/select/multiSelect.tsx @@ -84,7 +84,6 @@ export class MultiSelect extends React.PureComponent, IM extends React.PureComponent, IM Utils.safeInvoke(this.props.onItemSelect, item, evt); }; - private handleItemsPaste = (items: T[]) => { - Utils.safeInvoke(this.props.onItemsPaste, items); - }; - private handleQueryChange = (query: string, evt?: React.ChangeEvent) => { this.setState({ isOpen: query.length > 0 || !this.props.openOnKeyDown }); Utils.safeInvoke(this.props.onQueryChange, query, evt); From 3439afdf042db8d51e81ac124f1eb21be578e71a Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 22 Mar 2019 09:12:40 -0700 Subject: [PATCH 08/12] Respond to @adidahiya CR --- .../src/components/tag-input/tagInput.tsx | 5 ++-- .../src/components/query-list/queryList.tsx | 25 +++++++++---------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/core/src/components/tag-input/tagInput.tsx b/packages/core/src/components/tag-input/tagInput.tsx index d61602e221..f13a7f8852 100644 --- a/packages/core/src/components/tag-input/tagInput.tsx +++ b/packages/core/src/components/tag-input/tagInput.tsx @@ -16,9 +16,8 @@ import { Icon, IconName } from "../icon/icon"; import { ITagProps, Tag } from "../tag/tag"; /** - * A type reflecting the manner in which a `TagInput` value was added. - * - `"default"` - indicates that a value was added in the default fasion, via - * manual selection. + * The method in which a `TagInput` value was added. + * - `"default"` - indicates that a value was added by manual selection. * - `"blur"` - indicates that a value was added when the `TagInput` lost focus. * This is only possible when `addOnBlur=true`. * - `"paste"` - indicates that a value was added via paste. This is only diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index 016c9cf809..00a76c50b3 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -67,10 +67,10 @@ export interface IQueryListRendererProps // Omit `createNewItem`, because it * match an existing item will emit a new item as created via * `createNewItemFromQuery`. * - * If `itemPredicate` returns multiple matching items for a particular - * `value`, then only the first matching item will be emitted. + * If `itemPredicate` returns multiple matching items for a particular query + * in `queries`, then only the first matching item will be emitted. */ - handlePaste: (values: string[]) => void; + handlePaste: (queries: string[]) => void; /** * Keyboard handler for up/down arrow keys to shift the active item. @@ -369,7 +369,7 @@ export class QueryList extends React.Component, IQueryList } }; - private handlePaste = (values: string[]) => { + private handlePaste = (queries: string[]) => { const { createNewItemFromQuery, onItemsPaste } = this.props; let nextActiveItem: T | undefined; @@ -377,21 +377,21 @@ export class QueryList extends React.Component, IQueryList // Find an exising item that exactly matches each pasted value, or // create a new item if possible. Ignore unmatched values if creating // items is disabled. - const pastedItemsToEmit = values.reduce((agg, value) => { - const equalItem = getMatchingItem(value, this.props); + const pastedItemsToEmit = []; + + for (const query of queries) { + const equalItem = getMatchingItem(query, this.props); if (equalItem !== undefined) { nextActiveItem = equalItem; - agg.push(equalItem); + pastedItemsToEmit.push(equalItem); } else if (this.canCreateItems()) { - const newItem = Utils.safeInvoke(createNewItemFromQuery, value); + const newItem = Utils.safeInvoke(createNewItemFromQuery, query); if (newItem !== undefined) { - agg.push(newItem); + pastedItemsToEmit.push(newItem); } } - - return agg; - }, []); + } // UX nicety: update the active item if we matched with at least one // existing item. @@ -399,7 +399,6 @@ export class QueryList extends React.Component, IQueryList this.setActiveItem(nextActiveItem); } - // No need to update the active item. Utils.safeInvoke(onItemsPaste, pastedItemsToEmit); }; From 9fe79120722f8d47b29d82a96fa7cb2309f4ae81 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 22 Mar 2019 09:42:21 -0700 Subject: [PATCH 09/12] Fix build --- .../docs-app/src/examples/select-examples/multiSelectExample.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx index c346de9511..ab1f739416 100644 --- a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx +++ b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx @@ -13,7 +13,6 @@ import { areFilmsEqual, arrayContainsFilm, createFilm, - doesFilmEqualQuery, filmSelectProps, IFilm, maybeAddCreatedFilmToArrays, From e41e4bc8e6f40eb0cd2cd95e77d6d010d633d4b3 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 22 Mar 2019 10:38:07 -0700 Subject: [PATCH 10/12] Write tests --- .../core/test/tag-input/tagInputTests.tsx | 4 + packages/select/test/queryListTests.tsx | 121 +++++++++++++++++- 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/packages/core/test/tag-input/tagInputTests.tsx b/packages/core/test/tag-input/tagInputTests.tsx index 3efadb335f..bf3f8df112 100644 --- a/packages/core/test/tag-input/tagInputTests.tsx +++ b/packages/core/test/tag-input/tagInputTests.tsx @@ -128,6 +128,7 @@ describe("", () => { pressEnterInInput(wrapper, NEW_VALUE); assert.isTrue(onAdd.calledOnce); assert.deepEqual(onAdd.args[0][0], [NEW_VALUE]); + assert.deepEqual(onAdd.args[0][1], "default"); }); it("is invoked on blur when addOnBlur=true", done => { @@ -140,6 +141,8 @@ describe("", () => { // Need setTimeout here to wait for focus to change after blur event setTimeout(() => { assert.isTrue(onAdd.calledOnce); + assert.deepEqual(onAdd.args[0][0], [NEW_VALUE]); + assert.equal(onAdd.args[0][1], "blur"); done(); }); }); @@ -183,6 +186,7 @@ describe("", () => { wrapper.find("input").simulate("paste", { clipboardData: { getData: () => text } }); assert.isTrue(onAdd.calledOnce); assert.deepEqual(onAdd.args[0][0], ["pasted"]); + assert.equal(onAdd.args[0][1], "paste"); }); it("is not invoked on paste if the text does not include a delimiter", () => { diff --git a/packages/select/test/queryListTests.tsx b/packages/select/test/queryListTests.tsx index bff202fe01..ff468c9013 100644 --- a/packages/select/test/queryListTests.tsx +++ b/packages/select/test/queryListTests.tsx @@ -11,7 +11,14 @@ import * as sinon from "sinon"; // this is an awkward import across the monorepo, but we'd rather not introduce a cyclical dependency or create another package import { IQueryListProps } from "@blueprintjs/select"; import { IFilm, renderFilm, TOP_100_FILMS } from "../../docs-app/src/examples/select-examples/films"; -import { IQueryListRendererProps, IQueryListState, ItemListPredicate, ItemListRenderer, QueryList } from "../src/index"; +import { + IQueryListRendererProps, + IQueryListState, + ItemListPredicate, + ItemListRenderer, + ItemPredicate, + QueryList, +} from "../src/index"; type FilmQueryListWrapper = ReactWrapper, IQueryListState>; @@ -158,4 +165,116 @@ describe("", () => { describe("scrolling", () => { it("brings active item into view"); }); + + describe("pasting", () => { + const onItemsPaste = sinon.spy(); + + const itemPredicate: ItemPredicate = (query: string, film: IFilm, _i?: number, exactMatch?: boolean) => { + return exactMatch === true ? query.toLowerCase() === film.title.toLowerCase() : true; + }; + + function mountForPasteTest(overrideProps: Partial> = {}) { + // Placeholder. This will be overwritten by the mounted component. + let handlePaste: (queries: string[]) => void; + + const props: IQueryListProps = { + ...testProps, + itemPredicate, + onItemsPaste, + renderer: sinon.spy((listItemsProps: IQueryListRendererProps) => { + handlePaste = listItemsProps.handlePaste; + return testProps.renderer(listItemsProps); + }), + ...overrideProps, + }; + + const filmQueryList: FilmQueryListWrapper = mount(); + // `handlePaste` will have been set by now, because `props.renderer` + // will have been called. + return { filmQueryList, handlePaste: handlePaste! }; + } + + afterEach(() => { + onItemsPaste.resetHistory(); + }); + + it("converts 1 pasted value into an item", () => { + const { filmQueryList, handlePaste } = mountForPasteTest(); + + const pastedValue = TOP_100_FILMS[0].title; + handlePaste([pastedValue]); + + assert.isTrue(onItemsPaste.calledOnce); + assert.deepEqual(onItemsPaste.args[0][0], [TOP_100_FILMS[0]]); + assert.deepEqual(filmQueryList.state().activeItem, TOP_100_FILMS[0]); + }); + + it("convert multiple pasted values into items", () => { + const { filmQueryList, handlePaste } = mountForPasteTest(); + + // Paste items in unsorted order for fun. + const item1 = TOP_100_FILMS[6]; + const item2 = TOP_100_FILMS[0]; + const item3 = TOP_100_FILMS[3]; + + const pastedValue1 = item1.title; + const pastedValue2 = item2.title; + const pastedValue3 = item3.title; + + handlePaste([pastedValue1, pastedValue2, pastedValue3]); + + assert.isTrue(onItemsPaste.calledOnce); + // Emits all three items. + assert.deepEqual(onItemsPaste.args[0][0], [item1, item2, item3]); + // Highlight the last item pasted. + assert.deepEqual(filmQueryList.state().activeItem, item3); + }); + + it("ignores unrecognized values by default", () => { + const { filmQueryList, handlePaste } = mountForPasteTest(); + + const item1 = TOP_100_FILMS[6]; + const item3 = TOP_100_FILMS[3]; + + const pastedValue1 = item1.title; + const pastedValue2 = "unrecognized"; + const pastedValue3 = item3.title; + + handlePaste([pastedValue1, pastedValue2, pastedValue3]); + + assert.isTrue(onItemsPaste.calledOnce); + // Emits just the 2 valid items. + assert.deepEqual(onItemsPaste.args[0][0], [item1, item3]); + // Highlight the last item pasted. + assert.deepEqual(filmQueryList.state().activeItem, item3); + }); + + it("creates new items out of unrecognized values if 'Create item' option is enabled", () => { + const createdRank = 0; + const createdYear = 2019; + + const { filmQueryList, handlePaste } = mountForPasteTest({ + // Must pass these two props to enable the "Create item" option. + createNewItemFromQuery: query => ({ title: query, rank: createdRank, year: createdYear }), + createNewItemRenderer: () =>
Create item
, + }); + + const item1 = TOP_100_FILMS[6]; + const item2 = TOP_100_FILMS[3]; + + const pastedValue1 = item1.title; + const pastedValue2 = item2.title; + // Paste this item last. + const pastedValue3 = "unrecognized"; + + handlePaste([pastedValue1, pastedValue2, pastedValue3]); + const createdItem = { title: "unrecognized", rank: createdRank, year: createdYear }; + + assert.isTrue(onItemsPaste.calledOnce); + // Emits 2 existing items and 1 newly created item. + assert.deepEqual(onItemsPaste.args[0][0], [item1, item2, createdItem]); + // Highlight the last *already existing* item pasted. + assert.deepEqual(filmQueryList.state().activeItem, item2); + }); + }); }); From a739750ba215cb2df8301828256b437e897d71fd Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 22 Mar 2019 10:42:39 -0700 Subject: [PATCH 11/12] Fix duplicate docs in predicate.ts --- packages/select/src/common/predicate.ts | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/packages/select/src/common/predicate.ts b/packages/select/src/common/predicate.ts index 4f7e2d7fe8..473149894a 100644 --- a/packages/select/src/common/predicate.ts +++ b/packages/select/src/common/predicate.ts @@ -5,29 +5,13 @@ */ /** - * Customize querying of entire `items` array. Return new list of items. - * This method can reorder, add, or remove items at will. - * (Supports filter algorithms that operate on the entire set, rather than individual items.) - * - * If defined with `itemPredicate`, this prop takes priority and the other will be ignored. + * A custom predicate for returning an entirely new `items` array based on the provided query. + * See usage sites in `IListItemsProps`. */ export type ItemListPredicate = (query: string, items: T[]) => T[]; /** - * Customize querying of individual items. - * - * __Filtering a list of items.__ This function is invoked to filter the list of - * items as a query is typed. Return `true` to keep the item, or `false` to - * hide. This method is invoked once for each item, so it should be performant. - * For more complex queries, use `itemListPredicate` to operate once on the - * entire array. For the purposes of filtering the list, this prop is ignored if - * `itemListPredicate` is also defined. - * - * __Matching a pasted value to an item.__ This function is also invoked to - * match a pasted value to an existing item if possible. In this case, the - * function will receive `exactMatch=true`, and the function should return true - * only if the item _exactly_ matches the query. For the purposes of matching - * pasted values, this prop will be invoked even if `itemListPredicate` is - * defined. + * A custom predicate for filtering items based on the provided query. + * See usage sites in `IListItemsProps`. */ export type ItemPredicate = (query: string, item: T, index?: number, exactMatch?: boolean) => boolean; From e89272aad761834443f2e83ba4b2a1ea3d6b7708 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Fri, 22 Mar 2019 10:59:09 -0700 Subject: [PATCH 12/12] Concatenate unrecognized values into input query on paste --- .../src/components/query-list/queryList.tsx | 8 +++++++ packages/select/test/queryListTests.tsx | 23 +++++++++++-------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index 00a76c50b3..e24583c48e 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -373,6 +373,7 @@ export class QueryList extends React.Component, IQueryList const { createNewItemFromQuery, onItemsPaste } = this.props; let nextActiveItem: T | undefined; + const nextQueries = []; // Find an exising item that exactly matches each pasted value, or // create a new item if possible. Ignore unmatched values if creating @@ -390,9 +391,16 @@ export class QueryList extends React.Component, IQueryList if (newItem !== undefined) { pastedItemsToEmit.push(newItem); } + } else { + nextQueries.push(query); } } + // UX nicety: combine all unmatched queries into a single + // comma-separated query in the input, so we don't lose any information. + // And don't reset the active item; we'll do that ourselves below. + this.setQuery(nextQueries.join(", "), false); + // UX nicety: update the active item if we matched with at least one // existing item. if (nextActiveItem !== undefined) { diff --git a/packages/select/test/queryListTests.tsx b/packages/select/test/queryListTests.tsx index ff468c9013..26cd0ba8ea 100644 --- a/packages/select/test/queryListTests.tsx +++ b/packages/select/test/queryListTests.tsx @@ -207,6 +207,7 @@ describe("", () => { assert.isTrue(onItemsPaste.calledOnce); assert.deepEqual(onItemsPaste.args[0][0], [TOP_100_FILMS[0]]); assert.deepEqual(filmQueryList.state().activeItem, TOP_100_FILMS[0]); + assert.deepEqual(filmQueryList.state().query, ""); }); it("convert multiple pasted values into items", () => { @@ -228,25 +229,28 @@ describe("", () => { assert.deepEqual(onItemsPaste.args[0][0], [item1, item2, item3]); // Highlight the last item pasted. assert.deepEqual(filmQueryList.state().activeItem, item3); + assert.deepEqual(filmQueryList.state().query, ""); }); - it("ignores unrecognized values by default", () => { + it("concatenates unrecognized values into the ghost input by default", () => { const { filmQueryList, handlePaste } = mountForPasteTest(); - const item1 = TOP_100_FILMS[6]; - const item3 = TOP_100_FILMS[3]; + const item2 = TOP_100_FILMS[6]; + const item4 = TOP_100_FILMS[3]; - const pastedValue1 = item1.title; - const pastedValue2 = "unrecognized"; - const pastedValue3 = item3.title; + const pastedValue1 = "unrecognized1"; + const pastedValue2 = item2.title; + const pastedValue3 = "unrecognized2"; + const pastedValue4 = item4.title; - handlePaste([pastedValue1, pastedValue2, pastedValue3]); + handlePaste([pastedValue1, pastedValue2, pastedValue3, pastedValue4]); assert.isTrue(onItemsPaste.calledOnce); // Emits just the 2 valid items. - assert.deepEqual(onItemsPaste.args[0][0], [item1, item3]); + assert.deepEqual(onItemsPaste.args[0][0], [item2, item4]); // Highlight the last item pasted. - assert.deepEqual(filmQueryList.state().activeItem, item3); + assert.deepEqual(filmQueryList.state().activeItem, item4); + assert.deepEqual(filmQueryList.state().query, "unrecognized1, unrecognized2"); }); it("creates new items out of unrecognized values if 'Create item' option is enabled", () => { @@ -275,6 +279,7 @@ describe("", () => { assert.deepEqual(onItemsPaste.args[0][0], [item1, item2, createdItem]); // Highlight the last *already existing* item pasted. assert.deepEqual(filmQueryList.state().activeItem, item2); + assert.deepEqual(filmQueryList.state().query, ""); }); }); });