Skip to content

Commit 633eade

Browse files
KirtiRamchandaniKirtiRamchandani
committed
fix(web-components): refine radio upgrade handling
1 parent 29db4f2 commit 633eade

4 files changed

Lines changed: 110 additions & 99 deletions

File tree

‎packages/web-components/src/radio-group/radio-group.base.ts‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { attr, FASTElement, Observable, observable, Updates } from '@microsoft/fast-element';
22
import type { Radio } from '../radio/radio.js';
3+
import { isRadio } from '../radio/radio.options.js';
34
import { RadioGroupOrientation } from './radio-group.options.js';
45

56
/**
@@ -217,14 +218,6 @@ export class BaseRadioGroup extends FASTElement {
217218
@observable
218219
slottedRadios!: Radio[];
219220

220-
private static isUpgradedRadio(element: Element): element is Radio {
221-
return element.localName.endsWith('-radio') && '$fastController' in element;
222-
}
223-
224-
private getRadioDescendants(): Radio[] {
225-
return [...this.querySelectorAll('*')].filter(BaseRadioGroup.isUpgradedRadio);
226-
}
227-
228221
/**
229222
* Updates the radios collection when the slotted radios change.
230223
*
@@ -234,19 +227,22 @@ export class BaseRadioGroup extends FASTElement {
234227
slottedRadiosChanged(prev: Radio[] | undefined, next: Radio[]): void {
235228
Updates.enqueue(() => {
236229
const elements = [...this.querySelectorAll('*')];
237-
this.radios = this.getRadioDescendants();
238-
239-
for (const tagName of new Set(
240-
elements
241-
.filter(x => x.localName.endsWith('-radio') && !BaseRadioGroup.isUpgradedRadio(x))
242-
.map(x => x.localName),
243-
)) {
244-
customElements.whenDefined(tagName).then(() => {
245-
if (this.isConnected) {
246-
this.radios = this.getRadioDescendants();
247-
}
248-
});
230+
const isUpgradedRadio = (element: Element): element is Radio => isRadio(element) && '$fastController' in element;
231+
const radioElements: Element[] = elements.filter(element => isRadio(element));
232+
233+
this.radios = radioElements.filter(isUpgradedRadio);
234+
235+
const pendingRadioTagNames = [...new Set(radioElements.filter(x => !isUpgradedRadio(x)).map(x => x.localName))];
236+
237+
if (pendingRadioTagNames.length === 0) {
238+
return;
249239
}
240+
241+
Promise.all(pendingRadioTagNames.map(tagName => customElements.whenDefined(tagName))).then(() => {
242+
if (this.isConnected) {
243+
this.radios = [...this.querySelectorAll('*')].filter(isUpgradedRadio);
244+
}
245+
});
250246
});
251247
}
252248

‎packages/web-components/src/radio-group/radio-group.spec.ts‎

Lines changed: 21 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import { tagName as RadioTagName } from '../radio/radio.options.js';
44
import type { RadioGroup } from './radio-group.js';
55
import { tagName } from './radio-group.options.js';
66

7-
const sourceBaseUrl = `/@fs${new URL('../', import.meta.url).pathname}`;
8-
97
test.describe('RadioGroup', () => {
108
test.use({
119
tagName,
@@ -252,83 +250,6 @@ test.describe('RadioGroup', () => {
252250
await expect(radios.nth(2)).toHaveJSProperty('checked', false);
253251
});
254252

255-
test('should preserve checked state when radios upgrade after the group', async ({ page }) => {
256-
await page.goto('/');
257-
258-
const result = await page.evaluate(async sourceBaseUrl => {
259-
const id = Date.now().toString(36);
260-
const groupTagName = `upgrade-${id}-radio-group`;
261-
const radioTagName = `upgrade-${id}-radio`;
262-
263-
const importModule = (path: string): Promise<Record<string, unknown>> => import(path);
264-
const [
265-
radioGroupModule,
266-
radioGroupTemplateModule,
267-
radioGroupStylesModule,
268-
radioModule,
269-
radioTemplateModule,
270-
radioStylesModule,
271-
] = await Promise.all([
272-
importModule(`${sourceBaseUrl}radio-group/radio-group.ts`),
273-
importModule(`${sourceBaseUrl}radio-group/radio-group.template.ts`),
274-
importModule(`${sourceBaseUrl}radio-group/radio-group.styles.ts`),
275-
importModule(`${sourceBaseUrl}radio/radio.ts`),
276-
importModule(`${sourceBaseUrl}radio/radio.template.ts`),
277-
importModule(`${sourceBaseUrl}radio/radio.styles.ts`),
278-
]);
279-
280-
const RadioGroup = radioGroupModule.RadioGroup as {
281-
compose(options: Record<string, unknown>): { define(registry: CustomElementRegistry): void };
282-
};
283-
const Radio = radioModule.Radio as {
284-
compose(options: Record<string, unknown>): { define(registry: CustomElementRegistry): void };
285-
};
286-
287-
document.body.innerHTML = /* html */ `
288-
<${groupTagName} value="bar">
289-
<${radioTagName} value="foo"></${radioTagName}>
290-
<${radioTagName} value="bar"></${radioTagName}>
291-
<${radioTagName} value="baz"></${radioTagName}>
292-
</${groupTagName}>
293-
`;
294-
295-
RadioGroup.compose({
296-
name: groupTagName,
297-
template: radioGroupTemplateModule.template,
298-
styles: radioGroupStylesModule.styles,
299-
}).define(customElements);
300-
301-
const group = document.querySelector(groupTagName);
302-
if (!group) {
303-
throw new Error('Expected radio group to exist.');
304-
}
305-
306-
customElements.upgrade(group);
307-
308-
Radio.compose({
309-
name: radioTagName,
310-
template: radioTemplateModule.template,
311-
styles: radioStylesModule.styles,
312-
}).define(customElements);
313-
314-
await customElements.whenDefined(groupTagName);
315-
await customElements.whenDefined(radioTagName);
316-
await new Promise(requestAnimationFrame);
317-
318-
const checkedRadio = document.querySelector(`${radioTagName}[value="bar"]`) as HTMLElement & {
319-
checked: boolean;
320-
};
321-
322-
return {
323-
checked: checkedRadio.checked,
324-
hasOwnChecked: Object.prototype.hasOwnProperty.call(checkedRadio, 'checked'),
325-
};
326-
}, sourceBaseUrl);
327-
328-
expect(result.checked).toBe(true);
329-
expect(result.hasOwnChecked).toBe(false);
330-
});
331-
332253
test('radio should remain checked after it is set to disabled and uncheck when a new radio is checked', async ({
333254
fastPage,
334255
page,
@@ -869,3 +790,24 @@ test.describe('RadioGroup', () => {
869790
await expect(radios.nth(1)).toHaveJSProperty('checked', true);
870791
});
871792
});
793+
794+
test.describe('RadioGroup upgrade order', () => {
795+
test.use({
796+
tagName: '',
797+
});
798+
799+
test('should preserve checked state when radios upgrade after the group', async ({ fastPage }) => {
800+
await fastPage.page.goto('/test/radio-group-upgrade-order.html');
801+
802+
const result = await fastPage.page.evaluate(() => {
803+
return (
804+
window as unknown as {
805+
runRadioGroupUpgradeOrderTest(): Promise<{ checked: boolean; hasOwnChecked: boolean }>;
806+
}
807+
).runRadioGroupUpgradeOrderTest();
808+
});
809+
810+
expect(result.checked).toBe(true);
811+
expect(result.hasOwnChecked).toBe(false);
812+
});
813+
});

‎packages/web-components/test/radio-group-upgrade-order.html‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8" />
5+
<title>RadioGroup upgrade order</title>
6+
<script type="module" src="/src/radio-group-upgrade-order.js"></script>
7+
</head>
8+
<body></body>
9+
</html>

‎packages/web-components/test/src/radio-group-upgrade-order.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { Radio } from '../../src/radio/radio.js';
2+
import { styles as radioStyles } from '../../src/radio/radio.styles.js';
3+
import { template as radioTemplate } from '../../src/radio/radio.template.js';
4+
import { RadioGroup } from '../../src/radio-group/radio-group.js';
5+
import { styles as radioGroupStyles } from '../../src/radio-group/radio-group.styles.js';
6+
import { template as radioGroupTemplate } from '../../src/radio-group/radio-group.template.js';
7+
8+
type UpgradeOrderResult = {
9+
checked: boolean;
10+
hasOwnChecked: boolean;
11+
};
12+
13+
(
14+
window as unknown as {
15+
runRadioGroupUpgradeOrderTest(): Promise<UpgradeOrderResult>;
16+
}
17+
).runRadioGroupUpgradeOrderTest = async () => {
18+
const id = Date.now().toString(36);
19+
const groupTagName = `upgrade-${id}-radio-group`;
20+
const radioTagName = `upgrade-${id}-radio`;
21+
22+
document.body.innerHTML = `
23+
<${groupTagName} value="bar">
24+
<${radioTagName} value="foo"></${radioTagName}>
25+
<${radioTagName} value="bar"></${radioTagName}>
26+
<${radioTagName} value="baz"></${radioTagName}>
27+
</${groupTagName}>
28+
`;
29+
30+
RadioGroup.compose({
31+
name: groupTagName,
32+
template: radioGroupTemplate,
33+
styles: radioGroupStyles,
34+
}).define(customElements);
35+
36+
const group = document.querySelector(groupTagName);
37+
if (!group) {
38+
throw new Error('Expected radio group to exist.');
39+
}
40+
41+
customElements.upgrade(group);
42+
43+
Radio.compose({
44+
name: radioTagName,
45+
template: radioTemplate,
46+
styles: radioStyles,
47+
}).define(customElements);
48+
49+
await customElements.whenDefined(groupTagName);
50+
await customElements.whenDefined(radioTagName);
51+
await new Promise(requestAnimationFrame);
52+
53+
const checkedRadio = document.querySelector(`${radioTagName}[value="bar"]`);
54+
if (!checkedRadio) {
55+
throw new Error('Expected checked radio to exist.');
56+
}
57+
58+
const checkedRadioState = checkedRadio as unknown as { checked: boolean };
59+
60+
return {
61+
checked: checkedRadioState.checked,
62+
hasOwnChecked: Object.prototype.hasOwnProperty.call(checkedRadio, 'checked'),
63+
};
64+
};

0 commit comments

Comments
 (0)