Skip to content

Commit edb03f3

Browse files
vursenvursen
authored
refactor: add elements to id map immediately after their connection to DOM (#8299)
1 parent 892664d commit edb03f3

14 files changed

+166
-94
lines changed

‎packages/component-base/src/polylit-mixin.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,27 @@ const PolylitMixinImplementation = (superclass) => {
195195
return result;
196196
}
197197

198+
constructor() {
199+
super();
200+
this.__hasPolylitMixin = true;
201+
}
202+
203+
/** @protected */
204+
connectedCallback() {
205+
super.connectedCallback();
206+
207+
// Components like `vaadin-overlay` are teleported to the body element when opened.
208+
// If their opened state is set as an attribute, the teleportation happens immediately
209+
// after they are connected to the DOM. This means they will be outside the scope of
210+
// querySelectorAll in the parent component's `firstUpdated()`. To ensure their reference
211+
// is still registered in the $ map, we propagate the reference here.
212+
const parentHost = this.getRootNode().host;
213+
if (parentHost && parentHost.__hasPolylitMixin && this.id) {
214+
parentHost.$ ||= {};
215+
parentHost.$[this.id] = this;
216+
}
217+
}
218+
198219
/** @protected */
199220
firstUpdated() {
200221
super.firstUpdated();

‎packages/component-base/test/polylit-mixin.test.js‎

Lines changed: 78 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { expect } from '@vaadin/chai-plugins';
22
import { defineCE, fixtureSync, nextFrame } from '@vaadin/testing-helpers';
33
import sinon from 'sinon';
4-
import { html, LitElement } from 'lit';
4+
import { LitElement } from 'lit';
5+
import { html, unsafeStatic } from 'lit/static-html.js';
56
import { PolylitMixin } from '../src/polylit-mixin.js';
67

78
describe('PolylitMixin', () => {
@@ -37,7 +38,7 @@ describe('PolylitMixin', () => {
3738
const tag = defineCE(
3839
class extends PolylitMixin(LitElement) {
3940
render() {
40-
return html`<div id="foo">Component</div>`;
41+
return html`<div>Component</div>`;
4142
}
4243

4344
get(path, object) {
@@ -55,10 +56,6 @@ describe('PolylitMixin', () => {
5556
await element.updateComplete;
5657
});
5758

58-
it('should reference elements with id', () => {
59-
expect(element.$.foo).to.be.instanceOf(HTMLDivElement);
60-
});
61-
6259
it('should get the nested value', () => {
6360
expect(element.get('foo.bar', { foo: { bar: 'baz' } })).to.equal('baz');
6461
});
@@ -74,31 +71,6 @@ describe('PolylitMixin', () => {
7471
});
7572
});
7673

77-
describe('createRenderRoot', () => {
78-
let element;
79-
80-
const tag = defineCE(
81-
class extends PolylitMixin(LitElement) {
82-
render() {
83-
return html`<div id="foo">Component</div>`;
84-
}
85-
86-
createRenderRoot() {
87-
return this;
88-
}
89-
},
90-
);
91-
92-
beforeEach(async () => {
93-
element = fixtureSync(`<${tag}></${tag}>`);
94-
await element.updateComplete;
95-
});
96-
97-
it('should reference elements with id when rendering to light DOM', () => {
98-
expect(element.$.foo).to.be.instanceOf(HTMLDivElement);
99-
});
100-
});
101-
10274
describe('reflectToAttribute', () => {
10375
let element;
10476

@@ -1208,4 +1180,79 @@ describe('PolylitMixin', () => {
12081180
}).to.not.throw(Error);
12091181
});
12101182
});
1183+
1184+
describe('element id map', () => {
1185+
let element;
1186+
1187+
describe('basic', () => {
1188+
const teleportedTag = defineCE(
1189+
class extends PolylitMixin(LitElement) {
1190+
connectedCallback() {
1191+
super.connectedCallback();
1192+
1193+
if (this.parentNode !== document.body) {
1194+
document.body.appendChild(this);
1195+
}
1196+
}
1197+
1198+
render() {
1199+
return html`Teleported`;
1200+
}
1201+
},
1202+
);
1203+
1204+
const tag = defineCE(
1205+
class extends PolylitMixin(LitElement) {
1206+
render() {
1207+
return html`
1208+
<div id="title">Title</div>
1209+
<${unsafeStatic(teleportedTag)} id="teleported" />
1210+
`;
1211+
}
1212+
},
1213+
);
1214+
1215+
beforeEach(async () => {
1216+
element = fixtureSync(`<${tag}></${tag}>`);
1217+
await element.updateComplete;
1218+
});
1219+
1220+
afterEach(() => {
1221+
document.querySelector('#teleported').remove();
1222+
});
1223+
1224+
it('should register elements with id', () => {
1225+
expect(element.$.title).to.be.instanceOf(HTMLDivElement);
1226+
expect(element.$.title.id).to.equal('title');
1227+
});
1228+
1229+
it('should register teleported elements with id', () => {
1230+
expect(element.$.teleported).to.be.instanceOf(HTMLElement);
1231+
expect(element.$.teleported.id).to.equal('teleported');
1232+
});
1233+
});
1234+
1235+
describe('createRenderRoot', () => {
1236+
const tag = defineCE(
1237+
class extends PolylitMixin(LitElement) {
1238+
render() {
1239+
return html`<div id="foo">Component</div>`;
1240+
}
1241+
1242+
createRenderRoot() {
1243+
return this;
1244+
}
1245+
},
1246+
);
1247+
1248+
beforeEach(async () => {
1249+
element = fixtureSync(`<${tag}></${tag}>`);
1250+
await element.updateComplete;
1251+
});
1252+
1253+
it('should register elements with id when rendering to light DOM', () => {
1254+
expect(element.$.foo).to.be.instanceOf(HTMLDivElement);
1255+
});
1256+
});
1257+
});
12111258
});

‎packages/select/src/vaadin-lit-select-overlay.js‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,6 @@ class SelectOverlay extends SelectOverlayMixin(ThemableMixin(PolylitMixin(LitEle
6565
this.requestContentUpdate();
6666
}
6767
}
68-
69-
requestContentUpdate() {
70-
super.requestContentUpdate();
71-
72-
if (this.owner) {
73-
// Ensure menuElement reference is correct.
74-
const menuElement = this._getMenuElement();
75-
this.owner._assignMenuElement(menuElement);
76-
}
77-
}
7868
}
7969

8070
defineCustomElement(SelectOverlay);

‎packages/select/src/vaadin-lit-select.js‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ class Select extends SelectBaseMixin(ElementMixin(ThemableMixin(PolylitMixin(Lit
8484
</div>
8585
8686
<vaadin-select-overlay
87+
id="overlay"
8788
.owner="${this}"
8889
.positionTarget="${this._inputContainer}"
8990
.opened="${this.opened}"
@@ -103,15 +104,6 @@ class Select extends SelectBaseMixin(ElementMixin(ThemableMixin(PolylitMixin(Lit
103104
`;
104105
}
105106

106-
/** @protected */
107-
ready() {
108-
super.ready();
109-
110-
const overlay = this.shadowRoot.querySelector('vaadin-select-overlay');
111-
overlay.owner = this;
112-
this._overlayElement = overlay;
113-
}
114-
115107
/** @private */
116108
_onOpenedChanged(event) {
117109
this.opened = event.detail.value;

‎packages/select/src/vaadin-select-base-mixin.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ export const SelectBaseMixin = (superClass) =>
182182
ready() {
183183
super.ready();
184184

185+
this._overlayElement = this.$.overlay;
186+
185187
this._inputContainer = this.shadowRoot.querySelector('[part~="input-field"]');
186188

187189
this._valueButtonController = new ButtonController(this);

‎packages/select/src/vaadin-select-overlay-mixin.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,14 @@ export const SelectOverlayMixin = (superClass) =>
4444
}
4545
}
4646
}
47+
48+
requestContentUpdate() {
49+
super.requestContentUpdate();
50+
51+
if (this.owner) {
52+
// Ensure menuElement reference is correct.
53+
const menuElement = this._getMenuElement();
54+
this.owner._assignMenuElement(menuElement);
55+
}
56+
}
4757
};

‎packages/select/src/vaadin-select-overlay.js‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -54,27 +54,6 @@ export class SelectOverlay extends SelectOverlayMixin(ThemableMixin(PolymerEleme
5454
</div>
5555
`;
5656
}
57-
58-
/** @protected */
59-
ready() {
60-
super.ready();
61-
62-
// When setting `opened` as an attribute, the overlay is already teleported to body
63-
// by the time when `ready()` callback of the `vaadin-select` is executed by Polymer,
64-
// so querySelector() would return null. So we use this workaround to set properties.
65-
this.owner = this.__dataHost;
66-
this.owner._overlayElement = this;
67-
}
68-
69-
requestContentUpdate() {
70-
super.requestContentUpdate();
71-
72-
if (this.owner) {
73-
// Ensure menuElement reference is correct.
74-
const menuElement = this._getMenuElement();
75-
this.owner._assignMenuElement(menuElement);
76-
}
77-
}
7857
}
7958

8059
defineCustomElement(SelectOverlay);

‎packages/select/src/vaadin-select.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,8 @@ class Select extends SelectBaseMixin(ElementMixin(ThemableMixin(PolymerElement))
180180
</div>
181181
182182
<vaadin-select-overlay
183+
id="overlay"
184+
owner="[[__overlayOwner]]"
183185
position-target="[[_inputContainer]]"
184186
opened="{{opened}}"
185187
with-backdrop="[[_phone]]"
@@ -196,6 +198,17 @@ class Select extends SelectBaseMixin(ElementMixin(ThemableMixin(PolymerElement))
196198
`;
197199
}
198200

201+
static get properties() {
202+
return {
203+
/** @private */
204+
__overlayOwner: {
205+
value() {
206+
return this;
207+
},
208+
},
209+
};
210+
}
211+
199212
static get observers() {
200213
return ['_rendererChanged(renderer, _overlayElement)'];
201214
}

‎packages/select/test/dom/__snapshots__/select.test.snap.js‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ snapshots["vaadin-select host opened default"] =
307307

308308
snapshots["vaadin-select host opened overlay"] =
309309
`<vaadin-select-overlay
310+
id="overlay"
310311
opened=""
311312
start-aligned=""
312313
top-aligned=""
@@ -339,6 +340,7 @@ snapshots["vaadin-select host opened overlay"] =
339340
snapshots["vaadin-select host opened overlay class"] =
340341
`<vaadin-select-overlay
341342
class="custom select-overlay"
343+
id="overlay"
342344
opened=""
343345
start-aligned=""
344346
top-aligned=""
@@ -403,7 +405,7 @@ snapshots["vaadin-select shadow default"] =
403405
</slot>
404406
</div>
405407
</div>
406-
<vaadin-select-overlay>
408+
<vaadin-select-overlay id="overlay">
407409
<vaadin-select-list-box
408410
aria-orientation="vertical"
409411
role="listbox"
@@ -471,7 +473,7 @@ snapshots["vaadin-select shadow disabled"] =
471473
</slot>
472474
</div>
473475
</div>
474-
<vaadin-select-overlay>
476+
<vaadin-select-overlay id="overlay">
475477
<vaadin-select-list-box
476478
aria-orientation="vertical"
477479
role="listbox"
@@ -539,7 +541,7 @@ snapshots["vaadin-select shadow readonly"] =
539541
</slot>
540542
</div>
541543
</div>
542-
<vaadin-select-overlay>
544+
<vaadin-select-overlay id="overlay">
543545
<vaadin-select-list-box
544546
aria-orientation="vertical"
545547
role="listbox"
@@ -607,7 +609,7 @@ snapshots["vaadin-select shadow invalid"] =
607609
</slot>
608610
</div>
609611
</div>
610-
<vaadin-select-overlay>
612+
<vaadin-select-overlay id="overlay">
611613
<vaadin-select-list-box
612614
aria-orientation="vertical"
613615
role="listbox"
@@ -675,7 +677,10 @@ snapshots["vaadin-select shadow theme"] =
675677
</slot>
676678
</div>
677679
</div>
678-
<vaadin-select-overlay theme="align-right">
680+
<vaadin-select-overlay
681+
id="overlay"
682+
theme="align-right"
683+
>
679684
<vaadin-select-list-box
680685
aria-orientation="vertical"
681686
role="listbox"

‎packages/tooltip/src/vaadin-lit-tooltip.js‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class Tooltip extends TooltipMixin(ThemePropertyMixin(ElementMixin(PolylitMixin(
4545

4646
return html`
4747
<vaadin-tooltip-overlay
48+
id="overlay"
4849
.renderer="${this._renderer}"
4950
.owner="${this}"
5051
theme="${ifDefined(this._theme)}"
@@ -63,13 +64,6 @@ class Tooltip extends TooltipMixin(ThemePropertyMixin(ElementMixin(PolylitMixin(
6364
<slot name="sr-label"></slot>
6465
`;
6566
}
66-
67-
/** @protected */
68-
ready() {
69-
super.ready();
70-
71-
this._overlayElement = this.shadowRoot.querySelector('vaadin-tooltip-overlay');
72-
}
7367
}
7468

7569
defineCustomElement(Tooltip);

0 commit comments

Comments
 (0)