From de187f6763156ae791390d69b0b3f942e4164d67 Mon Sep 17 00:00:00 2001 From: Martin Hristov Date: Tue, 5 Nov 2019 13:01:15 +0200 Subject: [PATCH 1/2] fix(ui5-popover): restrict max content height when overflowing the screen FIXES: https://github.com/SAP/ui5-webcomponents/issues/853 --- packages/main/src/Popover.hbs | 2 +- packages/main/src/Popover.js | 10 ++++++++-- packages/main/src/themes/Popover.css | 1 + packages/main/test/pages/Popover.html | 8 ++++++++ packages/main/test/specs/Popover.spec.js | 15 +++++++++++++++ 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/main/src/Popover.hbs b/packages/main/src/Popover.hbs index fdb012218ea0..8b899de0b207 100644 --- a/packages/main/src/Popover.hbs +++ b/packages/main/src/Popover.hbs @@ -12,7 +12,7 @@ -
+
diff --git a/packages/main/src/Popover.js b/packages/main/src/Popover.js index e29282bc9dde..c13ee10e1988 100644 --- a/packages/main/src/Popover.js +++ b/packages/main/src/Popover.js @@ -151,6 +151,8 @@ const metadata = { * @private */ opened: { type: Boolean }, + + _maxContentHeight: { type: Integer }, }, slots: { /** @@ -545,8 +547,9 @@ class Popover extends UI5Element { let maxContentHeight = Math.round(maxHeight); - if (this.hasHeader) { - const headerDomRef = this.getPopupDomRef().querySelector(".ui5-popup-header"); + if (this.header.length) { + const headerDomRef = this.getPopupDomRef().querySelector(".ui5-popover-header-root"); + if (headerDomRef) { maxContentHeight = Math.round(maxHeight - headerDomRef.offsetHeight); } @@ -651,6 +654,9 @@ class Popover extends UI5Element { get styles() { return { + content: { + "max-height": `${this._maxContentHeight}px`, + }, arrow: { transform: `translate(${this.arrowTranslateX}px, ${this.arrowTranslateY}px)`, }, diff --git a/packages/main/src/themes/Popover.css b/packages/main/src/themes/Popover.css index 21e18104d827..0de32ef949dd 100644 --- a/packages/main/src/themes/Popover.css +++ b/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 { diff --git a/packages/main/test/pages/Popover.html b/packages/main/test/pages/Popover.html index bda4b58b4a5b..c210267b6b23 100644 --- a/packages/main/test/pages/Popover.html +++ b/packages/main/test/pages/Popover.html @@ -136,6 +136,10 @@

+ + + + diff --git a/packages/main/test/specs/Popover.spec.js b/packages/main/test/specs/Popover.spec.js index 05410884b6e8..beeb5b11a398 100644 --- a/packages/main/test/specs/Popover.spec.js +++ b/packages/main/test/specs/Popover.spec.js @@ -28,4 +28,19 @@ 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"); + }); }); From fc0f7c0708e1b29f4b169efc1260b78c3c052612 Mon Sep 17 00:00:00 2001 From: Martin Hristov Date: Tue, 5 Nov 2019 13:38:59 +0200 Subject: [PATCH 2/2] handle case when having a header --- packages/main/src/Popover.js | 7 +++++-- packages/main/test/pages/Popover.html | 22 +++++++++++++++++++--- packages/main/test/specs/Popover.spec.js | 18 ++++++++++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/packages/main/src/Popover.js b/packages/main/src/Popover.js index c13ee10e1988..a99abb4abda1 100644 --- a/packages/main/src/Popover.js +++ b/packages/main/src/Popover.js @@ -547,8 +547,11 @@ class Popover extends UI5Element { let maxContentHeight = Math.round(maxHeight); - if (this.header.length) { - const headerDomRef = this.getPopupDomRef().querySelector(".ui5-popover-header-root"); + 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); diff --git a/packages/main/test/pages/Popover.html b/packages/main/test/pages/Popover.html index c210267b6b23..592a0a47bde1 100644 --- a/packages/main/test/pages/Popover.html +++ b/packages/main/test/pages/Popover.html @@ -136,10 +136,18 @@

- - - + + +
+
+ + + Open Big Popover + + + + diff --git a/packages/main/test/specs/Popover.spec.js b/packages/main/test/specs/Popover.spec.js index beeb5b11a398..be32bdf756ba 100644 --- a/packages/main/test/specs/Popover.spec.js +++ b/packages/main/test/specs/Popover.spec.js @@ -42,5 +42,23 @@ describe("Popover general interaction", () => { 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"); }); });