From f8499b92826282e1a94122848ce97d4e301c48d0 Mon Sep 17 00:00:00 2001 From: KirtiRamchandani Date: Mon, 15 Jun 2026 06:29:38 +0000 Subject: [PATCH] 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 --- ...-6f78c014-b0c7-4dd8-8ad7-93c0521f8c76.json | 7 + .../web-components/src/accordion/accordion.ts | 24 ++-- .../src/listbox/listbox.spec.ts | 22 +++ .../web-components/src/listbox/listbox.ts | 13 +- .../src/menu-list/menu-list.base.ts | 18 ++- .../src/radio-group/radio-group.base.ts | 17 ++- .../src/tree-item/tree-item.base.ts | 42 ++---- packages/web-components/src/tree/tree.base.ts | 11 +- packages/web-components/src/tree/tree.spec.ts | 24 ++++ .../src/utils/custom-elements.ts | 41 ++++++ .../test/parent-child-upgrade-order.html | 9 ++ .../test/src/parent-child-upgrade-order.ts | 134 ++++++++++++++++++ 12 files changed, 310 insertions(+), 52 deletions(-) create mode 100644 change/@fluentui-web-components-6f78c014-b0c7-4dd8-8ad7-93c0521f8c76.json create mode 100644 packages/web-components/src/utils/custom-elements.ts create mode 100644 packages/web-components/test/parent-child-upgrade-order.html create mode 100644 packages/web-components/test/src/parent-child-upgrade-order.ts 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 eb6e6f99d8ae2b..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'; /** @@ -11,7 +12,7 @@ import { AccordionExpandMode } from './accordion.options.js'; * @tag fluent-accordion * * @slot - The default slot for the accordion items - * @fires { Event } change - Fires a custom 'change' event when the active item changes + * @fires change - Fires a custom 'change' event when the active item changes * * @public */ @@ -110,19 +111,26 @@ export class Accordion extends FASTElement { */ private setItems = (): void => { waitForConnectedDescendants(this, () => { - if (!this.slottedAccordionItems?.length) { + if (this.slottedAccordionItems.length === 0) { 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 01eb35094a0682..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. @@ -49,11 +50,6 @@ export class BaseMenuList extends FASTElement { */ public connectedCallback(): void { super.connectedCallback(); - - if (!this.slot && this.isNestedMenu()) { - this.slot = 'submenu'; - } - Updates.enqueue(() => { // wait until children have had a chance to // connect before setting/checking their props/attributes @@ -112,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 f924e7f850ff04..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,14 +1,13 @@ -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'; /** * A Base Radio Group Custom HTML Element. * Implements the {@link https://w3c.github.io/aria/#radiogroup | ARIA `radiogroup` role}. * - * @fires { Event } change - Fires a custom 'change' event when the checked radio changes - * * @public */ export class BaseRadioGroup extends FASTElement { @@ -227,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/tree-item/tree-item.base.ts b/packages/web-components/src/tree-item/tree-item.base.ts index b349a367db055c..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,17 +1,8 @@ import { attr, css, type ElementStyles, FASTElement, observable } from '@microsoft/fast-element'; import { toggleState } from '../utils/element-internals.js'; -import { maybeSetAutoFocus } from '../utils/autofocus.js'; +import { getUpgradedCustomElements, runAfterPendingDefinitions } from '../utils/custom-elements.js'; import { isTreeItem } from './tree-item.options.js'; -/** - * Base class for Tree Item Custom HTML Element. - * Based largely on the {@link https://developer.mozilla.org/en-US/docs/Web/HTML/Element/li | `
  • `} element. - * - * @fires { ToggleEvent } toggle - Fires when the expanded state changes - * @fires { Event } change - Fires when the selected state changes - * - * @public - */ export class BaseTreeItem extends FASTElement { /** * 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 { this.elementInternals.role = 'treeitem'; } - connectedCallback(): void { - super.connectedCallback(); - - this.tabIndex = Number(this.getAttribute('tabindex') || '0'); - - if (isTreeItem(this.parentElement)) { - this.slot ||= 'item'; - } - - maybeSetAutoFocus(this); - } - /** * When true, the control will be appear expanded by user interaction. * When true, the control will be appear expanded by user interaction. @@ -75,7 +54,7 @@ export class BaseTreeItem extends FASTElement { newState: next ? 'open' : 'closed', }); toggleState(this.elementInternals, 'expanded', next); - if (this.childTreeItems?.length) { + if (this.childTreeItems && this.childTreeItems.length > 0) { this.elementInternals.ariaExpanded = next ? 'true' : 'false'; // Update focusgroup attributes after subtree show/hide rendering is done. requestAnimationFrame(() => { @@ -115,11 +94,8 @@ export class BaseTreeItem extends FASTElement { */ protected selectedChanged(prev: boolean, next: boolean): void { this.$emit('change'); - - if (this.elementInternals) { - toggleState(this.elementInternals, 'selected', next); - this.elementInternals.ariaSelected = next ? 'true' : 'false'; - } + toggleState(this.elementInternals, 'selected', next); + this.elementInternals.ariaSelected = next ? 'true' : 'false'; } /** @@ -235,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/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'), + }; +};