Remove PoppperJS.Placement prop. Use Position in public API instead (#…

…1923) * Remove PoppperJS.Placement prop. Use Position in public API instead * Fix test references to Placement * Remove invalid warnings. Scrub tests * Clean imports * Update docs example * Update docs app and lint
palantir · Dec 14, 2017 · c9e9615 · c9e9615 · blueprint-bot · Dec 14, 2017
1 parent 8cf882d
commit c9e9615
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 67 deletions.
diff --git a/packages/core/src/common/errors.ts b/packages/core/src/common/errors.ts
@@ -56,7 +56,6 @@ export const POPOVER_WARN_UNCONTROLLED_ONINTERACTION = ns + ` <Popover> onIntera
 
 export const POPOVER2_WARN_DEPRECATED_IS_DISABLED = `${deprec} <Popover2> isDisabled is deprecated. Use disabled.`;
 export const POPOVER2_WARN_DEPRECATED_IS_MODAL = `${deprec} <Popover2> isModal is deprecated. Use hasBackdrop.`;
-export const POPOVER2_WARN_DEPRECATED_POSITION = `${deprec} <Popover2> position is deprecated. Use placement.`;
 
 export const PORTAL_CONTEXT_CLASS_NAME_STRING = ns + ` <Portal> context blueprintPortalClassName must be string`;
 

diff --git a/packages/core/src/components/popover2/popover2.tsx b/packages/core/src/components/popover2/popover2.tsx
@@ -10,7 +10,7 @@ import * as React from "react";
 import { Manager, Popper, Target } from "react-popper";
 
 export type PopperModifiers = PopperJS.Modifiers;
-export type Placement = PopperJS.Placement;
+type Placement = PopperJS.Placement;
 
 import { AbstractPureComponent } from "../../common/abstractPureComponent";
 import * as Classes from "../../common/classes";
@@ -133,12 +133,6 @@ export interface IPopover2Props extends IOverlayableProps, IProps {
      */
     openOnTargetFocus?: boolean;
 
-    /**
-     * The position (relative to the target) at which the popover should appear.
-     * @deprecated use `placement`
-     */
-    position?: Position;
-
     /**
      * A space-delimited string of class names that are applied to the popover (but not the target).
      */
@@ -167,11 +161,13 @@ export interface IPopover2Props extends IOverlayableProps, IProps {
 
     /**
      * The position (relative to the target) at which the popover should appear.
-     * 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.
+     *
+     * 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?: Position | "auto";
 
     /**
      * The name of the HTML tag to use when rendering the popover target wrapper element (`.pt-popover-target`).
@@ -199,7 +195,7 @@ export interface IPopover2State {
     hasBackdrop?: boolean;
 
     /** Migrated `placement` value that considers the `placement` and `position` props. */
-    placement?: Placement;
+    placement?: PopperJS.Placement;
 }
 
 export class Popover2 extends AbstractPureComponent<IPopover2Props, IPopover2State> {
@@ -374,9 +370,6 @@ export class Popover2 extends AbstractPureComponent<IPopover2Props, IPopover2Sta
         if (props.isModal !== undefined) {
             console.warn(Errors.POPOVER2_WARN_DEPRECATED_IS_MODAL);
         }
-        if (props.position !== undefined) {
-            console.warn(Errors.POPOVER2_WARN_DEPRECATED_POSITION);
-        }
     }
 
     private updateDarkParent() {
@@ -609,9 +602,7 @@ function getHasBackdrop(props: IPopover2Props): boolean {
 }
 
 function getPlacement(props: IPopover2Props): Placement {
-    if (props.placement !== undefined) {
-        return props.placement;
-    } else if (props.position !== undefined) {
+    if (props.position !== undefined) {
         return positionToPlacement(props.position);
     } else {
         return "auto";

diff --git a/packages/core/src/components/popover2/popoverMigrationUtils.ts b/packages/core/src/components/popover2/popoverMigrationUtils.ts
@@ -4,14 +4,14 @@
  * Licensed under the terms of the LICENSE file distributed with this project.
  */
 
+import PopperJS from "popper.js";
 import { Position } from "../../common/position";
-import { Placement } from "./popover2";
 
 /**
  * Convert a position to a placement.
  * @param position the position to convert
  */
-export function positionToPlacement(position: Position): Placement {
+export function positionToPlacement(position: Position | "auto"): PopperJS.Placement {
     switch (position) {
         case Position.TOP_LEFT:
             return "top-start";
@@ -37,6 +37,8 @@ export function positionToPlacement(position: Position): Placement {
             return "left";
         case Position.LEFT_TOP:
             return "left-start";
+        case "auto":
+            return "auto";
         default:
             return assertNever(position);
     }

diff --git a/packages/core/src/components/tooltip2/tooltip2.tsx b/packages/core/src/components/tooltip2/tooltip2.tsx
@@ -9,6 +9,7 @@ import PopperJS from "popper.js";
 import * as React from "react";
 
 import * as Classes from "../../common/classes";
+import { Position } from "../../common/position";
 import { IIntentProps, IProps } from "../../common/props";
 import { PopoverInteractionKind } from "../popover/popover"; // TODO: move this to popover2/ directory
 import { Popover2 } from "../popover2/popover2";
@@ -94,11 +95,13 @@ export interface ITooltip2Props extends IProps, IIntentProps {
 
     /**
      * The position (relative to the target) at which the popover should appear.
-     * 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.
+     *
+     * 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?: PopperJS.Placement;
+    position?: Position | "auto";
 
     /**
      * The name of the HTML tag to use when rendering the popover target wrapper element (`.pt-popover-target`).

diff --git a/packages/core/test/popover2/popover2Tests.tsx b/packages/core/test/popover2/popover2Tests.tsx
@@ -5,9 +5,9 @@
  */
 
 import { assert } from "chai";
-import { mount, ReactWrapper, shallow, ShallowWrapper } from "enzyme";
+import { mount, ReactWrapper, shallow } from "enzyme";
 import * as React from "react";
-import { Arrow, Popper } from "react-popper";
+import { Arrow } from "react-popper";
 import * as sinon from "sinon";
 
 import { dispatchMouseEvent, expectPropValidationError } from "@blueprintjs/test-commons";
@@ -19,12 +19,10 @@ import { Position } from "../../src/common/position";
 import * as Utils from "../../src/common/utils";
 import { Overlay } from "../../src/components/overlay/overlay";
 import { PopoverInteractionKind } from "../../src/components/popover/popover";
-import { IPopover2Props, IPopover2State, Placement, Popover2 } from "../../src/components/popover2/popover2";
+import { IPopover2Props, IPopover2State, Popover2 } from "../../src/components/popover2/popover2";
 import { Tooltip } from "../../src/components/tooltip/tooltip";
 import { Portal } from "../../src/index";
 
-type ShallowPopover2Wrapper = ShallowWrapper<IPopover2Props, IPopover2State>;
-
 describe("<Popover2>", () => {
     let testsContainerElement: HTMLElement;
     let wrapper: IPopoverWrapper;
@@ -563,7 +561,7 @@ describe("<Popover2>", () => {
         });
 
         it("computes arrow rotation", done => {
-            renderPopover({ isOpen: true, placement: "top" }).then(
+            renderPopover({ isOpen: true, position: Position.TOP }).then(
                 () => assert.equal(wrapper.state("arrowRotation"), 90),
                 done,
             );
@@ -584,18 +582,6 @@ describe("<Popover2>", () => {
     });
 
     describe("deprecated prop shims", () => {
-        it("should convert position to placement", () => {
-            const popover = shallow(
-                <Popover2 inline={true} position={Position.BOTTOM_LEFT}>
-                    child
-                </Popover2>,
-            );
-            assertPlacement(popover, "bottom-start");
-
-            popover.setProps({ position: Position.LEFT_BOTTOM });
-            assertPlacement(popover, "left-end");
-        });
-
         it("should convert isModal to hasBackdrop", () => {
             const popover = shallow(
                 <Popover2 inline={true} isModal={true}>
@@ -620,18 +606,6 @@ describe("<Popover2>", () => {
                 .assertIsOpen(true);
         });
 
-        it("placement should take precedence over position", () => {
-            const popover = shallow(
-                <Popover2 inline={true} placement="left-end" position={Position.BOTTOM_LEFT}>
-                    child
-                </Popover2>,
-            );
-            assertPlacement(popover, "left-end");
-
-            popover.setProps({ placement: "bottom-start", position: Position.LEFT_BOTTOM });
-            assertPlacement(popover, "bottom-start");
-        });
-
         it("hasBackdrop should take precedence over isModal", () => {
             const popover = shallow(
                 <Popover2 inline={true} hasBackdrop={true} isModal={false}>
@@ -700,8 +674,4 @@ describe("<Popover2>", () => {
         };
         return wrapper;
     }
-
-    function assertPlacement(popover: ShallowPopover2Wrapper, placement: Placement) {
-        assert.strictEqual(popover.find(Popper).prop("placement"), placement);
-    }
 });
diff --git a/packages/docs-app/src/components/navbarActions.tsx b/packages/docs-app/src/components/navbarActions.tsx
@@ -14,6 +14,7 @@ import {
     MenuDivider,
     MenuItem,
     Popover2,
+    Position,
 } from "@blueprintjs/core";
 import { IPackageInfo } from "@blueprintjs/docs-data";
 
@@ -32,7 +33,7 @@ export class NavbarActions extends React.PureComponent<INavbarActionsProps, {}>
         return (
             <div className={classNames(Classes.BUTTON_GROUP, Classes.MINIMAL)}>
                 <AnchorButton href="https://github.com/palantir/blueprint" target="_blank" text="GitHub" />
-                <Popover2 inline={true} content={this.renderReleasesMenu()} placement="bottom-end">
+                <Popover2 inline={true} content={this.renderReleasesMenu()} position={Position.BOTTOM_RIGHT}>
                     <AnchorButton rightIconName="caret-down" text="Releases" />
                 </Popover2>
                 <AnchorButton

diff --git a/packages/docs-app/src/examples/core-examples/tooltip2Example.tsx b/packages/docs-app/src/examples/core-examples/tooltip2Example.tsx
@@ -6,7 +6,7 @@
 
 import * as React from "react";
 
-import { Button, Intent, Popover2, Switch, Tooltip2 } from "@blueprintjs/core";
+import { Button, Intent, Popover2, Position, Switch, Tooltip2 } from "@blueprintjs/core";
 import { BaseExample } from "@blueprintjs/docs";
 
 export class Tooltip2Example extends BaseExample<{ isOpen: boolean }> {
@@ -69,7 +69,7 @@ export class Tooltip2Example extends BaseExample<{ isOpen: boolean }> {
                         content="Intent.PRIMARY"
                         inline={true}
                         intent={Intent.PRIMARY}
-                        placement="left"
+                        position={Position.LEFT}
                     >
                         Available
                     </Tooltip2>&nbsp;
@@ -78,7 +78,7 @@ export class Tooltip2Example extends BaseExample<{ isOpen: boolean }> {
                         content="Intent.SUCCESS"
                         inline={true}
                         intent={Intent.SUCCESS}
-                        placement="top"
+                        position={Position.TOP}
                     >
                         in the full
                     </Tooltip2>&nbsp;
@@ -87,7 +87,7 @@ export class Tooltip2Example extends BaseExample<{ isOpen: boolean }> {
                         content="Intent.WARNING"
                         inline={true}
                         intent={Intent.WARNING}
-                        placement="bottom"
+                        position={Position.BOTTOM}
                     >
                         range of
                     </Tooltip2>&nbsp;
@@ -96,14 +96,22 @@ export class Tooltip2Example extends BaseExample<{ isOpen: boolean }> {
                         content="Intent.DANGER"
                         inline={true}
                         intent={Intent.DANGER}
-                        placement="right"
+                        position={Position.RIGHT}
                     >
                         visual intents!
                     </Tooltip2>
                 </div>
                 <br />
-                <Popover2 content={<h1>Popover!</h1>} placement="right" popoverClassName="pt-popover-content-sizing">
-                    <Tooltip2 content={<span>This button also has a popover!</span>} placement="right" inline={true}>
+                <Popover2
+                    content={<h1>Popover!</h1>}
+                    position={Position.RIGHT}
+                    popoverClassName="pt-popover-content-sizing"
+                >
+                    <Tooltip2
+                        content={<span>This button also has a popover!</span>}
+                        position={Position.RIGHT}
+                        inline={true}
+                    >
                         <Button intent={Intent.SUCCESS} text="Hover and click me" />
                     </Tooltip2>
                 </Popover2>

diff --git a/packages/labs/test/timezonePickerTests.tsx b/packages/labs/test/timezonePickerTests.tsx
@@ -20,6 +20,7 @@ import {
     IPopoverState,
     MenuItem,
     Popover2,
+    Position,
 } from "@blueprintjs/core";
 import { IQueryListProps, IQueryListState, ISelectProps, ISelectState, QueryList, Select } from "@blueprintjs/select";
 import {
@@ -214,7 +215,7 @@ describe("<TimezonePicker>", () => {
         const popoverProps: IPopover2Props = {
             inline: true,
             isOpen: true,
-            placement: "right",
+            position: Position.RIGHT,
         };
         const timezonePicker = shallow(<TimezonePicker popoverProps={popoverProps} />);
         const popover = findPopover(timezonePicker);

diff --git a/packages/select/src/components/select/select.tsx b/packages/select/src/components/select/select.tsx
@@ -17,6 +17,7 @@ import {
     Keys,
     Menu,
     Popover2,
+    Position,
     Utils,
 } from "@blueprintjs/core";
 import * as Classes from "../../common/classes";
@@ -202,8 +203,8 @@ export class Select<T> extends React.PureComponent<ISelectProps<T>, ISelectState
                 autoFocus={false}
                 enforceFocus={false}
                 isOpen={this.state.isOpen}
-                placement="bottom-start"
                 disabled={disabled}
+                position={Position.BOTTOM_LEFT}
                 {...popoverProps}
                 className={classNames(listProps.className, popoverProps.className)}
                 onInteraction={this.handlePopoverInteraction}