refactor!: close popover without trigger on Esc and outside click (#8409

)
vaadin · Dec 31, 2024 · 10577e5 · 10577e5
1 parent 8e23ef6
commit 10577e5
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 27 deletions.
diff --git a/packages/popover/src/vaadin-popover.d.ts b/packages/popover/src/vaadin-popover.d.ts
@@ -221,7 +221,8 @@ declare class Popover extends PopoverPositionMixin(
    * - outside click (unless `noCloseOnOutsideClick` property is true)
    *
    * When setting `trigger` property to `null`, `undefined` or empty array, the popover
-   * can be only opened or closed programmatically by changing `opened` property.
+   * can be only opened programmatically by changing `opened` property. Note, closing
+   * on Escape press or outside click is still allowed unless explicitly disabled.
    */
   trigger: PopoverTrigger[] | null | undefined;
 

diff --git a/packages/popover/src/vaadin-popover.js b/packages/popover/src/vaadin-popover.js
@@ -372,7 +372,8 @@ class Popover extends PopoverPositionMixin(
        * - outside click (unless `noCloseOnOutsideClick` property is true)
        *
        * When setting `trigger` property to `null`, `undefined` or empty array, the popover
-       * can be only opened or closed programmatically by changing `opened` property.
+       * can be only opened programmatically by changing `opened` property. Note, closing
+       * on Escape press or outside click is still allowed unless explicitly disabled.
        */
       trigger: {
         type: Array,
@@ -611,7 +612,6 @@ class Popover extends PopoverPositionMixin(
   __onGlobalClick(event) {
     if (
       this.opened &&
-      !this.__isManual &&
       !this.modal &&
       !event.composedPath().some((el) => el === this._overlayElement || el === this.target) &&
       !this.noCloseOnOutsideClick &&
@@ -646,13 +646,7 @@ class Popover extends PopoverPositionMixin(
       return;
     }
 
-    if (
-      event.key === 'Escape' &&
-      !this.noCloseOnEsc &&
-      this.opened &&
-      !this.__isManual &&
-      isLastOverlay(this._overlayElement)
-    ) {
+    if (event.key === 'Escape' && !this.noCloseOnEsc && this.opened && isLastOverlay(this._overlayElement)) {
       // Prevent closing parent overlay (e.g. dialog)
       event.stopPropagation();
       this._openedStateController.close(true);
@@ -939,7 +933,7 @@ class Popover extends PopoverPositionMixin(
    * @private
    */
   __onEscapePress(e) {
-    if (this.noCloseOnEsc || this.__isManual) {
+    if (this.noCloseOnEsc) {
       e.preventDefault();
     }
   }
@@ -949,7 +943,7 @@ class Popover extends PopoverPositionMixin(
    * @private
    */
   __onOutsideClick(e) {
-    if (this.noCloseOnOutsideClick || this.__isManual) {
+    if (this.noCloseOnOutsideClick) {
       e.preventDefault();
     }
   }
@@ -959,11 +953,6 @@ class Popover extends PopoverPositionMixin(
     return Array.isArray(this.trigger) && this.trigger.includes(trigger);
   }
 
-  /** @private */
-  get __isManual() {
-    return this.trigger == null || (Array.isArray(this.trigger) && this.trigger.length === 0);
-  }
-
   /** @private */
   __updateDimension(overlay, dimension, value) {
     const prop = `--_vaadin-popover-content-${dimension}`;

diff --git a/packages/popover/test/trigger.test.js b/packages/popover/test/trigger.test.js
@@ -497,51 +497,51 @@ describe('trigger', () => {
           expect(overlay.opened).to.be.true;
         });
 
-        it(`should not close on global Escape press with trigger set to ${trigger}`, async () => {
+        it(`should close on global Escape press with trigger set to ${trigger}`, async () => {
           popover.opened = true;
           await nextRender();
 
           esc(document.body);
           await nextUpdate(popover);
-          expect(overlay.opened).to.be.true;
+          expect(overlay.opened).to.be.false;
         });
 
-        it(`should not close on target Escape press with trigger set to ${trigger}`, async () => {
+        it(`should close on target Escape press with trigger set to ${trigger}`, async () => {
           popover.opened = true;
           await nextRender();
 
           esc(target);
           await nextUpdate(popover);
-          expect(overlay.opened).to.be.true;
+          expect(overlay.opened).to.be.false;
         });
 
-        it(`should not close on global Escape press when modal with trigger set to ${trigger}`, async () => {
+        it(`should close on global Escape press when modal with trigger set to ${trigger}`, async () => {
           popover.modal = true;
           popover.opened = true;
           await nextRender();
 
           esc(document.body);
           await nextUpdate(popover);
-          expect(overlay.opened).to.be.true;
+          expect(overlay.opened).to.be.false;
         });
 
-        it(`should not close on outside click when not modal with trigger set to ${trigger}`, async () => {
+        it(`should close on outside click when not modal with trigger set to ${trigger}`, async () => {
           popover.opened = true;
           await nextRender();
 
           outsideClick();
           await nextUpdate(popover);
-          expect(overlay.opened).to.be.true;
+          expect(overlay.opened).to.be.false;
         });
 
-        it(`should not close on outside click when modal with trigger set to ${trigger}`, async () => {
+        it(`should close on outside click when modal with trigger set to ${trigger}`, async () => {
           popover.modal = true;
           popover.opened = true;
           await nextRender();
 
           outsideClick();
           await nextUpdate(popover);
-          expect(overlay.opened).to.be.true;
+          expect(overlay.opened).to.be.false;
         });
 
         it(`should close when setting opened to false with trigger set to ${trigger}`, async () => {