fix(ui5-popover): restrict max content height when overflowing the screen (#908)

FIXES: #853

Author: MapTo0

packages/main/src/Popover.hbs

@@ -12,7 +12,7 @@
 
 	<span class="first-fe" tabindex="0" @focusin={{forwardToLast}}></span>	
 
-	<div class="ui5-popover-content">
+	<div class="ui5-popover-content" style={{styles.content}}>
 		<slot></slot>
 	</div>
 

packages/main/src/Popover.js

@@ -151,6 +151,8 @@ const metadata = {
 		 * @private
 		 */
 		opened: { type: Boolean },
+
+		_maxContentHeight: { type: Integer },
 	},
 	slots: {
 		/**
@@ -545,8 +547,12 @@ class Popover extends UI5Element {
 
 		let maxContentHeight = Math.round(maxHeight);
 
-		if (this.hasHeader) {
-			const headerDomRef = this.getPopupDomRef().querySelector(".ui5-popup-header");
+		const hasHeader = this.header.length || this.headerText;
+
+		if (hasHeader) {
+			const headerDomRef = this.shadowRoot.querySelector(".ui5-popover-header-root")
+				|| this.shadowRoot.querySelector(".ui5-popup-header-text");
+
 			if (headerDomRef) {
 				maxContentHeight = Math.round(maxHeight - headerDomRef.offsetHeight);
 			}
@@ -651,6 +657,9 @@ class Popover extends UI5Element {
 
 	get styles() {
 		return {
+			content: {
+				"max-height": `${this._maxContentHeight}px`,
+			},
 			arrow: {
 				transform: `translate(${this.arrowTranslateX}px, ${this.arrowTranslateY}px)`,
 			},

packages/main/src/themes/Popover.css

@@ -137,6 +137,7 @@
 
 	/* Consider how to make this top level */
 	padding: var(--_ui5_popover_content_padding);
+	box-sizing: border-box;
 }
 
 :host([header-text]) .ui5-popup-header-text {

packages/main/test/pages/Popover.html

@@ -136,6 +136,18 @@
 	<br>
 	<br>
 
+	<ui5-select id="many-items"></ui5-select>
+
+	<br>
+	<br>
+
+	<ui5-button id="big-popover-button">
+		Open Big Popover
+	</ui5-button>
+
+	<ui5-popover placement-type="Bottom" id="big-popover" header-text="hello world">
+		<ui5-list id="bigList"></ui5-list>
+	</ui5-popover>
 
 	<script>
 
@@ -151,6 +163,18 @@
 			bigDangerPop.openBy(bigDanger);
 		});
 
+		document.getElementById("many-items").innerHTML = [].map.call("Ullamco ea ad fugiat enim elit dolore ullamco ad magna excepteur cupidatat.", function (item, index) {
+			return "<ui5-option>" + item + "</ui5-option>";
+		}).join("");
+
+		document.getElementById("bigList").innerHTML = [].map.call("Ullamco ea ad fugiat enim elit dolore ullamco ad magna excepteur cupidatat.", function (item, index) {
+			return "<ui5-li>" + item + "</ui5-li>";
+		}).join("");
+
+		document.getElementById("big-popover-button").addEventListener("click", function (event) {
+			document.getElementById("big-popover").openBy(event.target);
+		});
+
 	</script>
 </body>
 

packages/main/test/specs/Popover.spec.js

@@ -28,4 +28,37 @@ describe("Popover general interaction", () => {
 		btnInPopover.click();
 		assert.ok(popover.isDisplayedInViewport(), "Popover remains opened.");
 	});
+
+	it("tests if overflown content can be reached by scrolling", () => {
+		const manyItemsSelect = $("#many-items");
+		const items = manyItemsSelect.shadow$$("ui5-li");
+
+		manyItemsSelect.click();
+
+		const lastListItem = items[items.length - 1];
+		
+		assert.strictEqual(lastListItem.isDisplayedInViewport(), false, "Last item is not displayed after openining");
+
+		lastListItem.scrollIntoView();
+
+		assert.strictEqual(lastListItem.isDisplayedInViewport(), true, "Last item is displayed after scrolling");
+
+		manyItemsSelect.click();
+	});
+
+	it("tests if overflown content can be reached by scrolling (with header and arrow)", () => {
+		const bigPopover = $("#big-popover");
+		const items = bigPopover.$$("ui5-li");
+		const openBigPopoverButton = $("#big-popover-button")
+
+		openBigPopoverButton.click();
+
+		const lastListItem = items[items.length - 1];
+
+		assert.strictEqual(lastListItem.isDisplayedInViewport(), false, "Last item is not displayed after openining");
+
+		lastListItem.scrollIntoView();
+
+		assert.strictEqual(lastListItem.isDisplayedInViewport(), true, "Last item is displayed after scrolling");
+	});
 });