[select] popoverTargetProps and inputProps fixes & docs (#5730)

packages/select/src/common/selectPopoverProps.ts

@@ -21,14 +21,17 @@ import type { Popover2, Popover2Props } from "@blueprintjs/popover2";
  * and need to provide some degree of customization for that popover.
  */
 export interface SelectPopoverProps {
-    /** Props to spread to `Popover2` content wrapper eleemnt. */
+    /**
+     * HTML attributes to spread to the popover content container element.
+     */
     popoverContentProps?: React.HTMLAttributes<HTMLDivElement>;
 
     /**
      * Props to spread to `Popover2`.
      *
      * Note that `content` cannot be changed, but you may apply some props to the content wrapper element
-     * with `popoverContentProps`.
+     * with `popoverContentProps`. Likewise, `targetProps` is no longer supported as it was in Blueprint v4, but you
+     * may use `popoverTargetProps` instead.
      */
     popoverProps?: Partial<Omit<Popover2Props, "content" | "defaultIsOpen" | "disabled" | "fill" | "renderTarget">>;
 
@@ -37,4 +40,9 @@ export interface SelectPopoverProps {
      * This is sometimes useful to reposition the popover.
      */
     popoverRef?: React.RefObject<Popover2<React.HTMLProps<unknown>>>;
+
+    /**
+     * HTML attributes to add to the popover target element.
+     */
+    popoverTargetProps?: React.HTMLAttributes<HTMLElement>;
 }

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

@@ -46,7 +46,6 @@ export interface MultiSelect2Props<T> extends ListItemsProps<T>, SelectPopoverPr
 
     /**
      * Whether the component should take up the full width of its container.
-     * This overrides `popoverProps.fill` and `tagInputProps.fill`.
      */
     fill?: boolean;
 
@@ -92,25 +91,22 @@ export interface MultiSelect2Props<T> extends ListItemsProps<T>, SelectPopoverPr
      */
     placeholder?: string;
 
-    /**
-     * Props to add to the `div` that wraps the TagInput
-     */
-    popoverTargetProps?: React.HTMLAttributes<HTMLDivElement>;
-
     /** Controlled selected values. */
     selectedItems: T[];
 
     /**
-     * Props to spread to `TagInput`.
-     * If you wish to control the value of the input, use `query` and `onQueryChange` instead.
+     * Props to pass to the [TagInput component](##core/components/tag-input).
+     *
+     * Some properties are unavailable:
+     * - `tagInputProps.value`: use `query` instead
+     * - `tagInputProps.onChange`: use `onQueryChange` instead
      *
      * Notes for `tagInputProps.rightElement`:
-     * - you are responsible for disabling any elements you may render here when the overall
-     *   `MultiSelect2` is disabled.
+     * - you are responsible for disabling any elements you may render here when the overall `MultiSelect2` is disabled
      * - if the `onClear` prop is defined, this element will override/replace the default rightElement,
      *   which is a "clear" button that removes all items from the current selection.
      */
-    tagInputProps?: Partial<TagInputProps>;
+    tagInputProps?: Partial<Omit<TagInputProps, "value" | "onChange">>;
 
     /** Custom renderer to transform an item into tag content. */
     tagRenderer: (item: T) => React.ReactNode;
@@ -385,6 +381,8 @@ export class MultiSelect2<T> extends AbstractPureComponent2<MultiSelect2Props<T>
             if (this.state.isOpen && !isTargetingTagRemoveButton) {
                 handleQueryListKeyDown?.(e);
             }
+
+            this.props.popoverTargetProps?.onKeyDown?.(e);
         };
     };
 
@@ -397,6 +395,8 @@ export class MultiSelect2<T> extends AbstractPureComponent2<MultiSelect2Props<T>
             if (this.state.isOpen && isTargetingInput) {
                 handleQueryListKeyUp?.(e);
             }
+
+            this.props.popoverTargetProps?.onKeyDown?.(e);
         };
     };
 

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

@@ -54,8 +54,7 @@ export interface Select2Props<T> extends ListItemsProps<T>, SelectPopoverProps {
 
     /**
      * Whether the component should take up the full width of its container.
-     * This overrides `popoverProps.fill`. You also have to ensure that the child
-     * component has `fill` set to `true` or is styled appropriately.
+     * You also have to ensure that the child component has `fill` set to `true` or is styled appropriately.
      */
     fill?: boolean;
 
@@ -68,22 +67,19 @@ export interface Select2Props<T> extends ListItemsProps<T>, SelectPopoverProps {
     filterable?: boolean;
 
     /**
-     * Props to spread to the query `InputGroup`. Use `query` and
-     * `onQueryChange` instead of `inputProps.value` and `inputProps.onChange`
-     * to control this input.
+     * Props to pass to the query [InputGroup component](#core/components/text-inputs.input-group).
+     *
+     * Some properties are unavailable:
+     * - `inputProps.value`: use `query` instead
+     * - `inputProps.onChange`: use `onQueryChange` instead
      */
-    inputProps?: InputGroupProps2;
+    inputProps?: Partial<Omit<InputGroupProps2, "value" | "onChange">>;
 
     /**
-     * Props to spread to the `Menu` listbox containing the selectable options.
+     * HTML attributes to add to the `Menu` listbox containing the selectable options.
      */
     menuProps?: React.HTMLAttributes<HTMLUListElement>;
 
-    /**
-     * Props to add to the popover target wrapper element.
-     */
-    popoverTargetProps?: React.HTMLAttributes<HTMLDivElement>;
-
     /**
      * Whether the active item should be reset to the first matching item _when
      * the popover closes_. The query will also be reset to the empty string.
@@ -230,8 +226,11 @@ export class Select2<T> extends AbstractPureComponent2<Select2Props<T>, Select2S
                     // Normally, Popover2 would also need to attach its own `onKeyDown` handler via `targetProps`,
                     // but in our case we fully manage that interaction and listen for key events to open/close
                     // the popover, so we elide it from the DOM.
-                    onKeyDown: isOpen ? handleKeyDown : this.handleTargetKeyDown,
-                    onKeyUp: isOpen ? handleKeyUp : undefined,
+                    onKeyDown: this.withPopoverTargetPropsHandler(
+                        "keydown",
+                        isOpen ? handleKeyDown : this.handleTargetKeyDown,
+                    ),
+                    onKeyUp: this.withPopoverTargetPropsHandler("keyup", isOpen ? handleKeyUp : undefined),
                     ref,
                     role: "combobox",
                 },
@@ -251,6 +250,24 @@ export class Select2<T> extends AbstractPureComponent2<Select2Props<T>, Select2S
         ) : undefined;
     }
 
+    private withPopoverTargetPropsHandler = (
+        eventType: "keydown" | "keyup",
+        handler: React.KeyboardEventHandler<HTMLElement> | undefined,
+    ): React.KeyboardEventHandler<HTMLElement> => {
+        switch (eventType) {
+            case "keydown":
+                return event => {
+                    handler?.(event);
+                    this.props.popoverTargetProps?.onKeyDown?.(event);
+                };
+            case "keyup":
+                return event => {
+                    handler?.(event);
+                    this.props.popoverTargetProps?.onKeyUp?.(event);
+                };
+        }
+    };
+
     /**
      * Target wrapper element "keydown" handler while the popover is closed.
      */

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

@@ -35,7 +35,7 @@ import { Popover2, Popover2TargetProps, PopupKind } from "@blueprintjs/popover2"
 import { Classes, ListItemsProps, SelectPopoverProps } from "../../common";
 import { QueryList, QueryListRendererProps } from "../query-list/queryList";
 
-export interface Suggest2Props<T> extends ListItemsProps<T>, SelectPopoverProps {
+export interface Suggest2Props<T> extends ListItemsProps<T>, Omit<SelectPopoverProps, "popoverTargetProps"> {
     /**
      * Whether the popover should close after selecting an item.
      *
@@ -48,7 +48,6 @@ export interface Suggest2Props<T> extends ListItemsProps<T>, SelectPopoverProps
 
     /**
      * Whether the component should take up the full width of its container.
-     * This overrides `popoverProps.fill` and `inputProps.fill`.
      */
     fill?: boolean;
 
@@ -59,10 +58,13 @@ export interface Suggest2Props<T> extends ListItemsProps<T>, SelectPopoverProps
      * - `inputProps.value`: use `query` instead
      * - `inputProps.onChange`: use `onQueryChange` instead
      * - `inputProps.disabled`: use `disabled` instead
+     * - `inputProps.fill`: use `fill` instead
      *
-     * Note that `inputProps.tagName` will override `popoverProps.targetTagName`.
+     * Other notes:
+     * - `inputProps.tagName` will override `popoverProps.targetTagName`
+     * - `inputProps.className` will work as expected, but this is redundant with the simpler `className` prop
      */
-    inputProps?: Partial<Omit<InputGroupProps2, "disabled" | "value" | "onChange">>;
+    inputProps?: Partial<Omit<InputGroupProps2, "disabled" | "fill" | "value" | "onChange">>;
 
     /** Custom renderer to transform an item into a string for the input value. */
     inputValueRenderer: (item: T) => string;
@@ -81,7 +83,7 @@ export interface Suggest2Props<T> extends ListItemsProps<T>, SelectPopoverProps
     selectedItem?: T | null;
 
     /**
-     * Props to spread to the `Menu` listbox containing the selectable options.
+     * HTML attributes to add to the `Menu` listbox containing the selectable options.
      */
     menuProps?: React.HTMLAttributes<HTMLUListElement>;
 
@@ -225,7 +227,7 @@ export class Suggest2<T> extends AbstractPureComponent2<Suggest2Props<T>, Sugges
         ({
             // pull out `isOpen` so that it's not forwarded to the DOM
             isOpen: _isOpen,
-            // pull out `defaultValue` due to type incompatibility with InputGroup.
+            // pull out `defaultValue` due to type incompatibility with InputGroup
             defaultValue,
             ref,
             ...targetProps
@@ -252,6 +254,7 @@ export class Suggest2<T> extends AbstractPureComponent2<Suggest2Props<T>, Sugges
                     {...inputProps}
                     aria-autocomplete="list"
                     aria-expanded={isOpen}
+                    className={classNames(targetProps.className, inputProps.className)}
                     fill={fill}
                     inputRef={mergeRefs(this.handleInputRef, ref)}
                     onChange={listProps.handleQueryChange}

packages/select/test/select2Tests.tsx

@@ -94,6 +94,7 @@ describe("<Select2>", () => {
 
     it("inputProps value and onChange are ignored", () => {
         const inputProps = { value: "nailed it", onChange: sinon.spy() };
+        // @ts-expect-error - value and onChange are now omitted from the props type
         const input = select({ inputProps }).find("input");
         assert.notEqual(input.prop("onChange"), inputProps.onChange);
         assert.notEqual(input.prop("value"), inputProps.value);