[popover2] feat: new prop targetProps (#5802)

Co-authored-by: Adi Dahiya <adi.dahiya14@gmail.com>
Co-authored-by: Adi Dahiya <adahiya@palantir.com>

packages/popover2/src/errors.ts

@@ -26,3 +26,5 @@ export const POPOVER2_WARN_HAS_BACKDROP_INLINE = ns + ` <Popover2 usePortal={fal
 export const POPOVER2_WARN_PLACEMENT_AND_POSITION_MUTEX =
     ns + ` <Popover2> supports either placement or position prop, not both.`;
 export const POPOVER2_WARN_UNCONTROLLED_ONINTERACTION = ns + ` <Popover2> onInteraction is ignored when uncontrolled.`;
+export const POPOVER2_WARN_TARGET_PROPS_WITH_RENDER_TARGET =
+    ns + ` <Popover2> targetProps value is ignored when renderTarget API is used.`;

packages/popover2/src/index.ts

@@ -33,6 +33,8 @@ export {
     IPopover2TargetProps,
     Popover2SharedProps,
     Popover2TargetProps,
+    Popover2ClickTargetHandlers,
+    Popover2HoverTargetHandlers,
     PopperBoundary,
     PopperModifierOverrides,
     PopperCustomModifer,

packages/popover2/src/popover2.tsx

@@ -36,7 +36,12 @@ import { matchReferenceWidthModifier } from "./customModifiers";
 import * as Errors from "./errors";
 import { Popover2Arrow, POPOVER_ARROW_SVG_SIZE } from "./popover2Arrow";
 import { positionToPlacement } from "./popover2PlacementUtils";
-import { DefaultPopover2TargetHTMLProps, Popover2SharedProps } from "./popover2SharedProps";
+import {
+    DefaultPopover2TargetHTMLProps,
+    Popover2ClickTargetHandlers,
+    Popover2HoverTargetHandlers,
+    Popover2SharedProps,
+} from "./popover2SharedProps";
 import { PopupKind } from "./popupKind";
 import { ResizeSensor2 } from "./resizeSensor2";
 // eslint-disable-next-line import/no-cycle
@@ -137,7 +142,10 @@ export interface IPopover2State {
  * `DefaultPopover2TargetHTMLProps` type exported from @blueprintjs/popover2.
  * @see https://blueprintjs.com/docs/#popover2-package/popover2
  */
-export class Popover2<T> extends AbstractPureComponent2<Popover2Props<T>, IPopover2State> {
+export class Popover2<T extends DefaultPopover2TargetHTMLProps> extends AbstractPureComponent2<
+    Popover2Props<T>,
+    IPopover2State
+> {
     public static displayName = `${DISPLAYNAME_PREFIX}.Popover2`;
 
     public static defaultProps: Popover2Props = {
@@ -276,7 +284,7 @@ export class Popover2<T> extends AbstractPureComponent2<Popover2Props<T>, IPopov
         }
     }
 
-    protected validateProps(props: Popover2Props & { children?: React.ReactNode }) {
+    protected validateProps(props: Popover2Props<T> & { children?: React.ReactNode }) {
         if (props.isOpen == null && props.onInteraction != null) {
             console.warn(Errors.POPOVER2_WARN_UNCONTROLLED_ONINTERACTION);
         }
@@ -292,6 +300,7 @@ export class Popover2<T> extends AbstractPureComponent2<Popover2Props<T>, IPopov
 
         const childrenCount = React.Children.count(props.children);
         const hasRenderTargetProp = props.renderTarget !== undefined;
+        const hasTargetPropsProp = props.targetProps !== undefined;
 
         if (childrenCount === 0 && !hasRenderTargetProp) {
             console.warn(Errors.POPOVER2_REQUIRES_TARGET);
@@ -302,6 +311,9 @@ export class Popover2<T> extends AbstractPureComponent2<Popover2Props<T>, IPopov
         if (childrenCount > 0 && hasRenderTargetProp) {
             console.warn(Errors.POPOVER2_WARN_DOUBLE_TARGET);
         }
+        if (hasRenderTargetProp && hasTargetPropsProp) {
+            console.warn(Errors.POPOVER2_WARN_TARGET_PROPS_WITH_RENDER_TARGET);
+        }
     }
 
     /**
@@ -327,46 +339,58 @@ export class Popover2<T> extends AbstractPureComponent2<Popover2Props<T>, IPopov
 
         const ref = mergeRefs(popperChildRef, this.targetRef);
 
-        const targetEventHandlers = isHoverInteractionKind
-            ? {
-                  // HOVER handlers
-                  onBlur: this.handleTargetBlur,
-                  onContextMenu: this.handleTargetContextMenu,
-                  onFocus: this.handleTargetFocus,
-                  onMouseEnter: this.handleMouseEnter,
-                  onMouseLeave: this.handleMouseLeave,
-              }
-            : {
-                  // CLICK needs only one handler
-                  onClick: this.handleTargetClick,
-                  // For keyboard accessibility, trigger the same behavior as a click event upon pressing ENTER/SPACE
-                  onKeyDown: (event: React.KeyboardEvent<HTMLElement>) =>
-                      // eslint-disable-next-line deprecation/deprecation
-                      Keys.isKeyboardClick(event.keyCode) && this.handleTargetClick(event),
-              };
+        const targetEventHandlers: Popover2HoverTargetHandlers<T> | Popover2ClickTargetHandlers<T> =
+            isHoverInteractionKind
+                ? {
+                      // HOVER handlers
+                      onBlur: this.handleTargetBlur,
+                      onContextMenu: this.handleTargetContextMenu,
+                      onFocus: this.handleTargetFocus,
+                      onMouseEnter: this.handleMouseEnter,
+                      onMouseLeave: this.handleMouseLeave,
+                  }
+                : {
+                      // CLICK needs only one handler
+                      onClick: this.handleTargetClick,
+                      // For keyboard accessibility, trigger the same behavior as a click event upon pressing ENTER/SPACE
+                      onKeyDown: (event: React.KeyboardEvent<HTMLElement>) =>
+                          // eslint-disable-next-line deprecation/deprecation
+                          Keys.isKeyboardClick(event.keyCode) && this.handleTargetClick(event),
+                  };
         // Ensure target is focusable if relevant prop enabled
         const targetTabIndex = openOnTargetFocus && isHoverInteractionKind ? 0 : undefined;
-        const targetProps = {
+        const ownTargetProps = {
             "aria-haspopup":
                 this.props.popupKind ??
-                (this.props.interactionKind === Popover2InteractionKind.HOVER_TARGET_ONLY ? undefined : "true"),
+                (this.props.interactionKind === Popover2InteractionKind.HOVER_TARGET_ONLY
+                    ? undefined
+                    : ("true" as "true")),
             // N.B. this.props.className is passed along to renderTarget even though the user would have access to it.
             // If, instead, renderTarget is undefined and the target is provided as a child, this.props.className is
             // applied to the generated target wrapper element.
             className: classNames(className, Classes.POPOVER2_TARGET, {
                 [Classes.POPOVER2_OPEN]: isOpen,
                 // this class is mainly useful for button targets
-                [CoreClasses.ACTIVE]: !isControlled && isOpen && !isHoverInteractionKind,
+                [CoreClasses.ACTIVE]: isOpen && !isControlled && !isHoverInteractionKind,
             }),
             ref,
-            ...(targetEventHandlers as unknown as T),
+            ...targetEventHandlers,
+        };
+
+        const targetModifierClasses = {
+            // this class is mainly useful for Blueprint <Button> targets; we should only apply it for
+            // uncontrolled popovers when they are opened by a user interaction
+            [CoreClasses.ACTIVE]: isOpen && !isControlled && !isHoverInteractionKind,
+            // similarly, this class is mainly useful for targets like <Button>, <InputGroup>, etc.
+            [CoreClasses.FILL]: fill,
         };
 
         let target: JSX.Element | undefined;
 
         if (renderTarget !== undefined) {
             target = renderTarget({
-                ...targetProps,
+                ...ownTargetProps,
+                className: classNames(ownTargetProps.className, targetModifierClasses),
                 // if the consumer renders a tooltip target, it's their responsibility to disable that tooltip
                 // when *this* popover is open
                 isOpen,
@@ -379,20 +403,20 @@ export class Popover2<T> extends AbstractPureComponent2<Popover2Props<T>, IPopov
                 return null;
             }
 
-            const targetModifierClasses = {
-                // this class is mainly useful for Blueprint <Button> targets; we should only apply it for
-                // uncontrolled popovers when they are opened by a user interaction
-                [CoreClasses.ACTIVE]: isOpen && !isControlled && !isHoverInteractionKind,
-                // similarly, this class is mainly useful for targets like <Button>, <InputGroup>, etc.
-                [CoreClasses.FILL]: fill,
-            };
             const clonedTarget: JSX.Element = React.cloneElement(childTarget, {
                 className: classNames(childTarget.props.className, targetModifierClasses),
                 // force disable single Tooltip2 child when popover is open
                 disabled: isOpen && Utils.isElementOfType(childTarget, Tooltip2) ? true : childTarget.props.disabled,
                 tabIndex: childTarget.props.tabIndex ?? targetTabIndex,
             });
-            const wrappedTarget = React.createElement(targetTagName!, targetProps, clonedTarget);
+            const wrappedTarget = React.createElement(
+                targetTagName!,
+                {
+                    ...ownTargetProps,
+                    ...this.props.targetProps,
+                },
+                clonedTarget,
+            );
             target = wrappedTarget;
         }
 

packages/popover2/src/popover2SharedProps.ts

@@ -61,24 +61,39 @@ export type Popover2TargetProps = IPopover2TargetProps;
 /**
  * @deprecated use Popover2TargetProps
  */
-export interface IPopover2TargetProps {
+export interface IPopover2TargetProps
+    extends Pick<React.HTMLAttributes<HTMLElement>, "aria-haspopup" | "className" | "tabIndex"> {
     /** Target ref. */
     ref: React.Ref<any>;
 
     /** Whether the popover or tooltip is currently open. */
     isOpen: boolean;
 }
 
+/**
+ * Event handlers injected by Popover2 for hover interaction popovers.
+ */
+export type Popover2HoverTargetHandlers<
+    TProps extends DefaultPopover2TargetHTMLProps = DefaultPopover2TargetHTMLProps,
+> = Pick<TProps, "onBlur" | "onContextMenu" | "onFocus" | "onMouseEnter" | "onMouseLeave">;
+
+/**
+ * Event handlers injected by Popover2 for click interaction popovers.
+ */
+export type Popover2ClickTargetHandlers<
+    TProps extends DefaultPopover2TargetHTMLProps = DefaultPopover2TargetHTMLProps,
+> = Pick<TProps, "onClick" | "onKeyDown">;
+
 // eslint-disable-next-line deprecation/deprecation
-export type Popover2SharedProps<T> = IPopover2SharedProps<T>;
+export type Popover2SharedProps<T extends DefaultPopover2TargetHTMLProps> = IPopover2SharedProps<T>;
 /**
  * Props shared between `Popover2` and `Tooltip2`.
  *
  * @template TProps HTML props interface for target element,
  *                  defaults to props for HTMLElement in IPopover2Props and ITooltip2Props
  * @deprecated use Popover2SharedProps
  */
-export interface IPopover2SharedProps<TProps> extends OverlayableProps, Props {
+export interface IPopover2SharedProps<TProps extends DefaultPopover2TargetHTMLProps> extends OverlayableProps, Props {
     /** Interactive element which will trigger the popover. */
     children?: React.ReactNode;
 
@@ -223,7 +238,9 @@ export interface IPopover2SharedProps<TProps> extends OverlayableProps, Props {
      *
      * Mutually exclusive with `children` and `targetTagName` props.
      */
-    renderTarget?: (props: Popover2TargetProps & TProps) => JSX.Element;
+    renderTarget?: (
+        props: Popover2TargetProps & (Popover2HoverTargetHandlers<TProps> | Popover2ClickTargetHandlers<TProps>),
+    ) => JSX.Element;
 
     /**
      * A root boundary element supplied to the "flip" and "preventOverflow" modifiers.
@@ -283,6 +300,15 @@ export interface IPopover2SharedProps<TProps> extends OverlayableProps, Props {
      */
     targetTagName?: keyof JSX.IntrinsicElements;
 
+    /**
+     * HTML props for the target element. This is useful in some cases where you
+     * need to render some simple attributes on the generated target element.
+     *
+     * For more complex use cases, consider using the `renderTarget` API instead.
+     * This prop will be ignored if `renderTarget` is used.
+     */
+    targetProps?: TProps;
+
     /**
      * Whether the popover should be rendered inside a `Portal` attached to
      * `portalContainer` prop.

packages/popover2/test/popover2Tests.tsx

@@ -77,6 +77,11 @@ describe("<Popover2>", () => {
             assert.isTrue(warnSpy.calledWith(Errors.POPOVER2_WARN_DOUBLE_TARGET));
         });
 
+        it("warns if given targetProps and renderTarget", () => {
+            shallow(<Popover2 targetProps={{ role: "none" }} renderTarget={() => <span>"boom"</span>} />);
+            assert.isTrue(warnSpy.calledWith(Errors.POPOVER2_WARN_TARGET_PROPS_WITH_RENDER_TARGET));
+        });
+
         it("warns if attempting to open a popover with empty content", () => {
             shallow(
                 <Popover2 content={undefined} isOpen={true}>