From 233c243271fc8555cdfa3b30bc5f7fca45797fa0 Mon Sep 17 00:00:00 2001 From: Adam Date: Sat, 20 Jan 2018 02:16:18 +0100 Subject: [PATCH 1/4] feat(popup): verticalOffset Add a verticalOffset prop to the popup. --- src/modules/Popup/Popup.d.ts | 3 ++ src/modules/Popup/Popup.js | 13 ++++++- test/specs/modules/Popup/Popup-test.js | 47 +++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/modules/Popup/Popup.d.ts b/src/modules/Popup/Popup.d.ts index b764edc608..49463d7031 100644 --- a/src/modules/Popup/Popup.d.ts +++ b/src/modules/Popup/Popup.d.ts @@ -41,6 +41,9 @@ export interface PopupProps extends PortalProps { /** Horizontal offset in pixels to be applied to the popup. */ offset?: 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'>; diff --git a/src/modules/Popup/Popup.js b/src/modules/Popup/Popup.js index c62cd14acf..e97cf3e6bf 100644 --- a/src/modules/Popup/Popup.js +++ b/src/modules/Popup/Popup.js @@ -75,6 +75,9 @@ export default class Popup extends Component { /** Horizontal offset in pixels to be applied to the Popup. */ offset: PropTypes.number, + /** Vertical offset in pixels to be applied to the Popup. */ + verticalOffset: PropTypes.number, + /** Events triggering the popup. */ on: PropTypes.oneOfType([ PropTypes.oneOf(['hover', 'click', 'focus']), @@ -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 { offset, verticalOffset } = this.props const { pageYOffset, pageXOffset } = window const { clientWidth, clientHeight } = document.documentElement @@ -196,6 +199,14 @@ export default class Popup extends Component { } } + if (verticalOffset) { + if (_.isNumber(style.top)) { + style.top -= verticalOffset + } else { + style.bottom += verticalOffset + } + } + return style } diff --git a/test/specs/modules/Popup/Popup-test.js b/test/specs/modules/Popup/Popup-test.js index 41f0950ae5..09852f1188 100644 --- a/test/specs/modules/Popup/Popup-test.js +++ b/test/specs/modules/Popup/Popup-test.js @@ -108,6 +108,35 @@ describe('Popup', () => { }) }) + describe('verticalOffest', () => { + it('accepts a vertical offest to the top', () => { + wrapperMount( + foo} + />, + ) + + wrapper.find('button').simulate('click') + assertInBody('.ui.popup.visible') + }) + it('accepts a vertical offest to the bottom', () => { + wrapperMount( + foo} + />, + ) + + wrapper.find('button').simulate('click') + assertInBody('.ui.popup.visible') + }) + }) + describe('position', () => { POSITIONS.forEach((position) => { it('is always within the viewport', () => { @@ -129,7 +158,7 @@ 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( { expect(selectedPosition).to.equal(position) }) + + it('is the original if no vertical position fits within the viewport', () => { + wrapperMount( + foo} + on='click' + verticalOffset={3000} + />, + ) + wrapper.find('button').simulate('click') + const selectedPosition = wrapper.state('position') + + expect(selectedPosition).to.equal(position) + }) }) }) From 54c0b3b4e2c80389b95028891d8e72e4df28be95 Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 30 Jan 2018 15:25:07 +0100 Subject: [PATCH 2/4] feat(popup): Rename offset prop Rename `offset` to `horizontalOffset` for consistency. --- src/modules/Popup/Popup.d.ts | 2 +- src/modules/Popup/Popup.js | 10 +++++----- test/specs/modules/Popup/Popup-test.js | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/modules/Popup/Popup.d.ts b/src/modules/Popup/Popup.d.ts index 49463d7031..79897aee25 100644 --- a/src/modules/Popup/Popup.d.ts +++ b/src/modules/Popup/Popup.d.ts @@ -39,7 +39,7 @@ 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; diff --git a/src/modules/Popup/Popup.js b/src/modules/Popup/Popup.js index e97cf3e6bf..a9530dd388 100644 --- a/src/modules/Popup/Popup.js +++ b/src/modules/Popup/Popup.js @@ -73,7 +73,7 @@ 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, @@ -156,7 +156,7 @@ export default class Popup extends Component { // Do not access window/document when server side rendering if (!isBrowser()) return style - const { offset, verticalOffset } = this.props + const { horizontalOffset, verticalOffset } = this.props const { pageYOffset, pageXOffset } = window const { clientWidth, clientHeight } = document.documentElement @@ -191,11 +191,11 @@ export default class Popup extends Component { } } - if (offset) { + if (horizontalOffset) { if (_.isNumber(style.right)) { - style.right -= offset + style.right -= horizontalOffset } else { - style.left -= offset + style.left -= horizontalOffset } } diff --git a/test/specs/modules/Popup/Popup-test.js b/test/specs/modules/Popup/Popup-test.js index 09852f1188..9ab4f2cc70 100644 --- a/test/specs/modules/Popup/Popup-test.js +++ b/test/specs/modules/Popup/Popup-test.js @@ -83,7 +83,7 @@ describe('Popup', () => { it('accepts an offest to the left', () => { wrapperMount( foo} @@ -96,7 +96,7 @@ describe('Popup', () => { it('accepts an offest to the right', () => { wrapperMount( foo} @@ -165,7 +165,7 @@ describe('Popup', () => { position={position} trigger={} on='click' - offset={999} + horizontalOffset={999} />, ) wrapper.find('button').simulate('click') From aa7b655b86b5158dcc3a1f30a440494b704e92ad Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 30 Jan 2018 15:54:37 +0100 Subject: [PATCH 3/4] docs(popup): Update for offset Update docs for `horizontalOffset` and the new `verticalOffset` property. --- .../Popup/Variations/PopupExampleOffset.js | 16 ++++++++++++++-- .../Examples/modules/Popup/Variations/index.js | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/app/Examples/modules/Popup/Variations/PopupExampleOffset.js b/docs/app/Examples/modules/Popup/Variations/PopupExampleOffset.js index 952de9927f..960308995a 100644 --- a/docs/app/Examples/modules/Popup/Variations/PopupExampleOffset.js +++ b/docs/app/Examples/modules/Popup/Variations/PopupExampleOffset.js @@ -6,15 +6,27 @@ const PopupExampleOffset = () => ( } content='Way off to the left' - offset={50} + horizontalOffset={50} position='left center' /> } content='As expected this popup is way off to the right' - offset={50} + horizontalOffset={50} position='right center' /> + } + content='Way off to the top' + verticalOffset={50} + position='top center' + /> + } + content='As expected this popup is way off to the bottom' + verticalOffset={50} + position='bottom center' + /> ) diff --git a/docs/app/Examples/modules/Popup/Variations/index.js b/docs/app/Examples/modules/Popup/Variations/index.js index 917c384f4e..1e51dd78e7 100644 --- a/docs/app/Examples/modules/Popup/Variations/index.js +++ b/docs/app/Examples/modules/Popup/Variations/index.js @@ -36,7 +36,7 @@ const PopupVariationsExamples = () => ( /> Date: Fri, 2 Feb 2018 13:00:53 +0100 Subject: [PATCH 4/4] fix(popup): vertical offset bug Stop popup disappearing when vertical offset applied. --- src/modules/Popup/Popup.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/Popup/Popup.js b/src/modules/Popup/Popup.js index a9530dd388..ad427e7190 100644 --- a/src/modules/Popup/Popup.js +++ b/src/modules/Popup/Popup.js @@ -201,7 +201,7 @@ export default class Popup extends Component { if (verticalOffset) { if (_.isNumber(style.top)) { - style.top -= verticalOffset + style.top += verticalOffset } else { style.bottom += verticalOffset }