fix(web-components): remove dropdown manual slot assignment (microsoft#35125)

Author: radium-v

change/@fluentui-web-components-cae6cbef-0947-42e6-8459-4d3b2c3ec471.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "fix(dropdown): use slotchange events instead of manual slot assignment",
+  "packageName": "@fluentui/web-components",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

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

@@ -630,8 +630,6 @@ export class BaseDropdown extends FASTElement {
     // @internal
     listboxChanged(prev: Listbox | undefined, next: Listbox | undefined): void;
     // @internal
-    listboxSlot: HTMLSlotElement;
-    // @internal
     mousedownHandler(e: MouseEvent): boolean | void;
     multiple?: boolean;
     // @internal
@@ -651,6 +649,8 @@ export class BaseDropdown extends FASTElement {
     selectOption(index?: number, shouldEmit?: boolean): void;
     // @internal
     setValidity(flags?: Partial<ValidityState>, message?: string, anchor?: HTMLElement): void;
+    // @internal
+    slotchangeHandler(e: Event): boolean | void;
     type: DropdownType;
     // @internal
     typeChanged(prev: DropdownType | undefined, next: DropdownType | undefined): void;
@@ -2945,8 +2945,6 @@ export class Listbox extends FASTElement {
     // @internal
     beforetoggleHandler(e: ToggleEvent): boolean | undefined;
     clickHandler(e: PointerEvent): boolean | void;
-    // (undocumented)
-    connectedCallback(): void;
     // @internal
     dropdown?: BaseDropdown;
     // @internal
@@ -2966,6 +2964,7 @@ export class Listbox extends FASTElement {
     selectedIndex: number;
     get selectedOptions(): DropdownOption[];
     selectOption(index?: number): void;
+    slotchangeHandler(e: Event): void;
 }
 
 // @public

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

@@ -8,6 +8,7 @@ import { toggleState } from '../utils/element-internals.js';
 import { getLanguage } from '../utils/language.js';
 import { AnchorPositioningCSSSupported } from '../utils/support.js';
 import { uniqueId } from '../utils/unique-id.js';
+import { waitForConnectedDescendants } from '../utils/request-idle-callback.js';
 import { DropdownType } from './dropdown.options.js';
 import { dropdownButtonTemplate, dropdownInputTemplate } from './dropdown.template.js';
 
@@ -105,7 +106,6 @@ export class BaseDropdown extends FASTElement {
   public controlChanged(prev: HTMLInputElement | undefined, next: HTMLInputElement | undefined): void {
     if (next) {
       next.id = next.id || uniqueId('input-');
-      this.controlSlot?.assign(next);
     }
   }
 
@@ -224,7 +224,7 @@ export class BaseDropdown extends FASTElement {
       next.dropdown = this;
       next.popover = 'manual';
       next.tabIndex = -1;
-      this.listboxSlot.assign(next);
+
       const notifier = Observable.getNotifier(this);
       notifier.subscribe(next);
 
@@ -252,14 +252,6 @@ export class BaseDropdown extends FASTElement {
     }
   }
 
-  /**
-   * Reference to the listbox slot element.
-   *
-   * @internal
-   */
-  @observable
-  public listboxSlot!: HTMLSlotElement;
-
   /**
    * Indicates whether the dropdown allows multiple options to be selected.
    *
@@ -693,8 +685,6 @@ export class BaseDropdown extends FASTElement {
 
     this.elementInternals.role = 'presentation';
 
-    this.addEventListener('connected', this.listboxConnectedHandler);
-
     Updates.enqueue(() => {
       this.insertControl();
     });
@@ -974,6 +964,24 @@ export class BaseDropdown extends FASTElement {
     }
   }
 
+  /**
+   * Handles the `slotchange` event for the dropdown.
+   * Sets the `listbox` property when a valid listbox is assigned to the default slot.
+   *
+   * @param e - the slot change event
+   * @internal
+   */
+  public slotchangeHandler(e: Event): boolean | void {
+    const target = e.target as HTMLSlotElement;
+
+    waitForConnectedDescendants(this, () => {
+      const listbox = target.assignedElements().find((el): el is Listbox => isListbox(el));
+      if (listbox) {
+        this.listbox = listbox;
+      }
+    });
+  }
+
   /**
    * Updates the freeform option with the provided value.
    *
@@ -1009,20 +1017,6 @@ export class BaseDropdown extends FASTElement {
     super.disconnectedCallback();
   }
 
-  /**
-   * Handles the connected event for the listbox.
-   *
-   * @param e - the event object
-   * @internal
-   */
-  private listboxConnectedHandler(e: Event): void {
-    const target = e.target as HTMLElement;
-
-    if (isListbox(target)) {
-      this.listbox = target;
-    }
-  }
-
   /**
    * When anchor positioning isn't supported, an intersection observer is used to flip the listbox when it hits the
    * viewport bounds. One static observer is used for all dropdowns.

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

@@ -14,7 +14,4 @@ export const definition = Dropdown.compose({
   name: `${FluentDesignSystem.prefix}-dropdown`,
   template,
   styles,
-  shadowOptions: {
-    slotAssignment: 'manual',
-  },
 });

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

@@ -72,10 +72,22 @@ export default {
       control: 'text',
       table: { category: 'attributes' },
     },
+    disabled: {
+      control: 'boolean',
+      table: { category: 'attributes' },
+    },
     multiple: {
       control: 'boolean',
       table: { category: 'attributes' },
     },
+    name: {
+      control: 'text',
+      table: { category: 'attributes' },
+    },
+    required: {
+      control: 'boolean',
+      table: { category: 'attributes' },
+    },
     size: {
       control: 'select',
       options: ['', ...Object.values(DropdownSize)],

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

@@ -35,6 +35,7 @@ export const dropdownInputTemplate = html<BaseDropdown>`
     ?disabled="${x => x.disabled}"
     type="${x => x.type}"
     value="${x => x.valueAttribute}"
+    slot="control"
     ${ref('control')}
   />
 `;
@@ -57,6 +58,7 @@ export const dropdownButtonTemplate = html<BaseDropdown>`
     role="combobox"
     ?disabled="${x => x.disabled}"
     type="button"
+    slot="control"
     ${ref('control')}
   >
     ${x => x.displayValue}
@@ -83,7 +85,7 @@ export function dropdownTemplate<T extends BaseDropdown>(options: DropdownOption
         <slot name="control" ${ref('controlSlot')}></slot>
         <slot name="indicator" ${ref('indicatorSlot')}>${staticallyCompose(options.indicator)}</slot>
       </div>
-      <slot ${ref('listboxSlot')}></slot>
+      <slot @slotchange="${(x, c) => x.slotchangeHandler(c.event)}"></slot>
     </template>
   `;
 }

packages/web-components/src/listbox/listbox.template.ts

@@ -1,11 +1,9 @@
-import { ElementViewTemplate, html, slotted } from '@microsoft/fast-element';
-import { isDropdownOption } from '../option/option.options.js';
+import { type ElementViewTemplate, html } from '@microsoft/fast-element';
 import type { Listbox } from './listbox.js';
 
 /**
  * Generates a template for the {@link (Dropdown:class)} component.
  *
- * @param options - The {@link (DropdownOptions:interface)} to use for generating the template.
  * @returns The template object.
  *
  * @public
@@ -17,12 +15,7 @@ export function listboxTemplate<T extends Listbox>(): ElementViewTemplate<T> {
       @beforetoggle="${(x, c) => x.beforetoggleHandler(c.event as ToggleEvent)}"
       @click="${(x, c) => x.clickHandler(c.event as PointerEvent)}"
     >
-      <slot
-        ${slotted({
-          property: 'options',
-          filter: node => isDropdownOption(node),
-        })}
-      ></slot>
+      <slot @slotchange="${(x, c) => x.slotchangeHandler(c.event)}"></slot>
     </template>
   `;
 }

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

@@ -3,6 +3,7 @@ import type { BaseDropdown } from '../dropdown/dropdown.base.js';
 import type { DropdownOption } from '../option/option.js';
 import { isDropdownOption } from '../option/option.options.js';
 import { toggleState } from '../utils/element-internals.js';
+import { waitForConnectedDescendants } from '../utils/request-idle-callback.js';
 import { uniqueId } from '../utils/unique-id.js';
 
 /**
@@ -12,7 +13,6 @@ import { uniqueId } from '../utils/unique-id.js';
  * @tag fluent-listbox
  *
  * @slot - The default slot for the options.
- * @emits connected - Dispatched when the element is connected to the DOM.
  *
  * @remarks
  * The listbox component represents a list of options that can be selected.
@@ -165,12 +165,6 @@ export class Listbox extends FASTElement {
     this.elementInternals.role = 'listbox';
   }
 
-  connectedCallback(): void {
-    super.connectedCallback();
-
-    this.$emit('connected');
-  }
-
   /**
    * Handles observable subscriptions for the listbox.
    *
@@ -213,4 +207,21 @@ export class Listbox extends FASTElement {
 
     this.selectedIndex = selectedIndex;
   }
+
+  /**
+   * Handles the `slotchange` event for the default slot.
+   * Sets the `options` property to the list of slotted options.
+   *
+   * @param e - The slotchange event
+   * @public
+   */
+  public slotchangeHandler(e: Event): void {
+    waitForConnectedDescendants(this, () => {
+      const options = (e.target as HTMLSlotElement)
+        .assignedElements()
+        .filter<DropdownOption>((option): option is DropdownOption => isDropdownOption(option));
+
+      this.options = options;
+    });
+  }
 }

packages/web-components/src/option/option.spec.ts

@@ -77,6 +77,26 @@ test.describe('DropdownOption', () => {
     await expect(element).toHaveJSProperty('elementInternals.ariaDisabled', 'false');
   });
 
+  test('removing the `disabled` attribute should set the `disabled` property to false', async ({ fastPage }) => {
+    const { element } = fastPage;
+
+    await fastPage.setTemplate({ attributes: { disabled: true } });
+
+    await expect(element).toHaveJSProperty('disabled', true);
+
+    await expect(element).toHaveJSProperty('elementInternals.ariaDisabled', 'true');
+
+    await element.evaluate((node: DropdownOption) => {
+      node.removeAttribute('disabled');
+    });
+
+    await expect(element).not.toHaveAttribute('disabled');
+
+    await expect(element).toHaveJSProperty('disabled', false);
+
+    await expect(element).toHaveJSProperty('elementInternals.ariaDisabled', 'false');
+  });
+
   test('should NOT set the `selected` attribute to match the `selected` property when the `selected` attribute is NOT present', async ({
     fastPage,
   }) => {
@@ -134,4 +154,26 @@ test.describe('DropdownOption', () => {
 
     await expect(element).toHaveJSProperty('elementInternals.ariaSelected', 'false');
   });
+
+  test('should unset the form value when the option is disabled and selected', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+
+    await fastPage.setTemplate(/* html */ `
+      <form id="test-form" action="#">
+        <fluent-option name="option" value="hello" selected>Hello</fluent-option>
+      </form>
+    `);
+
+    const form = page.locator('#test-form');
+
+    expect(await form.evaluate((node: HTMLFormElement) => Array.from(new FormData(node).entries()))).toEqual([
+      ['option', 'hello'],
+    ]);
+
+    await element.evaluate((node: DropdownOption) => {
+      node.disabled = true;
+    });
+
+    expect(await form.evaluate((node: HTMLFormElement) => Array.from(new FormData(node).entries()))).toEqual([]);
+  });
 });

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

@@ -118,6 +118,7 @@ export class DropdownOption extends FASTElement implements Start {
   protected disabledChanged(prev: boolean | undefined, next: boolean | undefined): void {
     this.elementInternals.ariaDisabled = this.disabled ? 'true' : 'false';
     toggleState(this.elementInternals, 'disabled', this.disabled);
+    this.setFormValue(!this.disabled && this.selected ? this.value : null);
   }
 
   /**