Skip to content

Commit f8499b9

Browse files
fix(web-components): preserve radio state after deferred upgrade
Rebuild PR on current master after an earlier squash pulled in unrelated repository changes. Parent components now collect matching child tags only after FAST upgrades them, and refresh once pending definitions resolve. Covers RadioGroup, Accordion, Listbox, MenuList, Tree, and TreeItem. Fixes #36239
1 parent de337cf commit f8499b9

12 files changed

Lines changed: 310 additions & 52 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "fix(web-components): defer parent child collection until custom elements upgrade",
4+
"packageName": "@fluentui/web-components",
5+
"email": "kirtiar15502@gmail.com",
6+
"dependentChangeType": "patch"
7+
}

packages/web-components/src/accordion/accordion.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import { attr, FASTElement, Observable, observable, Updates } from '@microsoft/fast-element';
1+
import { attr, FASTElement, Observable, observable } from '@microsoft/fast-element';
22
import { BaseAccordionItem } from '../accordion-item/accordion-item.base.js';
3-
import { waitForConnectedDescendants } from '../utils/request-idle-callback.js';
43
import { isAccordionItem } from '../accordion-item/accordion-item.options.js';
4+
import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js';
5+
import { waitForConnectedDescendants } from '../utils/request-idle-callback.js';
56
import { AccordionExpandMode } from './accordion.options.js';
67

78
/**
@@ -11,7 +12,7 @@ import { AccordionExpandMode } from './accordion.options.js';
1112
* @tag fluent-accordion
1213
*
1314
* @slot - The default slot for the accordion items
14-
* @fires { Event } change - Fires a custom 'change' event when the active item changes
15+
* @fires change - Fires a custom 'change' event when the active item changes
1516
*
1617
* @public
1718
*/
@@ -110,19 +111,26 @@ export class Accordion extends FASTElement {
110111
*/
111112
private setItems = (): void => {
112113
waitForConnectedDescendants(this, () => {
113-
if (!this.slottedAccordionItems?.length) {
114+
if (this.slottedAccordionItems.length === 0) {
114115
return;
115116
}
116117

117-
// Get all existing children and remove event listeners
118+
this.removeItemListeners(this.accordionItems ?? []);
119+
118120
const children: Element[] = Array.from(this.children);
119-
this.removeItemListeners(children);
121+
const accordionItems = getUpgradedCustomElements(children, isAccordionItem);
122+
123+
runAfterPendingDefinitions(children, isAccordionItem, () => {
124+
if (this.isConnected) {
125+
this.setItems();
126+
}
127+
});
120128

121129
// Resubscribe to the `disabled` attribute of all children
122-
children.forEach((child: Element) => Observable.getNotifier(child).subscribe(this, 'disabled'));
130+
accordionItems.forEach((child: Element) => Observable.getNotifier(child).subscribe(this, 'disabled'));
123131

124132
// Add event listeners to each non-disabled AccordionItem
125-
this.accordionItems = children.filter(child => !child.hasAttribute('disabled'));
133+
this.accordionItems = accordionItems.filter(child => !child.hasAttribute('disabled'));
126134
this.accordionItems.forEach((item: Element, index: number) => {
127135
item.addEventListener('click', this.expandedChangedHandler);
128136
// Subscribe to the expanded attribute of the item

packages/web-components/src/listbox/listbox.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,25 @@ test.describe('Listbox', () => {
148148
await expect(element).toHaveJSProperty('dropdown', undefined);
149149
});
150150
});
151+
152+
test.describe('Listbox upgrade order', () => {
153+
test('should apply multiple state when options upgrade after the listbox', async ({ fastPage }) => {
154+
await fastPage.page.goto('/test/parent-child-upgrade-order.html');
155+
156+
const result = await fastPage.page.evaluate(async () => {
157+
return (
158+
window as unknown as {
159+
runListboxUpgradeOrderTest(): Promise<{
160+
firstOptionMultiple: boolean;
161+
hasOwnMultiple: boolean;
162+
optionsLength: number;
163+
}>;
164+
}
165+
).runListboxUpgradeOrderTest();
166+
});
167+
168+
expect(result.optionsLength).toBe(3);
169+
expect(result.firstOptionMultiple).toBe(true);
170+
expect(result.hasOwnMultiple).toBe(false);
171+
});
172+
});

packages/web-components/src/listbox/listbox.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { FASTElement, observable, Updates } from '@microsoft/fast-element';
22
import type { BaseDropdown } from '../dropdown/dropdown.base.js';
33
import type { DropdownOption } from '../option/option.js';
44
import { isDropdownOption } from '../option/option.options.js';
5+
import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js';
56
import { toggleState } from '../utils/element-internals.js';
67
import { waitForConnectedDescendants } from '../utils/request-idle-callback.js';
78
import { uniqueId } from '../utils/unique-id.js';
@@ -83,6 +84,7 @@ export class Listbox extends FASTElement {
8384
next?.forEach((option, index) => {
8485
option.elementInternals.ariaPosInSet = `${index + 1}`;
8586
option.elementInternals.ariaSetSize = `${next.length}`;
87+
option.multiple = !!this.multiple;
8688
});
8789
}
8890

@@ -240,11 +242,16 @@ export class Listbox extends FASTElement {
240242
public slotchangeHandler(e?: Event): void {
241243
waitForConnectedDescendants(this, () => {
242244
if (this.defaultSlot) {
243-
const options = this.defaultSlot
244-
.assignedElements()
245-
.filter<DropdownOption>((option): option is DropdownOption => isDropdownOption(option));
245+
const assignedElements = this.defaultSlot.assignedElements();
246+
const options = getUpgradedCustomElements(assignedElements, isDropdownOption);
246247

247248
this.options = options;
249+
250+
runAfterPendingDefinitions(assignedElements, isDropdownOption, () => {
251+
if (this.isConnected) {
252+
this.slotchangeHandler();
253+
}
254+
});
248255
}
249256
});
250257
}

packages/web-components/src/menu-list/menu-list.base.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { FASTElement, Observable, observable, Updates } from '@microsoft/fast-element';
2-
import { isHTMLElement } from '../utils/typings.js';
32
import type { MenuItemColumnCount } from '../menu-item/menu-item.js';
43
import type { MenuItem } from '../menu-item/menu-item.js';
54
import { isMenuItem, MenuItemRole } from '../menu-item/menu-item.options.js';
5+
import { isUpgradedCustomElement, runAfterPendingDefinitions } from '../utils/custom-elements.js';
6+
import { isHTMLElement } from '../utils/typings.js';
67

78
/**
89
* A Base MenuList Custom HTML Element.
@@ -49,11 +50,6 @@ export class BaseMenuList extends FASTElement {
4950
*/
5051
public connectedCallback(): void {
5152
super.connectedCallback();
52-
53-
if (!this.slot && this.isNestedMenu()) {
54-
this.slot = 'submenu';
55-
}
56-
5753
Updates.enqueue(() => {
5854
// wait until children have had a chance to
5955
// connect before setting/checking their props/attributes
@@ -112,7 +108,15 @@ export class BaseMenuList extends FASTElement {
112108
Observable.getNotifier(child).subscribe(this, 'hidden');
113109
});
114110

115-
this.menuChildren = children.filter(child => !child.hasAttribute('hidden'));
111+
runAfterPendingDefinitions(children, isMenuItem, () => {
112+
if (this.isConnected) {
113+
this.setItems();
114+
}
115+
});
116+
117+
this.menuChildren = children.filter(
118+
child => !child.hasAttribute('hidden') && (!isMenuItem(child) || isUpgradedCustomElement(child)),
119+
);
116120

117121
/**
118122
* Set the indent attribute on MenuItem elements based on their

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
1-
import { attr, FASTElement, Observable, observable } from '@microsoft/fast-element';
1+
import { attr, FASTElement, Observable, observable, Updates } from '@microsoft/fast-element';
22
import type { Radio } from '../radio/radio.js';
33
import { isRadio } from '../radio/radio.options.js';
4+
import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js';
45
import { RadioGroupOrientation } from './radio-group.options.js';
56

67
/**
78
* A Base Radio Group Custom HTML Element.
89
* Implements the {@link https://w3c.github.io/aria/#radiogroup | ARIA `radiogroup` role}.
910
*
10-
* @fires { Event } change - Fires a custom 'change' event when the checked radio changes
11-
*
1211
* @public
1312
*/
1413
export class BaseRadioGroup extends FASTElement {
@@ -227,7 +226,17 @@ export class BaseRadioGroup extends FASTElement {
227226
* @param next - the current slotted radios
228227
*/
229228
slottedRadiosChanged(prev: Radio[] | undefined, next: Radio[]): void {
230-
this.radios = [...this.querySelectorAll('*')].filter(x => isRadio(x)) as Radio[];
229+
Updates.enqueue(() => {
230+
const radioElements = [...this.querySelectorAll('*')].filter((element): element is Radio => isRadio(element));
231+
232+
this.radios = getUpgradedCustomElements(radioElements, isRadio);
233+
234+
runAfterPendingDefinitions(radioElements, isRadio, () => {
235+
if (this.isConnected) {
236+
this.radios = getUpgradedCustomElements([...this.querySelectorAll('*')], isRadio);
237+
}
238+
});
239+
});
231240
}
232241

233242
/**

packages/web-components/src/tree-item/tree-item.base.ts

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,8 @@
11
import { attr, css, type ElementStyles, FASTElement, observable } from '@microsoft/fast-element';
22
import { toggleState } from '../utils/element-internals.js';
3-
import { maybeSetAutoFocus } from '../utils/autofocus.js';
3+
import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js';
44
import { isTreeItem } from './tree-item.options.js';
55

6-
/**
7-
* Base class for Tree Item Custom HTML Element.
8-
* Based largely on the {@link https://developer.mozilla.org/en-US/docs/Web/HTML/Element/li | `<li>`} element.
9-
*
10-
* @fires { ToggleEvent } toggle - Fires when the expanded state changes
11-
* @fires { Event } change - Fires when the selected state changes
12-
*
13-
* @public
14-
*/
156
export class BaseTreeItem extends FASTElement {
167
/**
178
* The internal {@link https://developer.mozilla.org/docs/Web/API/ElementInternals | `ElementInternals`} instance for the component.
@@ -39,18 +30,6 @@ export class BaseTreeItem extends FASTElement {
3930
this.elementInternals.role = 'treeitem';
4031
}
4132

42-
connectedCallback(): void {
43-
super.connectedCallback();
44-
45-
this.tabIndex = Number(this.getAttribute('tabindex') || '0');
46-
47-
if (isTreeItem(this.parentElement)) {
48-
this.slot ||= 'item';
49-
}
50-
51-
maybeSetAutoFocus(this);
52-
}
53-
5433
/**
5534
* When true, the control will be appear expanded by user interaction.
5635
* When true, the control will be appear expanded by user interaction.
@@ -75,7 +54,7 @@ export class BaseTreeItem extends FASTElement {
7554
newState: next ? 'open' : 'closed',
7655
});
7756
toggleState(this.elementInternals, 'expanded', next);
78-
if (this.childTreeItems?.length) {
57+
if (this.childTreeItems && this.childTreeItems.length > 0) {
7958
this.elementInternals.ariaExpanded = next ? 'true' : 'false';
8059
// Update focusgroup attributes after subtree show/hide rendering is done.
8160
requestAnimationFrame(() => {
@@ -115,11 +94,8 @@ export class BaseTreeItem extends FASTElement {
11594
*/
11695
protected selectedChanged(prev: boolean, next: boolean): void {
11796
this.$emit('change');
118-
119-
if (this.elementInternals) {
120-
toggleState(this.elementInternals, 'selected', next);
121-
this.elementInternals.ariaSelected = next ? 'true' : 'false';
122-
}
97+
toggleState(this.elementInternals, 'selected', next);
98+
this.elementInternals.ariaSelected = next ? 'true' : 'false';
12399
}
124100

125101
/**
@@ -235,6 +211,14 @@ export class BaseTreeItem extends FASTElement {
235211

236212
/** @internal */
237213
public handleItemSlotChange() {
238-
this.childTreeItems = this.itemSlot.assignedElements().filter(el => isTreeItem(el));
214+
const assignedElements = this.itemSlot.assignedElements();
215+
216+
this.childTreeItems = getUpgradedCustomElements(assignedElements, isTreeItem);
217+
218+
runAfterPendingDefinitions(assignedElements, isTreeItem, () => {
219+
if (this.isConnected) {
220+
this.handleItemSlotChange();
221+
}
222+
});
239223
}
240224
}

packages/web-components/src/tree/tree.base.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { FASTElement, observable } from '@microsoft/fast-element';
22
import type { BaseTreeItem } from '../tree-item/tree-item.base.js';
33
import { isTreeItem } from '../tree-item/tree-item.options.js';
4+
import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js';
45

56
export class BaseTree extends FASTElement {
67
/**
@@ -160,7 +161,15 @@ export class BaseTree extends FASTElement {
160161

161162
/** @internal */
162163
public handleDefaultSlotChange() {
163-
this.childTreeItems = this.defaultSlot.assignedElements().filter(el => isTreeItem(el));
164+
const assignedElements = this.defaultSlot.assignedElements();
165+
166+
this.childTreeItems = getUpgradedCustomElements(assignedElements, isTreeItem);
167+
168+
runAfterPendingDefinitions(assignedElements, isTreeItem, () => {
169+
if (this.isConnected) {
170+
this.handleDefaultSlotChange();
171+
}
172+
});
164173
}
165174

166175
/**

packages/web-components/src/tree/tree.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,3 +424,27 @@ test.describe('Tree', () => {
424424
await expect(treeItems.nth(2)).toBeFocused();
425425
});
426426
});
427+
428+
test.describe('Tree upgrade order', () => {
429+
test('should apply tree state when tree items upgrade after the tree', async ({ fastPage }) => {
430+
await fastPage.page.goto('/test/parent-child-upgrade-order.html');
431+
432+
const result = await fastPage.page.evaluate(async () => {
433+
return (
434+
window as unknown as {
435+
runTreeUpgradeOrderTest(): Promise<{
436+
childTreeItemsLength: number;
437+
currentSelectedLocalName: string | undefined;
438+
firstItemSize: string;
439+
hasOwnSize: boolean;
440+
}>;
441+
}
442+
).runTreeUpgradeOrderTest();
443+
});
444+
445+
expect(result.childTreeItemsLength).toBe(2);
446+
expect(result.currentSelectedLocalName).toContain('tree-item');
447+
expect(result.firstItemSize).toBe('medium');
448+
expect(result.hasOwnSize).toBe(false);
449+
});
450+
});
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
type ElementPredicate<T extends Element> = (element: Element) => element is T;
2+
3+
/**
4+
* Returns true once FAST has upgraded the element instance.
5+
*/
6+
export function isUpgradedCustomElement(element: Element): boolean {
7+
return '$fastController' in element;
8+
}
9+
10+
/**
11+
* Filters matching custom elements down to instances that have finished upgrading.
12+
*/
13+
export function getUpgradedCustomElements<T extends Element>(
14+
elements: readonly Element[],
15+
predicate: ElementPredicate<T>,
16+
): T[] {
17+
return elements.filter((element): element is T => predicate(element) && isUpgradedCustomElement(element));
18+
}
19+
20+
/**
21+
* Runs a callback after all matching, still-pending custom element tag definitions resolve.
22+
*/
23+
export function runAfterPendingDefinitions<T extends Element>(
24+
elements: readonly Element[],
25+
predicate: ElementPredicate<T>,
26+
callback: () => void,
27+
): void {
28+
const pendingTagNames = [
29+
...new Set(
30+
elements
31+
.filter(element => predicate(element) && !isUpgradedCustomElement(element))
32+
.map(element => element.localName),
33+
),
34+
];
35+
36+
if (pendingTagNames.length === 0) {
37+
return;
38+
}
39+
40+
Promise.all(pendingTagNames.map(tagName => customElements.whenDefined(tagName))).then(callback);
41+
}

0 commit comments

Comments
 (0)