fix(popover): stay open on click when using polyfill (#3211)

Firefox ESR does not support the popover attribute and therefore uses the polyfill. Because the popover was nested in a shadow root, the polyfill did not work correctly (see #2925 (comment)). Not using shadow-dom for the post-popovercontainer fixes the issue.
swisspost · Jul 11, 2024 · 82032b9 · 82032b9
1 parent a2efb09
commit 82032b9
Show file tree

Hide file tree

Showing 10 changed files with 65 additions and 78 deletions.
diff --git a/.changeset/beige-cooks-switch.md b/.changeset/beige-cooks-switch.md
@@ -0,0 +1,5 @@
+---
+'@swisspost/design-system-components': patch
+---
+
+Fixed an issue with popovers on Firefox ESR that unexpectedly closed popovers when clicking on content.
diff --git a/packages/components/cypress/e2e/popover.cy.ts b/packages/components/cypress/e2e/popover.cy.ts
@@ -4,7 +4,7 @@ describe('popover', { baseUrl: null, includeShadowDom: true }, () => {
       cy.visit('./cypress/fixtures/post-popover.test.html');
       // Aria-expanded is set by the web component, therefore it's a good measure to indicate the component is ready
       cy.get('[data-popover-target="popover-one"][aria-expanded]').as('trigger');
-      cy.get('div.popover-container').as('popover');
+      cy.get('#testtext').as('popover');
     });
 
     it('should show up on click', () => {

diff --git a/packages/components/cypress/e2e/popovercontainer.cy.ts b/packages/components/cypress/e2e/popovercontainer.cy.ts
@@ -6,7 +6,7 @@ describe('popovercontainer', { baseUrl: null, includeShadowDom: true }, () => {
       // There is no dedicated docs page for the popovercontainer
       cy.visit('./cypress/fixtures/post-popover.test.html');
       cy.get('[data-popover-target="popover-one"][aria-expanded]').as('trigger');
-      cy.get('div.popover-container').as('container');
+      cy.get('#testtext').as('container');
     });
 
     it('should show up on click', () => {

diff --git a/packages/components/cypress/e2e/tooltip.cy.ts b/packages/components/cypress/e2e/tooltip.cy.ts
@@ -4,7 +4,7 @@ describe('tooltips', { baseUrl: null, includeShadowDom: true }, () => {
       cy.visit('./cypress/fixtures/post-tooltip.test.html');
       cy.get('#target1').as('target1');
       cy.get('#target2').as('target2');
-      cy.get('#tooltip-one').find('div[popover]').as('tooltip');
+      cy.get('#tooltip-one').find('post-popovercontainer[popover]').as('tooltip');
     });
 
     it('should display a tooltip', () => {
@@ -54,7 +54,7 @@ describe('tooltips', { baseUrl: null, includeShadowDom: true }, () => {
       cy.visit('./cypress/fixtures/post-tooltip.test.html');
       cy.get('#target-child-element').as('target');
       cy.get('#target-child-element span').as('target-child');
-      cy.get('#tooltip-one').find('div[popover]').as('tooltip');
+      cy.get('#tooltip-one').find('post-popovercontainer[popover]').as('tooltip');
     });
 
     it('should show tooltip on hovered child element', () => {

diff --git a/packages/components/cypress/fixtures/post-popover.test.html b/packages/components/cypress/fixtures/post-popover.test.html
@@ -9,7 +9,7 @@
   <body>
     <button data-popover-target="popover-one">toggle</button>
     <post-popover id="popover-one">
-      <p>This is a test</p>
+      <p id="testtext">This is a test</p>
     </post-popover>
   </body>
 </html>
diff --git a/packages/components/src/components/post-popover/post-popover.scss b/packages/components/src/components/post-popover/post-popover.scss
@@ -34,3 +34,7 @@
   padding: 0.5em;
   flex-grow: 1;
 }
+
+.btn-close {
+  color: inherit;
+}
diff --git a/packages/components/src/components/post-popovercontainer/post-popovercontainer.scss b/packages/components/src/components/post-popovercontainer/post-popovercontainer.scss
@@ -18,7 +18,7 @@
   }
 }
 
-.popover {
+:where(post-popovercontainer) {
   @include elevation-mx.elevation('elevation-3');
 
   position: fixed;
@@ -36,36 +36,36 @@
 
   // Keeps the little arrow visible
   overflow: visible;
-}
 
-.arrow {
-  $arrow-size: 0.5825rem;
-  position: absolute;
-  width: $arrow-size;
-  height: $arrow-size;
-  background-color: inherit;
-  rotate: 45deg;
-  pointer-events: none;
-  z-index: -1;
+  .arrow {
+    $arrow-size: 0.5825rem;
+    position: absolute;
+    width: $arrow-size;
+    height: $arrow-size;
+    background-color: inherit;
+    rotate: 45deg;
+    pointer-events: none;
+    z-index: -1;
 
-  // Create transparent border to be styled by and for the high contrast mode
-  &.top {
-    border-left: 2px solid transparent;
-    border-top: 2px solid transparent;
-  }
+    // Create transparent border to be styled by and for the high contrast mode
+    &.top {
+      border-left: 2px solid transparent;
+      border-top: 2px solid transparent;
+    }
 
-  &.right {
-    border-right: 2px solid transparent;
-    border-top: 2px solid transparent;
-  }
+    &.right {
+      border-right: 2px solid transparent;
+      border-top: 2px solid transparent;
+    }
 
-  &.left {
-    border-left: 2px solid transparent;
-    border-bottom: 2px solid transparent;
-  }
+    &.left {
+      border-left: 2px solid transparent;
+      border-bottom: 2px solid transparent;
+    }
 
-  &.bottom {
-    border-right: 2px solid transparent;
-    border-bottom: 2px solid transparent;
+    &.bottom {
+      border-right: 2px solid transparent;
+      border-bottom: 2px solid transparent;
+    }
   }
 }
diff --git a/packages/components/src/components/post-popovercontainer/post-popovercontainer.tsx b/packages/components/src/components/post-popovercontainer/post-popovercontainer.tsx
@@ -1,4 +1,4 @@
-import { Component, Element, Event, EventEmitter, h, Host, Method, Prop } from '@stencil/core';
+import { Component, Element, Event, EventEmitter, Host, Method, Prop, h } from '@stencil/core';
 import {
   arrow,
   autoUpdate,
@@ -39,11 +39,9 @@ export type PostPopoverElement = HTMLElement & PopoverElement;
 @Component({
   tag: 'post-popovercontainer',
   styleUrl: 'post-popovercontainer.scss',
-  shadow: true,
 })
 export class PostPopovercontainer {
   @Element() host: HTMLPostPopovercontainerElement;
-  private popoverRef: PostPopoverElement;
   private arrowRef: HTMLElement;
   private eventTarget: Element;
   private clearAutoUpdate: () => void;
@@ -67,16 +65,11 @@ export class PostPopovercontainer {
   @Prop() readonly arrow?: boolean = false;
 
   componentDidLoad() {
-    this.popoverRef.setAttribute('popover', '');
-    this.popoverRef.addEventListener('beforetoggle', this.handleToggle.bind(this));
+    this.host.setAttribute('popover', '');
+    this.host.addEventListener('beforetoggle', this.handleToggle.bind(this));
   }
 
   disconnectedCallback() {
-    if (this.popoverRef)
-      this.popoverRef.removeEventListener('beforetoggle', e =>
-        this.toggle(e.target as HTMLPostPopovercontainerElement),
-      );
-
     if (typeof this.clearAutoUpdate === 'function') this.clearAutoUpdate();
   }
 
@@ -89,7 +82,7 @@ export class PostPopovercontainer {
     if (!this.toggleTimeoutId) {
       this.eventTarget = target;
       this.calculatePosition();
-      this.popoverRef.showPopover();
+      this.host.showPopover();
     }
   }
 
@@ -100,7 +93,7 @@ export class PostPopovercontainer {
   async hide() {
     if (!this.toggleTimeoutId) {
       this.eventTarget = null;
-      this.popoverRef.hidePopover();
+      this.host.hidePopover();
     }
   }
 
@@ -115,10 +108,10 @@ export class PostPopovercontainer {
     if (!this.toggleTimeoutId) {
       this.eventTarget = target;
       this.calculatePosition();
-      this.popoverRef.togglePopover(force);
+      this.host.togglePopover(force);
       this.toggleTimeoutId = null;
     }
-    return this.popoverRef.matches(':popover-open');
+    return this.host.matches(':where(:popover-open, .popover-open');
   }
 
   /**
@@ -145,7 +138,7 @@ export class PostPopovercontainer {
   private startAutoupdates() {
     this.clearAutoUpdate = autoUpdate(
       this.eventTarget,
-      this.popoverRef,
+      this.host,
       this.calculatePosition.bind(this),
     );
   }
@@ -182,15 +175,15 @@ export class PostPopovercontainer {
       y,
       middlewareData,
       placement: currentPlacement,
-    } = await computePosition(this.eventTarget, this.popoverRef, {
+    } = await computePosition(this.eventTarget, this.host, {
       placement: this.placement || 'top',
       strategy: 'fixed',
       middleware,
     });
 
     // Tooltip
-    this.popoverRef.style.left = `${x}px`;
-    this.popoverRef.style.top = `${y}px`;
+    this.host.style.left = `${x}px`;
+    this.host.style.top = `${y}px`;
 
     // Arrow
     if (this.arrow) {
@@ -215,21 +208,15 @@ export class PostPopovercontainer {
   render() {
     return (
       <Host data-version={version}>
-        <div
-          class="popover"
-          part="popover"
-          ref={(el: HTMLDivElement & PostPopoverElement) => (this.popoverRef = el)}
-        >
-          {this.arrow && (
-            <span
-              class="arrow"
-              ref={el => {
-                this.arrowRef = el;
-              }}
-            ></span>
-          )}
-          <slot></slot>
-        </div>
+        {this.arrow && (
+          <span
+            class="arrow"
+            ref={el => {
+              this.arrowRef = el;
+            }}
+          ></span>
+        )}
+        <slot></slot>
       </Host>
     );
   }

diff --git a/packages/components/src/components/post-popovercontainer/readme.md b/packages/components/src/components/post-popovercontainer/readme.md
@@ -73,13 +73,6 @@ Type: `Promise<boolean>`
 |      | Default slot for placing content inside the popovercontainer. |
 
 
-## Shadow Parts
-
-| Part        | Description |
-| ----------- | ----------- |
-| `"popover"` |             |
-
-
 ## Dependencies
 
 ### Used by

diff --git a/packages/components/src/components/post-tooltip/post-tooltip.scss b/packages/components/src/components/post-tooltip/post-tooltip.scss
@@ -10,14 +10,12 @@
 }
 
 post-popovercontainer {
-  &::part(popover) {
-    padding: spacing.$size-micro spacing.$size-mini;
-    max-width: 2 * spacing.$size-bigger-giant - spacing.$size-mini;
-    min-height: spacing.$size-regular;
-  }
+  padding: spacing.$size-micro spacing.$size-mini;
+  max-width: 2 * spacing.$size-bigger-giant - spacing.$size-mini;
+  min-height: spacing.$size-regular;
 
   // Creates a safe space around the popovercontainer for save pointer crossing between trigger and tooltip
-  &.has-arrow::part(popover) {
+  &[arrow] {
     &::after {
       position: absolute;
       content: '';