Skip to content

Commit 561fefb

Browse files
authored
Merge branch 'main' into 2566-pf-radio-custom-form-control
2 parents 405c52e + 5940f37 commit 561fefb

File tree

7 files changed

+117
-55
lines changed

7 files changed

+117
-55
lines changed

.changeset/olive-rivers-stick.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
"@patternfly/pfe-core": patch
3+
---
4+
`SlotController`: `hasContent`/`isEmpty` now respects text nodes

.github/workflows/tests.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ jobs:
5252
- name: Configure node version
5353
uses: actions/setup-node@v4
5454
with:
55-
node-version: '20'
55+
node-version-file: '.nvmrc'
5656
cache: npm
5757

5858
- name: Install dependencies
@@ -265,7 +265,7 @@ jobs:
265265
close-previous: true
266266
title: "🧪 Tests are failing on main"
267267
body: "It looks like the build is currently failing on the main branch. See failed [action results](https://github.com/patternfly/patternfly-elements/actions/runs/${{ github.run_id }}) for more details."
268-
268+
269269
build-windows:
270270
name: Verify that build runs on Windows
271271
runs-on: windows-latest
@@ -275,9 +275,9 @@ jobs:
275275

276276
# Configures the node version used on GitHub-hosted runners
277277
- name: Configure node version
278-
uses: actions/setup-node@v3
278+
uses: actions/setup-node@v4
279279
with:
280-
node-version: 20
280+
node-version-file: .nvmrc
281281
cache: npm
282282

283283
- name: Install dependencies
Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
11
import type { ReactiveElement, ReactiveController } from 'lit';
22

33
export class CssVariableController implements ReactiveController {
4-
style: CSSStyleDeclaration;
4+
style?: CSSStyleDeclaration;
55

66
constructor(public host: ReactiveElement) {
7-
this.style = window.getComputedStyle(host);
7+
if (this.host.isConnected) {
8+
this.hostConnected();
9+
}
810
}
911

1012
private parseProperty(name: string) {
1113
return name.substring(0, 2) !== '--' ? `--${name}` : name;
1214
}
1315

1416
getVariable(name: string): string | null {
15-
return this.style.getPropertyValue(this.parseProperty(name)).trim() || null;
17+
return this.style?.getPropertyValue(this.parseProperty(name)).trim() || null;
1618
}
1719

18-
hostConnected?(): void;
20+
hostConnected(): void {
21+
this.style = window.getComputedStyle(this.host);
22+
};
1923
}

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { ReactiveController, ReactiveElement } from 'lit';
1+
import { isServer, type ReactiveController, type ReactiveElement } from 'lit';
22

33
import { Logger } from './logger.js';
44

@@ -143,11 +143,19 @@ export class SlotController implements ReactiveController {
143143
* @example this.hasSlotted('header');
144144
*/
145145
hasSlotted(...names: (string | null | undefined)[]): boolean {
146-
const slotNames = Array.from(names, x => x == null ? SlotController.default : x);
147-
if (!slotNames.length) {
148-
slotNames.push(SlotController.default);
146+
if (isServer) {
147+
return this.host
148+
.getAttribute('ssr-hint-has-slotted')
149+
?.split(',')
150+
.map(name => name.trim())
151+
.some(name => names.includes(name === 'default' ? null : name)) ?? false;
152+
} else {
153+
const slotNames = Array.from(names, x => x == null ? SlotController.default : x);
154+
if (!slotNames.length) {
155+
slotNames.push(SlotController.default);
156+
}
157+
return slotNames.some(x => this.#nodes.get(x)?.hasContent ?? false);
149158
}
150-
return slotNames.some(x => this.#nodes.get(x)?.hasContent ?? false);
151159
}
152160

153161
/**
@@ -193,7 +201,8 @@ export class SlotController implements ReactiveController {
193201
?? this.#getChildrenForSlot(name);
194202
const selector = slotName ? `slot[name="${slotName}"]` : 'slot:not([name])';
195203
const slot = this.host.shadowRoot?.querySelector?.<HTMLSlotElement>(selector) ?? null;
196-
const hasContent = !!elements.length;
204+
const nodes = slot?.assignedNodes?.();
205+
const hasContent = !!elements.length || !!nodes?.filter(x => x.textContent?.trim()).length;
197206
this.#nodes.set(name, { elements, name: slotName ?? '', hasContent, slot });
198207
this.#logger.debug(slotName, hasContent);
199208
};

elements/pf-card/test/pf-card.e2e.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { test } from '@playwright/test';
1+
import { test, expect } from '@playwright/test';
22
import { PfeDemoPage } from '@patternfly/pfe-tools/test/playwright/PfeDemoPage.js';
33
import { SSRPage } from '@patternfly/pfe-tools/test/playwright/SSRPage.js';
44

@@ -22,4 +22,22 @@ test.describe(tagName, () => {
2222
});
2323
await fixture.snapshots();
2424
});
25+
26+
test('ssr hints', async ({ browser }) => {
27+
const fixture = new SSRPage({
28+
tagName,
29+
browser,
30+
importSpecifiers: [`@patternfly/elements/${tagName}/${tagName}.js`],
31+
demoContent: /* html */ `
32+
<pf-card ssr-hint-has-slotted="header,default,footer">
33+
<h2 slot="header">Header</h2>
34+
<span>Body</span>
35+
<span slot="footer">Footer</span>
36+
</pf-card>
37+
`,
38+
});
39+
await fixture.updateCompleteFor('pf-card');
40+
await expect(fixture.page.locator('pf-card #title')).toHaveAttribute('hidden');
41+
await expect(fixture.page.locator('pf-card #header')).not.toHaveAttribute('hidden');
42+
});
2543
});

package.json

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,13 @@
113113
"files": [
114114
"tools/pfe-tools/*.ts",
115115
"tools/pfe-tools/**/*.ts",
116-
"!tools/pfe-tools/*.d.ts",
117-
"!tools/pfe-tools/**/*.d.ts"
116+
"!tools/**/*.d.ts"
118117
],
119118
"output": [
120119
"tools/pfe-tools/**/*.js",
121120
"tools/pfe-tools/**/*.map",
122121
"tools/pfe-tools/**/*.d.ts",
123-
"tools/*.tsbuildinfo"
122+
"tools/**/*.tsbuildinfo"
124123
]
125124
},
126125
"build:core": {
@@ -130,14 +129,14 @@
130129
],
131130
"files": [
132131
"core/**/*.ts",
133-
"!core/**/*.d.ts",
134-
"core/tsconfig.json"
132+
"core/tsconfig.json",
133+
"!core/**/*.d.ts"
135134
],
136135
"output": [
137136
"core/**/*.js",
138137
"core/**/*.map",
139138
"core/**/*.d.ts",
140-
"*.tsbuildinfo"
139+
"core/**/*.tsbuildinfo"
141140
]
142141
},
143142
"build:bundle": {

tools/pfe-tools/test/playwright/SSRPage.ts

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import Koa from 'koa';
99
import type { Browser, Page } from '@playwright/test';
1010
import type { Server } from 'node:http';
1111
import type { AddressInfo } from 'node:net';
12+
import type { LitElement } from 'lit';
1213

13-
interface SSRDemoConfig {
14-
demoDir: URL;
14+
interface SSRPageConfig {
15+
demoDir?: URL;
16+
demoContent?: string;
1517
importSpecifiers: string[];
1618
tagName: string;
1719
browser: Browser;
@@ -25,44 +27,64 @@ export class SSRPage {
2527
private app: Koa;
2628
private server!: Server;
2729
private host!: string;
28-
private page!: Page;
2930
private demoPaths!: string[];
3031

31-
constructor(
32-
private config: SSRDemoConfig,
33-
) {
32+
public page!: Page;
33+
34+
constructor(private config: SSRPageConfig) {
3435
this.app = new Koa();
35-
this.app.use(async (ctx, next) => {
36+
this.app.use(this.middleware(config));
37+
}
38+
39+
private middleware({ demoContent, demoDir, importSpecifiers }: SSRPageConfig) {
40+
return async (ctx: Koa.Context, next: Koa.Next) => {
3641
if (ctx.method === 'GET') {
37-
const origPath = ctx.request.path.replace(/^\//, '');
38-
const demoDir = config.demoDir.href;
39-
const fileUrl = resolve(demoDir, origPath);
40-
if (ctx.request.path.endsWith('.html')) {
42+
if (demoContent) {
4143
try {
42-
const content = await readFile(fileURLToPath(fileUrl), 'utf-8');
43-
ctx.response.body = await renderGlobal(content, this.config.importSpecifiers);
44+
ctx.response.body = await renderGlobal(
45+
demoContent,
46+
importSpecifiers,
47+
);
4448
} catch (e) {
4549
ctx.response.status = 500;
4650
ctx.response.body = (e as Error).stack;
4751
}
48-
} else {
49-
try {
50-
ctx.response.body = await readFile(fileURLToPath(fileUrl));
51-
} catch (e) {
52-
ctx.throw(500, e as Error);
52+
} else if (demoDir) {
53+
const origPath = ctx.request.path.replace(/^\//, '');
54+
const { href } = demoDir;
55+
const fileUrl = resolve(href, origPath);
56+
if (ctx.request.path.endsWith('.html')) {
57+
try {
58+
const content = await readFile(fileURLToPath(fileUrl), 'utf-8');
59+
ctx.response.body = await renderGlobal(content, importSpecifiers);
60+
} catch (e) {
61+
ctx.response.status = 500;
62+
ctx.response.body = (e as Error).stack;
63+
}
64+
} else {
65+
try {
66+
ctx.response.body = await readFile(fileURLToPath(fileUrl));
67+
} catch (e) {
68+
ctx.throw(500, e as Error);
69+
}
5370
}
71+
} else {
72+
throw new Error('SSRPage must either have a demoDir URL or a demoContent string');
5473
}
5574
} else {
5675
return next();
5776
}
58-
});
77+
};
5978
}
6079

6180
private async initPage() {
6281
this.page ??= await (await this.config.browser.newContext({
6382
javaScriptEnabled: false,
6483
}))
6584
.newPage();
85+
if (this.config.demoContent) {
86+
await this.page.goto(`${this.host}test.html`);
87+
}
6688
}
6789

6890
private async initServer() {
@@ -72,7 +94,7 @@ export class SSRPage {
7294
}
7395
const { address = 'localhost', port = 0 } = this.server.address() as AddressInfo;
7496
this.host ??= `http://${address.replace('::', 'localhost')}:${port}/`;
75-
this.demoPaths ??= (await readdir(this.config.demoDir))
97+
this.demoPaths ??= !this.config.demoDir ? [] : (await readdir(this.config.demoDir))
7698
.filter(x => x.endsWith('.html'))
7799
.map(x => new URL(x, this.host).href);
78100
}
@@ -82,6 +104,22 @@ export class SSRPage {
82104
!this.server ? rej('no server') : this.server?.close(e => e ? rej(e) : res()));
83105
}
84106

107+
/**
108+
* Take a visual regression snapshot and save it to disk
109+
* @param url url to the demo file
110+
*/
111+
private async snapshot(url: string) {
112+
const response = await this.page.goto(url, { waitUntil: 'load' });
113+
if (response?.status() === 404) {
114+
throw new Error(`Not Found: ${url}`);
115+
}
116+
expect(response?.status(), await response?.text())
117+
.toEqual(200);
118+
const snapshot = await this.page.screenshot({ fullPage: true });
119+
expect(snapshot, new URL(url).pathname)
120+
.toMatchSnapshot(`${this.config.tagName}-${basename(url)}.png`);
121+
}
122+
85123
/**
86124
* Creates visual regression snapshots for each demo in the server's `demoDir`
87125
*/
@@ -99,19 +137,9 @@ export class SSRPage {
99137
}
100138
}
101139

102-
/**
103-
* Take a visual regression snapshot and save it to disk
104-
* @param url url to the demo file
105-
*/
106-
private async snapshot(url: string) {
107-
const response = await this.page.goto(url, { waitUntil: 'load' });
108-
if (response?.status() === 404) {
109-
throw new Error(`Not Found: ${url}`);
110-
}
111-
expect(response?.status(), await response?.text())
112-
.toEqual(200);
113-
const snapshot = await this.page.screenshot({ fullPage: true });
114-
expect(snapshot, new URL(url).pathname)
115-
.toMatchSnapshot(`${this.config.tagName}-${basename(url)}.png`);
140+
async updateCompleteFor(tagName: string): Promise<void> {
141+
await this.initServer();
142+
await this.initPage();
143+
await this.page.$eval(tagName, el => (el as LitElement).updateComplete);
116144
}
117145
}

0 commit comments

Comments
 (0)