chore: small improvements to shared ref handler logic (#4475)

Follow-up to #4438

packages/core/src/common/refs.ts

@@ -39,6 +39,9 @@ export function getRef<T = HTMLElement>(ref: T | IRefObject<T> | null) {
     return (ref as IRefObject<T>).current ?? (ref as T);
 }
 
+/**
+ * Assign the given ref to a target, either a React ref object or a callback which takes the ref as its first argument.
+ */
 export function setRef<T extends HTMLElement>(refTarget: IRef<T>, ref: T | null) {
     if (isRefObject<T>(refTarget)) {
         refTarget.current = ref;
@@ -48,10 +51,25 @@ export function setRef<T extends HTMLElement>(refTarget: IRef<T>, ref: T | null)
     }
 }
 
+/**
+ * Creates a ref handler which assigns the ref returned by React for a mounted component to a field on the target object.
+ * The target object is usually a component class.
+ *
+ * If provided, it will also update the given `refProp` with the value of the ref.
+ */
+export function refHandler<T extends HTMLElement, K extends string>(
+    refTargetParent: { [k in K]: T | null },
+    refTargetKey: K,
+): IRefCallback<T>;
 export function refHandler<T extends HTMLElement, K extends string>(
+    refTargetParent: { [k in K]: T | IRefObject<T> | null },
+    refTargetKey: K,
     refProp: IRef<T> | undefined | null,
+): IRef<T>;
+export function refHandler<T extends HTMLElement, K extends string>(
     refTargetParent: { [k in K]: T | IRefObject<T> | null },
     refTargetKey: K,
+    refProp?: IRef<T> | undefined | null,
 ) {
     if (isRefObject<T>(refProp)) {
         refTargetParent[refTargetKey] = refProp;
@@ -64,7 +82,3 @@ export function refHandler<T extends HTMLElement, K extends string>(
         }
     };
 }
-
-export interface IRefHandlerContainer<T extends HTMLElement> {
-    [key: string]: IRef<T>;
-}

packages/core/src/components/button/abstractButton.tsx

@@ -32,7 +32,9 @@ import {
 import { Icon, IconName } from "../icon/icon";
 import { Spinner } from "../spinner/spinner";
 
-export interface IButtonProps extends IActionProps, IElementRefProps<any> {
+export interface IButtonProps<E extends HTMLButtonElement | HTMLAnchorElement = HTMLButtonElement>
+    extends IActionProps,
+        IElementRefProps<E> {
     /**
      * If set to `true`, the button will display in an active state.
      * This is equivalent to setting `className={Classes.ACTIVE}`.
@@ -90,8 +92,11 @@ export interface IButtonState {
     isActive: boolean;
 }
 
-export abstract class AbstractButton<H extends React.HTMLAttributes<HTMLElement>> extends AbstractPureComponent2<
-    IButtonProps & H,
+export abstract class AbstractButton<E extends HTMLButtonElement | HTMLAnchorElement> extends AbstractPureComponent2<
+    IButtonProps<E> &
+        (E extends HTMLButtonElement
+            ? React.ButtonHTMLAttributes<HTMLButtonElement>
+            : React.AnchorHTMLAttributes<HTMLAnchorElement>),
     IButtonState
 > {
     public state = {

packages/core/src/components/button/buttons.tsx

@@ -25,13 +25,13 @@ import { AbstractButton, IButtonProps } from "./abstractButton";
 
 export { IButtonProps };
 
-export class Button extends AbstractButton<React.ButtonHTMLAttributes<HTMLButtonElement>> {
+export class Button extends AbstractButton<HTMLButtonElement> {
     public static displayName = `${DISPLAYNAME_PREFIX}.Button`;
 
     // need to keep this ref so that we can access it in AbstractButton#handleKeyUp
     public buttonRef: HTMLButtonElement | IRefObject<HTMLButtonElement> | null = null;
 
-    protected handleRef: IRef<HTMLButtonElement> = refHandler(this.props.elementRef, this, "buttonRef");
+    protected handleRef: IRef<HTMLButtonElement> = refHandler(this, "buttonRef", this.props.elementRef);
 
     public render() {
         return (
@@ -47,13 +47,13 @@ export class Button extends AbstractButton<React.ButtonHTMLAttributes<HTMLButton
     }
 }
 
-export class AnchorButton extends AbstractButton<React.AnchorHTMLAttributes<HTMLAnchorElement>> {
+export class AnchorButton extends AbstractButton<HTMLAnchorElement> {
     public static displayName = `${DISPLAYNAME_PREFIX}.AnchorButton`;
 
     // need to keep this ref so that we can access it in AbstractButton#handleKeyUp
     public buttonRef: HTMLAnchorElement | IRefObject<HTMLAnchorElement> | null = null;
 
-    protected handleRef: IRef<HTMLAnchorElement> = refHandler(this.props.elementRef, this, "buttonRef");
+    protected handleRef: IRef<HTMLAnchorElement> = refHandler(this, "buttonRef", this.props.elementRef);
 
     public render() {
         const { href, tabIndex = 0 } = this.props;

packages/core/src/components/dialog/multistepDialog.tsx

@@ -22,7 +22,7 @@ import { AbstractPureComponent2, Classes, Utils } from "../../common";
 import { DISPLAYNAME_PREFIX } from "../../common/props";
 import { Button, IButtonProps } from "../button/buttons";
 import { Dialog, IDialogProps } from "./dialog";
-import { IDialogStepProps, DialogStep, DialogStepId } from "./dialogStep";
+import { DialogStep, DialogStepId, IDialogStepProps } from "./dialogStep";
 
 type DialogStepElement = React.ReactElement<IDialogStepProps & { children: React.ReactNode }>;
 

packages/core/src/components/forms/controls.tsx

@@ -247,7 +247,7 @@ export class Checkbox extends AbstractPureComponent2<ICheckboxProps, ICheckboxSt
     // must maintain internal reference for `indeterminate` support
     public input: HTMLInputElement | null = null;
 
-    private handleInputRef: IRef<HTMLInputElement> = refHandler(this.props.inputRef, this, "input");
+    private handleInputRef: IRef<HTMLInputElement> = refHandler(this, "input", this.props.inputRef);
 
     public render() {
         const { defaultIndeterminate, indeterminate, ...controlProps } = this.props;

packages/core/src/components/forms/numericInput.tsx

@@ -319,7 +319,7 @@ export class NumericInput extends AbstractPureComponent2<HTMLInputProps & INumer
 
     public inputElement: HTMLInputElement | null = null;
 
-    private inputRef: IRef<HTMLInputElement> = refHandler(this.props.inputRef, this, "inputElement");
+    private inputRef: IRef<HTMLInputElement> = refHandler(this, "inputElement", this.props.inputRef);
 
     private intervalId?: number;
 

packages/core/src/components/forms/textArea.tsx

@@ -23,7 +23,6 @@ import {
     Classes,
     getRef,
     IRef,
-    IRefHandlerContainer,
     IRefObject,
     isRefCallback,
     isRefObject,
@@ -70,19 +69,17 @@ export class TextArea extends AbstractPureComponent2<ITextAreaProps, ITextAreaSt
 
     public state: ITextAreaState = {};
 
-    // keep our own ref so that we can measure and set the height of the component on first mount
-    public textareaRef: HTMLTextAreaElement | IRefObject<HTMLTextAreaElement> | null = null;
+    // used to measure and set the height of the component on first mount
+    public textareaElement: HTMLTextAreaElement | IRefObject<HTMLTextAreaElement> | null = null;
 
-    private refHandlers: IRefHandlerContainer<HTMLTextAreaElement> = {
-        textarea: refHandler(this.props.inputRef, this, "textareaRef"),
-    };
+    private handleRef: IRef<HTMLTextAreaElement> = refHandler(this, "textareaElement", this.props.inputRef);
 
     public componentDidMount() {
-        if (this.props.growVertically && this.textareaRef !== null) {
+        if (this.props.growVertically && this.textareaElement !== null) {
             // HACKHACK: this should probably be done in getSnapshotBeforeUpdate
             /* eslint-disable-next-line react/no-did-mount-set-state */
             this.setState({
-                height: getRef(this.textareaRef)!.scrollHeight,
+                height: getRef(this.textareaElement)!.scrollHeight,
             });
         }
     }
@@ -91,10 +88,10 @@ export class TextArea extends AbstractPureComponent2<ITextAreaProps, ITextAreaSt
         const { inputRef } = this.props;
         if (prevProps.inputRef !== inputRef) {
             if (isRefObject<HTMLTextAreaElement>(inputRef)) {
-                inputRef.current = (this.textareaRef as IRefObject<HTMLTextAreaElement>).current;
-                this.textareaRef = inputRef;
+                inputRef.current = (this.textareaElement as IRefObject<HTMLTextAreaElement>).current;
+                this.textareaElement = inputRef;
             } else if (isRefCallback<HTMLTextAreaElement>(inputRef)) {
-                inputRef(this.textareaRef as HTMLTextAreaElement | null);
+                inputRef(this.textareaElement as HTMLTextAreaElement | null);
             }
         }
     }
@@ -129,7 +126,7 @@ export class TextArea extends AbstractPureComponent2<ITextAreaProps, ITextAreaSt
                 {...htmlProps}
                 className={rootClasses}
                 onChange={this.handleChange}
-                ref={this.refHandlers.textarea}
+                ref={this.handleRef}
                 style={style}
             />
         );

packages/core/src/components/popover/popover.tsx

@@ -20,7 +20,7 @@ import * as React from "react";
 import { polyfill } from "react-lifecycles-compat";
 import { Manager, Popper, PopperChildrenProps, Reference, ReferenceChildrenProps } from "react-popper";
 
-import { AbstractPureComponent2, Classes, IRef, IRefHandlerContainer, refHandler } from "../../common";
+import { AbstractPureComponent2, Classes, IRef, refHandler } from "../../common";
 import * as Errors from "../../common/errors";
 import { DISPLAYNAME_PREFIX, HTMLDivProps } from "../../common/props";
 import * as Utils from "../../common/utils";
@@ -151,10 +151,9 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
     // Reference to the Poppper.scheduleUpdate() function, this changes every time the popper is mounted
     private popperScheduleUpdate?: () => void;
 
-    private refHandlers: IRefHandlerContainer<HTMLElement> = {
-        popover: refHandler(this.props.popoverRef, this, "popoverElement"),
-        target: (ref: HTMLElement | null) => (this.targetElement = ref),
-    };
+    private handlePopoverRef: IRef<HTMLElement> = refHandler(this, "popoverElement", this.props.popoverRef);
+
+    private handleTargetRef = (ref: HTMLElement | null) => (this.targetElement = ref);
 
     public render() {
         // rename wrapper tag to begin with uppercase letter so it's recognized
@@ -182,7 +181,7 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
         const wrapper = React.createElement(
             wrapperTagName!,
             { className: wrapperClasses },
-            <Reference innerRef={this.refHandlers.target}>{this.renderTarget}</Reference>,
+            <Reference innerRef={this.handleTargetRef}>{this.renderTarget}</Reference>,
             <Overlay
                 autoFocus={this.props.autoFocus}
                 backdropClassName={Classes.POPOVER_BACKDROP}
@@ -204,7 +203,7 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
                 portalContainer={this.props.portalContainer}
             >
                 <Popper
-                    innerRef={this.refHandlers.popover}
+                    innerRef={this.handlePopoverRef}
                     placement={positionToPlacement(this.props.position!)}
                     modifiers={this.getPopperModifiers()}
                 >

packages/core/src/components/tag-input/tagInput.tsx

@@ -18,7 +18,7 @@ import classNames from "classnames";
 import * as React from "react";
 import { polyfill } from "react-lifecycles-compat";
 
-import { AbstractPureComponent2, Classes, IRefHandlerContainer, Keys, refHandler, Utils } from "../../common";
+import { AbstractPureComponent2, Classes, getRef, IRef, IRefObject, Keys, refHandler, Utils } from "../../common";
 import { DISPLAYNAME_PREFIX, HTMLInputProps, IIntentProps, IProps, MaybeElement } from "../../common/props";
 import { Icon, IconName } from "../icon/icon";
 import { ITagProps, Tag } from "../tag/tag";
@@ -222,11 +222,9 @@ export class TagInput extends AbstractPureComponent2<ITagInputProps, ITagInputSt
         isInputFocused: false,
     };
 
-    public inputElement: HTMLInputElement | null = null;
+    public inputElement: HTMLInputElement | IRefObject<HTMLInputElement> | null = null;
 
-    private refHandlers: IRefHandlerContainer<HTMLInputElement> = {
-        input: refHandler(this.props.inputRef, this, "inputElement"),
-    };
+    private handleRef: IRef<HTMLInputElement> = refHandler(this, "inputElement", this.props.inputRef);
 
     public render() {
         const { className, disabled, fill, inputProps, intent, large, leftIcon, placeholder, values } = this.props;
@@ -268,7 +266,7 @@ export class TagInput extends AbstractPureComponent2<ITagInputProps, ITagInputSt
                         onKeyUp={this.handleInputKeyUp}
                         onPaste={this.handleInputPaste}
                         placeholder={resolvedPlaceholder}
-                        ref={this.refHandlers.input}
+                        ref={this.handleRef}
                         className={classNames(Classes.INPUT_GHOST, inputProps?.className)}
                         disabled={disabled}
                     />
@@ -349,9 +347,7 @@ export class TagInput extends AbstractPureComponent2<ITagInputProps, ITagInputSt
     }
 
     private handleContainerClick = () => {
-        if (this.inputElement != null) {
-            this.inputElement.focus();
-        }
+        getRef(this.inputElement)?.focus();
     };
 
     private handleContainerBlur = ({ currentTarget }: React.FocusEvent<HTMLDivElement>) => {

packages/core/test/buttons/buttonTests.tsx

@@ -110,7 +110,7 @@ function buttonTestSuite(component: React.ComponentClass<any>, tagName: string)
 
         if (typeof React.createRef !== "undefined") {
             it("matches buttonRef with elementRef using createRef", done => {
-                const elementRef = React.createRef<HTMLElement>();
+                const elementRef = React.createRef<HTMLButtonElement>();
                 const wrapper = button({ elementRef }, true);
 
                 // wait for the whole lifecycle to run
@@ -123,13 +123,9 @@ function buttonTestSuite(component: React.ComponentClass<any>, tagName: string)
 
         it("matches buttonRef with elementRef using callback", done => {
             let elementRef: HTMLElement | null = null;
-            const buttonRefCallback = (ref: HTMLElement) => {
-                elementRef = ref;
-            };
-
             const wrapper = button(
                 {
-                    elementRef: buttonRefCallback,
+                    elementRef: (ref: HTMLButtonElement | null) => (elementRef = ref),
                 },
                 true,
             );

packages/datetime/src/dateInput.tsx

@@ -29,7 +29,7 @@ import {
     Intent,
     IPopoverProps,
     IProps,
-    IRefHandlerContainer,
+    IRefCallback,
     IRefObject,
     Keys,
     Popover,
@@ -181,17 +181,21 @@ export class DateInput extends AbstractPureComponent2<IDateInputProps, IDateInpu
         valueString: null,
     };
 
-    public inputEl: HTMLInputElement | IRefObject<HTMLInputElement> | null = null;
+    public inputElement: HTMLInputElement | IRefObject<HTMLInputElement> | null = null;
 
-    private popoverContentEl: HTMLElement | null = null;
+    public popoverContentElement: HTMLDivElement | null = null;
 
     // Last element in popover that is tabbable, and the one that triggers popover closure
     // when the user press TAB on it
     private lastTabbableElement: HTMLElement | null = null;
 
-    private refHandlers: IRefHandlerContainer<HTMLInputElement> = {
-        input: refHandler(this.props.inputProps?.inputRef, this, "inputEl"),
-    };
+    private handleInputRef = refHandler<HTMLInputElement, "inputElement">(
+        this,
+        "inputElement",
+        this.props.inputProps?.inputRef,
+    );
+
+    private handlePopoverContentRef: IRefCallback<HTMLDivElement> = refHandler(this, "popoverContentElement");
 
     public componentWillUnmount() {
         this.unregisterPopoverBlurHandler();
@@ -225,7 +229,7 @@ export class DateInput extends AbstractPureComponent2<IDateInputProps, IDateInpu
         };
 
         const wrappedPopoverContent = (
-            <div ref={ref => (this.popoverContentEl = ref)}>
+            <div ref={this.handlePopoverContentRef}>
                 <DatePicker
                     {...this.props}
                     dayPickerProps={dayPickerProps}
@@ -260,7 +264,7 @@ export class DateInput extends AbstractPureComponent2<IDateInputProps, IDateInpu
                     type="text"
                     {...inputProps}
                     disabled={this.props.disabled}
-                    inputRef={this.refHandlers.input}
+                    inputRef={this.handleInputRef}
                     onBlur={this.handleInputBlur}
                     onChange={this.handleInputChange}
                     onClick={this.handleInputClick}
@@ -409,15 +413,15 @@ export class DateInput extends AbstractPureComponent2<IDateInputProps, IDateInpu
             this.setState({ isOpen: false });
         } else if (e.which === Keys.ESCAPE) {
             this.setState({ isOpen: false });
-            getRef(this.inputEl).blur();
+            getRef(this.inputElement).blur();
         }
         this.safeInvokeInputProp("onKeyDown", e);
     };
 
     private getLastTabbableElement = () => {
         // Popover contents are well structured, but the selector will need
         // to be updated if more focusable components are added in the future
-        const tabbableElements = this.popoverContentEl?.querySelectorAll("input, [tabindex]:not([tabindex='-1'])");
+        const tabbableElements = this.popoverContentElement?.querySelectorAll("input, [tabindex]:not([tabindex='-1'])");
         const numOfElements = tabbableElements?.length ?? 0;
         // Keep track of the last focusable element in popover and add
         // a blur handler, so that when:
@@ -436,10 +440,10 @@ export class DateInput extends AbstractPureComponent2<IDateInputProps, IDateInpu
             relatedTarget = document.activeElement as HTMLElement;
         }
         const eventTarget = e.target as HTMLElement;
-        // Beware: this.popoverContentEl is sometimes null under Chrome
+        // Beware: this.popoverContentElement is sometimes null under Chrome
         if (
             relatedTarget == null ||
-            (this.popoverContentEl != null && !this.popoverContentEl.contains(relatedTarget))
+            (this.popoverContentElement != null && !this.popoverContentElement.contains(relatedTarget))
         ) {
             // Exclude the following blur operations that makes "body" the activeElement
             // and would close the Popover unexpectedly
@@ -459,7 +463,7 @@ export class DateInput extends AbstractPureComponent2<IDateInputProps, IDateInpu
     };
 
     private registerPopoverBlurHandler = () => {
-        if (this.popoverContentEl != null) {
+        if (this.popoverContentElement != null) {
             this.unregisterPopoverBlurHandler();
             this.lastTabbableElement = this.getLastTabbableElement();
             this.lastTabbableElement?.addEventListener("blur", this.handlePopoverBlur);