[select] fix: ItemRendererProps type regression

packages/core/src/common/props.ts

@@ -65,23 +65,28 @@ export type IntentProps = IIntentProps;
  * Interface for a clickable action, such as a button or menu item.
  * These props can be spready directly to a `<Button>` or `<MenuItem>` element.
  *
- * @deprecated use ActionProps
+ * @template T type of the DOM element rendered by this component
  */
-export interface IActionProps extends IntentProps, Props {
+export interface ActionProps<T extends HTMLElement = HTMLElement> extends IntentProps, Props {
     /** Whether this action is non-interactive. */
     disabled?: boolean;
 
     /** Name of a Blueprint UI icon (or an icon element) to render before the text. */
     icon?: IconName | MaybeElement;
 
     /** Click event handler. */
-    onClick?: (event: React.MouseEvent<HTMLElement>) => void;
+    onClick?: (event: React.MouseEvent<T>) => void;
+
+    /** Focus event handler. */
+    onFocus?: (event: React.FocusEvent<T>) => void;
 
     /** Action text. Can be any single React renderable. */
     text?: React.ReactNode;
 }
-// eslint-disable-next-line deprecation/deprecation
-export type ActionProps = IActionProps;
+/**
+ * @deprecated use ActionProps
+ */
+export type IActionProps = ActionProps;
 
 /**
  * Interface for a link, with support for customizing target window.

packages/core/src/components/menu/menuItem.tsx

@@ -29,7 +29,7 @@ import { Menu, MenuProps } from "./menu";
 
 export type MenuItemProps = IMenuItemProps;
 /** @deprecated use MenuItemProps */
-export interface IMenuItemProps extends ActionProps, LinkProps, IElementRefProps<HTMLLIElement> {
+export interface IMenuItemProps extends ActionProps<HTMLAnchorElement>, LinkProps, IElementRefProps<HTMLLIElement> {
     /** Item text, required for usability. */
     text: React.ReactNode;
 

packages/docs-app/src/examples/select-examples/multiSelectExample.tsx

@@ -16,9 +16,9 @@
 
 import * as React from "react";
 
-import { Code, H5, Intent, MenuItem, Switch, TagProps } from "@blueprintjs/core";
+import { Code, H5, Intent, Switch, TagProps } from "@blueprintjs/core";
 import { Example, ExampleProps } from "@blueprintjs/docs-theme";
-import { Popover2 } from "@blueprintjs/popover2";
+import { MenuItem2, Popover2 } from "@blueprintjs/popover2";
 import { ItemRenderer, MultiSelect2 } from "@blueprintjs/select";
 import {
     areFilmsEqual,
@@ -105,7 +105,7 @@ export class MultiSelectExample extends React.PureComponent<ExampleProps, IMulti
         });
 
         const initialContent = this.state.hasInitialContent ? (
-            <MenuItem disabled={true} text={`${TOP_100_FILMS.length} items loaded.`} roleStructure="listoption" />
+            <MenuItem2 disabled={true} text={`${TOP_100_FILMS.length} items loaded.`} roleStructure="listoption" />
         ) : // explicit undefined (not null) for default behavior (show full list)
         undefined;
 
@@ -121,7 +121,7 @@ export class MultiSelectExample extends React.PureComponent<ExampleProps, IMulti
                     items={this.state.items}
                     itemsEqual={areFilmsEqual}
                     menuProps={{ "aria-label": "films" }}
-                    noResults={<MenuItem disabled={true} text="No results." roleStructure="listoption" />}
+                    noResults={<MenuItem2 disabled={true} text="No results." roleStructure="listoption" />}
                     onClear={this.state.showClearButton ? this.handleClear : undefined}
                     onItemSelect={this.handleFilmSelect}
                     onItemsPaste={this.handleFilmsPaste}
@@ -231,7 +231,7 @@ export class MultiSelectExample extends React.PureComponent<ExampleProps, IMulti
         }
 
         return (
-            <MenuItem
+            <MenuItem2
                 {...getFilmItemProps(film, props)}
                 selected={this.isFilmSelected(film)}
                 shouldDismissPopover={false}

packages/popover2/src/menuItem2.tsx

@@ -35,7 +35,7 @@ import * as Classes from "./classes";
 import { Popover2, Popover2Props } from "./popover2";
 
 // eslint-disable-next-line deprecation/deprecation
-export interface MenuItem2Props extends ActionProps, LinkProps, IElementRefProps<HTMLLIElement> {
+export interface MenuItem2Props extends ActionProps<HTMLAnchorElement>, LinkProps, IElementRefProps<HTMLLIElement> {
     /** Item text, required for usability. */
     text: React.ReactNode;
 

packages/select/src/__examples__/filmSelect.tsx

@@ -16,7 +16,8 @@
 
 import * as React from "react";
 
-import { Button, MenuItem } from "@blueprintjs/core";
+import { Button } from "@blueprintjs/core";
+import { MenuItem2 } from "@blueprintjs/popover2";
 
 import type { ItemRenderer } from "../common";
 import { Select2, Select2Props } from "../components/select/select2";
@@ -65,7 +66,7 @@ export function FilmSelect({ allowCreate = false, fill, ...restProps }: FilmSele
             if (!props.modifiers.matchesPredicate) {
                 return null;
             }
-            return <MenuItem {...getFilmItemProps(film, props)} selected={film === selectedFilm} />;
+            return <MenuItem2 {...getFilmItemProps(film, props)} selected={film === selectedFilm} />;
         },
         [selectedFilm],
     );
@@ -80,7 +81,7 @@ export function FilmSelect({ allowCreate = false, fill, ...restProps }: FilmSele
             items={items}
             itemsEqual={areFilmsEqual}
             menuProps={{ "aria-label": "films" }}
-            noResults={<MenuItem disabled={true} text="No results." roleStructure="listoption" />}
+            noResults={<MenuItem2 disabled={true} text="No results." roleStructure="listoption" />}
             onItemSelect={handleItemSelect}
             {...restProps}
         >

packages/select/src/__examples__/films.tsx

@@ -16,7 +16,7 @@
 
 import * as React from "react";
 
-import { MenuItem, MenuItemProps } from "@blueprintjs/core";
+import { MenuItem2, MenuItem2Props } from "@blueprintjs/popover2";
 
 import type { ItemPredicate, ItemRenderer, ItemRendererProps } from "../common";
 
@@ -141,7 +141,7 @@ export const TOP_100_FILMS: Film[] = [
 export function getFilmItemProps(
     film: Film,
     { handleClick, handleFocus, modifiers, ref, query }: ItemRendererProps,
-): MenuItemProps & React.Attributes & React.HTMLAttributes<HTMLAnchorElement> {
+): MenuItem2Props & React.Attributes {
     return {
         active: modifiers.active,
         disabled: modifiers.disabled,
@@ -162,7 +162,7 @@ export const renderFilm: ItemRenderer<Film> = (film, props) => {
     if (!props.modifiers.matchesPredicate) {
         return null;
     }
-    return <MenuItem {...getFilmItemProps(film, props)} />;
+    return <MenuItem2 {...getFilmItemProps(film, props)} />;
 };
 
 /**
@@ -173,7 +173,7 @@ export const renderCreateFilmMenuItem = (
     active: boolean,
     handleClick: React.MouseEventHandler<HTMLElement>,
 ) => (
-    <MenuItem
+    <MenuItem2
         icon="add"
         text={`Create "${query}"`}
         roleStructure="listoption"
@@ -191,7 +191,7 @@ export const renderCreateFilmsMenuItem = (
     active: boolean,
     handleClick: React.MouseEventHandler<HTMLElement>,
 ) => (
-    <MenuItem
+    <MenuItem2
         icon="add"
         text={`Create ${printReadableList(query)}`}
         roleStructure="listoption"

packages/select/src/common/itemRenderer.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { MouseEventHandler, Ref } from "react";
+import * as React from "react";
 
 /** @deprecated use ItemModifiers */
 export type IItemModifiers = ItemModifiers;
@@ -36,18 +36,28 @@ export type IItemRendererProps = ItemRendererProps;
 /**
  * An object describing how to render a particular item.
  * An `itemRenderer` receives the item as its first argument, and this object as its second argument.
+ *
+ * Make sure to forward the provided `ref` to the rendered element (usually via the `elementRef` prop on `MenuItem`/`MenuItem2`)
+ * to ensure that scrolling to active items works correctly.
+ *
+ * @template T type of the DOM element to attach a ref to, usually the component's root container (defaults to MenuItem's HTMLLIElement)
+ * @template U type of the DOM element to attach a click handler to, usually the component's root container (defaults to MenuItem's HTMLAnchorElement)
  */
-export interface ItemRendererProps {
-    /** A ref that receives the native HTML element rendered by this item. */
-    ref: Ref<any>;
+export interface ItemRendererProps<T extends HTMLElement = HTMLLIElement, U extends HTMLElement = HTMLAnchorElement> {
+    /**
+     * A ref attached the native HTML element rendered by this item.
+     *
+     * N.B. this is optional to preserve backwards-compatibilty with @blueprintjs/select version < 4.9.0
+     */
+    ref?: React.Ref<T>;
 
     /** Click event handler to select this item. */
-    handleClick: MouseEventHandler<HTMLElement>;
+    handleClick: React.MouseEventHandler<U>;
 
     /**
      * Focus event handler to set this as the "active" item.
      *
-     * N.B. this is optional to preserve back-compat; it will become required in the next major version.
+     * N.B. this is optional to preserve backwards-compatibility with @blueprintjs/select version < 4.2.0
      */
     handleFocus?: () => void;
 
@@ -60,5 +70,9 @@ export interface ItemRendererProps {
     query: string;
 }
 
-/** Type alias for a function that receives an item and props and renders a JSX element (or `null`). */
+/**
+ * Type alias for a function that receives an item and props and renders a JSX element (or `null`).
+ *
+ * @template T list item data type
+ */
 export type ItemRenderer<T> = (item: T, itemProps: ItemRendererProps) => JSX.Element | null;

packages/select/src/components/select/select2.md

@@ -278,8 +278,15 @@ const FilmSelect: React.FC = () => (
 @### Item renderer
 
 `Select2`'s `itemRenderer` will be called for each item and receives the item and a props object containing data specific
-to rendering this item in this frame. The renderer is called for all items, so don't forget to respect
-`modifiers.matchesPredicate` to hide items that don't match the predicate. Also, don't forget to define a `key` for each item, or face React's console wrath!
+to rendering this item in this frame.
+
+A few things to keep in mind:
+
+-   The renderer is called for all items, so don't forget to respect `modifiers.matchesPredicate` to hide items which
+    do not match the predicate.
+-   Make sure to forward the provided `ref` to the rendered element (usually via the `elementRef` prop on
+    `MenuItem` or `MenuItem2`) to ensure that scrolling to active items works correctly.
+-   Also, don't forget to define a `key` for each item, or face React's console wrath!
 
 ```tsx
 import { Classes } from "@blueprintjs/core";

packages/select/test/index.ts

@@ -11,6 +11,7 @@ import "@blueprintjs/popover2/lib/css/blueprint-popover2.css";
 // tslint:enable no-submodule-imports
 import "../lib/css/blueprint-select.css";
 
+import "./itemRendererTests";
 import "./listItemsPropsTests";
 import "./multiSelectTests";
 import "./multiSelect2Tests";

packages/select/test/itemRendererTests.tsx

@@ -0,0 +1,87 @@
+/*
+ * Copyright 2023 Palantir Technologies, Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { shallow } from "enzyme";
+import * as React from "react";
+
+import { Menu } from "@blueprintjs/core";
+import { MenuItem2 } from "@blueprintjs/popover2";
+
+import { ItemRenderer, ItemRendererProps } from "../src";
+import { Film, TOP_100_FILMS } from "../src/__examples__";
+
+describe("ItemRenderer", () => {
+    // N.B. don't use `renderFilm` here from the src/__examples__ directory, since we are specifically trying to
+    // test the ergonomics and type definitions of the item renderer API by defining custom renderers.
+    describe("allows defining basic item renderers", () => {
+        const myItemRenderer: ItemRenderer<Film> = (item, { ref, modifiers, handleClick, handleFocus }) => {
+            return (
+                <MenuItem2
+                    active={modifiers.active}
+                    elementRef={ref}
+                    key={item.title}
+                    onClick={handleClick}
+                    onFocus={handleFocus}
+                    text={item.title}
+                />
+            );
+        };
+
+        it("without ref prop", () => {
+            function getItemProps(_item: Film): ItemRendererProps {
+                return {
+                    handleClick: (_event: React.MouseEvent<HTMLElement>) => {
+                        /* noop */
+                    },
+                    modifiers: {
+                        active: false,
+                        disabled: false,
+                        matchesPredicate: true,
+                    },
+                    query: "",
+                };
+            }
+            const MyList: React.FC = () => {
+                return <Menu>{TOP_100_FILMS.map(item => myItemRenderer(item, getItemProps(item)))}</Menu>;
+            };
+            shallow(<MyList />);
+        });
+
+        it("with ref prop", () => {
+            function getItemProps(_item: Film): ItemRendererProps {
+                const ref = React.createRef<HTMLLIElement>();
+                return {
+                    // @ts-expect-error -- extra properties should not be allowed
+                    blah: "foo",
+                    handleClick: (_event: React.MouseEvent<HTMLElement>) => {
+                        /* noop */
+                    },
+                    modifiers: {
+                        active: false,
+                        disabled: false,
+                        matchesPredicate: true,
+                    },
+                    query: "",
+                    ref,
+                };
+            }
+            const MyList: React.FC = () => {
+                return <Menu>{TOP_100_FILMS.map(item => myItemRenderer(item, getItemProps(item)))}</Menu>;
+            };
+            shallow(<MyList />);
+        });
+    });
+});

packages/select/test/suggest2Tests.tsx

@@ -19,8 +19,8 @@ import { mount, ReactWrapper } from "enzyme";
 import * as React from "react";
 import * as sinon from "sinon";
 
-import { InputGroup, Keys, MenuItem } from "@blueprintjs/core";
-import { Popover2, Popover2Props } from "@blueprintjs/popover2";
+import { InputGroup, Keys } from "@blueprintjs/core";
+import { MenuItem2, Popover2, Popover2Props } from "@blueprintjs/popover2";
 
 import { ItemRendererProps, QueryList } from "../src";
 import { Film, renderFilm, TOP_100_FILMS } from "../src/__examples__";
@@ -78,7 +78,7 @@ describe("Suggest2", () => {
             const popover = wrapper.find(Popover2);
             assert.lengthOf(wrapper.find(InputGroup), 1, "should render InputGroup");
             assert.lengthOf(popover, 1, "should render Popover2");
-            assert.lengthOf(popover.find(MenuItem), 100, "should render 100 items in popover");
+            assert.lengthOf(popover.find(MenuItem2), 100, "should render 100 items in popover");
         });
 
         describe("when ESCAPE key pressed", () => {

packages/select/test/suggestTests.tsx

@@ -26,7 +26,8 @@ import { mount, ReactWrapper } from "enzyme";
 import * as React from "react";
 import * as sinon from "sinon";
 
-import { InputGroup, IPopoverProps, Keys, MenuItem, Popover } from "@blueprintjs/core";
+import { InputGroup, IPopoverProps, Keys, Popover } from "@blueprintjs/core";
+import { MenuItem2 } from "@blueprintjs/popover2";
 
 import { ItemRendererProps, QueryList } from "../src";
 import { Film, renderFilm, TOP_100_FILMS } from "../src/__examples__";
@@ -71,7 +72,7 @@ describe("Suggest", () => {
             const popover = wrapper.find(Popover);
             assert.lengthOf(wrapper.find(InputGroup), 1, "should render InputGroup");
             assert.lengthOf(popover, 1, "should render Popover");
-            assert.lengthOf(popover.find(MenuItem), 100, "should render 100 items in popover");
+            assert.lengthOf(popover.find(MenuItem2), 100, "should render 100 items in popover");
         });
 
         describe("when ESCAPE key pressed", () => {