Select: use shouldReturnFocusOnClose

packages/core/test/popover/popoverTests.tsx

@@ -255,8 +255,8 @@ describe("<Popover>", () => {
         it("moves focus to overlay when opened", done => {
             function handleOpened() {
                 assert.notEqual(document.activeElement, document.body, "body element should not have focus");
-                assert.isTrue(
-                    document.activeElement?.closest(`.${Classes.OVERLAY}`) !== null,
+                assert.isNotNull(
+                    document.activeElement?.closest(`.${Classes.OVERLAY}`),
                     "focus should be inside overlay",
                 );
                 done();
@@ -271,10 +271,7 @@ describe("<Popover>", () => {
             function handleClosed(wrapper2: PopoverWrapper) {
                 wrapper2.assertIsOpen(false);
                 assert.notEqual(document.activeElement, document.body, "body element should not have focus");
-                assert.isTrue(
-                    document.activeElement?.closest(`.${targetClassName}`) != null,
-                    "focus should be on target",
-                );
+                assert.isNotNull(document.activeElement?.closest(`.${targetClassName}`), "focus should be on target");
             }
 
             wrapper = renderPopover(commonProps);

packages/select/src/common/selectPopoverProps.ts

@@ -36,7 +36,12 @@ export interface SelectPopoverProps {
      * N.B. `disabled` is supported here, as this can be distinct from disabling the entire select button / input
      * control element. There are some cases where we only want to disable the popover interaction.
      */
-    popoverProps?: Partial<Omit<PopoverProps, "content" | "defaultIsOpen" | "fill" | "renderTarget">>;
+    popoverProps?: Partial<
+        Omit<
+            PopoverProps,
+            "content" | "defaultIsOpen" | "fill" | "popupKind" | "ref" | "renderTarget" | "shouldReturnFocusOnClose"
+        >
+    >;
 
     /**
      * Optional ref for the Popover component instance.

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

@@ -122,8 +122,6 @@ export class Select<T> extends AbstractPureComponent<SelectProps<T>, SelectState
 
     private queryList: QueryList<T> | null = null;
 
-    private previousFocusedElement: HTMLElement | undefined;
-
     private handleInputRef: React.Ref<HTMLInputElement> = refHandler(
         this,
         "inputElement",
@@ -212,6 +210,7 @@ export class Select<T> extends AbstractPureComponent<SelectProps<T>, SelectState
                 popupKind={PopupKind.LISTBOX}
                 ref={popoverRef}
                 renderTarget={this.getPopoverTargetRenderer(listProps, this.state.isOpen)}
+                shouldReturnFocusOnClose={true}
             />
         );
     };
@@ -314,9 +313,6 @@ export class Select<T> extends AbstractPureComponent<SelectProps<T>, SelectState
     };
 
     private handlePopoverOpening = (node: HTMLElement) => {
-        // save currently focused element before popover steals focus, so we can restore it when closing.
-        this.previousFocusedElement = (Utils.getActiveElement(this.inputElement) as HTMLElement | null) ?? undefined;
-
         if (this.props.resetOnClose) {
             this.resetQuery();
         }
@@ -342,16 +338,6 @@ export class Select<T> extends AbstractPureComponent<SelectProps<T>, SelectState
     };
 
     private handlePopoverClosing = (node: HTMLElement) => {
-        // restore focus to saved element.
-        // timeout allows popover to begin closing and remove focus handlers beforehand.
-        /* istanbul ignore next */
-        this.requestAnimationFrame(() => {
-            if (this.previousFocusedElement !== undefined) {
-                this.previousFocusedElement.focus();
-                this.previousFocusedElement = undefined;
-            }
-        });
-
         this.props.popoverProps?.onClosing?.(node);
     };
 

packages/select/test/selectTests.tsx

@@ -122,6 +122,23 @@ describe("<Select>", () => {
         assert.strictEqual(wrapper.find(Popover).prop("isOpen"), true);
     });
 
+    it("when filterable, filter input receives focus on popover open, focus is returned to target element on popover close", () => {
+        const wrapper = select({ filterable: true, popoverProps: { usePortal: false } });
+        const target = wrapper.find("[role=combobox]").hostNodes().getDOMNode<HTMLElement>();
+        target.focus();
+
+        assert.strictEqual(wrapper.find(Popover).prop("isOpen"), false);
+        findTargetButton(wrapper).simulate("keydown", { key: "ArrowDown" });
+        assert.strictEqual(wrapper.find(Popover).prop("isOpen"), true);
+
+        const filterInput = wrapper.find("input");
+        assert.equal(document.activeElement, filterInput.hostNodes().getDOMNode());
+        filterInput.simulate("keydown", { key: "Esc" });
+        assert.strictEqual(wrapper.find(Popover).prop("isOpen"), false);
+
+        assert.equal(document.activeElement, target);
+    });
+
     it("invokes onItemSelect when clicking first MenuItem", () => {
         const wrapper = select();
         // N.B. need to trigger interaction on nested <a> element, where item onClick is actually attached to the DOM