diff --git a/change/@fluentui-web-components-6f78c014-b0c7-4dd8-8ad7-93c0521f8c76.json b/change/@fluentui-web-components-6f78c014-b0c7-4dd8-8ad7-93c0521f8c76.json new file mode 100644 index 00000000000000..5258cce163d5ce --- /dev/null +++ b/change/@fluentui-web-components-6f78c014-b0c7-4dd8-8ad7-93c0521f8c76.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "fix(web-components): defer parent child collection until custom elements upgrade", + "packageName": "@fluentui/web-components", + "email": "kirtiar15502@gmail.com", + "dependentChangeType": "patch" +} diff --git a/packages/web-components/src/accordion/accordion.ts b/packages/web-components/src/accordion/accordion.ts index cdfcbc24b14731..0e498296a48bc4 100644 --- a/packages/web-components/src/accordion/accordion.ts +++ b/packages/web-components/src/accordion/accordion.ts @@ -1,7 +1,8 @@ -import { attr, FASTElement, Observable, observable, Updates } from '@microsoft/fast-element'; +import { attr, FASTElement, Observable, observable } from '@microsoft/fast-element'; import { BaseAccordionItem } from '../accordion-item/accordion-item.base.js'; -import { waitForConnectedDescendants } from '../utils/request-idle-callback.js'; import { isAccordionItem } from '../accordion-item/accordion-item.options.js'; +import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js'; +import { waitForConnectedDescendants } from '../utils/request-idle-callback.js'; import { AccordionExpandMode } from './accordion.options.js'; /** @@ -114,15 +115,22 @@ export class Accordion extends FASTElement { return; } - // Get all existing children and remove event listeners + this.removeItemListeners(this.accordionItems ?? []); + const children: Element[] = Array.from(this.children); - this.removeItemListeners(children); + const accordionItems = getUpgradedCustomElements(children, isAccordionItem); + + runAfterPendingDefinitions(children, isAccordionItem, () => { + if (this.isConnected) { + this.setItems(); + } + }); // Resubscribe to the `disabled` attribute of all children - children.forEach((child: Element) => Observable.getNotifier(child).subscribe(this, 'disabled')); + accordionItems.forEach((child: Element) => Observable.getNotifier(child).subscribe(this, 'disabled')); // Add event listeners to each non-disabled AccordionItem - this.accordionItems = children.filter(child => !child.hasAttribute('disabled')); + this.accordionItems = accordionItems.filter(child => !child.hasAttribute('disabled')); this.accordionItems.forEach((item: Element, index: number) => { item.addEventListener('click', this.expandedChangedHandler); // Subscribe to the expanded attribute of the item diff --git a/packages/web-components/src/listbox/listbox.spec.ts b/packages/web-components/src/listbox/listbox.spec.ts index ad4d49a7271e58..8175fcb4f9c054 100644 --- a/packages/web-components/src/listbox/listbox.spec.ts +++ b/packages/web-components/src/listbox/listbox.spec.ts @@ -148,3 +148,25 @@ test.describe('Listbox', () => { await expect(element).toHaveJSProperty('dropdown', undefined); }); }); + +test.describe('Listbox upgrade order', () => { + test('should apply multiple state when options upgrade after the listbox', async ({ fastPage }) => { + await fastPage.page.goto('/test/parent-child-upgrade-order.html'); + + const result = await fastPage.page.evaluate(async () => { + return ( + window as unknown as { + runListboxUpgradeOrderTest(): Promise<{ + firstOptionMultiple: boolean; + hasOwnMultiple: boolean; + optionsLength: number; + }>; + } + ).runListboxUpgradeOrderTest(); + }); + + expect(result.optionsLength).toBe(3); + expect(result.firstOptionMultiple).toBe(true); + expect(result.hasOwnMultiple).toBe(false); + }); +}); diff --git a/packages/web-components/src/listbox/listbox.ts b/packages/web-components/src/listbox/listbox.ts index 73fac80ce4f09b..ca2d7144564ab2 100644 --- a/packages/web-components/src/listbox/listbox.ts +++ b/packages/web-components/src/listbox/listbox.ts @@ -2,6 +2,7 @@ import { FASTElement, observable, Updates } from '@microsoft/fast-element'; import type { BaseDropdown } from '../dropdown/dropdown.base.js'; import type { DropdownOption } from '../option/option.js'; import { isDropdownOption } from '../option/option.options.js'; +import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js'; import { toggleState } from '../utils/element-internals.js'; import { waitForConnectedDescendants } from '../utils/request-idle-callback.js'; import { uniqueId } from '../utils/unique-id.js'; @@ -83,6 +84,7 @@ export class Listbox extends FASTElement { next?.forEach((option, index) => { option.elementInternals.ariaPosInSet = `${index + 1}`; option.elementInternals.ariaSetSize = `${next.length}`; + option.multiple = !!this.multiple; }); } @@ -240,11 +242,16 @@ export class Listbox extends FASTElement { public slotchangeHandler(e?: Event): void { waitForConnectedDescendants(this, () => { if (this.defaultSlot) { - const options = this.defaultSlot - .assignedElements() - .filter((option): option is DropdownOption => isDropdownOption(option)); + const assignedElements = this.defaultSlot.assignedElements(); + const options = getUpgradedCustomElements(assignedElements, isDropdownOption); this.options = options; + + runAfterPendingDefinitions(assignedElements, isDropdownOption, () => { + if (this.isConnected) { + this.slotchangeHandler(); + } + }); } }); } diff --git a/packages/web-components/src/menu-list/menu-list.base.ts b/packages/web-components/src/menu-list/menu-list.base.ts index 6ebcd314be5c0d..e8d8cc1a779c99 100644 --- a/packages/web-components/src/menu-list/menu-list.base.ts +++ b/packages/web-components/src/menu-list/menu-list.base.ts @@ -1,8 +1,9 @@ import { FASTElement, Observable, observable, Updates } from '@microsoft/fast-element'; -import { isHTMLElement } from '../utils/typings.js'; import type { MenuItemColumnCount } from '../menu-item/menu-item.js'; import type { MenuItem } from '../menu-item/menu-item.js'; import { isMenuItem, MenuItemRole } from '../menu-item/menu-item.options.js'; +import { isUpgradedCustomElement, runAfterPendingDefinitions } from '../utils/custom-elements.js'; +import { isHTMLElement } from '../utils/typings.js'; /** * A Base MenuList Custom HTML Element. @@ -107,7 +108,15 @@ export class BaseMenuList extends FASTElement { Observable.getNotifier(child).subscribe(this, 'hidden'); }); - this.menuChildren = children.filter(child => !child.hasAttribute('hidden')); + runAfterPendingDefinitions(children, isMenuItem, () => { + if (this.isConnected) { + this.setItems(); + } + }); + + this.menuChildren = children.filter( + child => !child.hasAttribute('hidden') && (!isMenuItem(child) || isUpgradedCustomElement(child)), + ); /** * Set the indent attribute on MenuItem elements based on their diff --git a/packages/web-components/src/radio-group/radio-group.base.ts b/packages/web-components/src/radio-group/radio-group.base.ts index 48161953041618..2f1f36f9ea6748 100644 --- a/packages/web-components/src/radio-group/radio-group.base.ts +++ b/packages/web-components/src/radio-group/radio-group.base.ts @@ -1,6 +1,7 @@ -import { attr, FASTElement, Observable, observable } from '@microsoft/fast-element'; +import { attr, FASTElement, Observable, observable, Updates } from '@microsoft/fast-element'; import type { Radio } from '../radio/radio.js'; import { isRadio } from '../radio/radio.options.js'; +import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js'; import { RadioGroupOrientation } from './radio-group.options.js'; /** @@ -225,7 +226,17 @@ export class BaseRadioGroup extends FASTElement { * @param next - the current slotted radios */ slottedRadiosChanged(prev: Radio[] | undefined, next: Radio[]): void { - this.radios = [...this.querySelectorAll('*')].filter(x => isRadio(x)) as Radio[]; + Updates.enqueue(() => { + const radioElements = [...this.querySelectorAll('*')].filter((element): element is Radio => isRadio(element)); + + this.radios = getUpgradedCustomElements(radioElements, isRadio); + + runAfterPendingDefinitions(radioElements, isRadio, () => { + if (this.isConnected) { + this.radios = getUpgradedCustomElements([...this.querySelectorAll('*')], isRadio); + } + }); + }); } /** diff --git a/packages/web-components/src/radio-group/radio-group.spec.ts b/packages/web-components/src/radio-group/radio-group.spec.ts index 6b581d079c232b..ddfb70c69f94ca 100644 --- a/packages/web-components/src/radio-group/radio-group.spec.ts +++ b/packages/web-components/src/radio-group/radio-group.spec.ts @@ -790,3 +790,24 @@ test.describe('RadioGroup', () => { await expect(radios.nth(1)).toHaveJSProperty('checked', true); }); }); + +test.describe('RadioGroup upgrade order', () => { + test.use({ + tagName: '', + }); + + test('should preserve checked state when radios upgrade after the group', async ({ fastPage }) => { + await fastPage.page.goto('/test/radio-group-upgrade-order.html'); + + const result = await fastPage.page.evaluate(() => { + return ( + window as unknown as { + runRadioGroupUpgradeOrderTest(): Promise<{ checked: boolean; hasOwnChecked: boolean }>; + } + ).runRadioGroupUpgradeOrderTest(); + }); + + expect(result.checked).toBe(true); + expect(result.hasOwnChecked).toBe(false); + }); +}); diff --git a/packages/web-components/src/tree-item/tree-item.base.ts b/packages/web-components/src/tree-item/tree-item.base.ts index 3137b81a88504c..fd7e45e0fbb731 100644 --- a/packages/web-components/src/tree-item/tree-item.base.ts +++ b/packages/web-components/src/tree-item/tree-item.base.ts @@ -1,5 +1,6 @@ import { attr, css, type ElementStyles, FASTElement, observable } from '@microsoft/fast-element'; import { toggleState } from '../utils/element-internals.js'; +import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js'; import { isTreeItem } from './tree-item.options.js'; export class BaseTreeItem extends FASTElement { @@ -210,6 +211,14 @@ export class BaseTreeItem extends FASTElement { /** @internal */ public handleItemSlotChange() { - this.childTreeItems = this.itemSlot.assignedElements().filter(el => isTreeItem(el)); + const assignedElements = this.itemSlot.assignedElements(); + + this.childTreeItems = getUpgradedCustomElements(assignedElements, isTreeItem); + + runAfterPendingDefinitions(assignedElements, isTreeItem, () => { + if (this.isConnected) { + this.handleItemSlotChange(); + } + }); } } diff --git a/packages/web-components/src/tree/tree.base.ts b/packages/web-components/src/tree/tree.base.ts index b836dc312a3a8a..9bf703b2d000db 100644 --- a/packages/web-components/src/tree/tree.base.ts +++ b/packages/web-components/src/tree/tree.base.ts @@ -1,6 +1,7 @@ import { FASTElement, observable } from '@microsoft/fast-element'; import type { BaseTreeItem } from '../tree-item/tree-item.base.js'; import { isTreeItem } from '../tree-item/tree-item.options.js'; +import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js'; export class BaseTree extends FASTElement { /** @@ -160,7 +161,15 @@ export class BaseTree extends FASTElement { /** @internal */ public handleDefaultSlotChange() { - this.childTreeItems = this.defaultSlot.assignedElements().filter(el => isTreeItem(el)); + const assignedElements = this.defaultSlot.assignedElements(); + + this.childTreeItems = getUpgradedCustomElements(assignedElements, isTreeItem); + + runAfterPendingDefinitions(assignedElements, isTreeItem, () => { + if (this.isConnected) { + this.handleDefaultSlotChange(); + } + }); } /** diff --git a/packages/web-components/src/tree/tree.spec.ts b/packages/web-components/src/tree/tree.spec.ts index 3e065dc9d60f5b..9f150cd10816eb 100644 --- a/packages/web-components/src/tree/tree.spec.ts +++ b/packages/web-components/src/tree/tree.spec.ts @@ -424,3 +424,27 @@ test.describe('Tree', () => { await expect(treeItems.nth(2)).toBeFocused(); }); }); + +test.describe('Tree upgrade order', () => { + test('should apply tree state when tree items upgrade after the tree', async ({ fastPage }) => { + await fastPage.page.goto('/test/parent-child-upgrade-order.html'); + + const result = await fastPage.page.evaluate(async () => { + return ( + window as unknown as { + runTreeUpgradeOrderTest(): Promise<{ + childTreeItemsLength: number; + currentSelectedLocalName: string | undefined; + firstItemSize: string; + hasOwnSize: boolean; + }>; + } + ).runTreeUpgradeOrderTest(); + }); + + expect(result.childTreeItemsLength).toBe(2); + expect(result.currentSelectedLocalName).toContain('tree-item'); + expect(result.firstItemSize).toBe('medium'); + expect(result.hasOwnSize).toBe(false); + }); +}); diff --git a/packages/web-components/src/utils/custom-elements.ts b/packages/web-components/src/utils/custom-elements.ts new file mode 100644 index 00000000000000..c610901c16cd4e --- /dev/null +++ b/packages/web-components/src/utils/custom-elements.ts @@ -0,0 +1,41 @@ +type ElementPredicate = (element: Element) => element is T; + +/** + * Returns true once FAST has upgraded the element instance. + */ +export function isUpgradedCustomElement(element: Element): boolean { + return '$fastController' in element; +} + +/** + * Filters matching custom elements down to instances that have finished upgrading. + */ +export function getUpgradedCustomElements( + elements: readonly Element[], + predicate: ElementPredicate, +): T[] { + return elements.filter((element): element is T => predicate(element) && isUpgradedCustomElement(element)); +} + +/** + * Runs a callback after all matching, still-pending custom element tag definitions resolve. + */ +export function runAfterPendingDefinitions( + elements: readonly Element[], + predicate: ElementPredicate, + callback: () => void, +): void { + const pendingTagNames = [ + ...new Set( + elements + .filter(element => predicate(element) && !isUpgradedCustomElement(element)) + .map(element => element.localName), + ), + ]; + + if (pendingTagNames.length === 0) { + return; + } + + Promise.all(pendingTagNames.map(tagName => customElements.whenDefined(tagName))).then(callback); +} diff --git a/packages/web-components/test/parent-child-upgrade-order.html b/packages/web-components/test/parent-child-upgrade-order.html new file mode 100644 index 00000000000000..9140b6a90c7b36 --- /dev/null +++ b/packages/web-components/test/parent-child-upgrade-order.html @@ -0,0 +1,9 @@ + + + + + Parent child upgrade order + + + + diff --git a/packages/web-components/test/radio-group-upgrade-order.html b/packages/web-components/test/radio-group-upgrade-order.html new file mode 100644 index 00000000000000..4666a66c2e2d43 --- /dev/null +++ b/packages/web-components/test/radio-group-upgrade-order.html @@ -0,0 +1,9 @@ + + + + + RadioGroup upgrade order + + + + diff --git a/packages/web-components/test/src/parent-child-upgrade-order.ts b/packages/web-components/test/src/parent-child-upgrade-order.ts new file mode 100644 index 00000000000000..572e7b182b9d3d --- /dev/null +++ b/packages/web-components/test/src/parent-child-upgrade-order.ts @@ -0,0 +1,134 @@ +import { Listbox } from '../../src/listbox/listbox.js'; +import { styles as listboxStyles } from '../../src/listbox/listbox.styles.js'; +import { template as listboxTemplate } from '../../src/listbox/listbox.template.js'; +import { DropdownOption } from '../../src/option/option.js'; +import { styles as optionStyles } from '../../src/option/option.styles.js'; +import { template as optionTemplate } from '../../src/option/option.template.js'; +import { TreeItem } from '../../src/tree-item/tree-item.js'; +import { styles as treeItemStyles } from '../../src/tree-item/tree-item.styles.js'; +import { template as treeItemTemplate } from '../../src/tree-item/tree-item.template.js'; +import { Tree } from '../../src/tree/tree.js'; +import { styles as treeStyles } from '../../src/tree/tree.styles.js'; +import { template as treeTemplate } from '../../src/tree/tree.template.js'; + +type ListboxUpgradeOrderResult = { + firstOptionMultiple: boolean; + hasOwnMultiple: boolean; + optionsLength: number; +}; + +type TreeUpgradeOrderResult = { + childTreeItemsLength: number; + currentSelectedLocalName: string | undefined; + firstItemSize: string; + hasOwnSize: boolean; +}; + +const nextFrame = () => new Promise(resolve => requestAnimationFrame(() => resolve())); + +( + window as unknown as { + runListboxUpgradeOrderTest(): Promise; + } +).runListboxUpgradeOrderTest = async () => { + const id = Date.now().toString(36); + const listboxTagName = `upgrade-${id}-listbox`; + const optionTagName = `upgrade-${id}-option`; + + document.body.innerHTML = ` + <${listboxTagName}> + <${optionTagName}>Apple + <${optionTagName}>Banana + <${optionTagName}>Orange + + `; + + Listbox.compose({ + name: listboxTagName, + template: listboxTemplate, + styles: listboxStyles, + }).define(customElements); + + const listbox = document.querySelector(listboxTagName); + if (!listbox) { + throw new Error('Expected listbox to exist.'); + } + + customElements.upgrade(listbox); + listbox.multiple = true; + + DropdownOption.compose({ + name: optionTagName, + template: optionTemplate, + styles: optionStyles, + }).define(customElements); + + await customElements.whenDefined(listboxTagName); + await customElements.whenDefined(optionTagName); + await nextFrame(); + await nextFrame(); + + const firstOption = document.querySelector(optionTagName); + if (!firstOption) { + throw new Error('Expected option to exist.'); + } + + return { + firstOptionMultiple: firstOption.multiple, + hasOwnMultiple: Object.prototype.hasOwnProperty.call(firstOption, 'multiple'), + optionsLength: listbox.options.length, + }; +}; + +( + window as unknown as { + runTreeUpgradeOrderTest(): Promise; + } +).runTreeUpgradeOrderTest = async () => { + const id = Date.now().toString(36); + const treeTagName = `upgrade-${id}-tree`; + const treeItemTagName = `upgrade-${id}-tree-item`; + + document.body.innerHTML = ` + <${treeTagName} size="medium"> + <${treeItemTagName} selected>One + <${treeItemTagName}>Two + + `; + + Tree.compose({ + name: treeTagName, + template: treeTemplate, + styles: treeStyles, + }).define(customElements); + + const tree = document.querySelector(treeTagName); + if (!tree) { + throw new Error('Expected tree to exist.'); + } + + customElements.upgrade(tree); + + TreeItem.compose({ + name: treeItemTagName, + template: treeItemTemplate, + styles: treeItemStyles, + }).define(customElements); + + await customElements.whenDefined(treeTagName); + await customElements.whenDefined(treeItemTagName); + await nextFrame(); + await nextFrame(); + + const firstItem = document.querySelector(treeItemTagName); + if (!firstItem) { + throw new Error('Expected tree item to exist.'); + } + + return { + childTreeItemsLength: tree.childTreeItems.length, + currentSelectedLocalName: tree.currentSelected?.localName, + firstItemSize: firstItem.size, + hasOwnSize: Object.prototype.hasOwnProperty.call(firstItem, 'size'), + }; +}; diff --git a/packages/web-components/test/src/radio-group-upgrade-order.ts b/packages/web-components/test/src/radio-group-upgrade-order.ts new file mode 100644 index 00000000000000..ebc063ea8f763c --- /dev/null +++ b/packages/web-components/test/src/radio-group-upgrade-order.ts @@ -0,0 +1,64 @@ +import { Radio } from '../../src/radio/radio.js'; +import { styles as radioStyles } from '../../src/radio/radio.styles.js'; +import { template as radioTemplate } from '../../src/radio/radio.template.js'; +import { RadioGroup } from '../../src/radio-group/radio-group.js'; +import { styles as radioGroupStyles } from '../../src/radio-group/radio-group.styles.js'; +import { template as radioGroupTemplate } from '../../src/radio-group/radio-group.template.js'; + +type UpgradeOrderResult = { + checked: boolean; + hasOwnChecked: boolean; +}; + +( + window as unknown as { + runRadioGroupUpgradeOrderTest(): Promise; + } +).runRadioGroupUpgradeOrderTest = async () => { + const id = Date.now().toString(36); + const groupTagName = `upgrade-${id}-radio-group`; + const radioTagName = `upgrade-${id}-radio`; + + document.body.innerHTML = ` + <${groupTagName} value="bar"> + <${radioTagName} value="foo"> + <${radioTagName} value="bar"> + <${radioTagName} value="baz"> + + `; + + RadioGroup.compose({ + name: groupTagName, + template: radioGroupTemplate, + styles: radioGroupStyles, + }).define(customElements); + + const group = document.querySelector(groupTagName); + if (!group) { + throw new Error('Expected radio group to exist.'); + } + + customElements.upgrade(group); + + Radio.compose({ + name: radioTagName, + template: radioTemplate, + styles: radioStyles, + }).define(customElements); + + await customElements.whenDefined(groupTagName); + await customElements.whenDefined(radioTagName); + await new Promise(requestAnimationFrame); + + const checkedRadio = document.querySelector(`${radioTagName}[value="bar"]`); + if (!checkedRadio) { + throw new Error('Expected checked radio to exist.'); + } + + const checkedRadioState = checkedRadio as unknown as { checked: boolean }; + + return { + checked: checkedRadioState.checked, + hasOwnChecked: Object.prototype.hasOwnProperty.call(checkedRadio, 'checked'), + }; +};