Skip to content

Commit 16da9b1

Browse files
fix(web-components): defer child collection until upgrade
1 parent 633eade commit 16da9b1

12 files changed

Lines changed: 297 additions & 25 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: 14 additions & 6 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
/**
@@ -114,15 +115,22 @@ export class Accordion extends FASTElement {
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 & 2 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.
@@ -107,7 +108,15 @@ export class BaseMenuList extends FASTElement {
107108
Observable.getNotifier(child).subscribe(this, 'hidden');
108109
});
109110

110-
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+
);
111120

112121
/**
113122
* Set the indent attribute on MenuItem elements based on their

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
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
/**
@@ -226,21 +227,13 @@ export class BaseRadioGroup extends FASTElement {
226227
*/
227228
slottedRadiosChanged(prev: Radio[] | undefined, next: Radio[]): void {
228229
Updates.enqueue(() => {
229-
const elements = [...this.querySelectorAll('*')];
230-
const isUpgradedRadio = (element: Element): element is Radio => isRadio(element) && '$fastController' in element;
231-
const radioElements: Element[] = elements.filter(element => isRadio(element));
230+
const radioElements = [...this.querySelectorAll('*')].filter((element): element is Radio => isRadio(element));
232231

233-
this.radios = radioElements.filter(isUpgradedRadio);
232+
this.radios = getUpgradedCustomElements(radioElements, isRadio);
234233

235-
const pendingRadioTagNames = [...new Set(radioElements.filter(x => !isUpgradedRadio(x)).map(x => x.localName))];
236-
237-
if (pendingRadioTagNames.length === 0) {
238-
return;
239-
}
240-
241-
Promise.all(pendingRadioTagNames.map(tagName => customElements.whenDefined(tagName))).then(() => {
234+
runAfterPendingDefinitions(radioElements, isRadio, () => {
242235
if (this.isConnected) {
243-
this.radios = [...this.querySelectorAll('*')].filter(isUpgradedRadio);
236+
this.radios = getUpgradedCustomElements([...this.querySelectorAll('*')], isRadio);
244237
}
245238
});
246239
});

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { attr, css, type ElementStyles, FASTElement, observable } from '@microsoft/fast-element';
22
import { toggleState } from '../utils/element-internals.js';
3+
import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js';
34
import { isTreeItem } from './tree-item.options.js';
45

56
export class BaseTreeItem extends FASTElement {
@@ -210,6 +211,14 @@ export class BaseTreeItem extends FASTElement {
210211

211212
/** @internal */
212213
public handleItemSlotChange() {
213-
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+
});
214223
}
215224
}

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)