From 8f5925a2d76c3170a2c141a8e8dcaaf5ae1fc358 Mon Sep 17 00:00:00 2001 From: Gregory Douglas <2968519+ggdouglas@users.noreply.github.com> Date: Mon, 14 Oct 2024 16:07:38 -0400 Subject: [PATCH 1/4] Use generic for specifying select item mutability --- .../select/src/common/itemListRenderer.ts | 14 +++-- packages/select/src/common/listItemsProps.ts | 12 ++-- packages/select/src/common/predicate.ts | 2 +- .../components/multi-select/multiSelect.tsx | 29 +++++----- .../select/src/components/omnibar/omnibar.tsx | 8 +-- .../src/components/query-list/queryList.tsx | 55 ++++++++++++------- .../select/src/components/select/select.tsx | 16 +++--- .../select/src/components/suggest/suggest.tsx | 21 ++++--- 8 files changed, 90 insertions(+), 67 deletions(-) diff --git a/packages/select/src/common/itemListRenderer.ts b/packages/select/src/common/itemListRenderer.ts index 180f762fad..5368ad2511 100644 --- a/packages/select/src/common/itemListRenderer.ts +++ b/packages/select/src/common/itemListRenderer.ts @@ -20,7 +20,7 @@ import type { CreateNewItem } from "./listItemsUtils"; * An object describing how to render the list of items. * An `itemListRenderer` receives this object as its sole argument. */ -export interface ItemListRendererProps { +export interface ItemListRendererProps { /** * The currently focused item (for keyboard interactions), or `null` to * indicate that no item is active. @@ -35,13 +35,13 @@ export interface ItemListRendererProps { * map each item in this array through `renderItem`, with support for * optional `noResults` and `initialContent` states. */ - filteredItems: T[]; + filteredItems: A; /** * Array of all items in the list. * See `filteredItems` for a filtered array based on `query` and predicate props. */ - items: T[]; + items: A; /** * The current query string. @@ -75,15 +75,17 @@ export interface ItemListRendererProps { } /** Type alias for a function that renders the list of items. */ -export type ItemListRenderer = (itemListProps: ItemListRendererProps) => React.JSX.Element | null; +export type ItemListRenderer = ( + itemListProps: ItemListRendererProps, +) => React.JSX.Element | null; /** * `ItemListRenderer` helper method for rendering each item in `filteredItems`, * with optional support for `noResults` (when filtered items is empty) * and `initialContent` (when query is empty). */ -export function renderFilteredItems( - props: ItemListRendererProps, +export function renderFilteredItems( + props: ItemListRendererProps, noResults?: React.ReactNode, initialContent?: React.ReactNode | null, ): React.ReactNode { diff --git a/packages/select/src/common/listItemsProps.ts b/packages/select/src/common/listItemsProps.ts index 2beb4431b3..23eb60de40 100644 --- a/packages/select/src/common/listItemsProps.ts +++ b/packages/select/src/common/listItemsProps.ts @@ -34,7 +34,7 @@ export type ItemsEqualComparator = (itemA: T, itemB: T) => boolean; export type ItemsEqualProp = ItemsEqualComparator | keyof T; /** Reusable generic props for a component that operates on a filterable, selectable list of `items`. */ -export interface ListItemsProps extends Props { +export interface ListItemsProps extends Props { /** * The currently focused item for keyboard interactions, or `null` to * indicate that no item is active. If omitted or `undefined`, this prop will be @@ -44,7 +44,7 @@ export interface ListItemsProps extends Props { activeItem?: T | CreateNewItem | null; /** Array of items in the list. */ - items: T[]; + items: A; /** * Specifies how to test if two items are equal. By default, simple strict @@ -75,7 +75,7 @@ export interface ListItemsProps extends Props { * * If `itemPredicate` is also defined, this prop takes priority and the other will be ignored. */ - itemListPredicate?: ItemListPredicate; + itemListPredicate?: ItemListPredicate; /** * Customize querying of individual items. @@ -110,7 +110,7 @@ export interface ListItemsProps extends Props { * and wraps them all in a `Menu` element. If the query is empty then `initialContent` is returned, * and if there are no items that match the predicate then `noResults` is returned. */ - itemListRenderer?: ItemListRenderer; + itemListRenderer?: ItemListRenderer; /** * React content to render when query is empty. @@ -157,7 +157,7 @@ export interface ListItemsProps extends Props { /** * Callback invoked when multiple items are selected at once via pasting. */ - onItemsPaste?: (items: T[]) => void; + onItemsPaste?: (items: A) => void; /** * Callback invoked when the query string changes. @@ -170,7 +170,7 @@ export interface ListItemsProps extends Props { * created, either by pressing the `Enter` key or by clicking on the "Create * Item" option. It transforms a query string into one or many items type. */ - createNewItemFromQuery?: (query: string) => T | T[]; + createNewItemFromQuery?: (query: string) => T | A; /** * Custom renderer to transform the current query string into a selectable diff --git a/packages/select/src/common/predicate.ts b/packages/select/src/common/predicate.ts index 8011cbe033..e308689bcf 100644 --- a/packages/select/src/common/predicate.ts +++ b/packages/select/src/common/predicate.ts @@ -18,7 +18,7 @@ * A custom predicate for returning an entirely new `items` array based on the provided query. * See usage sites in `ListItemsProps`. */ -export type ItemListPredicate = (query: string, items: T[]) => T[]; +export type ItemListPredicate = (query: string, items: A) => A; /** * A custom predicate for filtering items based on the provided query. diff --git a/packages/select/src/components/multi-select/multiSelect.tsx b/packages/select/src/components/multi-select/multiSelect.tsx index 1d69815585..39db60641a 100644 --- a/packages/select/src/components/multi-select/multiSelect.tsx +++ b/packages/select/src/components/multi-select/multiSelect.tsx @@ -39,12 +39,12 @@ import { Cross } from "@blueprintjs/icons"; import { Classes, type ListItemsProps, type SelectPopoverProps } from "../../common"; import { QueryList, type QueryListRendererProps } from "../query-list/queryList"; -export interface MultiSelectProps extends ListItemsProps, SelectPopoverProps { +export interface MultiSelectProps extends ListItemsProps, SelectPopoverProps { /** * Element which triggers the multiselect popover. Providing this prop will replace the default TagInput * target thats rendered and move the search functionality to within the Popover. */ - customTarget?: (selectedItems: T[], isOpen: boolean) => React.ReactNode; + customTarget?: (selectedItems: A, isOpen: boolean) => React.ReactNode; /** * Whether the component is non-interactive. @@ -104,7 +104,7 @@ export interface MultiSelectProps extends ListItemsProps, SelectPopoverPro placeholder?: string; /** Controlled selected values. */ - selectedItems: T[]; + selectedItems: A; /** * Props to pass to the [TagInput component](##core/components/tag-input). @@ -142,7 +142,10 @@ export interface MultiSelectState { * * @see https://blueprintjs.com/docs/#select/multi-select */ -export class MultiSelect extends AbstractPureComponent, MultiSelectState> { +export class MultiSelect extends AbstractPureComponent< + MultiSelectProps, + MultiSelectState +> { public static displayName = `${DISPLAYNAME_PREFIX}.MultiSelect`; private listboxId = Utils.uniqueId("listbox"); @@ -164,19 +167,19 @@ export class MultiSelect extends AbstractPureComponent, M public input: HTMLInputElement | null = null; - public queryList: QueryList | null = null; + public queryList: QueryList | null = null; private refHandlers: { input: React.RefCallback; popover: React.RefObject; - queryList: React.RefCallback>; + queryList: React.RefCallback>; } = { input: refHandler(this, "input", this.props.tagInputProps?.inputRef), popover: React.createRef(), - queryList: (ref: QueryList | null) => (this.queryList = ref), + queryList: (ref: QueryList | null) => (this.queryList = ref), }; - public componentDidUpdate(prevProps: MultiSelectProps) { + public componentDidUpdate(prevProps: MultiSelectProps) { if (prevProps.tagInputProps?.inputRef !== this.props.tagInputProps?.inputRef) { setRef(prevProps.tagInputProps?.inputRef, null); this.refHandlers.input = refHandler(this, "input", this.props.tagInputProps?.inputRef); @@ -195,7 +198,7 @@ export class MultiSelect extends AbstractPureComponent, M const { menuProps, openOnKeyDown, popoverProps, tagInputProps, customTarget, ...restProps } = this.props; return ( - + {...restProps} menuProps={{ "aria-label": "selectable options", @@ -211,7 +214,7 @@ export class MultiSelect extends AbstractPureComponent, M ); } - private renderQueryList = (listProps: QueryListRendererProps) => { + private renderQueryList = (listProps: QueryListRendererProps) => { const { disabled, popoverContentProps = {}, popoverProps = {} } = this.props; const { handleKeyDown, handleKeyUp } = listProps; @@ -267,7 +270,7 @@ export class MultiSelect extends AbstractPureComponent, M // the "fill" prop. Note that we must take `isOpen` as an argument to force this render function to be called // again after that state changes. private getPopoverTargetRenderer = - (listProps: QueryListRendererProps, isOpen: boolean) => + (listProps: QueryListRendererProps, isOpen: boolean) => // N.B. pull out `isOpen` so that it's not forwarded to the DOM, but remember not to use it directly // since it may be stale (`renderTarget` is not re-invoked on this.state changes). // eslint-disable-next-line react/display-name @@ -304,7 +307,7 @@ export class MultiSelect extends AbstractPureComponent, M ); }; - private getTagInput = (listProps: QueryListRendererProps, className?: string) => { + private getTagInput = (listProps: QueryListRendererProps, className?: string) => { const { disabled, fill, onClear, placeholder, selectedItems, tagInputProps = {} } = this.props; const maybeClearButton = @@ -408,7 +411,7 @@ export class MultiSelect extends AbstractPureComponent, M }; private getTagInputAddHandler = - (listProps: QueryListRendererProps) => (values: any[], method: TagInputAddMethod) => { + (listProps: QueryListRendererProps) => (values: any[], method: TagInputAddMethod) => { if (method === "paste") { listProps.handlePaste(values); } diff --git a/packages/select/src/components/omnibar/omnibar.tsx b/packages/select/src/components/omnibar/omnibar.tsx index 8491a76b8d..2d251161d5 100644 --- a/packages/select/src/components/omnibar/omnibar.tsx +++ b/packages/select/src/components/omnibar/omnibar.tsx @@ -23,7 +23,7 @@ import { Search } from "@blueprintjs/icons"; import { Classes, type ListItemsProps } from "../../common"; import { QueryList, type QueryListRendererProps } from "../query-list/queryList"; -export interface OmnibarProps extends ListItemsProps { +export interface OmnibarProps extends ListItemsProps { /** * Props to spread to the query `InputGroup`. Use `query` and * `onQueryChange` instead of `inputProps.value` and `inputProps.onChange` @@ -58,7 +58,7 @@ export interface OmnibarProps extends ListItemsProps { * * @see https://blueprintjs.com/docs/#select/omnibar */ -export class Omnibar extends React.PureComponent> { +export class Omnibar extends React.PureComponent> { public static displayName = `${DISPLAYNAME_PREFIX}.Omnibar`; public static ofType() { @@ -71,7 +71,7 @@ export class Omnibar extends React.PureComponent> { const initialContent = "initialContent" in this.props ? this.props.initialContent : null; return ( - + {...restProps} // Omnibar typically does not keep track of and/or show its selection state like other // select components, so it's more of a menu than a listbox. This means that users should return @@ -83,7 +83,7 @@ export class Omnibar extends React.PureComponent> { ); } - private renderQueryList = (listProps: QueryListRendererProps) => { + private renderQueryList = (listProps: QueryListRendererProps) => { const { inputProps = {}, isOpen, overlayProps = {} } = this.props; const { handleKeyDown, handleKeyUp } = listProps; const handlers = isOpen ? { onKeyDown: handleKeyDown, onKeyUp: handleKeyUp } : {}; diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index e326843711..20b0974f60 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -30,7 +30,7 @@ import { renderFilteredItems, } from "../../common"; -export interface QueryListProps extends ListItemsProps { +export interface QueryListProps extends ListItemsProps { /** * Initial active item, useful if the parent component is controlling its selectedItem but * not activeItem. @@ -60,7 +60,7 @@ export interface QueryListProps extends ListItemsProps { * Customize rendering of the component. * Receives an object with props that should be applied to elements as necessary. */ - renderer: (listProps: QueryListRendererProps) => React.JSX.Element; + renderer: (listProps: QueryListRendererProps) => React.JSX.Element; /** * Whether the list is disabled. @@ -74,8 +74,8 @@ export interface QueryListProps extends ListItemsProps { * An object describing how to render a `QueryList`. * A `QueryList` `renderer` receives this object as its sole argument. */ -export interface QueryListRendererProps // Omit `createNewItem`, because it's used strictly for internal tracking. - extends Pick, "activeItem" | "filteredItems" | "query">, +export interface QueryListRendererProps // Omit `createNewItem`, because it's used strictly for internal tracking. + extends Pick, "activeItem" | "filteredItems" | "query">, Props { /** * Selection handler that should be invoked when a new item has been chosen, @@ -124,7 +124,7 @@ export interface QueryListRendererProps // Omit `createNewItem`, because it's } /** Exported for testing, not part of public API */ -export interface QueryListState { +export interface QueryListState { /** The currently focused item (for keyboard interactions). */ activeItem: T | CreateNewItem | null; @@ -134,10 +134,10 @@ export interface QueryListState { * this element will be used to hide the "Create Item" option if its value * matches the current `query`. */ - createNewItem: T | T[] | undefined; + createNewItem: T | A | undefined; /** The original `items` array filtered by `itemListPredicate` or `itemPredicate`. */ - filteredItems: T[]; + filteredItems: A; /** The current query string. */ query: string; @@ -148,7 +148,10 @@ export interface QueryListState { * * @see https://blueprintjs.com/docs/#select/query-list */ -export class QueryList extends AbstractComponent, QueryListState> { +export class QueryList extends AbstractComponent< + QueryListProps, + QueryListState +> { public static displayName = `${DISPLAYNAME_PREFIX}.QueryList`; public static defaultProps = { @@ -198,7 +201,7 @@ export class QueryList extends AbstractComponent, QueryList */ private isEnterKeyPressed = false; - public constructor(props: QueryListProps) { + public constructor(props: QueryListProps) { super(props); const { query = "" } = props; @@ -238,7 +241,7 @@ export class QueryList extends AbstractComponent, QueryList }); } - public componentDidUpdate(prevProps: QueryListProps) { + public componentDidUpdate(prevProps: QueryListProps) { if (this.props.activeItem !== undefined && this.props.activeItem !== this.state.activeItem) { this.shouldCheckActiveItemInViewport = true; this.setState({ activeItem: this.props.activeItem }); @@ -354,7 +357,7 @@ export class QueryList extends AbstractComponent, QueryList } /** default `itemListRenderer` implementation */ - private renderItemList = (listProps: ItemListRendererProps) => { + private renderItemList = (listProps: ItemListRendererProps) => { const { initialContent, noResults } = this.props; // omit noResults if createNewItemFromQuery and createNewItemRenderer are both supplied, and query is not empty @@ -485,7 +488,7 @@ export class QueryList extends AbstractComponent, QueryList // 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 = []; + const pastedItemsToEmit: T[] = []; for (const query of queries) { const equalItem = getMatchingItem(query, this.props); @@ -515,7 +518,7 @@ export class QueryList extends AbstractComponent, QueryList this.setActiveItem(nextActiveItem); } - onItemsPaste?.(pastedItemsToEmit); + onItemsPaste?.(pastedItemsToEmit as readonly T[] as A); }; private handleKeyDown = (event: React.KeyboardEvent) => { @@ -584,7 +587,7 @@ export class QueryList extends AbstractComponent, QueryList * @param createNewItem Checks if this item would match the current query. Cannot check this.state.createNewItem * every time since state may not have been updated yet. */ - private isCreateItemRendered(createNewItem?: T | T[]): boolean { + private isCreateItemRendered(createNewItem?: T | A): boolean { return ( this.canCreateItems() && this.state.query !== "" && @@ -603,7 +606,7 @@ export class QueryList extends AbstractComponent, QueryList return this.props.createNewItemFromQuery != null && this.props.createNewItemRenderer != null; } - private wouldCreatedItemMatchSomeExistingItem(createNewItem?: T | T[]) { + private wouldCreatedItemMatchSomeExistingItem(createNewItem?: T | A) { // 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 => { @@ -623,7 +626,10 @@ function pxToNumber(value: string | null) { return value == null ? 0 : parseInt(value.slice(0, -2), 10); } -function getMatchingItem(query: string, { items, itemPredicate }: QueryListProps): T | undefined { +function getMatchingItem( + query: string, + { items, itemPredicate }: QueryListProps, +): 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. @@ -637,12 +643,15 @@ function getMatchingItem(query: string, { items, itemPredicate }: QueryListPr return undefined; } -function getFilteredItems(query: string, { items, itemPredicate, itemListPredicate }: QueryListProps) { +function getFilteredItems( + query: string, + { items, itemPredicate, itemListPredicate }: QueryListProps, +) { if (Utils.isFunction(itemListPredicate)) { // note that implementations can reorder the items here return itemListPredicate(query, items); } else if (Utils.isFunction(itemPredicate)) { - return items.filter((item, index) => itemPredicate(query, item, index)); + return items.filter((item, index) => itemPredicate(query, item, index)) as readonly T[] as A; } return items; } @@ -657,7 +666,11 @@ function wrapNumber(value: number, min: number, max: number) { return value; } -function isItemDisabled(item: T | null, index: number, itemDisabled?: ListItemsProps["itemDisabled"]) { +function isItemDisabled( + item: T | null, + index: number, + itemDisabled?: ListItemsProps["itemDisabled"], +) { if (itemDisabled == null || item == null) { return false; } else if (Utils.isFunction(itemDisabled)) { @@ -675,8 +688,8 @@ function isItemDisabled(item: T | null, index: number, itemDisabled?: ListIte * @param direction amount to move in each iteration, typically +/-1 * @param startIndex which index to begin moving from */ -export function getFirstEnabledItem( - items: T[], +export function getFirstEnabledItem( + items: A, itemDisabled?: keyof T | ((item: T, index: number) => boolean), direction = 1, startIndex = items.length - 1, diff --git a/packages/select/src/components/select/select.tsx b/packages/select/src/components/select/select.tsx index 827217a1ed..5cdb9245fd 100644 --- a/packages/select/src/components/select/select.tsx +++ b/packages/select/src/components/select/select.tsx @@ -37,7 +37,7 @@ import { Cross, Search } from "@blueprintjs/icons"; import { Classes, type ListItemsProps, type SelectPopoverProps } from "../../common"; import { QueryList, type QueryListRendererProps } from "../query-list/queryList"; -export interface SelectProps extends ListItemsProps, SelectPopoverProps { +export interface SelectProps extends ListItemsProps, SelectPopoverProps { /** * Element which triggers the select popover. In most cases, you should display * the name or label of the curently selected item here. @@ -108,7 +108,7 @@ export interface SelectState { * * @see https://blueprintjs.com/docs/#select/select */ -export class Select extends AbstractPureComponent, SelectState> { +export class Select extends AbstractPureComponent, SelectState> { public static displayName = `${DISPLAYNAME_PREFIX}.Select`; /** @deprecated no longer necessary now that the TypeScript parser supports type arguments on JSX element tags */ @@ -120,7 +120,7 @@ export class Select extends AbstractPureComponent, SelectState public inputElement: HTMLInputElement | null = null; - private queryList: QueryList | null = null; + private queryList: QueryList | null = null; private previousFocusedElement: HTMLElement | undefined; @@ -130,7 +130,7 @@ export class Select extends AbstractPureComponent, SelectState this.props.inputProps?.inputRef, ); - private handleQueryListRef = (ref: QueryList | null) => (this.queryList = ref); + private handleQueryListRef = (ref: QueryList | null) => (this.queryList = ref); private listboxId = Utils.uniqueId("listbox"); @@ -139,7 +139,7 @@ export class Select extends AbstractPureComponent, SelectState const { filterable, inputProps, menuProps, popoverProps, ...restProps } = this.props; return ( - + {...restProps} menuProps={{ "aria-label": "selectable options", ...menuProps, id: this.listboxId }} onItemSelect={this.handleItemSelect} @@ -149,7 +149,7 @@ export class Select extends AbstractPureComponent, SelectState ); } - public componentDidUpdate(prevProps: SelectProps, prevState: SelectState) { + public componentDidUpdate(prevProps: SelectProps, prevState: SelectState) { if (prevProps.inputProps?.inputRef !== this.props.inputProps?.inputRef) { setRef(prevProps.inputProps?.inputRef, null); this.handleInputRef = refHandler(this, "inputElement", this.props.inputProps?.inputRef); @@ -161,7 +161,7 @@ export class Select extends AbstractPureComponent, SelectState } } - private renderQueryList = (listProps: QueryListRendererProps) => { + private renderQueryList = (listProps: QueryListRendererProps) => { // not using defaultProps cuz they're hard to type with generics (can't use on static members) const { filterable = true, @@ -220,7 +220,7 @@ export class Select extends AbstractPureComponent, SelectState // the "fill" prop. Note that we must take `isOpen` as an argument to force this render function to be called // again after that state changes. private getPopoverTargetRenderer = - (listProps: QueryListRendererProps, isOpen: boolean) => + (listProps: QueryListRendererProps, isOpen: boolean) => // N.B. pull out `isOpen` so that it's not forwarded to the DOM, but remember not to use it directly // since it may be stale (`renderTarget` is not re-invoked on this.state changes). // eslint-disable-next-line react/display-name diff --git a/packages/select/src/components/suggest/suggest.tsx b/packages/select/src/components/suggest/suggest.tsx index 9cc3b87173..1424ee1f4a 100644 --- a/packages/select/src/components/suggest/suggest.tsx +++ b/packages/select/src/components/suggest/suggest.tsx @@ -35,7 +35,9 @@ import { import { Classes, type ListItemsProps, type SelectPopoverProps } from "../../common"; import { QueryList, type QueryListRendererProps } from "../query-list/queryList"; -export interface SuggestProps extends ListItemsProps, Omit { +export interface SuggestProps + extends ListItemsProps, + Omit { /** * Whether the popover should close after selecting an item. * @@ -117,7 +119,10 @@ export interface SuggestState { * * @see https://blueprintjs.com/docs/#select/suggest */ -export class Suggest extends AbstractPureComponent, SuggestState> { +export class Suggest extends AbstractPureComponent< + SuggestProps, + SuggestState +> { public static displayName = `${DISPLAYNAME_PREFIX}.Suggest`; public static defaultProps: Partial> = { @@ -139,7 +144,7 @@ export class Suggest extends AbstractPureComponent, SuggestSt public inputElement: HTMLInputElement | null = null; - private queryList: QueryList | null = null; + private queryList: QueryList | null = null; private handleInputRef: React.Ref = refHandler( this, @@ -147,7 +152,7 @@ export class Suggest extends AbstractPureComponent, SuggestSt this.props.inputProps?.inputRef, ); - private handleQueryListRef = (ref: QueryList | null) => (this.queryList = ref); + private handleQueryListRef = (ref: QueryList | null) => (this.queryList = ref); private listboxId = Utils.uniqueId("listbox"); @@ -156,7 +161,7 @@ export class Suggest extends AbstractPureComponent, SuggestSt const { disabled, inputProps, menuProps, popoverProps, ...restProps } = this.props; return ( - + {...restProps} menuProps={{ "aria-label": "selectable options", ...menuProps, id: this.listboxId }} initialActiveItem={this.props.selectedItem ?? undefined} @@ -167,7 +172,7 @@ export class Suggest extends AbstractPureComponent, SuggestSt ); } - public componentDidUpdate(prevProps: SuggestProps, prevState: SuggestState) { + public componentDidUpdate(prevProps: SuggestProps, prevState: SuggestState) { if (prevProps.inputProps?.inputRef !== this.props.inputProps?.inputRef) { setRef(prevProps.inputProps?.inputRef, null); this.handleInputRef = refHandler(this, "inputElement", this.props.inputProps?.inputRef); @@ -191,7 +196,7 @@ export class Suggest extends AbstractPureComponent, SuggestSt } } - private renderQueryList = (listProps: QueryListRendererProps) => { + private renderQueryList = (listProps: QueryListRendererProps) => { const { popoverContentProps = {}, popoverProps = {}, popoverRef } = this.props; const { isOpen } = this.state; const { handleKeyDown, handleKeyUp } = listProps; @@ -226,7 +231,7 @@ export class Suggest extends AbstractPureComponent, SuggestSt // the "fill" prop. Note that we must take `isOpen` as an argument to force this render function to be called // again after that state changes. private getPopoverTargetRenderer = - (listProps: QueryListRendererProps, isOpen: boolean) => + (listProps: QueryListRendererProps, isOpen: boolean) => // eslint-disable-next-line react/display-name ({ // pull out `isOpen` so that it's not forwarded to the DOM From 19cdbeec25bdc51d8382f611920f4f6660bd5347 Mon Sep 17 00:00:00 2001 From: Gregory Douglas Date: Wed, 30 Oct 2024 14:28:24 -0400 Subject: [PATCH 2/4] Update examples and tests to use readonly array --- .../select-examples/multiSelectExample.tsx | 20 +++++++++-------- .../select-examples/omnibarExample.tsx | 2 +- .../select-examples/selectExample.tsx | 10 ++++----- .../select-examples/suggestExample.tsx | 6 ++--- .../select/src/__examples__/filmSelect.tsx | 8 +++---- packages/select/src/__examples__/films.tsx | 22 +++++++++---------- packages/select/test/multiSelectTests.tsx | 12 +++++----- packages/select/test/queryListTests.tsx | 4 +++- packages/select/test/selectComponentSuite.tsx | 4 ++-- packages/select/test/selectTests.tsx | 6 ++--- packages/select/test/suggestTests.tsx | 6 ++--- 11 files changed, 52 insertions(+), 48 deletions(-) diff --git a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx index ff5d0129c2..c9c6cae76c 100644 --- a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx +++ b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx @@ -40,13 +40,13 @@ const INTENTS = [Intent.NONE, Intent.PRIMARY, Intent.SUCCESS, Intent.DANGER, Int export interface MultiSelectExampleState { allowCreate: boolean; - createdItems: Film[]; + createdItems: readonly Film[]; disabled: boolean; fill: boolean; - films: Film[]; + films: readonly Film[]; hasInitialContent: boolean; intent: boolean; - items: Film[]; + items: readonly Film[]; matchTargetWidth: boolean; openOnKeyDown: boolean; popoverMinimal: boolean; @@ -124,7 +124,7 @@ export class MultiSelectExample extends React.PureComponent - + ; + private renderCustomTarget = (selectedItems: readonly Film[]) => ( + + ); private renderTag = (film: Film) => film.title; @@ -287,7 +289,7 @@ export class MultiSelectExample extends React.PureComponent { let nextCreatedItems = createdItems.slice(); let nextFilms = films.slice(); @@ -295,8 +297,8 @@ export class MultiSelectExample extends React.PureComponent { const results = maybeAddCreatedFilmToArrays(nextItems, nextCreatedItems, film); - nextItems = results.items; - nextCreatedItems = results.createdItems; + nextItems = results.items.slice(); + nextCreatedItems = results.createdItems.slice(); // Avoid re-creating an item that is already selected (the "Create // Item" option will be shown even if it matches an already selected // item). @@ -336,7 +338,7 @@ export class MultiSelectExample extends React.PureComponent { + private handleFilmsPaste = (films: readonly Film[]) => { // On paste, don't bother with deselecting already selected values, just // add the new ones. this.selectFilms(films); diff --git a/packages/docs-app/src/examples/select-examples/omnibarExample.tsx b/packages/docs-app/src/examples/select-examples/omnibarExample.tsx index 19d4596ef0..b30c197123 100644 --- a/packages/docs-app/src/examples/select-examples/omnibarExample.tsx +++ b/packages/docs-app/src/examples/select-examples/omnibarExample.tsx @@ -92,7 +92,7 @@ export class OmnibarExample extends React.PureComponent - + { + private getGroupedItems = (filteredItems: readonly Film[]) => { return filteredItems.reduce>( (acc, item, index) => { const group = this.getGroup(item); @@ -193,7 +193,7 @@ export class SelectExample extends React.PureComponent { + private groupedItemListPredicate = (query: string, items: readonly Film[]) => { return items .filter((item, index) => filterFilm(query, item, index)) .sort((a, b) => this.getGroup(a).localeCompare(this.getGroup(b))); @@ -208,7 +208,7 @@ export class SelectExample extends React.PureComponent this.state.disableItems && film.year < 2000; - private renderGroupedItemList = (listProps: ItemListRendererProps) => { + private renderGroupedItemList = (listProps: ItemListRendererProps) => { const initialContent = this.getInitialContent(); const noResults = ; @@ -231,7 +231,7 @@ export class SelectExample extends React.PureComponent, + listProps: ItemListRendererProps, noResults?: React.ReactNode, initialContent?: React.ReactNode | null, ) => { diff --git a/packages/docs-app/src/examples/select-examples/suggestExample.tsx b/packages/docs-app/src/examples/select-examples/suggestExample.tsx index 835ae1411a..4e6d3941ad 100644 --- a/packages/docs-app/src/examples/select-examples/suggestExample.tsx +++ b/packages/docs-app/src/examples/select-examples/suggestExample.tsx @@ -34,10 +34,10 @@ import { export interface SuggestExampleState { allowCreate: boolean; closeOnSelect: boolean; - createdItems: Film[]; + createdItems: readonly Film[]; disabled: boolean; fill: boolean; - items: Film[]; + items: readonly Film[]; matchTargetWidth: boolean; minimal: boolean; openOnKeyDown: boolean; @@ -92,7 +92,7 @@ export class SuggestExample extends React.PureComponent - + , + SelectProps, | "createNewItemFromQuery" | "createNewItemRenderer" | "itemPredicate" @@ -49,8 +49,8 @@ type FilmSelectProps = Omit< }; export function FilmSelect({ allowCreate = false, fill, ...restProps }: FilmSelectProps) { - const [items, setItems] = React.useState([...TOP_100_FILMS]); - const [createdItems, setCreatedItems] = React.useState([]); + const [items, setItems] = React.useState([...TOP_100_FILMS]); + const [createdItems, setCreatedItems] = React.useState([]); const [selectedFilm, setSelectedFilm] = React.useState(undefined); const handleItemSelect = React.useCallback( (newFilm: Film) => { @@ -82,7 +82,7 @@ export function FilmSelect({ allowCreate = false, fill, ...restProps }: FilmSele ); return ( - + ", () => { testsContainerElement?.remove(); }); - selectComponentSuite, SelectState>(props => + selectComponentSuite, SelectState>(props => mount(", () => { assert.isTrue(wrapper.find(Popover).prop("isOpen")); }); - function select(props: Partial> = {}, query?: string) { + function select(props: Partial> = {}, query?: string) { const wrapper = mount( - {...defaultProps} {...handlers} {...props}> + , { attachTo: testsContainerElement }, diff --git a/packages/select/test/suggestTests.tsx b/packages/select/test/suggestTests.tsx index 2002c8dc29..b9f16836ab 100644 --- a/packages/select/test/suggestTests.tsx +++ b/packages/select/test/suggestTests.tsx @@ -57,7 +57,7 @@ describe("Suggest", () => { testsContainerElement?.remove(); }); - selectComponentSuite, SuggestState>(props => + selectComponentSuite, SuggestState>(props => mount( { }); }); - function suggest(props: Partial> = {}) { - return mount>( {...defaultProps} {...handlers} {...props} />, { + function suggest(props: Partial> = {}) { + return mount>(, { attachTo: testsContainerElement, }); } From 5b8c385361d358ba90e18980189eb5ddc683f6b1 Mon Sep 17 00:00:00 2001 From: Gregory Douglas Date: Thu, 31 Oct 2024 11:09:15 -0400 Subject: [PATCH 3/4] Remove slice around readonly arrays --- .../examples/select-examples/multiSelectExample.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx index c9c6cae76c..5006a5ac08 100644 --- a/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx +++ b/packages/docs-app/src/examples/select-examples/multiSelectExample.tsx @@ -291,14 +291,14 @@ export class MultiSelectExample extends React.PureComponent { - let nextCreatedItems = createdItems.slice(); - let nextFilms = films.slice(); - let nextItems = items.slice(); + let nextCreatedItems = createdItems; + let nextFilms = films; + let nextItems = items; filmsToSelect.forEach(film => { const results = maybeAddCreatedFilmToArrays(nextItems, nextCreatedItems, film); - nextItems = results.items.slice(); - nextCreatedItems = results.createdItems.slice(); + 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). From 25f1aa5b6cc2534a17ee53d4be39f462d2cc7c4d Mon Sep 17 00:00:00 2001 From: Gregory Douglas Date: Thu, 31 Oct 2024 11:18:10 -0400 Subject: [PATCH 4/4] Fix typo in Suggest tests --- packages/select/test/suggestTests.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/select/test/suggestTests.tsx b/packages/select/test/suggestTests.tsx index b9f16836ab..64d58e3c23 100644 --- a/packages/select/test/suggestTests.tsx +++ b/packages/select/test/suggestTests.tsx @@ -339,7 +339,7 @@ describe("Suggest", () => { }); function suggest(props: Partial> = {}) { - return mount>(, { + return mount>(, { attachTo: testsContainerElement, }); }