[select] feat(Suggest): sync activeItem with selectedItem on popover close (#3934)

Author: skurger

packages/select/src/components/query-list/queryList.tsx

@@ -30,6 +30,12 @@ import {
 } from "../../common";
 
 export interface IQueryListProps<T> extends IListItemsProps<T> {
+    /**
+     * Initial active item, useful if the parent component is controlling its selectedItem but
+     * not activeItem.
+     */
+    initialActiveItem?: T;
+
     /**
      * Callback invoked when user presses a key, after processing `QueryList`'s own key events
      * (up/down to navigate active item). This callback is passed to `renderer` and (along with
@@ -171,7 +177,7 @@ export class QueryList<T> extends AbstractComponent2<IQueryListProps<T>, IQueryL
             activeItem:
                 props.activeItem !== undefined
                     ? props.activeItem
-                    : getFirstEnabledItem(filteredItems, props.itemDisabled),
+                    : props.initialActiveItem ?? getFirstEnabledItem(filteredItems, props.itemDisabled),
             createNewItem,
             filteredItems,
             query,
@@ -289,6 +295,21 @@ export class QueryList<T> extends AbstractComponent2<IQueryListProps<T>, IQueryL
         }
     }
 
+    public setActiveItem(activeItem: T | ICreateNewItem | null) {
+        this.expectedNextActiveItem = activeItem;
+        if (this.props.activeItem === undefined) {
+            // indicate that the active item may need to be scrolled into view after update.
+            this.shouldCheckActiveItemInViewport = true;
+            this.setState({ activeItem });
+        }
+
+        if (isCreateNewItem(activeItem)) {
+            Utils.safeInvoke(this.props.onActiveItemChange, null, true);
+        } else {
+            Utils.safeInvoke(this.props.onActiveItemChange, activeItem, false);
+        }
+    }
+
     /** default `itemListRenderer` implementation */
     private renderItemList = (listProps: IItemListRendererProps<T>) => {
         const { initialContent, noResults } = this.props;
@@ -486,21 +507,6 @@ export class QueryList<T> extends AbstractComponent2<IQueryListProps<T>, IQueryL
         return getFirstEnabledItem(this.state.filteredItems, this.props.itemDisabled, direction, startIndex);
     }
 
-    private setActiveItem(activeItem: T | ICreateNewItem | null) {
-        this.expectedNextActiveItem = activeItem;
-        if (this.props.activeItem === undefined) {
-            // indicate that the active item may need to be scrolled into view after update.
-            this.shouldCheckActiveItemInViewport = true;
-            this.setState({ activeItem });
-        }
-
-        if (isCreateNewItem(activeItem)) {
-            Utils.safeInvoke(this.props.onActiveItemChange, null, true);
-        } else {
-            Utils.safeInvoke(this.props.onActiveItemChange, activeItem, false);
-        }
-    }
-
     private isCreateItemRendered(): boolean {
         return (
             this.canCreateItems() &&

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

@@ -125,10 +125,10 @@ export class Suggest<T> extends React.PureComponent<ISuggestProps<T>, ISuggestSt
     public render() {
         // omit props specific to this component, spread the rest.
         const { disabled, inputProps, popoverProps, ...restProps } = this.props;
-
         return (
             <this.TypedQueryList
                 {...restProps}
+                initialActiveItem={this.props.selectedItem ?? undefined}
                 onItemSelect={this.handleItemSelect}
                 ref={this.refHandlers.queryList}
                 renderer={this.renderQueryList}
@@ -142,6 +142,14 @@ export class Suggest<T> extends React.PureComponent<ISuggestProps<T>, ISuggestSt
             this.setState({ selectedItem: this.props.selectedItem });
         }
 
+        if (this.state.isOpen === false && prevState.isOpen === true) {
+            // just closed, likely by keyboard interaction
+            // wait until the transition ends so there isn't a flash of content in the popover
+            setTimeout(() => {
+                this.maybeResetActiveItemToSelectedItem();
+            }, this.props.popoverProps?.transitionDuration ?? Popover.defaultProps.transitionDuration);
+        }
+
         if (this.state.isOpen && !prevState.isOpen && this.queryList != null) {
             this.queryList.scrollActiveItemIntoView();
         }
@@ -320,4 +328,13 @@ export class Suggest<T> extends React.PureComponent<ISuggestProps<T>, ISuggestSt
             Utils.safeInvokeMember(this.props.inputProps, "onKeyUp", evt);
         };
     };
+
+    private maybeResetActiveItemToSelectedItem() {
+        const shouldResetActiveItemToSelectedItem =
+            this.props.activeItem === undefined && this.state.selectedItem !== null && !this.props.resetOnSelect;
+
+        if (this.queryList !== null && shouldResetActiveItemToSelectedItem) {
+            this.queryList.setActiveItem(this.props.selectedItem ?? this.state.selectedItem);
+        }
+    }
 }

packages/select/test/suggestTests.tsx

@@ -22,7 +22,7 @@ import * as sinon from "sinon";
 
 import { IFilm, renderFilm, TOP_100_FILMS } from "../../docs-app/src/examples/select-examples/films";
 import { ISuggestProps, ISuggestState, Suggest } from "../src/components/select/suggest";
-import { IItemRendererProps } from "../src/index";
+import { IItemRendererProps, QueryList } from "../src/index";
 import { selectComponentSuite } from "./selectComponentSuite";
 
 describe("Suggest", () => {
@@ -99,6 +99,35 @@ describe("Suggest", () => {
             assert.strictEqual(scrollActiveItemIntoViewSpy.callCount, 1, "should call scrollActiveItemIntoView");
         });
 
+        it("sets active item to the selected item when the popover is closed", done => {
+            // transition duration shorter than timeout below to ensure it's done
+            const wrapper = suggest({ selectedItem: TOP_100_FILMS[10], popoverProps: { transitionDuration: 5 } });
+            const queryList = ((wrapper.instance() as Suggest<IFilm>) as any).queryList as QueryList<IFilm>; // private ref
+
+            assert.deepEqual(
+                queryList.state.activeItem,
+                wrapper.state().selectedItem,
+                "QueryList activeItem should be set to the controlled selectedItem if prop is provided",
+            );
+
+            simulateFocus(wrapper);
+            assert.isTrue(wrapper.state().isOpen);
+
+            const newActiveItem = TOP_100_FILMS[11];
+            queryList.setActiveItem(newActiveItem);
+            assert.deepEqual(queryList.state.activeItem, newActiveItem);
+
+            simulateKeyDown(wrapper, Keys.ESCAPE);
+            assert.isFalse(wrapper.state().isOpen);
+
+            wrapper.update();
+            wrapper.find(QueryList).update();
+            setTimeout(() => {
+                assert.deepEqual(queryList.state.activeItem, wrapper.state().selectedItem);
+                done();
+            }, 10);
+        });
+
         function checkKeyDownDoesNotOpenPopover(wrapper: ReactWrapper<any, any>, which: number) {
             simulateKeyDown(wrapper, which);
             assert.isFalse(wrapper.state().isOpen, "should not open popover");