[select] fix: various fixes, class name deprecation

packages/select/src/common/classes.ts

@@ -23,6 +23,8 @@ export const MULTISELECT_POPOVER = `${MULTISELECT}-popover`;
 export const MULTISELECT_TAG_INPUT_INPUT = `${MULTISELECT}-tag-input-input`;
 export const OMNIBAR = `${NS}-omnibar`;
 export const OMNIBAR_OVERLAY = `${OMNIBAR}-overlay`;
+/** @deprecated this class name is not rendered by any component in this package */
 export const SELECT = `${NS}-select`;
-export const SELECT_POPOVER = `${SELECT}-popover`;
-export const SELECT_MATCH_TARGET_WIDTH = `${SELECT}-match-target-width`;
+export const SELECT_POPOVER = `${NS}-select-popover`;
+export const SELECT_MATCH_TARGET_WIDTH = `${NS}-select-match-target-width`;
+export const SUGGEST_POPOVER = `${NS}-suggest-popover`;

packages/select/src/common/selectPopoverProps.ts

@@ -28,7 +28,9 @@ export interface SelectPopoverProps {
      * Props to spread to `Popover2`.
      * Note that `content` cannot be changed aside from utilizing `popoverContentProps`.
      */
-    popoverProps?: Partial<Omit<Popover2Props, "content">>;
+    popoverProps?: Partial<
+        Omit<Popover2Props, "content" | "defaultIsOpen" | "disabled" | "fill" | "renderTarget" | "targetTagName">
+    >;
 
     /**
      * Optional ref for the Popover2 component instance.

packages/select/src/components/multi-select/multiSelect2.tsx

@@ -35,8 +35,6 @@ import { Popover2, Popover2TargetProps, PopupKind } from "@blueprintjs/popover2"
 import { Classes, IListItemsProps, SelectPopoverProps } from "../../common";
 import { IQueryListRendererProps, QueryList } from "../query-list/queryList";
 
-// N.B. selectedItems should really be a required prop, but is left optional for backwards compatibility
-
 export interface MultiSelect2Props<T> extends IListItemsProps<T>, SelectPopoverProps {
     /**
      * Whether the component is non-interactive.
@@ -95,7 +93,7 @@ export interface MultiSelect2Props<T> extends IListItemsProps<T>, SelectPopoverP
     popoverTargetProps?: React.HTMLAttributes<HTMLDivElement>;
 
     /** Controlled selected values. */
-    selectedItems?: T[];
+    selectedItems: T[];
 
     /**
      * Props to spread to `TagInput`.
@@ -233,7 +231,7 @@ export class MultiSelect2<T> extends AbstractPureComponent2<MultiSelect2Props<T>
                 onClear,
                 placeholder,
                 popoverTargetProps = {},
-                selectedItems = [],
+                selectedItems,
                 tagInputProps = {},
             } = this.props;
             const { handleKeyDown, handleKeyUp } = listProps;
@@ -331,7 +329,7 @@ export class MultiSelect2<T> extends AbstractPureComponent2<MultiSelect2Props<T>
     };
 
     private handleTagRemove = (tag: React.ReactNode, index: number) => {
-        const { selectedItems = [], onRemove, tagInputProps } = this.props;
+        const { selectedItems, onRemove, tagInputProps } = this.props;
         onRemove?.(selectedItems[index], index);
         tagInputProps?.onRemove?.(tag, index);
         this.refHandlers.popover.current?.reposition(); // reposition when size of input changes

packages/select/src/components/select/select2.tsx

@@ -28,7 +28,6 @@ import {
     InputGroupProps2,
     IRef,
     Keys,
-    Position,
     refHandler,
     setRef,
     Utils,
@@ -147,7 +146,6 @@ export class Select2<T> extends AbstractPureComponent2<Select2Props<T>, Select2S
     private renderQueryList = (listProps: IQueryListRendererProps<T>) => {
         // not using defaultProps cuz they're hard to type with generics (can't use <T> on static members)
         const {
-            fill,
             filterable = true,
             disabled = false,
             inputProps = {},
@@ -156,10 +154,6 @@ export class Select2<T> extends AbstractPureComponent2<Select2Props<T>, Select2S
             popoverRef,
         } = this.props;
 
-        if (fill) {
-            popoverProps.fill = true;
-        }
-
         const input = (
             <InputGroup
                 aria-autocomplete="list"
@@ -174,13 +168,15 @@ export class Select2<T> extends AbstractPureComponent2<Select2Props<T>, Select2S
         );
 
         const { handleKeyDown, handleKeyUp } = listProps;
+
+        // N.B. no need to set `fill` since that is unused with the `renderTarget` API
         return (
             <Popover2
                 autoFocus={false}
                 enforceFocus={false}
                 isOpen={this.state.isOpen}
                 disabled={disabled}
-                position={Position.BOTTOM_LEFT}
+                placement={popoverProps.position || popoverProps.placement ? undefined : "bottom-start"}
                 {...popoverProps}
                 className={classNames(listProps.className, popoverProps.className)}
                 content={

packages/select/src/components/suggest/suggest2.tsx

@@ -27,7 +27,6 @@ import {
     IRef,
     Keys,
     mergeRefs,
-    Position,
     refHandler,
     setRef,
     Utils,
@@ -185,7 +184,7 @@ export class Suggest2<T> extends AbstractPureComponent2<Suggest2Props<T>, Sugges
                 autoFocus={false}
                 enforceFocus={false}
                 isOpen={isOpen}
-                position={Position.BOTTOM_LEFT}
+                placement={popoverProps.position || popoverProps.placement ? undefined : "bottom-start"}
                 {...popoverProps}
                 className={classNames(listProps.className, popoverProps.className)}
                 content={
@@ -197,7 +196,7 @@ export class Suggest2<T> extends AbstractPureComponent2<Suggest2Props<T>, Sugges
                 onInteraction={this.handlePopoverInteraction}
                 onOpened={this.handlePopoverOpened}
                 onOpening={this.handlePopoverOpening}
-                popoverClassName={classNames(Classes.SELECT_POPOVER, popoverProps.popoverClassName)}
+                popoverClassName={classNames(Classes.SUGGEST_POPOVER, popoverProps.popoverClassName)}
                 popupKind={PopupKind.LISTBOX}
                 ref={popoverRef}
                 renderTarget={this.getPopoverTargetRenderer(listProps, isOpen)}

packages/select/test/multiSelect2Tests.tsx

@@ -51,7 +51,14 @@ describe("<MultiSelect2>", () => {
     });
 
     selectComponentSuite<MultiSelect2Props<IFilm>, MultiSelect2State>(props =>
-        mount(<MultiSelect2 {...props} popoverProps={{ isOpen: true, usePortal: false }} tagRenderer={renderTag} />),
+        mount(
+            <MultiSelect2
+                selectedItems={[]}
+                {...props}
+                popoverProps={{ isOpen: true, usePortal: false }}
+                tagRenderer={renderTag}
+            />,
+        ),
     );
 
     it("placeholder can be controlled with placeholder prop", () => {
@@ -76,12 +83,6 @@ describe("<MultiSelect2>", () => {
         assert.equal(wrapper.find(Tag).find("strong").length, 1);
     });
 
-    // N.B. this is not good behavior, we shouldn't support this since the component is controlled.
-    // we keep it around for backcompat but expect that nobody actually uses the component this way.
-    it("selectedItems is optional", () => {
-        assert.doesNotThrow(() => multiselect({ selectedItems: undefined }));
-    });
-
     it("only triggers QueryList key up events when focus is on TagInput's <input>", () => {
         const itemSelectSpy = sinon.spy();
         const wrapper = multiselect({