Skip to content

Commit 678b431

Browse files
committed
fix(overlay): set listenerHost correctly in trigger directive
1 parent ba3cae7 commit 678b431

File tree

3 files changed

+93
-1
lines changed

3 files changed

+93
-1
lines changed

.changeset/eight-coats-read.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@spectrum-web-components/overlay': patch
3+
---
4+
5+
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.

1st-gen/packages/overlay/src/overlay-trigger-directive.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export class OverlayTriggerDirective extends SlottableRequestDirective {
9090
this.target = part.element as HTMLElement;
9191
newTarget = true;
9292
}
93+
this.listenerHost = this.target;
9394
if (newTarget || newStrategy) {
9495
this.strategy?.abort();
9596
this.strategy = new strategies[

1st-gen/packages/overlay/test/overlay-trigger-directive.test.ts

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,13 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import { elementUpdated, expect, fixture, oneEvent } from '@open-wc/testing';
13+
import {
14+
elementUpdated,
15+
expect,
16+
fixture,
17+
nextFrame,
18+
oneEvent,
19+
} from '@open-wc/testing';
1420
import { html, TemplateResult } from '@spectrum-web-components/base';
1521
import { stub } from 'sinon';
1622
import { trigger } from '@spectrum-web-components/overlay/src/overlay-trigger-directive.js';
@@ -20,6 +26,9 @@ import '@spectrum-web-components/dialog/sp-dialog.js';
2026
import '@spectrum-web-components/slider/sp-slider.js';
2127
import '@spectrum-web-components/tooltip/sp-tooltip.js';
2228
import { Overlay } from '@spectrum-web-components/overlay/src/Overlay.js';
29+
import { LitElement } from '@spectrum-web-components/base';
30+
import { cache } from 'lit/directives/cache.js';
31+
import { Button } from '@spectrum-web-components/button';
2332

2433
describe('Overlay trigger directive', () => {
2534
describe('dev mode', () => {
@@ -88,4 +97,81 @@ describe('Overlay trigger directive', () => {
8897
});
8998
});
9099
});
100+
101+
it('does not throw when directive reconnects before overlay exists', async function () {
102+
// Define a test component that uses cache() to trigger reconnection.
103+
class CachedOverlayTriggerTest extends LitElement {
104+
private show = true;
105+
106+
protected override render(): TemplateResult {
107+
const cachedButton = html`
108+
<sp-button
109+
${trigger(
110+
() => html`
111+
<sp-popover>
112+
<sp-dialog no-divider>
113+
Cached popover
114+
</sp-dialog>
115+
</sp-popover>
116+
`,
117+
{ triggerInteraction: 'click' }
118+
)}
119+
>
120+
Cached Trigger
121+
</sp-button>
122+
`;
123+
124+
return html`
125+
<sp-button
126+
@click=${() => {
127+
this.show = !this.show;
128+
this.requestUpdate();
129+
}}
130+
>
131+
Toggle cached host
132+
</sp-button>
133+
${cache(this.show ? cachedButton : html``)}
134+
`;
135+
}
136+
}
137+
customElements.define(
138+
'cached-overlay-trigger-test',
139+
CachedOverlayTriggerTest
140+
);
141+
142+
const wrapper = await fixture<CachedOverlayTriggerTest>(html`
143+
<cached-overlay-trigger-test></cached-overlay-trigger-test>
144+
`);
145+
await elementUpdated(wrapper);
146+
147+
const shadowRoot = wrapper.shadowRoot as ShadowRoot;
148+
const toggleButton = shadowRoot.querySelector('sp-button') as Button;
149+
expect(toggleButton).to.exist;
150+
151+
// Get the cached trigger button (initially visible).
152+
let cachedTrigger = shadowRoot.querySelectorAll(
153+
'sp-button'
154+
)[1] as Button;
155+
expect(cachedTrigger, 'cached trigger should exist initially').to.exist;
156+
157+
// Toggle off to disconnect the cached button.
158+
toggleButton.click();
159+
await nextFrame();
160+
await elementUpdated(wrapper);
161+
162+
// Toggle back on to reconnect the cached button.
163+
toggleButton.click();
164+
await nextFrame();
165+
await elementUpdated(wrapper);
166+
167+
// Get the reconnected cached trigger button.
168+
cachedTrigger = shadowRoot.querySelectorAll('sp-button')[1] as Button;
169+
expect(cachedTrigger, 'cached trigger should exist after toggle').to
170+
.exist;
171+
172+
// Now click the cached trigger to open the overlay.
173+
const opened = oneEvent(cachedTrigger, 'sp-opened');
174+
cachedTrigger.click();
175+
await opened;
176+
});
91177
});

0 commit comments

Comments
 (0)