From f04c25f90075f39e6567f67b85339fea5d19a09d Mon Sep 17 00:00:00 2001 From: TarunAdobe Date: Tue, 11 Nov 2025 14:10:08 +0530 Subject: [PATCH 1/3] fix(overlay): set listenerHost correctly in trigger directive --- .changeset/eight-coats-read.md | 5 ++ .../overlay/src/overlay-trigger-directive.ts | 1 + .../test/overlay-trigger-directive.test.ts | 88 ++++++++++++++++++- 3 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 .changeset/eight-coats-read.md diff --git a/.changeset/eight-coats-read.md b/.changeset/eight-coats-read.md new file mode 100644 index 00000000000..248ae328a44 --- /dev/null +++ b/.changeset/eight-coats-read.md @@ -0,0 +1,5 @@ +--- +'@spectrum-web-components/overlay': patch +--- + +Sets this.listenerHost = this.target in overlay-trigger-directive’s overridden update() when a listenerHost is not already defined. This prevents type errors during the reconnected flow when listenerHost is required but unset. diff --git a/1st-gen/packages/overlay/src/overlay-trigger-directive.ts b/1st-gen/packages/overlay/src/overlay-trigger-directive.ts index 09f35a539a9..70339cd279c 100644 --- a/1st-gen/packages/overlay/src/overlay-trigger-directive.ts +++ b/1st-gen/packages/overlay/src/overlay-trigger-directive.ts @@ -90,6 +90,7 @@ export class OverlayTriggerDirective extends SlottableRequestDirective { this.target = part.element as HTMLElement; newTarget = true; } + this.listenerHost = this.target; if (newTarget || newStrategy) { this.strategy?.abort(); this.strategy = new strategies[ diff --git a/1st-gen/packages/overlay/test/overlay-trigger-directive.test.ts b/1st-gen/packages/overlay/test/overlay-trigger-directive.test.ts index 669d71b3b01..66302da399b 100644 --- a/1st-gen/packages/overlay/test/overlay-trigger-directive.test.ts +++ b/1st-gen/packages/overlay/test/overlay-trigger-directive.test.ts @@ -10,7 +10,13 @@ * governing permissions and limitations under the License. */ -import { elementUpdated, expect, fixture, oneEvent } from '@open-wc/testing'; +import { + elementUpdated, + expect, + fixture, + nextFrame, + oneEvent, +} from '@open-wc/testing'; import { html, TemplateResult } from '@spectrum-web-components/base'; import { stub } from 'sinon'; import { trigger } from '@spectrum-web-components/overlay/src/overlay-trigger-directive.js'; @@ -20,6 +26,9 @@ import '@spectrum-web-components/dialog/sp-dialog.js'; import '@spectrum-web-components/slider/sp-slider.js'; import '@spectrum-web-components/tooltip/sp-tooltip.js'; import { Overlay } from '@spectrum-web-components/overlay/src/Overlay.js'; +import { LitElement } from '@spectrum-web-components/base'; +import { cache } from 'lit/directives/cache.js'; +import { Button } from '@spectrum-web-components/button'; describe('Overlay trigger directive', () => { describe('dev mode', () => { @@ -88,4 +97,81 @@ describe('Overlay trigger directive', () => { }); }); }); + + it('does not throw when directive reconnects before overlay exists', async function () { + // Define a test component that uses cache() to trigger reconnection. + class CachedOverlayTriggerTest extends LitElement { + private show = true; + + protected override render(): TemplateResult { + const cachedButton = html` + html` + + + Cached popover + + + `, + { triggerInteraction: 'click' } + )} + > + Cached Trigger + + `; + + return html` + { + this.show = !this.show; + this.requestUpdate(); + }} + > + Toggle cached host + + ${cache(this.show ? cachedButton : html``)} + `; + } + } + customElements.define( + 'cached-overlay-trigger-test', + CachedOverlayTriggerTest + ); + + const wrapper = await fixture(html` + + `); + await elementUpdated(wrapper); + + const shadowRoot = wrapper.shadowRoot as ShadowRoot; + const toggleButton = shadowRoot.querySelector('sp-button') as Button; + expect(toggleButton).to.exist; + + // Get the cached trigger button (initially visible). + let cachedTrigger = shadowRoot.querySelectorAll( + 'sp-button' + )[1] as Button; + expect(cachedTrigger, 'cached trigger should exist initially').to.exist; + + // Toggle off to disconnect the cached button. + toggleButton.click(); + await nextFrame(); + await elementUpdated(wrapper); + + // Toggle back on to reconnect the cached button. + toggleButton.click(); + await nextFrame(); + await elementUpdated(wrapper); + + // Get the reconnected cached trigger button. + cachedTrigger = shadowRoot.querySelectorAll('sp-button')[1] as Button; + expect(cachedTrigger, 'cached trigger should exist after toggle').to + .exist; + + // Now click the cached trigger to open the overlay. + const opened = oneEvent(cachedTrigger, 'sp-opened'); + cachedTrigger.click(); + await opened; + }); }); From a50062273f6b39283e50c5b3f9b9908b24186c6e Mon Sep 17 00:00:00 2001 From: TarunAdobe Date: Mon, 17 Nov 2025 18:59:37 +0530 Subject: [PATCH 2/3] chore: add review changes --- .../packages/overlay/src/overlay-trigger-directive.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/1st-gen/packages/overlay/src/overlay-trigger-directive.ts b/1st-gen/packages/overlay/src/overlay-trigger-directive.ts index 70339cd279c..d3145c06da7 100644 --- a/1st-gen/packages/overlay/src/overlay-trigger-directive.ts +++ b/1st-gen/packages/overlay/src/overlay-trigger-directive.ts @@ -90,7 +90,6 @@ export class OverlayTriggerDirective extends SlottableRequestDirective { this.target = part.element as HTMLElement; newTarget = true; } - this.listenerHost = this.target; if (newTarget || newStrategy) { this.strategy?.abort(); this.strategy = new strategies[ @@ -136,6 +135,15 @@ export class OverlayTriggerDirective extends SlottableRequestDirective { insertionEl.insertAdjacentElement(where, this.overlay); } } + + override reconnected(): void { + // Do not re-initialize until an overlay instance exists. + // The overlay-ready callback is responsible for wiring the initial listener. + if (!this.overlay) { + return; + } + this.init(); + } } export const trigger = directive(OverlayTriggerDirective); From 1c8d9c08e2218fe6d38f630f2946dacaf8bfcbde Mon Sep 17 00:00:00 2001 From: TarunAdobe Date: Tue, 18 Nov 2025 14:55:09 +0530 Subject: [PATCH 3/3] chore: update overlay directive story --- .changeset/eight-coats-read.md | 2 +- 1st-gen/packages/overlay/stories/overlay-directive.stories.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.changeset/eight-coats-read.md b/.changeset/eight-coats-read.md index 248ae328a44..870a410bdc9 100644 --- a/.changeset/eight-coats-read.md +++ b/.changeset/eight-coats-read.md @@ -2,4 +2,4 @@ '@spectrum-web-components/overlay': patch --- -Sets this.listenerHost = this.target in overlay-trigger-directive’s overridden update() when a listenerHost is not already defined. This prevents type errors during the reconnected flow when listenerHost is required but unset. +Guards `OverlayTriggerDirective`'s `reconnected()` implementation so it only calls `init()` after an overlay instance exists. This prevents type errors during the reconnected flow when `listenerHost` would otherwise be unset because the overlay is not yet ready. diff --git a/1st-gen/packages/overlay/stories/overlay-directive.stories.ts b/1st-gen/packages/overlay/stories/overlay-directive.stories.ts index 4a3e123d6ed..e5696e7e8bc 100644 --- a/1st-gen/packages/overlay/stories/overlay-directive.stories.ts +++ b/1st-gen/packages/overlay/stories/overlay-directive.stories.ts @@ -53,6 +53,7 @@ import './overlay-story-components.js'; import { tooltip } from '@spectrum-web-components/tooltip/src/tooltip-directive.js'; import { ifDefined } from '@spectrum-web-components/base/src/directives.js'; import { state } from '@spectrum-web-components/base/src/decorators.js'; +import { cache } from 'lit/directives/cache.js'; const storyStyles = html`