From e13eed4b769bd9edf73e6a8eb3c83b444d33d693 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Wed, 18 Oct 2017 09:54:55 +0300 Subject: [PATCH 01/25] added test for scrolling --- test/components/popup.spec.tsx | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index b7d7595a5..c0ddcb3a8 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -30,8 +30,8 @@ describe('', () => { anchor.style.height = '150px'; anchor.style.border = '1px solid blue'; }); - afterEach(() => {clientRenderer.cleanup(); }); - after(() => {document.body.removeChild(anchor); }); + // afterEach(() => {clientRenderer.cleanup(); }); + // after(() => {document.body.removeChild(anchor); }); describe('The popup user', () => { it('clicks on the parent and the popup opens and closes after another click', async () => { @@ -156,6 +156,35 @@ describe('', () => { expect([div, popup.root]).to.be.inVerticalSequence(); }); }); + + it('listens to internal scrolling and adjusts the popup location accordingly', async () => { + let anchorDiv: HTMLDivElement; + let scrollDiv: HTMLDivElement; + const {waitForDom} = clientRenderer.render( +
+
scrollDiv = elem} style={{height: '100px', overflow: 'scroll'}}> +
Filler
+
anchorDiv = elem}>Anchor
+
+
+ ); + + await waitForDom(() => expect(anchorDiv).to.be.present()); + + const {driver: popup} = clientRenderer.render( + + Popup Body + ).withDriver(PopupTestDriver); + + scrollDiv!.scrollTop = 100; + + return waitForDom(() => { + expect([anchorDiv, popup.root]).to.be.inVerticalSequence(); + }); + }); }); describe('Layout tests', () => { From 09439bdbe721e34602911cb1ea697ea48ad6d290 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Wed, 18 Oct 2017 11:22:25 +0300 Subject: [PATCH 02/25] initial solution --- src/components/popup/popup.tsx | 14 ++++++++++++++ test/components/popup.spec.tsx | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index 8e66b66c9..f448b8870 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -45,10 +45,24 @@ export class Popup extends React.Component { return null; } + public componentDidMount() { + window.addEventListener('scroll', this.test, true); + } + + public componentWillUnmount() { + window.removeEventListener('scroll', this.test); + } + public getPortal(): Portal | null { return this.portal; } + private test = (e: any) => { + if (e.target.contains(this.props.anchor)) { + this.forceUpdate(); + } + }; + private createStyle(): React.CSSProperties { if (!this.props.anchor) { return {}; diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index c0ddcb3a8..328b60b4f 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -30,8 +30,8 @@ describe('', () => { anchor.style.height = '150px'; anchor.style.border = '1px solid blue'; }); - // afterEach(() => {clientRenderer.cleanup(); }); - // after(() => {document.body.removeChild(anchor); }); + afterEach(() => {clientRenderer.cleanup(); }); + after(() => {document.body.removeChild(anchor); }); describe('The popup user', () => { it('clicks on the parent and the popup opens and closes after another click', async () => { From 061266131f1e907f84e4043166d7378f3c807dfb Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Thu, 19 Oct 2017 09:47:59 +0300 Subject: [PATCH 03/25] renamed callback --- src/components/popup/popup.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index a40417dd9..7d0d12cc7 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -49,18 +49,18 @@ export class Popup extends React.Component { } public componentDidMount() { - window.addEventListener('scroll', this.test, true); + window.addEventListener('scroll', this.onScroll, true); } public componentWillUnmount() { - window.removeEventListener('scroll', this.test); + window.removeEventListener('scroll', this.onScroll); } public getPortal(): Portal | null { return this.portal; } - private test = (e: any) => { + private onScroll = (e: any) => { if (e.target.contains(this.props.anchor)) { this.forceUpdate(); } From c78a9e168b113f3d3eaba385c433e031de269fcb Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Mon, 23 Oct 2017 16:58:58 +0300 Subject: [PATCH 04/25] initial collision solution --- demo/components/popup-demo.tsx | 46 +++++++++++++++++++------------- src/components/popup/popup.tsx | 26 ++++++++++++++++-- src/components/portal/portal.tsx | 7 +++++ test/components/popup.spec.tsx | 32 +++++++++++++++++++++- 4 files changed, 89 insertions(+), 22 deletions(-) diff --git a/demo/components/popup-demo.tsx b/demo/components/popup-demo.tsx index 6eeea3c5b..55cf82f93 100644 --- a/demo/components/popup-demo.tsx +++ b/demo/components/popup-demo.tsx @@ -38,31 +38,36 @@ export class PopupDemo extends React.Component<{}, DemoState> { {value: 'top', labelText: 'top'}, {value: 'center', labelText: 'center'}, {value: 'bottom', labelText: 'bottom'} - ]; + ]; const hPos: RadioGroupDataSchemaProps[] = [ {value: 'left', labelText: 'left'}, {value: 'center', labelText: 'center'}, {value: 'right', labelText: 'right'} - ]; + ]; return (
- - this.popup = popup} - > -
Hello!
-
+
+
+ +
+ this.popup = popup} + onExitBounds={this.onExitBounds} + > +
Hello!
+
+

Popup position - vertical

@@ -113,4 +118,7 @@ export class PopupDemo extends React.Component<{}, DemoState> { this.setState({aHorizontal: e.value}); } + private onExitBounds = () => { + this.setState({isOpen: false}); + } } diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index 7d0d12cc7..30052da13 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -2,6 +2,7 @@ import * as React from 'react'; import {properties} from 'wix-react-tools'; import {Point} from '../../types'; import {Portal} from '../portal'; +import {noop} from '../../utils'; export type PopupVerticalPosition = 'top' | 'center' | 'bottom'; export type PopupHorizontalPosition = 'left' | 'center' | 'right'; @@ -16,6 +17,7 @@ export interface PopupProps extends properties.Props { anchorPosition?: PopupPositionPoint; popupPosition?: PopupPositionPoint; syncWidth?: boolean; + onExitBounds?: () => void; children?: React.ReactNode; } @@ -29,12 +31,15 @@ export class Popup extends React.Component { open: false, anchorPosition: {vertical: 'bottom', horizontal: 'left'}, popupPosition: {vertical: 'top', horizontal: 'left'}, - syncWidth: true + syncWidth: true, + onExitBounds: noop }; private portal: Portal | null; + private isOutOfBounds = false; public render() { + this.isOutOfBounds = false; if (this.props.anchor && this.props.open) { return ( { window.removeEventListener('scroll', this.onScroll); } + public componentDidUpdate() { + if (this.isOutOfBounds) { + this.props.onExitBounds!(); + } + } + public getPortal(): Portal | null { return this.portal; } @@ -64,7 +75,7 @@ export class Popup extends React.Component { if (e.target.contains(this.props.anchor)) { this.forceUpdate(); } - }; + } private createStyle(): React.CSSProperties { if (!this.props.anchor) { @@ -104,6 +115,10 @@ export class Popup extends React.Component { break; } + const rect = this.portal && this.portal.getPortal && this.portal.getPortal()!.getBoundingClientRect(); + if (rect) { + this.isOutOfBounds = isOutOfBounds(newStyle.top, newStyle.left, rect.height, rect.width) ? true : false; + } return newStyle; } } @@ -132,3 +147,10 @@ function addTransform(style: React.CSSProperties, transformation: string) { function isPoint(elem: Element | Point): elem is Point { return elem.hasOwnProperty('x') && elem.hasOwnProperty('y'); } + +function isOutOfBounds(top: number, left: number, height: number, width: number): boolean { + return top < 0 || + left < 0 || + top + height - window.pageYOffset > (window.innerHeight || document.documentElement.clientHeight) || + left + width - window.pageXOffset > (window.innerWidth || document.documentElement.clientWidth); +} diff --git a/src/components/portal/portal.tsx b/src/components/portal/portal.tsx index 798529593..720cb9da7 100644 --- a/src/components/portal/portal.tsx +++ b/src/components/portal/portal.tsx @@ -35,6 +35,13 @@ export class Portal extends React.PureComponent { } } + public getPortal() { + if (this.container) { + return this.container.children[0]; + } + return null; + } + private renderPortal() { ReactDOM.unstable_renderSubtreeIntoContainer(this, this.portalContent, this.getContainer()); } diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index 328b60b4f..b3a247d6c 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -1,6 +1,6 @@ import React = require('react'); import ReactDOM = require('react-dom'); -import {ClientRenderer, DriverBase, expect, waitFor} from 'test-drive-react'; +import {ClientRenderer, DriverBase, expect, sinon, waitFor} from 'test-drive-react'; import {PopupDemo} from '../../demo/components/popup-demo'; import {Popup, PopupPositionPoint} from '../../src/components/'; import {PopupTestDriver} from '../../test-kit/components/popup-driver'; @@ -185,6 +185,36 @@ describe('', () => { expect([anchorDiv, popup.root]).to.be.inVerticalSequence(); }); }); + + it('calls onExitBounds when the popup leaves the viewport', async () => { + let div: HTMLDivElement; + let scrollDiv: HTMLDivElement; + const onExitBounds = sinon.spy(); + const {waitForDom} = clientRenderer.render( +
scrollDiv = elem} style={{height: '1000px', overflow: 'scroll'}}> +
div = elem} style={{height: '50px'}}>Anchor
+
+
+ ); + + await waitForDom(() => expect(div).to.be.present()); + + const {driver: popup} = clientRenderer.render( + +
Body
+
).withDriver(PopupTestDriver); + + await waitForDom(() => expect(popup.root).to.be.present()); + scrollDiv!.scrollTop = 51; + + return waitFor(() => { + expect(onExitBounds).to.have.been.called; + }); + }); }); describe('Layout tests', () => { From c9e8347c36f99c2ee28d0e2bafbe09b0fa02b1c0 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Tue, 24 Oct 2017 11:51:48 +0300 Subject: [PATCH 05/25] removed changes from popup demo --- demo/components/popup-demo.tsx | 40 +++++++++++++++------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/demo/components/popup-demo.tsx b/demo/components/popup-demo.tsx index 55cf82f93..e0a6936f7 100644 --- a/demo/components/popup-demo.tsx +++ b/demo/components/popup-demo.tsx @@ -46,28 +46,24 @@ export class PopupDemo extends React.Component<{}, DemoState> { ]; return (
-
-
- -
- this.popup = popup} - onExitBounds={this.onExitBounds} - > -
Hello!
-
-
+ + this.popup = popup} + onExitBounds={this.onExitBounds} + > +
Hello!
+

Popup position - vertical

From 1f4902c6101318d16e64ad0e5a908efc80839ea9 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Tue, 24 Oct 2017 12:02:22 +0300 Subject: [PATCH 06/25] removed clientHeight --- src/components/popup/popup.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index 30052da13..b507aa9ff 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -151,6 +151,6 @@ function isPoint(elem: Element | Point): elem is Point { function isOutOfBounds(top: number, left: number, height: number, width: number): boolean { return top < 0 || left < 0 || - top + height - window.pageYOffset > (window.innerHeight || document.documentElement.clientHeight) || - left + width - window.pageXOffset > (window.innerWidth || document.documentElement.clientWidth); + top + height - window.pageYOffset > window.innerHeight || + left + width - window.pageXOffset > window.innerWidth; } From c08fc0a9d53b0721fb9185d174ff2e1ff4a15e10 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Tue, 24 Oct 2017 12:04:55 +0300 Subject: [PATCH 07/25] fixed some imports --- src/components/popup/popup.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index b507aa9ff..cf85b12fc 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -1,8 +1,8 @@ import * as React from 'react'; import {properties} from 'wix-react-tools'; import {Point} from '../../types'; -import {Portal} from '../portal'; import {noop} from '../../utils'; +import {Portal} from '../portal'; export type PopupVerticalPosition = 'top' | 'center' | 'bottom'; export type PopupHorizontalPosition = 'left' | 'center' | 'right'; From d4efd185eccabdbe38213f0dfb7efc5439f493a5 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Tue, 24 Oct 2017 15:32:30 +0300 Subject: [PATCH 08/25] changed scrolling values in tests --- test/components/popup.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index b3a247d6c..1fbc53527 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -179,7 +179,7 @@ describe('', () => { Popup Body ).withDriver(PopupTestDriver); - scrollDiv!.scrollTop = 100; + scrollDiv!.scrollTop = 200; return waitForDom(() => { expect([anchorDiv, popup.root]).to.be.inVerticalSequence(); @@ -209,7 +209,7 @@ describe('', () => { ).withDriver(PopupTestDriver); await waitForDom(() => expect(popup.root).to.be.present()); - scrollDiv!.scrollTop = 51; + scrollDiv!.scrollTop = 500; return waitFor(() => { expect(onExitBounds).to.have.been.called; From 7f8ea23306a33f43c6b0e2054a3f8bbd9bcea79d Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Tue, 24 Oct 2017 16:05:31 +0300 Subject: [PATCH 09/25] changed overflow to auto for safari --- test/components/popup.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index 1fbc53527..325929ed5 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -162,7 +162,7 @@ describe('', () => { let scrollDiv: HTMLDivElement; const {waitForDom} = clientRenderer.render(
-
scrollDiv = elem} style={{height: '100px', overflow: 'scroll'}}> +
scrollDiv = elem} style={{height: '100px', overflow: 'auto'}}>
Filler
anchorDiv = elem}>Anchor
@@ -191,7 +191,7 @@ describe('', () => { let scrollDiv: HTMLDivElement; const onExitBounds = sinon.spy(); const {waitForDom} = clientRenderer.render( -
scrollDiv = elem} style={{height: '1000px', overflow: 'scroll'}}> +
scrollDiv = elem} style={{height: '1000px', overflow: 'auto'}}>
div = elem} style={{height: '50px'}}>Anchor
From 8b6b389ed2349328c938c28f6ff370313da929b2 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Tue, 24 Oct 2017 16:17:07 +0300 Subject: [PATCH 10/25] added webkit overflow touch for mobile --- test/components/popup.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index 325929ed5..49cae290e 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -162,7 +162,7 @@ describe('', () => { let scrollDiv: HTMLDivElement; const {waitForDom} = clientRenderer.render(
-
scrollDiv = elem} style={{height: '100px', overflow: 'auto'}}> +
scrollDiv = elem} style={{height: '100px', overflow: 'auto', webKitOverFlow: 'touch'}}>
Filler
anchorDiv = elem}>Anchor
@@ -191,7 +191,7 @@ describe('', () => { let scrollDiv: HTMLDivElement; const onExitBounds = sinon.spy(); const {waitForDom} = clientRenderer.render( -
scrollDiv = elem} style={{height: '1000px', overflow: 'auto'}}> +
scrollDiv = elem} style={{height: '1000px', overflow: 'auto', webKitOverFlow: 'touch'}}>
div = elem} style={{height: '50px'}}>Anchor
From bf5b67e72bc2a216eb648a9b2adfc38fd0a0d72e Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Tue, 24 Oct 2017 16:22:35 +0300 Subject: [PATCH 11/25] fixed lint error --- test/components/popup.spec.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index 49cae290e..d431e7f57 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -162,7 +162,10 @@ describe('', () => { let scrollDiv: HTMLDivElement; const {waitForDom} = clientRenderer.render(
-
scrollDiv = elem} style={{height: '100px', overflow: 'auto', webKitOverFlow: 'touch'}}> +
scrollDiv = elem} + style={{height: '100px', overflow: 'auto', webKitOverFlow: 'touch'}} + >
Filler
anchorDiv = elem}>Anchor
@@ -191,7 +194,10 @@ describe('', () => { let scrollDiv: HTMLDivElement; const onExitBounds = sinon.spy(); const {waitForDom} = clientRenderer.render( -
scrollDiv = elem} style={{height: '1000px', overflow: 'auto', webKitOverFlow: 'touch'}}> +
scrollDiv = elem} + style={{height: '1000px', overflow: 'auto', webKitOverFlow: 'touch'}} + >
div = elem} style={{height: '50px'}}>Anchor
From 1fc90d27d5b626661fbe09d736d74f3a148bdd2a Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Tue, 24 Oct 2017 16:32:07 +0300 Subject: [PATCH 12/25] fixed wrong style name in tests --- test/components/popup.spec.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index d431e7f57..fffaa855d 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -164,7 +164,7 @@ describe('', () => {
scrollDiv = elem} - style={{height: '100px', overflow: 'auto', webKitOverFlow: 'touch'}} + style={{height: '100px', overflow: 'auto', WebkitOverflowScrolling: 'touch'}} >
Filler
anchorDiv = elem}>Anchor
@@ -196,7 +196,7 @@ describe('', () => { const {waitForDom} = clientRenderer.render(
scrollDiv = elem} - style={{height: '1000px', overflow: 'auto', webKitOverFlow: 'touch'}} + style={{height: '1000px', overflow: 'auto', WebkitOverflowScrolling: 'touch'}} >
div = elem} style={{height: '50px'}}>Anchor
From d3322856ea6743709a6aaa35c1351a0cee75f8e3 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Wed, 25 Oct 2017 15:21:39 +0300 Subject: [PATCH 13/25] found the problem? --- test/components/popup.spec.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index fffaa855d..0d4d3664f 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -126,9 +126,12 @@ describe('', () => { after(() => { document.body.removeChild(scroll); + }); + + afterEach(() => { document.body.scrollTop = 0; document.body.scrollLeft = 0; - }); + }) it('renders the popup in the right location when it is out of view', async () => { let div: HTMLDivElement; From ca210c286f64d5bbe41530429c94559237751c5d Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Wed, 25 Oct 2017 15:53:23 +0300 Subject: [PATCH 14/25] added missing semicolon --- test/components/popup.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index 0d4d3664f..7ff834770 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -131,7 +131,7 @@ describe('', () => { afterEach(() => { document.body.scrollTop = 0; document.body.scrollLeft = 0; - }) + }); it('renders the popup in the right location when it is out of view', async () => { let div: HTMLDivElement; From 1a0d940382fb9a18509a38b46a809d76458253ce Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Wed, 25 Oct 2017 16:13:56 +0300 Subject: [PATCH 15/25] changed tests values to better reflect test --- test/components/popup.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index 7ff834770..40e08b8d0 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -218,7 +218,7 @@ describe('', () => { ).withDriver(PopupTestDriver); await waitForDom(() => expect(popup.root).to.be.present()); - scrollDiv!.scrollTop = 500; + scrollDiv!.scrollTop = 51; return waitFor(() => { expect(onExitBounds).to.have.been.called; From 834c0d6e77cf980709c543a7b309ff1e011a99d8 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Wed, 25 Oct 2017 16:53:05 +0300 Subject: [PATCH 16/25] some small changes and dealing with points --- src/components/popup/popup.tsx | 14 ++++++++++---- src/components/portal/portal.tsx | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index cf85b12fc..397768999 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -39,7 +39,6 @@ export class Popup extends React.Component { private isOutOfBounds = false; public render() { - this.isOutOfBounds = false; if (this.props.anchor && this.props.open) { return ( { if (this.isOutOfBounds) { this.props.onExitBounds!(); } + this.isOutOfBounds = false; } public getPortal(): Portal | null { return this.portal; } - private onScroll = (e: any) => { - if (e.target.contains(this.props.anchor)) { - this.forceUpdate(); + private onScroll = (e: Event) => { + if (this.props.anchor) { + if (isPoint(this.props.anchor)) { + if (isOutOfBounds(this.props.anchor.y, this.props.anchor.x, 0, 0)) { + this.forceUpdate(); + } + } else if ((e.target as Node).contains(this.props.anchor)) { + this.forceUpdate(); + } } } diff --git a/src/components/portal/portal.tsx b/src/components/portal/portal.tsx index 720cb9da7..76ac01fe0 100644 --- a/src/components/portal/portal.tsx +++ b/src/components/portal/portal.tsx @@ -35,8 +35,8 @@ export class Portal extends React.PureComponent { } } - public getPortal() { - if (this.container) { + public getPortal(): Element | null { + if (this.container && this.container.children.length > 0) { return this.container.children[0]; } return null; From 1173b3e4ebb83e479d1cc98d3ebdd3d6ea01f89b Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Thu, 26 Oct 2017 15:23:16 +0300 Subject: [PATCH 17/25] some small fixes --- src/components/popup/popup.tsx | 23 ++++++++++++++--------- src/components/portal/portal.tsx | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index 4cda7ba6e..c9a5d7169 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -37,7 +37,7 @@ export class Popup extends React.Component { }; private portal: Portal | null; - private isOutOfBounds = false; + private isExitingBounds = false; public render() { if (this.props.anchor && this.props.open) { @@ -62,10 +62,10 @@ export class Popup extends React.Component { } public componentDidUpdate() { - if (this.isOutOfBounds) { + if (this.isExitingBounds) { this.props.onExitBounds!(); } - this.isOutOfBounds = false; + this.isExitingBounds = false; } public getPortal(): Portal | null { @@ -73,9 +73,10 @@ export class Popup extends React.Component { } private onScroll = (e: Event) => { - if (this.props.anchor) { - if (isPoint(this.props.anchor)) { - if (isOutOfBounds(this.props.anchor.y, this.props.anchor.x, 0, 0)) { + if (this.props.anchor && this.props.open) { + if (e.target === document || isPoint(this.props.anchor)) { + const rect = this.getPortalRect(); + if (rect && isFullyContainedWithinWindow(rect.top, rect.left, rect.height, rect.width)) { this.forceUpdate(); } } else if ((e.target as Node).contains(this.props.anchor)) { @@ -122,12 +123,16 @@ export class Popup extends React.Component { break; } - const rect = this.portal && this.portal.getPortal && this.portal.getPortal()!.getBoundingClientRect(); + const rect = this.getPortalRect(); if (rect) { - this.isOutOfBounds = isOutOfBounds(newStyle.top, newStyle.left, rect.height, rect.width) ? true : false; + this.isExitingBounds = isFullyContainedWithinWindow(newStyle.top, newStyle.left, rect.height, rect.width); } return newStyle; } + + private getPortalRect(): ClientRect | null { + return this.portal && this.portal.getPortal && this.portal.getPortal()!.getBoundingClientRect(); + } } function getVerticalReference(rect: ClientRect, anchorPosition: PopupVerticalPosition): number { @@ -155,7 +160,7 @@ function isPoint(elem: Element | Point): elem is Point { return elem.hasOwnProperty('x') && elem.hasOwnProperty('y'); } -function isOutOfBounds(top: number, left: number, height: number, width: number): boolean { +function isFullyContainedWithinWindow(top: number, left: number, height: number, width: number): boolean { return top < 0 || left < 0 || top + height - window.pageYOffset > window.innerHeight || diff --git a/src/components/portal/portal.tsx b/src/components/portal/portal.tsx index 76ac01fe0..082670c71 100644 --- a/src/components/portal/portal.tsx +++ b/src/components/portal/portal.tsx @@ -37,7 +37,7 @@ export class Portal extends React.PureComponent { public getPortal(): Element | null { if (this.container && this.container.children.length > 0) { - return this.container.children[0]; + return this.container.firstElementChild; } return null; } From c1bc77851bc14500176dfaaf9e5b96829d55c741 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Sun, 29 Oct 2017 10:45:21 +0200 Subject: [PATCH 18/25] removed point and document if from scroll event --- src/components/popup/popup.tsx | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index c9a5d7169..ead27d0d4 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -74,12 +74,7 @@ export class Popup extends React.Component { private onScroll = (e: Event) => { if (this.props.anchor && this.props.open) { - if (e.target === document || isPoint(this.props.anchor)) { - const rect = this.getPortalRect(); - if (rect && isFullyContainedWithinWindow(rect.top, rect.left, rect.height, rect.width)) { - this.forceUpdate(); - } - } else if ((e.target as Node).contains(this.props.anchor)) { + if (!isPoint(this.props.anchor) && (e.target as Node).contains(this.props.anchor)) { this.forceUpdate(); } } From 67587b827a8e630fb1eb741911c234bf2ed2ab57 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Sun, 29 Oct 2017 14:41:35 +0200 Subject: [PATCH 19/25] some small PR changes --- src/components/popup/popup.tsx | 8 ++++---- src/components/portal/portal.tsx | 5 +---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index ead27d0d4..9ab991bbf 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -73,10 +73,10 @@ export class Popup extends React.Component { } private onScroll = (e: Event) => { - if (this.props.anchor && this.props.open) { - if (!isPoint(this.props.anchor) && (e.target as Node).contains(this.props.anchor)) { - this.forceUpdate(); - } + debugger; + if (this.props.anchor && this.props.open + && !isPoint(this.props.anchor) && (e.target as Node).contains(this.props.anchor)) { + this.forceUpdate(); } } diff --git a/src/components/portal/portal.tsx b/src/components/portal/portal.tsx index 082670c71..0e47f6286 100644 --- a/src/components/portal/portal.tsx +++ b/src/components/portal/portal.tsx @@ -36,10 +36,7 @@ export class Portal extends React.PureComponent { } public getPortal(): Element | null { - if (this.container && this.container.children.length > 0) { - return this.container.firstElementChild; - } - return null; + return this.container && this.container.firstElementChild; } private renderPortal() { From 3e194e5da14f1d513c52f98af7990aa43e142d57 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Sun, 29 Oct 2017 14:57:18 +0200 Subject: [PATCH 20/25] removed debugger --- src/components/popup/popup.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index 9ab991bbf..fe2368429 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -73,7 +73,6 @@ export class Popup extends React.Component { } private onScroll = (e: Event) => { - debugger; if (this.props.anchor && this.props.open && !isPoint(this.props.anchor) && (e.target as Node).contains(this.props.anchor)) { this.forceUpdate(); From dba5ea02cd4cb550f9c37ce9e4480ede7c1c9257 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Wed, 1 Nov 2017 13:10:25 +0200 Subject: [PATCH 21/25] PR fixes --- src/components/popup/popup.tsx | 6 +++--- test/components/popup.spec.tsx | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index fe2368429..c461b8151 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -117,15 +117,15 @@ export class Popup extends React.Component { break; } - const rect = this.getPortalRect(); + const rect = this.getContentRect(); if (rect) { this.isExitingBounds = isFullyContainedWithinWindow(newStyle.top, newStyle.left, rect.height, rect.width); } return newStyle; } - private getPortalRect(): ClientRect | null { - return this.portal && this.portal.getPortal && this.portal.getPortal()!.getBoundingClientRect(); + private getContentRect(): ClientRect | null { + return this.portal && this.portal.getPortal() && this.portal.getPortal()!.getBoundingClientRect(); } } diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index 40e08b8d0..e5a378c69 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -193,7 +193,7 @@ describe('', () => { }); it('calls onExitBounds when the popup leaves the viewport', async () => { - let div: HTMLDivElement; + let anchorDiv: HTMLDivElement; let scrollDiv: HTMLDivElement; const onExitBounds = sinon.spy(); const {waitForDom} = clientRenderer.render( @@ -201,16 +201,16 @@ describe('', () => { ref={(elem: HTMLDivElement) => scrollDiv = elem} style={{height: '1000px', overflow: 'auto', WebkitOverflowScrolling: 'touch'}} > -
div = elem} style={{height: '50px'}}>Anchor
+
anchorDiv = elem} style={{height: '50px'}}>Anchor
); - await waitForDom(() => expect(div).to.be.present()); + await waitForDom(() => expect(anchorDiv).to.be.present()); const {driver: popup} = clientRenderer.render( @@ -218,10 +218,11 @@ describe('', () => { ).withDriver(PopupTestDriver); await waitForDom(() => expect(popup.root).to.be.present()); + expect(onExitBounds).to.not.have.been.called; scrollDiv!.scrollTop = 51; return waitFor(() => { - expect(onExitBounds).to.have.been.called; + expect(onExitBounds).to.have.been.calledOnce; }); }); }); From 22c9a01cd5cee2c40d9a5bd9c227f110ce42132f Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Wed, 1 Nov 2017 15:43:29 +0200 Subject: [PATCH 22/25] added some more tests for out of bounds --- src/components/popup/popup.tsx | 4 +- test/components/popup.spec.tsx | 104 ++++++++++++++++++++++++++++++++- 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index c461b8151..032ecb4a6 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -155,8 +155,8 @@ function isPoint(elem: Element | Point): elem is Point { } function isFullyContainedWithinWindow(top: number, left: number, height: number, width: number): boolean { - return top < 0 || - left < 0 || + return top < window.pageYOffset || + left < window.pageXOffset || top + height - window.pageYOffset > window.innerHeight || left + width - window.pageXOffset > window.innerWidth; } diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index e5a378c69..e602de37d 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -192,7 +192,7 @@ describe('', () => { }); }); - it('calls onExitBounds when the popup leaves the viewport', async () => { + it('calls onExitBounds when the popup leaves the viewport from top', async () => { let anchorDiv: HTMLDivElement; let scrollDiv: HTMLDivElement; const onExitBounds = sinon.spy(); @@ -225,6 +225,108 @@ describe('', () => { expect(onExitBounds).to.have.been.calledOnce; }); }); + + it('calls onExitBounds when the popup leaves the viewport from bottom', async () => { + let anchorDiv: HTMLDivElement; + let scrollDiv: HTMLDivElement; + const onExitBounds = sinon.spy(); + const {waitForDom} = clientRenderer.render( +
scrollDiv = elem} + style={{height: '1000px', overflow: 'auto', WebkitOverflowScrolling: 'touch'}} + > +
+
anchorDiv = elem} style={{height: '50px'}}>Anchor
+
+
+ ); + scrollDiv!.scrollTop = 3100; + await waitForDom(() => expect(anchorDiv).to.be.present()); + + const {driver: popup} = clientRenderer.render( + +
Body
+
).withDriver(PopupTestDriver); + + await waitForDom(() => expect(popup.root).to.be.present()); + expect(onExitBounds).to.not.have.been.called; + scrollDiv!.scrollTop = 2000; + return waitFor(() => { + expect(onExitBounds).to.have.been.calledOnce; + }); + }); + + it('calls onExitBounds when the popup leaves the viewport from left', async () => { + let anchorDiv: HTMLDivElement; + let scrollDiv: HTMLDivElement; + const onExitBounds = sinon.spy(); + const {waitForDom} = clientRenderer.render( +
scrollDiv = elem} + style={{width: '100%', overflow: 'auto', WebkitOverflowScrolling: 'touch'}} + > +
anchorDiv = elem} style={{width: '50px'}}>Anchor
+
+
+ ); + + await waitForDom(() => expect(anchorDiv).to.be.present()); + + const {driver: popup} = clientRenderer.render( + +
Body
+
).withDriver(PopupTestDriver); + + await waitForDom(() => expect(popup.root).to.be.present()); + expect(onExitBounds).to.not.have.been.called; + scrollDiv!.scrollLeft = 1; + + return waitFor(() => { + expect(onExitBounds).to.have.been.calledOnce; + }); + }); + + it('calls onExitBounds when the popup leaves the viewport from right', async () => { + let anchorDiv: HTMLDivElement; + let scrollDiv: HTMLDivElement; + const onExitBounds = sinon.spy(); + const {waitForDom} = clientRenderer.render( +
scrollDiv = elem} + style={{width: '100%', overflow: 'auto', WebkitOverflowScrolling: 'touch'}} + > +
anchorDiv = elem} style={{width: '200%'}}>Anchor
+
+ ); + + await waitForDom(() => expect(anchorDiv).to.be.present()); + scrollDiv!.scrollLeft = 50; + const {driver: popup} = clientRenderer.render( + +
Body
+
).withDriver(PopupTestDriver); + + await waitForDom(() => expect(popup.root).to.be.present()); + expect(onExitBounds).to.not.have.been.called; + scrollDiv!.scrollLeft = 0; + + return waitFor(() => { + expect(onExitBounds).to.have.been.calledOnce; + }); + }); }); describe('Layout tests', () => { From d5795982f0fc646dfa6c0cd797b2a9746c9a0a86 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Thu, 2 Nov 2017 14:27:19 +0200 Subject: [PATCH 23/25] added check that popup is in the correct position for out of bounds test --- test/components/popup.spec.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/components/popup.spec.tsx b/test/components/popup.spec.tsx index e602de37d..5f10fa9c3 100644 --- a/test/components/popup.spec.tsx +++ b/test/components/popup.spec.tsx @@ -217,7 +217,10 @@ describe('', () => {
Body
).withDriver(PopupTestDriver); - await waitForDom(() => expect(popup.root).to.be.present()); + await waitForDom(() => { + expect(popup.root).to.be.present(); + expect(popup.root.getBoundingClientRect().top).to.equal(50); + }); expect(onExitBounds).to.not.have.been.called; scrollDiv!.scrollTop = 51; From ea505678a17f7482b6a7fdc877229aae06d79515 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Sun, 5 Nov 2017 15:06:04 +0200 Subject: [PATCH 24/25] changed portal to use a callback --- src/components/popup/popup.tsx | 12 +++++++----- src/components/portal/portal.tsx | 20 ++++++++++++++------ test/components/portal.spec.tsx | 24 +++++++++++++++++++++++- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/components/popup/popup.tsx b/src/components/popup/popup.tsx index 032ecb4a6..368da8a15 100644 --- a/src/components/popup/popup.tsx +++ b/src/components/popup/popup.tsx @@ -38,6 +38,7 @@ export class Popup extends React.Component { private portal: Portal | null; private isExitingBounds = false; + private portalRect: ClientRect | null = null; public render() { if (this.props.anchor && this.props.open) { @@ -45,6 +46,7 @@ export class Popup extends React.Component { this.portal = portal} + onLayout={this.onPortalLayout} > {this.props.children} ); @@ -117,15 +119,15 @@ export class Popup extends React.Component { break; } - const rect = this.getContentRect(); - if (rect) { - this.isExitingBounds = isFullyContainedWithinWindow(newStyle.top, newStyle.left, rect.height, rect.width); + if (this.portalRect) { + this.isExitingBounds = isFullyContainedWithinWindow(newStyle.top, + newStyle.left, this.portalRect.height, this.portalRect.width); } return newStyle; } - private getContentRect(): ClientRect | null { - return this.portal && this.portal.getPortal() && this.portal.getPortal()!.getBoundingClientRect(); + private onPortalLayout = (rect: ClientRect | null) => { + this.portalRect = rect; } } diff --git a/src/components/portal/portal.tsx b/src/components/portal/portal.tsx index 0e47f6286..dd5d4bf76 100644 --- a/src/components/portal/portal.tsx +++ b/src/components/portal/portal.tsx @@ -1,17 +1,22 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; import {globalId} from 'wix-react-tools'; +import {noop} from '../../utils'; export interface PortalProps extends React.HTMLAttributes { children: React.ReactNode; + onLayout?(layout: ClientRect | null): void; } export class Portal extends React.PureComponent { + public static defaultProps: Partial = { + onLayout: noop + }; private container: HTMLDivElement | null; private portalContent: React.ReactElement>; public render() { - const {children, ...rest} = this.props; + const {children, onLayout, ...rest} = this.props; const uniqueId = globalId.getRootId(this); this.portalContent = (
{children}
@@ -35,15 +40,18 @@ export class Portal extends React.PureComponent { } } - public getPortal(): Element | null { - return this.container && this.container.firstElementChild; - } - private renderPortal() { - ReactDOM.unstable_renderSubtreeIntoContainer(this, this.portalContent, this.getContainer()); + ReactDOM.unstable_renderSubtreeIntoContainer(this, this.portalContent, this.getContainer(), this.onRender); } private getContainer() { return this.container = this.container || document.body.appendChild(document.createElement('div')); } + + private onRender = () => { + if (this.props.onLayout !== noop) { + const portalContent = this.container && this.container.firstElementChild; + this.props.onLayout!(portalContent ? portalContent.getBoundingClientRect() : null); + } + } } diff --git a/test/components/portal.spec.tsx b/test/components/portal.spec.tsx index 16d9273b0..39515efa2 100644 --- a/test/components/portal.spec.tsx +++ b/test/components/portal.spec.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; -import {ClientRenderer, expect, waitFor} from 'test-drive-react'; +import {ClientRenderer, expect, waitFor, sinon} from 'test-drive-react'; import {Portal} from '../../src'; import {PortalTestDriver} from '../../test-kit'; @@ -123,4 +123,26 @@ describe('', () => { await waitFor(() => expect(driver.portal as Element).to.have.attribute('class', 'test-class')); }); + + it('calls the onLayout callback after rendering with the correct layout', async () => { + const onLayout = sinon.spy(); + const layout = { + top: 0, + left: 0, + right: 102, + bottom: 101, + height: 101, + width: 102 + }; + clientRenderer.render( + +
1
+
+ ); + + await waitFor(() => { + expect(onLayout).to.have.been.calledOnce; + expect(onLayout.getCall(0).args[0]).to.include(layout); + }); + }); }); From 932f6428704989f12d95bb6ebb84bcac6d525fe4 Mon Sep 17 00:00:00 2001 From: "WIXPRESS\\danielstr" Date: Sun, 5 Nov 2017 15:21:15 +0200 Subject: [PATCH 25/25] fixed imports --- test/components/portal.spec.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/components/portal.spec.tsx b/test/components/portal.spec.tsx index 39515efa2..d4f08bd11 100644 --- a/test/components/portal.spec.tsx +++ b/test/components/portal.spec.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import * as ReactDOM from 'react-dom'; -import {ClientRenderer, expect, waitFor, sinon} from 'test-drive-react'; +import {ClientRenderer, expect, sinon, waitFor} from 'test-drive-react'; import {Portal} from '../../src'; import {PortalTestDriver} from '../../test-kit';