[popover2] feat: add position prop for improved backcompat

packages/core/src/common/errors.ts

@@ -77,6 +77,8 @@ export const POPOVER_WARN_DOUBLE_TARGET =
     ns + ` <Popover> with children ignores target prop; use either prop or children.`;
 export const POPOVER_WARN_EMPTY_CONTENT = ns + ` Disabling <Popover> with empty/whitespace content...`;
 export const POPOVER_WARN_HAS_BACKDROP_INLINE = ns + ` <Popover usePortal={false}> ignores hasBackdrop`;
+export const POPOVER_WARN_PLACEMENT_AND_POSITION_MUTEX =
+    ns + ` <Popover> supports either placement or position prop, not both.`;
 export const POPOVER_WARN_UNCONTROLLED_ONINTERACTION = ns + ` <Popover> onInteraction is ignored when uncontrolled.`;
 
 export const PORTAL_CONTEXT_CLASS_NAME_STRING = ns + ` <Portal> context blueprintPortalClassName must be string`;

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

@@ -116,9 +116,8 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
         minimal: false,
         modifiers: {},
         openOnTargetFocus: true,
-        // N.B. we don't set a default for `placement` here because that would override
-        // the deprecated `position` prop
-        position: "auto",
+        // N.B. we don't set a default for `placement` or `position` here because that would trigger
+        // a warning in validateProps if the other prop is specified by a user of this component
         targetTagName: "span",
         transitionDuration: 300,
         usePortal: true,
@@ -163,7 +162,7 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
         // as JSX component instead of intrinsic element. but because of its
         // type, tsc actually recognizes that it is _any_ intrinsic element, so
         // it can typecheck the HTML props!!
-        const { className, disabled, fill, placement } = this.props;
+        const { className, disabled, fill, placement, position = "auto" } = this.props;
         const { isOpen } = this.state;
         let { wrapperTagName } = this.props;
         if (fill) {
@@ -207,8 +206,7 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
             >
                 <Popper
                     innerRef={this.handlePopoverRef}
-                    // eslint-disable-next-line deprecation/deprecation
-                    placement={placement ?? positionToPlacement(this.props.position!)}
+                    placement={placement ?? positionToPlacement(position)}
                     modifiers={this.getPopperModifiers()}
                 >
                     {this.renderPopover}
@@ -260,6 +258,9 @@ export class Popover extends AbstractPureComponent2<IPopoverProps, IPopoverState
         if (props.hasBackdrop && props.interactionKind !== PopoverInteractionKind.CLICK) {
             console.error(Errors.POPOVER_HAS_BACKDROP_INTERACTION);
         }
+        if (props.placement !== undefined && props.position !== undefined) {
+            console.warn(Errors.POPOVER_WARN_PLACEMENT_AND_POSITION_MUTEX);
+        }
 
         const childrenCount = React.Children.count(props.children);
         const hasContentProp = props.content !== undefined;

packages/core/src/components/popover/popoverSharedProps.ts

@@ -136,6 +136,7 @@ export interface IPopoverSharedProps extends IOverlayableProps, IProps {
 
     /**
      * The placement (relative to the target) at which the popover should appear.
+     * Mutually exclusive with `position` prop.
      *
      * The default value of `"auto"` will choose the best placement when opened
      * and will allow the popover to reposition itself to remain onscreen as the
@@ -153,13 +154,13 @@ export interface IPopoverSharedProps extends IOverlayableProps, IProps {
 
     /**
      * The position (relative to the target) at which the popover should appear.
+     * Mutually exclusive with `placement` prop.
      *
      * The default value of `"auto"` will choose the best position when opened
      * and will allow the popover to reposition itself to remain onscreen as the
      * user scrolls around.
      *
      * @default "auto"
-     * @deprecated use placement instead
      */
     position?: PopoverPosition;
 

packages/popover2/src/errors.ts

@@ -23,4 +23,6 @@ export const POPOVER2_WARN_DOUBLE_TARGET =
     ns + ` <Popover2> with children ignores renderTarget prop; use either prop or children.`;
 export const POPOVER2_WARN_EMPTY_CONTENT = ns + ` Disabling <Popover2> with empty/whitespace content...`;
 export const POPOVER2_WARN_HAS_BACKDROP_INLINE = ns + ` <Popover2 usePortal={false}> ignores hasBackdrop`;
+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.`;

packages/popover2/src/popover2.tsx

@@ -34,6 +34,7 @@ import {
 import * as Classes from "./classes";
 import * as Errors from "./errors";
 import { POPOVER_ARROW_SVG_SIZE, Popover2Arrow } from "./popover2Arrow";
+import { positionToPlacement } from "./popover2PlacementUtils";
 import { IPopover2SharedProps } from "./popover2SharedProps";
 // eslint-disable-next-line import/no-cycle
 import { Tooltip2 } from "./tooltip2";
@@ -122,7 +123,8 @@ export class Popover2<T> extends AbstractPureComponent2<IPopover2Props<T>, IPopo
         interactionKind: Popover2InteractionKind.CLICK,
         minimal: false,
         openOnTargetFocus: true,
-        placement: "auto",
+        // N.B. we don't set a default for `placement` or `position` here because that would trigger
+        // a warning in validateProps if the other prop is specified by a user of this component
         positioningStrategy: "absolute",
         renderTarget: undefined as any,
         targetTagName: "span",
@@ -188,7 +190,7 @@ export class Popover2<T> extends AbstractPureComponent2<IPopover2Props<T>, IPopo
     }
 
     public render() {
-        const { disabled, content } = this.props;
+        const { disabled, content, placement, position = "auto", positioningStrategy } = this.props;
         const { isOpen } = this.state;
 
         const isContentEmpty = content == null || (typeof content === "string" && content.trim() === "");
@@ -207,8 +209,8 @@ export class Popover2<T> extends AbstractPureComponent2<IPopover2Props<T>, IPopo
                 <Reference>{this.renderTarget}</Reference>
                 <Popper
                     innerRef={this.refHandlers.popover}
-                    placement={this.props.placement}
-                    strategy={this.props.positioningStrategy}
+                    placement={placement ?? positionToPlacement(position)}
+                    strategy={positioningStrategy}
                     modifiers={this.computePopperModifiers()}
                 >
                     {this.renderPopover}
@@ -248,6 +250,9 @@ export class Popover2<T> extends AbstractPureComponent2<IPopover2Props<T>, IPopo
         if (props.hasBackdrop && props.interactionKind !== Popover2InteractionKind.CLICK) {
             console.warn(Errors.POPOVER2_HAS_BACKDROP_INTERACTION);
         }
+        if (props.placement !== undefined && props.position !== undefined) {
+            console.warn(Errors.POPOVER2_WARN_PLACEMENT_AND_POSITION_MUTEX);
+        }
 
         const childrenCount = React.Children.count(props.children);
         const hasRenderTargetPropp = props.renderTarget !== undefined;

packages/popover2/src/popover2PlacementUtils.ts

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2021 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 { Placement } from "@popperjs/core";
+
+import { PopoverPosition } from "@blueprintjs/core";
+
+/**
+ * Convert a position to a placement.
+ *
+ * @param position the position to convert
+ */
+export function positionToPlacement(position: PopoverPosition): Placement {
+    /* istanbul ignore next */
+    switch (position) {
+        case PopoverPosition.TOP_LEFT:
+            return "top-start";
+        case PopoverPosition.TOP:
+            return "top";
+        case PopoverPosition.TOP_RIGHT:
+            return "top-end";
+        case PopoverPosition.RIGHT_TOP:
+            return "right-start";
+        case PopoverPosition.RIGHT:
+            return "right";
+        case PopoverPosition.RIGHT_BOTTOM:
+            return "right-end";
+        case PopoverPosition.BOTTOM_RIGHT:
+            return "bottom-end";
+        case PopoverPosition.BOTTOM:
+            return "bottom";
+        case PopoverPosition.BOTTOM_LEFT:
+            return "bottom-start";
+        case PopoverPosition.LEFT_BOTTOM:
+            return "left-end";
+        case PopoverPosition.LEFT:
+            return "left";
+        case PopoverPosition.LEFT_TOP:
+            return "left-start";
+        case "auto":
+        case "auto-start":
+        case "auto-end":
+            // Return the string unchanged.
+            return position;
+        default:
+            return assertNever(position);
+    }
+}
+
+/* istanbul ignore next */
+function assertNever(x: never): never {
+    throw new Error("Unexpected position: " + x);
+}

packages/popover2/src/popover2SharedProps.ts

@@ -17,7 +17,7 @@
 import { Boundary, Placement, placements, RootBoundary, StrictModifiers } from "@popperjs/core";
 import { StrictModifier } from "react-popper";
 
-import { IOverlayableProps, IProps } from "@blueprintjs/core";
+import { IOverlayableProps, IProps, PopoverPosition } from "@blueprintjs/core";
 
 export { Boundary as PopperBoundary, Placement, placements as PlacementOptions };
 // copied from @popperjs/core, where it is not exported as public
@@ -161,21 +161,34 @@ export interface IPopover2SharedProps<TProps> extends IOverlayableProps, IProps
      */
     rootBoundary?: RootBoundary;
 
+    /**
+     * The placement (relative to the target) at which the popover should appear.
+     * Mutually exclusive with `position` prop.
+     *
+     * The default value of `"auto"` will choose the best placement when opened
+     * and will allow the popover to reposition itself to remain onscreen as the
+     * user scrolls around.
+     *
+     * @default "auto"
+     */
+    placement?: Placement;
+
     /**
      * A space-delimited string of class names applied to the popover element.
      */
     popoverClassName?: string;
 
     /**
-     * The placement (relative to the target) at which the popover should appear.
+     * The position (relative to the target) at which the popover should appear.
+     * Mutually exclusive with `placement` prop.
      *
-     * The default value of `"auto"` will choose the best placement when opened
+     * The default value of `"auto"` will choose the best position when opened
      * and will allow the popover to reposition itself to remain onscreen as the
      * user scrolls around.
      *
      * @default "auto"
      */
-    placement?: Placement;
+    position?: PopoverPosition;
 
     /**
      * HTML tag name for the target element. This must be an HTML element to