Skip to content

Commit c2bf30e

Browse files
authored
fix(web-components): descendant check perf regressions (#36018)
1 parent c93849c commit c2bf30e

9 files changed

Lines changed: 213 additions & 140 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: improve SSR compatibility and component lifecycle management for Menu, Accordion, Dropdown, Tablist, and waitForConnectedDescendants",
4+
"packageName": "@fluentui/web-components",
5+
"email": "863023+radium-v@users.noreply.github.com",
6+
"dependentChangeType": "patch"
7+
}

packages/web-components/docs/web-components.api.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3243,11 +3243,14 @@ export class Menu extends FASTElement {
32433243
persistOnItemClick?: boolean;
32443244
persistOnItemClickChanged(oldValue: boolean, newValue: boolean): void;
32453245
primaryAction: HTMLSlotElement;
3246+
// @deprecated
32463247
setComponent(): void;
3247-
slottedMenuList: MenuList[];
3248+
slottedMenuList: HTMLElement[];
32483249
// @internal
3249-
slottedMenuListChanged(prev: MenuList[] | undefined, next: MenuList[] | undefined): void;
3250+
slottedMenuListChanged(prev: HTMLElement[] | undefined, next: HTMLElement[] | undefined): void;
32503251
slottedTriggers: HTMLElement[];
3252+
// @internal
3253+
slottedTriggersChanged(prev: HTMLElement[] | undefined, next: HTMLElement[] | undefined): void;
32513254
split?: boolean;
32523255
toggleHandler: (e: Event) => void;
32533256
toggleMenu: () => void;

packages/web-components/playwright.config.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { PlaywrightTestConfig } from '@playwright/test';
22
import { devices } from '@playwright/test';
33

44
const config: PlaywrightTestConfig = {
5-
reporter: 'list',
5+
reporter: process.env.CI ? 'github' : 'list',
66
retries: 3,
77
fullyParallel: process.env.CI ? false : true,
88
timeout: process.env.CI ? 10000 : 30000,
@@ -27,6 +27,8 @@ const config: PlaywrightTestConfig = {
2727
command: 'yarn vite preview test/harness',
2828
port: 5173,
2929
reuseExistingServer: true,
30+
stderr: 'pipe',
31+
stdout: 'pipe',
3032
},
3133
};
3234

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

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export class Accordion extends FASTElement {
4343
}
4444

4545
// Clean up single expand mode behavior
46-
(expandedItem as BaseAccordionItem)?.expandbutton.removeAttribute('aria-disabled');
46+
(expandedItem as BaseAccordionItem)?.expandbutton?.removeAttribute('aria-disabled');
4747
}
4848

4949
/**
@@ -61,11 +61,15 @@ export class Accordion extends FASTElement {
6161
* @internal
6262
*/
6363
public slottedAccordionItemsChanged(oldValue: HTMLElement[], newValue: HTMLElement[]): void {
64-
if (this.$fastController.isConnected) {
65-
this.setItems();
66-
}
64+
this.setItems();
6765
}
6866

67+
/**
68+
* Guard flag to prevent re-entrant calls to setSingleExpandMode.
69+
* @internal
70+
*/
71+
private _isAdjusting: boolean = false;
72+
6973
/**
7074
* Watch for changes to the slotted accordion items `disabled` and `expanded` attributes
7175
* @internal
@@ -76,8 +80,10 @@ export class Accordion extends FASTElement {
7680
} else if (propertyName === 'expanded') {
7781
// we only need to manage single expanded instances
7882
// such as scenarios where a child is programatically expanded
79-
if (source.expanded && this.isSingleExpandMode()) {
83+
if (source.expanded && this.isSingleExpandMode() && !this._isAdjusting) {
84+
this._isAdjusting = true;
8085
this.setSingleExpandMode(source);
86+
this._isAdjusting = false;
8187
}
8288
}
8389
}
@@ -146,27 +152,25 @@ export class Accordion extends FASTElement {
146152
* @returns {void}
147153
*/
148154
private setSingleExpandMode(expandedItem: Element): void {
149-
requestAnimationFrame(() => {
150-
if (this.accordionItems.length === 0) {
151-
return;
152-
}
153-
const currentItems = Array.from(this.accordionItems);
154-
this.activeItemIndex = currentItems.indexOf(expandedItem);
155-
156-
currentItems.forEach((item: Element, index: number) => {
157-
if (isAccordionItem(item)) {
158-
if (this.activeItemIndex === index) {
159-
item.expanded = true;
160-
item.expandbutton.setAttribute('aria-disabled', 'true');
161-
} else {
162-
item.expanded = false;
163-
164-
if (!item.hasAttribute('disabled')) {
165-
item.expandbutton.removeAttribute('aria-disabled');
166-
}
155+
if (this.accordionItems.length === 0) {
156+
return;
157+
}
158+
const currentItems = Array.from(this.accordionItems);
159+
this.activeItemIndex = currentItems.indexOf(expandedItem);
160+
161+
currentItems.forEach((item: Element, index: number) => {
162+
if (isAccordionItem(item)) {
163+
if (this.activeItemIndex === index) {
164+
item.expanded = true;
165+
item.expandbutton?.setAttribute('aria-disabled', 'true');
166+
} else {
167+
item.expanded = false;
168+
169+
if (!item.hasAttribute('disabled')) {
170+
item.expandbutton?.removeAttribute('aria-disabled');
167171
}
168172
}
169-
});
173+
}
170174
});
171175
}
172176

@@ -206,9 +210,7 @@ export class Accordion extends FASTElement {
206210
connectedCallback(): void {
207211
super.connectedCallback();
208212

209-
requestAnimationFrame(() => {
210-
this.expandmode = this.expandmode || AccordionExpandMode.multi;
211-
this.setItems();
212-
});
213+
this.expandmode = this.expandmode || AccordionExpandMode.multi;
214+
this.setItems();
213215
}
214216
}

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

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -232,20 +232,24 @@ export class BaseDropdown extends FASTElement {
232232

233233
notifier.notify('multiple');
234234

235-
Updates.enqueue(() => {
236-
this.options.forEach(option => {
237-
option.disabled = option.disabledAttribute || this.disabled;
238-
option.name = this.name;
239-
});
240-
241-
this.enabledOptions
242-
.filter(x => x.defaultSelected)
243-
.forEach((x, i) => {
244-
x.selected = this.multiple || i === 0;
235+
waitForConnectedDescendants(
236+
next,
237+
() => {
238+
this.options.forEach(option => {
239+
option.disabled = option.disabledAttribute || this.disabled;
240+
option.name = this.name;
245241
});
246242

247-
this.setValidity();
248-
});
243+
this.enabledOptions
244+
.filter(x => x.defaultSelected)
245+
.forEach((x, i) => {
246+
x.selected = this.multiple || i === 0;
247+
});
248+
249+
this.setValidity();
250+
},
251+
{ idleCallback: true },
252+
);
249253

250254
if (AnchorPositioningCSSSupported) {
251255
// The `anchor-name` property seems to not be isolated between instances in Safari Technology Preview 220 (18.4).
@@ -801,6 +805,12 @@ export class BaseDropdown extends FASTElement {
801805
return true;
802806
}
803807

808+
/**
809+
* Guard flag to prevent reentrant calls to `insertControl`.
810+
* @internal
811+
*/
812+
private _insertingControl = false;
813+
804814
/**
805815
* Inserts the control element based on the dropdown type.
806816
*
@@ -809,6 +819,11 @@ export class BaseDropdown extends FASTElement {
809819
* This method can be overridden in derived classes to provide custom control elements, though this is not recommended.
810820
*/
811821
protected insertControl(): void {
822+
if (this._insertingControl) {
823+
return;
824+
}
825+
826+
this._insertingControl = true;
812827
this.controlSlot?.assignedNodes().forEach(x => this.removeChild(x));
813828

814829
if (this.type === DropdownType.combobox) {
@@ -817,6 +832,7 @@ export class BaseDropdown extends FASTElement {
817832
}
818833

819834
dropdownButtonTemplate.render(this, this);
835+
this._insertingControl = false;
820836
}
821837

822838
/**
@@ -930,7 +946,9 @@ export class BaseDropdown extends FASTElement {
930946
*/
931947
public selectOption(index: number = this.selectedIndex, shouldEmit: boolean = false): void {
932948
this.listbox.selectOption(index);
933-
this.control.value = this.displayValue;
949+
if (this.control) {
950+
this.control.value = this.displayValue;
951+
}
934952

935953
this.setValidity();
936954

@@ -951,20 +969,22 @@ export class BaseDropdown extends FASTElement {
951969
* @internal
952970
*/
953971
public setValidity(flags?: Partial<ValidityState>, message?: string, anchor?: HTMLElement): void {
954-
if (this.$fastController.isConnected) {
955-
if (this.disabled || !this.required) {
956-
this.elementInternals.setValidity({});
957-
return;
958-
}
959-
960-
const valueMissing = this.required && this.listbox.selectedOptions.length === 0;
972+
if (!this.elementInternals) {
973+
return;
974+
}
961975

962-
this.elementInternals.setValidity(
963-
{ valueMissing, ...flags },
964-
message ?? this.validationMessage,
965-
anchor ?? this.control,
966-
);
976+
if (this.disabled || !this.required) {
977+
this.elementInternals.setValidity({});
978+
return;
967979
}
980+
981+
const valueMissing = this.required && this.listbox.selectedOptions.length === 0;
982+
983+
this.elementInternals.setValidity(
984+
{ valueMissing, ...flags },
985+
message ?? this.validationMessage,
986+
anchor ?? this.control,
987+
);
968988
}
969989

970990
/**

packages/web-components/src/dropdown/dropdown.styles.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
typographyBody2Styles,
55
typographyCaption1Styles,
66
} from '../styles/partials/typography.partials.js';
7-
import { flipBlockState, openState, placeholderShownState } from '../styles/states/index.js';
7+
import { openState, placeholderShownState } from '../styles/states/index.js';
88
import {
99
borderRadiusMedium,
1010
borderRadiusNone,
@@ -233,6 +233,7 @@ export const styles = css`
233233
color: ${colorNeutralForegroundDisabled};
234234
}
235235
236+
::slotted(:not([slot]):not([popover])),
236237
::slotted([popover]:not(:popover-open)) {
237238
display: none;
238239
}

0 commit comments

Comments
 (0)