Repository: mapbox/mr-ui (ts)

Issue:
hasPointer prop for popover positioner doesn't work
hasPointer isn't actually used for anything: https://github.com/mapbox/mr-ui/blob/master/src/components/popover/popover-positioner.js#L37


Pull request:
correctly pass hasPointer prop from popover
also adds test coverage for hasPointer to popover-positioner.test.js and popover.test.js. Closes #76

I need to hide the pointer in studio 😭 

@davidtheclark for review plz

Patch:
diff --git a/package-lock.json b/package-lock.json
index 34299dc..9c43c7f 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -6514,8 +6514,7 @@
         "ansi-regex": {
           "version": "2.1.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "aproba": {
           "version": "1.2.0",
@@ -6536,14 +6535,12 @@
         "balanced-match": {
           "version": "1.0.0",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "brace-expansion": {
           "version": "1.1.11",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "balanced-match": "^1.0.0",
             "concat-map": "0.0.1"
@@ -6558,20 +6555,17 @@
         "code-point-at": {
           "version": "1.1.0",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "concat-map": {
           "version": "0.0.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "console-control-strings": {
           "version": "1.1.0",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "core-util-is": {
           "version": "1.0.2",
@@ -6688,8 +6682,7 @@
         "inherits": {
           "version": "2.0.3",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "ini": {
           "version": "1.3.5",
@@ -6701,7 +6694,6 @@
           "version": "1.0.0",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "number-is-nan": "^1.0.0"
           }
@@ -6716,7 +6708,6 @@
           "version": "3.0.4",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "brace-expansion": "^1.1.7"
           }
@@ -6724,14 +6715,12 @@
         "minimist": {
           "version": "0.0.8",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "minipass": {
           "version": "2.2.4",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "safe-buffer": "^5.1.1",
             "yallist": "^3.0.0"
@@ -6750,7 +6739,6 @@
           "version": "0.5.1",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "minimist": "0.0.8"
           }
@@ -6831,8 +6819,7 @@
         "number-is-nan": {
           "version": "1.0.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "object-assign": {
           "version": "4.1.1",
@@ -6844,7 +6831,6 @@
           "version": "1.4.0",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "wrappy": "1"
           }
@@ -6930,8 +6916,7 @@
         "safe-buffer": {
           "version": "5.1.1",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "safer-buffer": {
           "version": "2.1.2",
@@ -6967,7 +6952,6 @@
           "version": "1.0.2",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "code-point-at": "^1.0.0",
             "is-fullwidth-code-point": "^1.0.0",
@@ -6987,7 +6971,6 @@
           "version": "3.0.1",
           "bundled": true,
           "dev": true,
-          "optional": true,
           "requires": {
             "ansi-regex": "^2.0.0"
           }
@@ -7031,14 +7014,12 @@
         "wrappy": {
           "version": "1.0.2",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         },
         "yallist": {
           "version": "3.0.2",
           "bundled": true,
-          "dev": true,
-          "optional": true
+          "dev": true
         }
       }
     },
diff --git a/src/components/popover/popover-positioner.js b/src/components/popover/popover-positioner.js
index 822d7b7..58fcf07 100644
--- a/src/components/popover/popover-positioner.js
+++ b/src/components/popover/popover-positioner.js
@@ -103,6 +103,7 @@ class PopoverPositioner extends React.PureComponent {
       getAnchorElement: props.getAnchorElement,
       placement: props.placement,
       alignment: props.alignment,
+      hasPointer: props.hasPointer,
       offsetFromAnchor: props.offsetFromAnchor,
       pointerBase: props.pointerBase,
       pointerAltitude: props.pointerAltitude,


Test patch:
diff --git a/src/components/popover/__tests__/__snapshots__/popover.test.js.snap b/src/components/popover/__tests__/__snapshots__/popover.test.js.snap
index 7f0470c..d34145b 100644
--- a/src/components/popover/__tests__/__snapshots__/popover.test.js.snap
+++ b/src/components/popover/__tests__/__snapshots__/popover.test.js.snap
@@ -16,7 +16,7 @@ exports[`Popover all props renders 1`] = `
 >
   <div
     className="color-text shadow-darken25 round"
-    data-popover-id={4}
+    data-popover-id={5}
     data-test={true}
     key="body"
     onFocus={[Function]}
@@ -110,6 +110,42 @@ exports[`Popover dark renders 1`] = `
 </Displaced>
 `;
 
+exports[`Popover no pointer renders 1`] = `
+<Displaced
+  alignment="top"
+  allowPlacementAxisChange={true}
+  containWithinViewport={true}
+  getAnchorElement={[Function]}
+  hasPointer={false}
+  hideWhenAnchorIsOffscreen={false}
+  mounted={true}
+  onElement={[Function]}
+  placement="right"
+  pointerColor="#fff"
+>
+  <div
+    className="color-text shadow-darken25 round px12 py12"
+    data-popover-id={4}
+    key="body"
+    onFocus={[Function]}
+    onKeyDown={[Function]}
+    role="dialog"
+    style={
+      Object {
+        "backgroundColor": "#fff",
+      }
+    }
+    tabIndex={-1}
+  >
+    <div
+      className="w240 h240"
+    >
+      test popover
+    </div>
+  </div>
+</Displaced>
+`;
+
 exports[`Popover warning renders 1`] = `
 <Displaced
   alignment="top"
diff --git a/src/components/popover/__tests__/popover-positioner.test.js b/src/components/popover/__tests__/popover-positioner.test.js
index f2ccee2..8343de9 100644
--- a/src/components/popover/__tests__/popover-positioner.test.js
+++ b/src/components/popover/__tests__/popover-positioner.test.js
@@ -137,6 +137,7 @@ describe('PopoverPositioner', () => {
       pointerBase: 12,
       pointerAltitude: 3,
       pointerColor: '#000',
+      hasPointer: false,
       hideWhenAnchorIsOffscreen: true,
       allowPlacementAxisChange: false,
       containWithinViewport: false,
@@ -152,6 +153,7 @@ describe('PopoverPositioner', () => {
       getAnchorElement: mockGetAnchorElement,
       placement: extraProps.placement,
       alignment: extraProps.alignment,
+      hasPointer: extraProps.hasPointer,
       offsetFromAnchor: extraProps.offsetFromAnchor,
       pointerBase: extraProps.pointerBase,
       pointerAltitude: extraProps.pointerAltitude,
diff --git a/src/components/popover/__tests__/popover-test-cases.js b/src/components/popover/__tests__/popover-test-cases.js
index b9be811..eb9acd2 100644
--- a/src/components/popover/__tests__/popover-test-cases.js
+++ b/src/components/popover/__tests__/popover-test-cases.js
@@ -254,6 +254,18 @@ testCases.warning = {
   )
 };
 
+testCases.noPointer = {
+  description: 'no pointer',
+  element: (
+    <PopoverTrigger
+      popoverProps={{
+        hasPointer: false
+      }}
+      content="No pointer here"
+    />
+  )
+};
+
 const noDisplayCases = {};
 
 noDisplayCases.basic = {
@@ -285,6 +297,16 @@ noDisplayCases.warning = {
   }
 };
 
+noDisplayCases.noPointer = {
+  description: 'no pointer',
+  component: Popover,
+  props: {
+    getAnchorElement: () => {},
+    children: getPopoverContent(),
+    hasPointer: false
+  }
+};
+
 noDisplayCases.allProps = {
   description: 'all props',
   component: Popover,
diff --git a/src/components/popover/__tests__/popover.test.js b/src/components/popover/__tests__/popover.test.js
index eea6fa0..bedf737 100644
--- a/src/components/popover/__tests__/popover.test.js
+++ b/src/components/popover/__tests__/popover.test.js
@@ -45,6 +45,19 @@ describe('Popover', () => {
     });
   });
 
+  describe(noDisplayCases.noPointer.description, () => {
+    beforeEach(() => {
+      testCase = noDisplayCases.noPointer;
+      wrapper = shallow(
+        React.createElement(testCase.component, testCase.props)
+      );
+    });
+
+    test('renders', () => {
+      expect(wrapper).toMatchSnapshot();
+    });
+  });
+
   describe(noDisplayCases.allProps.description, () => {
     beforeEach(() => {
       testCase = noDisplayCases.allProps;