fix(popup): now watching anchor size for position correctness (#1236)

* fix(popup): now watching anchor size for position correctness

Closes #1234

* Linting fixes

* update story

* Fix the tests

* Fix linting

* Refactor tests

Co-authored-by: Rina <rina.oksman@vonage.com>

components/popup/src/vwc-popup-base.ts

@@ -122,14 +122,8 @@ export class VWCPopupBase extends LitElement {
     	this.open = false;
     }
 
-    // TODO: Make the observer work on positioning for the first time
-    // new `IntersectionObserver` constructor
-    private observer = new IntersectionObserver((entries) => {
-    	for (const entry of entries) {
-    		const bounds = entry.boundingClientRect;
-    		console.log('anchor element bounds ', bounds);
-    		requestAnimationFrame(() => this.updatePosition());
-    	}
+    private sizeObserver = new ResizeObserver(() => {
+    	return this.updatePosition();
     });
 
     override connectedCallback(): void {
@@ -143,19 +137,21 @@ export class VWCPopupBase extends LitElement {
     	window.removeEventListener('scroll', this.updatePosition);
     	window.removeEventListener('resize', this.onResizeWindow);
     	// Disconnect the observer to stop from running in the background
-    	this.observer.disconnect();
+    	this.sizeObserver.disconnect();
     }
 
     protected override firstUpdated(_changedProperties: PropertyValues): void {
     	super.firstUpdated(_changedProperties);
     	this.anchorEl = this.getAnchorById();
-    	if(this.anchorEl) this.observer.observe(this.anchorEl);
+    	if(this.anchorEl) this.sizeObserver.observe(this.anchorEl);
     }
 
     protected override updated(changes: Map<string, boolean>): void {
     	super.updated(changes);
     	if (changes.has('anchor')) {
+    		this.sizeObserver.disconnect();
     		this.anchorEl = this.getAnchorById();
+    		if(this.anchorEl) this.sizeObserver.observe(this.anchorEl);
     	}
     	this.updatePosition();
     }

components/popup/stories/popup.stories.js

@@ -34,7 +34,7 @@ const Template = args => html`
 		}
 	</style>
 	<div class="popup-wrapper">
-		<vwc-button id="buttonAnchor" layout="outlined" outlined aria-haspopup="true" aria-describedby="popup" @click=${onClick}>Click to open popup</vwc-button>
+		<vwc-button id="buttonAnchor" layout="outlined" outlined aria-haspopup="true" aria-describedby="popup" @click=${onClick}>Click on me</vwc-button>
 		<vwc-popup id="popup" anchor="buttonAnchor" ...=${spread(args)}>
 			<div class="content">
 				<vwc-text font-face="body-1-bold" tight><p class="line">Popup title</p></vwc-text>
@@ -45,15 +45,16 @@ const Template = args => html`
 	`;
 
 export const Basic = Template.bind({});
+Basic.args = { open: true };
 
 export const WithArrow = Template.bind({});
-WithArrow.args = { arrow: true, corner: "right" };
+WithArrow.args = { open: true, arrow: true, corner: "right" };
 
 export const Alternate = Template.bind({});
-Alternate.args = { arrow: true, corner: "bottom", alternate: true };
+Alternate.args = { open: true, arrow: true, corner: "bottom", alternate: true };
 
 export const Dismissible = Template.bind({});
-Dismissible.args = { dismissible: true, corner: "top" };
+Dismissible.args = { open: true, dismissible: true, corner: "top" };
 
 function onClick() {
 	popup.open = !popup.open;

components/popup/test/popup.test.js

@@ -4,9 +4,9 @@ import 'chai-dom';
 import {
 	waitNextTask,
 	textToDomToParent,
-	isolatedElementsCreation,
+	isolatedElementsCreation, waitInterval, noop,
 } from '../../../test/test-helpers.js';
-import { chaiDomDiff } from '@open-wc/semantic-dom-diff';
+import {chaiDomDiff} from '@open-wc/semantic-dom-diff';
 
 chai.use(chaiDomDiff);
 
@@ -39,7 +39,7 @@ describe('popup', () => {
 			await actualElement.updateComplete;
 			expect(actualElement.anchor, 'anchor should be ""')
 				.to
-				.equal("");
+				.equal('');
 			expect(actualElement.open, 'open should be false')
 				.to
 				.equal(false);
@@ -66,7 +66,7 @@ describe('popup', () => {
 			const [actualElement] = addElement(
 				textToDomToParent(`<${COMPONENT_NAME}></${COMPONENT_NAME}>`)
 			);
-			actualElement.anchor = "anchor";
+			actualElement.anchor = 'anchor';
 			await actualElement.updateComplete;
 
 			actualElement.show();
@@ -94,11 +94,40 @@ describe('popup', () => {
 	});
 
 	describe(`anchor`, () => {
-		it(`should not open the popup if anchor does not exist`, async () => {
+		async function flushResizeEvents() {
+			await waitNextTask();
+		}
+
+		function triggerResize(element) {
+			element.style.height = '20px';
+		}
+
+		function expectASingleUpdatePositionCall(updatePositionCallCount, anchorElement) {
+			const observer = new ResizeObserver(async function () {
+				await waitNextTask();
+				expect(updatePositionCallCount).to.equal(1);
+			});
+
+			observer.observe(anchorElement);
+		}
+
+		async function setPopupAndAnchor() {
+			const [anchorElement] = addElement(
+				textToDomToParent(`<vwc-button style="position: absolute; left: 100px;" layout="outlined" id="anchor">Button</vwc-button>`)
+			);
+			const [actualElement] = addElement(
+				textToDomToParent(`<${COMPONENT_NAME} arrow anchor="anchor" open><div>This is my popup</div></${COMPONENT_NAME}>`)
+			);
+			actualElement.updatePosition = noop;
+			await actualElement.updateComplete;
+			return {anchorElement, actualElement};
+		}
+
+		it(`should not set popup open if anchor element does not exist`, async () => {
 			const [actualElement] = addElement(
 				textToDomToParent(`<${COMPONENT_NAME}></${COMPONENT_NAME}>`)
 			);
-			actualElement.anchor = "anchor";
+			actualElement.anchor = 'anchor';
 			await actualElement.updateComplete;
 
 			actualElement.show();
@@ -109,16 +138,52 @@ describe('popup', () => {
 				.equal(false);
 		});
 
-		it(`should not open the popup if anchor does not exist`, async () => {
+		it(`should init the popup as open if anchor element does not exist`, async () => {
 			const [actualElement] = addElement(
 				textToDomToParent(`<${COMPONENT_NAME} open></${COMPONENT_NAME}>`)
 			);
-			actualElement.anchor = "anchor";
+			actualElement.anchor = 'anchor';
 			await actualElement.updateComplete;
 
 			expect(actualElement.open)
 				.to
 				.equal(false);
 		});
+
+		it(`should reposition when the anchor changes its size`, async function () {
+
+			const {anchorElement, actualElement} = await setPopupAndAnchor();
+
+			await flushResizeEvents();
+
+			let updatePositionCallCount = 0;
+			actualElement.updatePosition = function () {
+				updatePositionCallCount++;
+			};
+
+			triggerResize(anchorElement);
+
+			expectASingleUpdatePositionCall(updatePositionCallCount, anchorElement);
+
+		});
+
+		it(`should stop observing the anchor when changing an anchor`, async function () {
+			const {anchorElement, actualElement} = await setPopupAndAnchor();
+			await flushResizeEvents();
+
+			actualElement.anchor = '';
+			await waitNextTask();
+
+			let updatePositionCallCount = 0;
+			actualElement.updatePosition = function () {
+				updatePositionCallCount++;
+			};
+
+			anchorElement.style.height = '20px';
+
+			await waitNextTask();
+
+			expect(updatePositionCallCount).to.equal(0);
+		});
 	});
 });

test/test-helpers.js

@@ -14,9 +14,14 @@ export {
 	isFirefox,
 	getFrameLoadedInjected,
 	cleanFrame,
-	getRandom
+	getRandom,
+	noop
 }
 
+const noop = () => {
+	const x = [];
+	x.push(5);
+};
 const tmpTemple = document.createElement('template');
 
 function listenToSubmission(formElement) {
@@ -289,4 +294,4 @@ class TestComponent extends HTMLElement {
 	}
 }
 
-window.customElements.define('vivid-tests-component', TestComponent);
+window.customElements.define('vivid-tests-component', TestComponent);

ui-tests/index.ts

@@ -148,6 +148,7 @@ async function setDevServer() {
 		open: true
 	};
 
+	//@ts-ignore
 	const devServer = new WebpackDevServer(devServerOptions, compiler);
 
 	devServer.listen(webpackConfig.devServer.port, '127.0.0.1', () => {

ui-tests/tests/vwc-popup/index.js

@@ -0,0 +1,45 @@
+import '@vonage/vwc-popup';
+import '@vonage/vwc-button';
+import '@vonage/vwc-text';
+
+export async function createElementVariations(wrapper) {
+	const textElementWrapper = document.createElement('div');
+	textElementWrapper.innerHTML = `
+	<style>
+	.popup-wrapper {
+		width: 100%;
+		height: 400px;
+		display: flex;
+		align-items: center;
+		justify-content: center;
+		background-color: var(--vvd-color-neutral-10);
+	}
+	.content {
+		width: 200px;
+		text-align: left;
+		padding: 1rem;
+	}
+	.line {
+		border-bottom: 1px solid var(--vvd-color-neutral-40);
+		padding-bottom: 0.5rem;
+		margin-bottom: 0.5rem;
+	}
+</style>
+<div class="popup-wrapper">
+	<vwc-button id="buttonAnchor" layout="outlined" outlined aria-haspopup="true" aria-describedby="popup">Click to open popup</vwc-button>
+	<vwc-popup id="popup" open corner="left" dismissible anchor="buttonAnchor">
+		<div class="content">
+			<vwc-text font-face="body-1-bold" tight><p class="line">Popup title</p></vwc-text>
+			<vwc-text font-face="body-2" tight>Lorem ipsum dolor sit amet, consectetur adipiscing elit.</vwc-text>
+		</div>
+	</vwc-popup>
+	<vwc-popup arrow alternate corner="right" id="popup" open anchor="buttonAnchor">
+		<div class="content">
+			<vwc-text font-face="body-1-bold" tight><p class="line">Popup title</p></vwc-text>
+			<vwc-text font-face="body-2" tight>Lorem ipsum dolor sit amet, consectetur adipiscing elit.</vwc-text>
+		</div>
+	</vwc-popup>
+</div>`;
+wrapper.appendChild(textElementWrapper);
+
+}