breaking(popup): verticalOffset (Semantic-Org#2450)

* feat(popup): verticalOffset

Add a verticalOffset prop to the popup.

* feat(popup): Rename offset prop

Rename `offset` to `horizontalOffset` for consistency.

* docs(popup): Update for offset

Update docs for `horizontalOffset` and the new `verticalOffset` property.

* fix(popup): vertical offset bug

Stop popup disappearing when vertical offset applied.

docs/app/Examples/modules/Popup/Variations/PopupExampleOffset.js

@@ -6,15 +6,27 @@ const PopupExampleOffset = () => (
     <Popup
       trigger={<Icon size='large' name='heart' circular />}
       content='Way off to the left'
-      offset={50}
+      horizontalOffset={50}
       position='left center'
     />
     <Popup
       trigger={<Icon size='large' name='heart' circular />}
       content='As expected this popup is way off to the right'
-      offset={50}
+      horizontalOffset={50}
       position='right center'
     />
+    <Popup
+      trigger={<Icon size='large' name='heart' circular />}
+      content='Way off to the top'
+      verticalOffset={50}
+      position='top center'
+    />
+    <Popup
+      trigger={<Icon size='large' name='heart' circular />}
+      content='As expected this popup is way off to the bottom'
+      verticalOffset={50}
+      position='bottom center'
+    />
   </div>
 )
 

docs/app/Examples/modules/Popup/Variations/index.js

@@ -36,7 +36,7 @@ const PopupVariationsExamples = () => (
     />
     <ComponentExample
       title='Offset'
-      description='A popup position can be adjusted manually by specifying an offset property.'
+      description='A popup position can be offset from its position.'
       examplePath='modules/Popup/Variations/PopupExampleOffset'
     />
     <ComponentExample

src/modules/Popup/Popup.d.ts

@@ -39,7 +39,10 @@ export interface PopupProps extends PortalProps {
   inverted?: boolean;
 
   /** Horizontal offset in pixels to be applied to the popup. */
-  offset?: number;
+  horizontalOffset?: number;
+
+  /** Vertical offset in pixels to be applied to the popup. */
+  verticalOffset?: number;
 
   /** Events triggering the popup. */
   on?: 'hover' | 'click' | 'focus' | Array<'hover' | 'click' | 'focus'>;

src/modules/Popup/Popup.js

@@ -73,7 +73,10 @@ export default class Popup extends Component {
     inverted: PropTypes.bool,
 
     /** Horizontal offset in pixels to be applied to the Popup. */
-    offset: PropTypes.number,
+    horizontalOffset: PropTypes.number,
+
+    /** Vertical offset in pixels to be applied to the Popup. */
+    verticalOffset: PropTypes.number,
 
     /** Events triggering the popup. */
     on: PropTypes.oneOfType([
@@ -153,7 +156,7 @@ export default class Popup extends Component {
     // Do not access window/document when server side rendering
     if (!isBrowser()) return style
 
-    const { offset } = this.props
+    const { horizontalOffset, verticalOffset } = this.props
     const { pageYOffset, pageXOffset } = window
     const { clientWidth, clientHeight } = document.documentElement
 
@@ -188,11 +191,19 @@ export default class Popup extends Component {
       }
     }
 
-    if (offset) {
+    if (horizontalOffset) {
       if (_.isNumber(style.right)) {
-        style.right -= offset
+        style.right -= horizontalOffset
+      } else {
+        style.left -= horizontalOffset
+      }
+    }
+
+    if (verticalOffset) {
+      if (_.isNumber(style.top)) {
+        style.top += verticalOffset
       } else {
-        style.left -= offset
+        style.bottom += verticalOffset
       }
     }
 

test/specs/modules/Popup/Popup-test.js

@@ -83,7 +83,7 @@ describe('Popup', () => {
     it('accepts an offest to the left', () => {
       wrapperMount(
         <Popup
-          offset={50}
+          horizontalOffset={50}
           position='bottom right'
           content='foo'
           trigger={<button>foo</button>}
@@ -96,7 +96,7 @@ describe('Popup', () => {
     it('accepts an offest to the right', () => {
       wrapperMount(
         <Popup
-          offset={50}
+          horizontalOffset={50}
           position='bottom left'
           content='foo'
           trigger={<button>foo</button>}
@@ -108,6 +108,35 @@ describe('Popup', () => {
     })
   })
 
+  describe('verticalOffest', () => {
+    it('accepts a vertical offest to the top', () => {
+      wrapperMount(
+        <Popup
+          verticalOffset={50}
+          position='bottom right'
+          content='foo'
+          trigger={<button>foo</button>}
+        />,
+      )
+
+      wrapper.find('button').simulate('click')
+      assertInBody('.ui.popup.visible')
+    })
+    it('accepts a vertical offest to the bottom', () => {
+      wrapperMount(
+        <Popup
+          verticalOffset={50}
+          position='top left'
+          content='foo'
+          trigger={<button>foo</button>}
+        />,
+      )
+
+      wrapper.find('button').simulate('click')
+      assertInBody('.ui.popup.visible')
+    })
+  })
+
   describe('position', () => {
     POSITIONS.forEach((position) => {
       it('is always within the viewport', () => {
@@ -129,14 +158,30 @@ describe('Popup', () => {
         expect(bottom).to.be.at.most(document.documentElement.clientHeight)
         expect(right).to.be.at.most(document.documentElement.clientWidth)
       })
-      it('is the original if no position fits within the viewport', () => {
+      it('is the original if no horizontal position fits within the viewport', () => {
+        wrapperMount(
+          <Popup
+            content='_'
+            position={position}
+            trigger={<button>foo</button>}
+            on='click'
+            horizontalOffset={999}
+          />,
+        )
+        wrapper.find('button').simulate('click')
+        const selectedPosition = wrapper.state('position')
+
+        expect(selectedPosition).to.equal(position)
+      })
+
+      it('is the original if no vertical position fits within the viewport', () => {
         wrapperMount(
           <Popup
             content='_'
             position={position}
             trigger={<button>foo</button>}
             on='click'
-            offset={999}
+            verticalOffset={3000}
           />,
         )
         wrapper.find('button').simulate('click')