From 1f377f8b881409cde383ed7fd3ce98fa6ae7a41e Mon Sep 17 00:00:00 2001 From: Filip Siderov Date: Wed, 28 Oct 2020 12:14:49 +0200 Subject: [PATCH 1/4] feat(ui5-popover): implement hide-block-layer property --- packages/main/src/Dialog.js | 4 ++++ packages/main/src/Popover.js | 15 +++++++++++++++ packages/main/src/Popup.js | 13 +++++++++++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/main/src/Dialog.js b/packages/main/src/Dialog.js index 3a4e05adc45e..7db180d515fc 100644 --- a/packages/main/src/Dialog.js +++ b/packages/main/src/Dialog.js @@ -174,6 +174,10 @@ class Dialog extends Popup { return true; } + get shouldHideBlockLayer() { // Required by Popup.js + return false; + } + get _ariaLabelledBy() { // Required by Popup.js return this.ariaLabel ? undefined : "ui5-popup-header"; } diff --git a/packages/main/src/Popover.js b/packages/main/src/Popover.js index ad49cfb37504..dd555b06d244 100644 --- a/packages/main/src/Popover.js +++ b/packages/main/src/Popover.js @@ -106,6 +106,17 @@ const metadata = { type: Boolean, }, + /** + * Defines whether the block layer will be shown if modal property is set to true. + * @type {Boolean} + * @defaultvalue false + * @public + * @since 1.0.0-rc.10 + */ + hideBlockLayer: { + type: Boolean, + }, + /** * Determines whether the ui5-popover arrow is hidden. * @@ -620,6 +631,10 @@ class Popover extends Popup { return this.modal; } + get shouldHideBlockLayer() { // Required by Popup.js + return this.hideBlockLayer; + } + get _ariaLabelledBy() { // Required by Popup.js return this.ariaLabel ? undefined : "ui5-popup-header"; } diff --git a/packages/main/src/Popup.js b/packages/main/src/Popup.js index 716d1c0d184c..baecf09d267e 100644 --- a/packages/main/src/Popup.js +++ b/packages/main/src/Popup.js @@ -66,7 +66,7 @@ const metadata = { * Defines the aria-label attribute for the popup * * @type {String} - * @defaultvalue: "" + * @defaultvalue "" * @private * @since 1.0.0-rc.8 */ @@ -308,7 +308,7 @@ class Popup extends UI5Element { return; } - if (this.isModal) { + if (this.isModal && !this.hideBlockLayer) { // create static area item ref for block layer this.getStaticAreaItemDomRef(); this._blockLayerHidden = false; @@ -425,6 +425,15 @@ class Popup extends UI5Element { */ get isModal() {} // eslint-disable-line + /** + * Implement this getter with relevant logic in order to hide the block layer (f.e. based on a public property) + * + * @protected + * @abstract + * @returns {boolean} + */ + get shouldHideBlockLayer() {} // eslint-disable-line + /** * Return the ID of an element in the shadow DOM that is going to label this popup * From ac5ededd717644484dbd229be1e078935838f902 Mon Sep 17 00:00:00 2001 From: Filip Siderov Date: Fri, 30 Oct 2020 11:20:25 +0200 Subject: [PATCH 2/4] fix comments & add test --- packages/main/src/Popover.js | 2 +- packages/main/src/Popup.js | 2 +- packages/main/test/pages/Popover.html | 22 ++++++++++++++++++++++ packages/main/test/specs/Popover.spec.js | 24 ++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/packages/main/src/Popover.js b/packages/main/src/Popover.js index dd555b06d244..4584c452aa91 100644 --- a/packages/main/src/Popover.js +++ b/packages/main/src/Popover.js @@ -108,7 +108,7 @@ const metadata = { /** * Defines whether the block layer will be shown if modal property is set to true. - * @type {Boolean} + * @type {boolean} * @defaultvalue false * @public * @since 1.0.0-rc.10 diff --git a/packages/main/src/Popup.js b/packages/main/src/Popup.js index baecf09d267e..747957a78d57 100644 --- a/packages/main/src/Popup.js +++ b/packages/main/src/Popup.js @@ -308,7 +308,7 @@ class Popup extends UI5Element { return; } - if (this.isModal && !this.hideBlockLayer) { + if (this.isModal && !this.shouldHideBlockLayer) { // create static area item ref for block layer this.getStaticAreaItemDomRef(); this._blockLayerHidden = false; diff --git a/packages/main/test/pages/Popover.html b/packages/main/test/pages/Popover.html index 0e3c001a6af9..3db49172ed03 100644 --- a/packages/main/test/pages/Popover.html +++ b/packages/main/test/pages/Popover.html @@ -171,6 +171,20 @@ + Opens modal popover with no block layer! + + + + Hello + Again + Wooooooooooooooooooooooooooooorld + + +
+ Close +
+
+ Opens popover with initial focus! @@ -374,6 +388,14 @@ modalPopover.openBy(btnPopModal); }); + btnPopModalNoLayer.addEventListener("click", function() { + modalPopoverNoLayer.openBy(btnPopModalNoLayer); + }); + + modalPopoverNoLayerClose.addEventListener("click", function() { + modalPopoverNoLayer.close(); + }); + modalPopoverClose.addEventListener("click", function (event) { modalPopover.close(); }); diff --git a/packages/main/test/specs/Popover.spec.js b/packages/main/test/specs/Popover.spec.js index 785a78095c74..79adce988209 100644 --- a/packages/main/test/specs/Popover.spec.js +++ b/packages/main/test/specs/Popover.spec.js @@ -137,6 +137,30 @@ describe("Popover general interaction", () => { assert.ok(!popover.isDisplayedInViewport(), "Popover is closed."); }); + it("tests modal popover with no block layer", () => { + const btnOpenPopover = $("#btnPopModalNoLayer"); + const popover = $("#modalPopoverNoLayer"); + + btnOpenPopover.click(); + assert.ok(popover.getProperty("opened"), "Popover is opened."); + + const blockLayerIsCreated = browser.execute( () => { + const staticAreaItems = document.querySelectorAll("ui5-static-area-item"); + let result = false; + + staticAreaItems.forEach(item => { + if (item.shadowRoot.querySelector(".ui5-block-layer")) { + result = true; + } + }); + + return result + }); + assert.notOk(blockLayerIsCreated, "Block layer is not created."); + + browser.keys("Escape"); + }); + it("tests initial focus", () => { const focusedButton = $("#focusMe"); const btnOpenPopover = $("#btnPopFocus"); From 76d8ec203fa4df85dc0744a0d7e140f7878ed596 Mon Sep 17 00:00:00 2001 From: Filip Siderov Date: Mon, 2 Nov 2020 11:41:02 +0200 Subject: [PATCH 3/4] fix test --- packages/main/test/specs/Popover.spec.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/main/test/specs/Popover.spec.js b/packages/main/test/specs/Popover.spec.js index 79adce988209..ce762f5cebb6 100644 --- a/packages/main/test/specs/Popover.spec.js +++ b/packages/main/test/specs/Popover.spec.js @@ -140,22 +140,24 @@ describe("Popover general interaction", () => { it("tests modal popover with no block layer", () => { const btnOpenPopover = $("#btnPopModalNoLayer"); const popover = $("#modalPopoverNoLayer"); + const popoverId = popover.getProperty("_id"); btnOpenPopover.click(); assert.ok(popover.getProperty("opened"), "Popover is opened."); - const blockLayerIsCreated = browser.execute( () => { + const blockLayerIsCreated = browser.execute( (popoverId) => { const staticAreaItems = document.querySelectorAll("ui5-static-area-item"); let result = false; staticAreaItems.forEach(item => { - if (item.shadowRoot.querySelector(".ui5-block-layer")) { + if (item.shadowRoot.querySelector(".ui5-block-layer") && item.classList.contains(popoverId)) { result = true; } }); return result - }); + }, popoverId); + assert.notOk(blockLayerIsCreated, "Block layer is not created."); browser.keys("Escape"); From 59387223688c326143b95191ac00784492f2441d Mon Sep 17 00:00:00 2001 From: Filip Siderov Date: Fri, 6 Nov 2020 10:50:48 +0200 Subject: [PATCH 4/4] fix comment --- packages/main/src/Dialog.js | 2 +- packages/main/src/Popover.js | 6 +++--- packages/main/src/Popup.js | 4 ++-- packages/main/test/pages/Popover.html | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/main/src/Dialog.js b/packages/main/src/Dialog.js index 7db180d515fc..436baa692c41 100644 --- a/packages/main/src/Dialog.js +++ b/packages/main/src/Dialog.js @@ -174,7 +174,7 @@ class Dialog extends Popup { return true; } - get shouldHideBlockLayer() { // Required by Popup.js + get shouldHideBackdrop() { // Required by Popup.js return false; } diff --git a/packages/main/src/Popover.js b/packages/main/src/Popover.js index 4584c452aa91..12ec2757acf8 100644 --- a/packages/main/src/Popover.js +++ b/packages/main/src/Popover.js @@ -113,7 +113,7 @@ const metadata = { * @public * @since 1.0.0-rc.10 */ - hideBlockLayer: { + hideBackdrop: { type: Boolean, }, @@ -631,8 +631,8 @@ class Popover extends Popup { return this.modal; } - get shouldHideBlockLayer() { // Required by Popup.js - return this.hideBlockLayer; + get shouldHideBackdrop() { // Required by Popup.js + return this.hideBackdrop; } get _ariaLabelledBy() { // Required by Popup.js diff --git a/packages/main/src/Popup.js b/packages/main/src/Popup.js index 747957a78d57..68263cd59220 100644 --- a/packages/main/src/Popup.js +++ b/packages/main/src/Popup.js @@ -308,7 +308,7 @@ class Popup extends UI5Element { return; } - if (this.isModal && !this.shouldHideBlockLayer) { + if (this.isModal && !this.shouldHideBackdrop) { // create static area item ref for block layer this.getStaticAreaItemDomRef(); this._blockLayerHidden = false; @@ -432,7 +432,7 @@ class Popup extends UI5Element { * @abstract * @returns {boolean} */ - get shouldHideBlockLayer() {} // eslint-disable-line + get shouldHideBackdrop() {} // eslint-disable-line /** * Return the ID of an element in the shadow DOM that is going to label this popup diff --git a/packages/main/test/pages/Popover.html b/packages/main/test/pages/Popover.html index 3db49172ed03..a54fb1b8b138 100644 --- a/packages/main/test/pages/Popover.html +++ b/packages/main/test/pages/Popover.html @@ -173,7 +173,7 @@ Opens modal popover with no block layer! - + Hello Again