fix(overlay): ensure events are only bound once

adobe · Dec 4, 2023 · abe57ce · abe57ce
1 parent 421cc44
commit abe57ce
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 4 deletions.
diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts
@@ -504,6 +504,8 @@ export class Overlay extends OverlayFeatures {
 
     protected bindEvents(): void {
         if (!this.hasNonVirtualTrigger) return;
+        // Clean up listeners if they've already been bound
+        this.abortController?.abort();
         this.abortController = new AbortController();
         const nextTriggerElement = this.triggerElement as HTMLElement;
         switch (this.triggerInteraction) {
@@ -583,7 +585,9 @@ export class Overlay extends OverlayFeatures {
         );
     }
 
-    protected manageTriggerElement(triggerElement: HTMLElement | null): void {
+    protected manageTriggerElement(
+        triggerElement: HTMLElement | undefined
+    ): void {
         if (triggerElement) {
             this.unbindEvents();
             this.releaseAriaDescribedby();
@@ -928,13 +932,17 @@ export class Overlay extends OverlayFeatures {
                 | 'hover'
                 | undefined;
         }
-        const oldTrigger = this.triggerElement as HTMLElement;
+        // Merge multiple possible calls to manageTriggerElement().
+        let oldTrigger: HTMLElement | false | undefined = false;
         if (changes.has(elementResolverUpdatedSymbol)) {
+            oldTrigger = this.triggerElement as HTMLElement;
             this.triggerElement = this.elementResolver.element;
-            this.manageTriggerElement(oldTrigger);
         }
         if (changes.has('triggerElement')) {
-            this.manageTriggerElement(changes.get('triggerElement'));
+            oldTrigger = changes.get('triggerElement');
+        }
+        if (oldTrigger !== false) {
+            this.manageTriggerElement(oldTrigger);
         }
     }
 

diff --git a/packages/overlay/stories/overlay-element.stories.ts b/packages/overlay/stories/overlay-element.stories.ts
@@ -26,6 +26,7 @@ import '@spectrum-web-components/icons-workflow/icons/sp-icon-rect-select.js';
 import { Placement } from '@floating-ui/dom';
 import { OverlayTypes } from '../src/overlay-types.js';
 import { notAgain } from '../../dialog/stories/dialog-base.stories.js';
+import './overlay-story-components.js';
 
 export default {
     title: 'Overlay Element',
@@ -587,3 +588,11 @@ export const actionGroupWithFilters = ({
         </sp-overlay>
     `;
 };
+
+// Test #3795 in browser
+export const transientHover = (): TemplateResult => html`
+    <transient-hover></transient-hover>
+`;
+transientHover.swc_vrt = {
+    skip: true,
+};
diff --git a/packages/overlay/stories/overlay-story-components.ts b/packages/overlay/stories/overlay-story-components.ts
@@ -344,3 +344,35 @@ declare global {
         'popover-content': PopoverContent;
     }
 }
+
+export default class TransientHover extends LitElement {
+    @property()
+    open = false;
+
+    protected override render(): TemplateResult {
+        return html`
+            <sp-button variant="primary" id="triggerButton">
+                Button popover
+            </sp-button>
+            <sp-overlay
+                type="auto"
+                trigger="triggerButton@click"
+                @sp-opened=${() => {
+                    this.open = true;
+                }}
+            >
+                <sp-popover>My Popover</sp-popover>
+            </sp-overlay>
+
+            ${!this.open
+                ? html`
+                      <sp-overlay trigger="triggerButton@hover" type="hint">
+                          <sp-tooltip placement="right">My tooltip</sp-tooltip>
+                      </sp-overlay>
+                  `
+                : html``}
+        `;
+    }
+}
+
+customElements.define('transient-hover', TransientHover);