Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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"
}
24 changes: 16 additions & 8 deletions packages/web-components/src/accordion/accordion.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand All @@ -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
*/
Expand Down Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions packages/web-components/src/listbox/listbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
13 changes: 10 additions & 3 deletions packages/web-components/src/listbox/listbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
});
}

Expand Down Expand Up @@ -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<DropdownOption>((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();
}
});
}
});
}
Expand Down
18 changes: 11 additions & 7 deletions packages/web-components/src/menu-list/menu-list.base.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 13 additions & 4 deletions packages/web-components/src/radio-group/radio-group.base.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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);
}
});
});
}

/**
Expand Down
42 changes: 13 additions & 29 deletions packages/web-components/src/tree-item/tree-item.base.ts
Original file line number Diff line number Diff line change
@@ -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 | `<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.
Expand Down Expand Up @@ -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.
Expand All @@ -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(() => {
Expand Down Expand Up @@ -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';
}

/**
Expand Down Expand Up @@ -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();
}
});
}
}
11 changes: 10 additions & 1 deletion packages/web-components/src/tree/tree.base.ts
Original file line number Diff line number Diff line change
@@ -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 {
/**
Expand Down Expand Up @@ -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();
}
});
}

/**
Expand Down
24 changes: 24 additions & 0 deletions packages/web-components/src/tree/tree.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
41 changes: 41 additions & 0 deletions packages/web-components/src/utils/custom-elements.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
type ElementPredicate<T extends Element> = (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<T extends Element>(
elements: readonly Element[],
predicate: ElementPredicate<T>,
): 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<T extends Element>(
elements: readonly Element[],
predicate: ElementPredicate<T>,
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);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<title>Parent child upgrade order</title>
<script type="module" src="/src/parent-child-upgrade-order.js"></script>
</head>
<body></body>
</html>
Loading