Skip to content

Commit fefd8bb

Browse files
bennypowersbennypowers
authored
fix(core): initialize slotcontroller map after update (#2903)
* fix(core): initialize slotcontroller map after update * test(core): slot controller tests wip * fix(core): slot controller
1 parent 940d1e3 commit fefd8bb

File tree

3 files changed

+236
-64
lines changed

3 files changed

+236
-64
lines changed

‎.changeset/fresh-things-burn.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@patternfly/pfe-core": patch
3+
---
4+
5+
`SlotController`: correctly report slot content after updating
6+

‎core/pfe-core/controllers/slot-controller.ts‎

Lines changed: 82 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,16 @@ export function isObjectSpread(config: SlotControllerArgs): config is [SlotsConf
3838
return config.length === 1 && typeof config[0] === 'object' && config[0] !== null;
3939
}
4040

41-
/**
42-
* If it's a named slot, return its children,
43-
* for the default slot, look for direct children not assigned to a slot
44-
* @param n slot name
45-
*/
46-
const isSlot =
47-
<T extends Element = Element>(n: string | typeof SlotController.default) =>
48-
(child: Element): child is T =>
49-
n === SlotController.default ? !child.hasAttribute('slot')
50-
: child.getAttribute('slot') === n;
41+
function isContent(node: Node) {
42+
switch (node.nodeType) {
43+
case Node.TEXT_NODE:
44+
return !!node.textContent?.trim();
45+
case Node.COMMENT_NODE:
46+
return false;
47+
default:
48+
return true;
49+
}
50+
}
5151

5252
export declare class SlotControllerPublicAPI implements ReactiveController {
5353
static default: symbol;
@@ -98,25 +98,67 @@ export declare class SlotControllerPublicAPI implements ReactiveController {
9898
isEmpty(...names: (string | null | undefined)[]): boolean;
9999
}
100100

101+
class SlotRecord {
102+
constructor(
103+
public slot: HTMLSlotElement,
104+
public name: string | symbol,
105+
private host: ReactiveElement,
106+
) {}
107+
108+
get elements() {
109+
return this.slot?.assignedElements?.();
110+
}
111+
112+
get hasContent() {
113+
if (this.name === SlotController.default) {
114+
return !!this.elements.length
115+
|| !![...this.host.childNodes]
116+
.some(node => {
117+
if (node instanceof Element) {
118+
return !node.hasAttribute('slot');
119+
} else {
120+
return isContent(node);
121+
}
122+
});
123+
} else {
124+
return !!this.slot.assignedNodes()
125+
.some(isContent);
126+
}
127+
}
128+
}
129+
101130
export class SlotController implements SlotControllerPublicAPI {
102131
public static default = Symbol('default slot') satisfies symbol as symbol;
103132

104133
/** @deprecated use `default` */
105134
public static anonymous: symbol = this.default;
106135

107-
#nodes = new Map<string | typeof SlotController.default, Slot>();
108-
109-
#slotMapInitialized = false;
136+
#slotRecords = new Map<string | typeof SlotController.default, SlotRecord>();
110137

111-
#slotNames: (string | null)[] = [];
138+
#slotNames: (string | symbol | null)[] = [];
112139

113140
#deprecations: Record<string, string> = {};
114141

115-
#mo = new MutationObserver(this.#initSlotMap.bind(this));
142+
#initSlotMap = async () => {
143+
const { host } = this;
144+
await host.updateComplete;
145+
const slotRecords = this.#slotRecords;
146+
// Loop over the properties provided by the schema
147+
for (let slotName of this.#slotNames.concat(Object.values(this.#deprecations))) {
148+
slotName ||= SlotController.default;
149+
const slot = this.#getSlotElement(slotName);
150+
if (slot) {
151+
slotRecords.set(slotName, new SlotRecord(slot, slotName, host));
152+
}
153+
}
154+
host.requestUpdate();
155+
};
156+
157+
#mo = new MutationObserver(this.#initSlotMap);
116158

117159
constructor(public host: ReactiveElement, ...args: SlotControllerArgs) {
118-
this.#initialize(...args);
119160
host.addController(this);
161+
this.#initialize(...args);
120162
if (!this.#slotNames.length) {
121163
this.#slotNames = [null];
122164
}
@@ -133,59 +175,27 @@ export class SlotController implements SlotControllerPublicAPI {
133175
}
134176
}
135177

178+
#getSlotElement(slotId: string | symbol) {
179+
const selector =
180+
slotId === SlotController.default ? 'slot:not([name])' : `slot[name="${slotId as string}"]`;
181+
return this.host.shadowRoot?.querySelector?.<HTMLSlotElement>(selector) ?? null;
182+
}
183+
136184
async hostConnected(): Promise<void> {
137185
this.#mo.observe(this.host, { childList: true });
138186
// Map the defined slots into an object that is easier to query
139-
this.#nodes.clear();
187+
this.#slotRecords.clear();
188+
await this.host.updateComplete;
140189
this.#initSlotMap();
141190
// insurance for framework integrations
142191
await this.host.updateComplete;
143192
this.host.requestUpdate();
144193
}
145194

146-
hostUpdated(): void {
147-
if (!this.#slotMapInitialized) {
148-
this.#initSlotMap();
149-
}
150-
}
151-
152195
hostDisconnected(): void {
153196
this.#mo.disconnect();
154197
}
155198

156-
#initSlotMap() {
157-
// Loop over the properties provided by the schema
158-
for (const slotName of this.#slotNames
159-
.concat(Object.values(this.#deprecations))) {
160-
const slotId = slotName || SlotController.default;
161-
const name = slotName ?? '';
162-
const elements = this.#getChildrenForSlot(slotId);
163-
const slot = this.#getSlotElement(slotId);
164-
const hasContent =
165-
!!elements.length || !!slot?.assignedNodes?.()?.filter(x => x.textContent?.trim()).length;
166-
this.#nodes.set(slotId, { elements, name, hasContent, slot });
167-
}
168-
this.host.requestUpdate();
169-
this.#slotMapInitialized = true;
170-
}
171-
172-
#getSlotElement(slotId: string | symbol) {
173-
const selector =
174-
slotId === SlotController.default ? 'slot:not([name])' : `slot[name="${slotId as string}"]`;
175-
return this.host.shadowRoot?.querySelector?.<HTMLSlotElement>(selector) ?? null;
176-
}
177-
178-
#getChildrenForSlot<T extends Element = Element>(
179-
name: string | typeof SlotController.default,
180-
): T[] {
181-
if (this.#nodes.has(name)) {
182-
return (this.#nodes.get(name)!.slot?.assignedElements?.() ?? []) as T[];
183-
} else {
184-
const children = Array.from(this.host.children) as T[];
185-
return children.filter(isSlot(name));
186-
}
187-
}
188-
189199
/**
190200
* Given a slot name or slot names, returns elements assigned to the requested slots as an array.
191201
* If no value is provided, it returns all children not assigned to a slot (without a slot attribute).
@@ -203,12 +213,12 @@ export class SlotController implements SlotControllerPublicAPI {
203213
* this.getSlotted();
204214
* ```
205215
*/
206-
getSlotted<T extends Element = Element>(...slotNames: string[]): T[] {
207-
if (!slotNames.length) {
208-
return (this.#nodes.get(SlotController.default)?.elements ?? []) as T[];
216+
public getSlotted<T extends Element = Element>(...slotNames: string[] | [null]): T[] {
217+
if (!slotNames.length || slotNames.length === 1 && slotNames.at(0) === null) {
218+
return (this.#slotRecords.get(SlotController.default)?.elements ?? []) as T[];
209219
} else {
210220
return slotNames.flatMap(slotName =>
211-
this.#nodes.get(slotName)?.elements ?? []) as T[];
221+
this.#slotRecords.get(slotName ?? SlotController.default)?.elements ?? []) as T[];
212222
}
213223
}
214224

@@ -217,12 +227,20 @@ export class SlotController implements SlotControllerPublicAPI {
217227
* @param names The slot names to check.
218228
* @example this.hasSlotted('header');
219229
*/
220-
hasSlotted(...names: (string | null | undefined)[]): boolean {
221-
const slotNames = Array.from(names, x => x == null ? SlotController.default : x);
230+
public hasSlotted(...names: (string | null | undefined)[]): boolean {
231+
const slotNames = Array.from(names, x =>
232+
x == null ? SlotController.default : x);
222233
if (!slotNames.length) {
223234
slotNames.push(SlotController.default);
224235
}
225-
return slotNames.some(x => this.#nodes.get(x)?.hasContent ?? false);
236+
return slotNames.some(slotName => {
237+
const slot = this.#slotRecords.get(slotName);
238+
if (!slot) {
239+
return false;
240+
} else {
241+
return slot.hasContent;
242+
}
243+
});
226244
}
227245

228246
/**
@@ -232,7 +250,7 @@ export class SlotController implements SlotControllerPublicAPI {
232250
* @example this.isEmpty();
233251
* @returns
234252
*/
235-
isEmpty(...names: (string | null | undefined)[]): boolean {
253+
public isEmpty(...names: (string | null | undefined)[]): boolean {
236254
return !this.hasSlotted(...names);
237255
}
238256
}

‎core/pfe-core/controllers/test/slot-controller.spec.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import { expect, fixture, nextFrame } from '@open-wc/testing';
2+
3+
import { customElement } from 'lit/decorators/custom-element.js';
4+
import { LitElement, html, type TemplateResult } from 'lit';
5+
6+
import { SlotController } from '../slot-controller.js';
7+
8+
@customElement('test-slot-controller-with-named-and-anonymous')
9+
class TestSlotControllerWithNamedAndAnonymous extends LitElement {
10+
controller = new SlotController(this, 'a', null);
11+
render(): TemplateResult {
12+
return html`
13+
<slot name="a"></slot>
14+
<slot name="b"></slot>
15+
<slot></slot>
16+
`;
17+
}
18+
}
19+
20+
describe('SlotController', function() {
21+
describe('with named and anonymous slots', function() {
22+
describe('with no content', function() {
23+
let element: TestSlotControllerWithNamedAndAnonymous;
24+
beforeEach(async function() {
25+
element = await fixture(html`
26+
<test-slot-controller-with-named-and-anonymous></test-slot-controller-with-named-and-anonymous>
27+
`);
28+
});
29+
it('reports empty named slots', function() {
30+
expect(element.controller.hasSlotted('a')).to.be.false;
31+
expect(element.controller.isEmpty('a')).to.be.true;
32+
});
33+
it('reports empty default slot', function() {
34+
expect(element.controller.hasSlotted(null)).to.be.false;
35+
expect(element.controller.isEmpty(null)).to.be.true;
36+
});
37+
it('reports empty default slot with no arguments', function() {
38+
expect(element.controller.hasSlotted()).to.be.false;
39+
expect(element.controller.isEmpty()).to.be.true;
40+
});
41+
it('returns empty list for getSlotted("a")', function() {
42+
expect(element.controller.getSlotted('a')).to.be.empty;
43+
});
44+
it('returns empty list for getSlotted(null)', function() {
45+
expect(element.controller.getSlotted(null)).to.be.empty;
46+
});
47+
it('returns empty list for getSlotted()', function() {
48+
expect(element.controller.getSlotted()).to.be.empty;
49+
});
50+
});
51+
52+
describe('with element content in default slot', function() {
53+
let element: TestSlotControllerWithNamedAndAnonymous;
54+
beforeEach(async function() {
55+
element = await fixture(html`
56+
<test-slot-controller-with-named-and-anonymous>
57+
<p>element</p>
58+
</test-slot-controller-with-named-and-anonymous>
59+
`);
60+
});
61+
it('reports empty named slots', function() {
62+
expect(element.controller.hasSlotted('a')).to.be.false;
63+
expect(element.controller.isEmpty('a')).to.be.true;
64+
});
65+
it('reports non-empty default slot', function() {
66+
expect(element.controller.hasSlotted(null)).to.be.true;
67+
expect(element.controller.isEmpty(null)).to.be.false;
68+
});
69+
it('reports non-empty default slot with no arguments', function() {
70+
expect(element.controller.hasSlotted()).to.be.true;
71+
expect(element.controller.isEmpty()).to.be.false;
72+
});
73+
it('returns empty list for getSlotted("a")', function() {
74+
expect(element.controller.getSlotted('a')).to.be.empty;
75+
});
76+
it('returns lengthy list for getSlotted(null)', function() {
77+
expect(element.controller.getSlotted(null)).to.not.be.empty;
78+
});
79+
it('returns lengthy list for getSlotted()', function() {
80+
expect(element.controller.getSlotted()).to.not.be.empty;
81+
});
82+
});
83+
84+
describe('with element content in named slot', function() {
85+
let element: TestSlotControllerWithNamedAndAnonymous;
86+
beforeEach(async function() {
87+
element = await fixture(html`
88+
<test-slot-controller-with-named-and-anonymous>
89+
<p slot="a">element</p>
90+
</test-slot-controller-with-named-and-anonymous>
91+
`);
92+
});
93+
it('reports non-empty named slots', function() {
94+
expect(element.controller.hasSlotted('a')).to.be.true;
95+
expect(element.controller.isEmpty('a')).to.be.false;
96+
});
97+
it('reports empty default slot', function() {
98+
expect(element.controller.hasSlotted(null)).to.be.false;
99+
expect(element.controller.isEmpty(null)).to.be.true;
100+
});
101+
it('reports empty default slot with no arguments', function() {
102+
expect(element.controller.hasSlotted()).to.be.false;
103+
expect(element.controller.isEmpty()).to.be.true;
104+
});
105+
it('returns lengthy list for getSlotted("a")', function() {
106+
expect(element.controller.getSlotted('a')).to.not.be.empty;
107+
});
108+
it('returns empty list for getSlotted(null)', function() {
109+
expect(element.controller.getSlotted(null)).to.be.empty;
110+
});
111+
it('returns empty list for getSlotted()', function() {
112+
expect(element.controller.getSlotted()).to.be.empty;
113+
});
114+
});
115+
116+
describe('with text content in default slot', function() {
117+
let element: TestSlotControllerWithNamedAndAnonymous;
118+
beforeEach(async function() {
119+
element = await fixture(html`
120+
<test-slot-controller-with-named-and-anonymous>
121+
text
122+
</test-slot-controller-with-named-and-anonymous>
123+
`);
124+
});
125+
it('reports empty named slots', function() {
126+
expect(element.controller.hasSlotted('a')).to.be.false;
127+
expect(element.controller.isEmpty('a')).to.be.true;
128+
});
129+
it('reports non-empty default slot', function() {
130+
expect(element.controller.hasSlotted(null)).to.be.true;
131+
expect(element.controller.isEmpty(null)).to.be.false;
132+
});
133+
it('reports non-empty default slot with no arguments', function() {
134+
expect(element.controller.hasSlotted()).to.be.true;
135+
expect(element.controller.isEmpty()).to.be.false;
136+
});
137+
it('returns empty list for getSlotted("a")', function() {
138+
expect(element.controller.getSlotted('a')).to.be.empty;
139+
});
140+
it('returns lengthy list for getSlotted(null)', function() {
141+
expect(element.controller.getSlotted(null)).to.be.empty;
142+
});
143+
it('returns lengthy list for getSlotted()', function() {
144+
expect(element.controller.getSlotted()).to.be.empty;
145+
});
146+
});
147+
});
148+
});

0 commit comments

Comments
 (0)