[fix]: menu-items programmatically added to menu-list do not properl… (#35765)

Author: eljefe223

change/@fluentui-web-components-c44e3301-2ce6-40fc-9255-59b428cc9984.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "[fix]: menu-items programmatically added to menu-list do not  properly get assigned data indent.",
+  "packageName": "@fluentui/web-components",
+  "email": "jes@microsoft.com",
+  "dependentChangeType": "patch"
+}

packages/web-components/src/menu-list/menu-list.spec.ts

@@ -493,6 +493,99 @@ test.describe('Menu', () => {
     }
   });
 
+  test('should set the data-indent attribute correctly when menu items are dynamically appended', async ({
+    fastPage,
+  }) => {
+    const { element } = fastPage;
+
+    await fastPage.setTemplate({ innerHTML: '' });
+
+    await element.evaluate(node => {
+      const items = ['item 1', 'item 2', 'item 3'];
+
+      items.forEach(item => {
+        const menuItem = document.createElement('fluent-menu-item');
+        menuItem.role = 'menuitemradio';
+        menuItem.textContent = item;
+        node.append(menuItem);
+      });
+    });
+
+    const menuItems = element.locator('fluent-menu-item');
+    await expect(menuItems).toHaveCount(3);
+
+    for (const item of await menuItems.all()) {
+      await expect(item).toHaveAttribute('data-indent', '1');
+    }
+  });
+
+  test('should set the data-indent attribute correctly when menu items are appended via a DocumentFragment', async ({
+    fastPage,
+  }) => {
+    const { element } = fastPage;
+
+    await fastPage.setTemplate({ innerHTML: '' });
+
+    await element.evaluate(node => {
+      const fragment = document.createDocumentFragment();
+      const items = ['item 1', 'item 2', 'item 3'];
+
+      items.forEach(item => {
+        const menuItem = document.createElement('fluent-menu-item');
+        menuItem.role = 'menuitemradio';
+        menuItem.textContent = item;
+        fragment.append(menuItem);
+      });
+
+      node.append(fragment);
+    });
+
+    const menuItems = element.locator('fluent-menu-item');
+    await expect(menuItems).toHaveCount(3);
+
+    for (const item of await menuItems.all()) {
+      await expect(item).toHaveAttribute('data-indent', '1');
+    }
+  });
+
+  test('should update data-indent on existing items when a menuitemradio is appended and removed', async ({
+    fastPage,
+  }) => {
+    const { element } = fastPage;
+    const menuItems = element.locator('fluent-menu-item');
+
+    await test.step('all plain menuitems should start with data-indent 0', async () => {
+      for (const item of await menuItems.all()) {
+        await expect(item).toHaveAttribute('data-indent', '0');
+      }
+    });
+
+    await test.step('appending a menuitemradio should update all items to data-indent 1', async () => {
+      await element.evaluate(node => {
+        const menuItem = document.createElement('fluent-menu-item');
+        menuItem.role = 'menuitemradio';
+        menuItem.textContent = 'Radio item';
+        node.append(menuItem);
+      });
+
+      await expect(menuItems).toHaveCount(5);
+
+      for (const item of await menuItems.all()) {
+        await expect(item).toHaveAttribute('data-indent', '1');
+      }
+    });
+
+    await test.step('removing the menuitemradio should revert all items to data-indent 0', async () => {
+      await menuItems.last().evaluate(node => node.remove());
+
+      await expect(menuItems).toHaveCount(4);
+
+      for (const item of await menuItems.all()) {
+        await expect(item).toHaveAttribute('data-indent', '0');
+      }
+    });
+  });
+
   test.describe('`change` event', () => {
     test('should emit `change` event when `checked` property changed', async ({ fastPage }) => {
       const { element } = fastPage;

packages/web-components/src/menu-list/menu-list.ts

@@ -161,7 +161,7 @@ export class MenuList extends FASTElement {
   }
 
   private static elementIndent(el: HTMLElement): MenuItemColumnCount {
-    const role = el.getAttribute('role');
+    const role = el.role;
     const startSlot = el.querySelector('[slot=start]');
 
     if (role && role !== MenuItemRole.menuitem) {
@@ -237,7 +237,7 @@ export class MenuList extends FASTElement {
     if (changedMenuItem.role === 'menuitemradio' && changedMenuItem.checked === true) {
       for (let i = changeItemIndex - 1; i >= 0; --i) {
         const item: Element = this.menuItems[i];
-        const role: string | null = item.getAttribute('role');
+        const role: string | null = (item as HTMLElement).role;
         if (role === MenuItemRole.menuitemradio) {
           (item as MenuItem).checked = false;
         }
@@ -248,7 +248,7 @@ export class MenuList extends FASTElement {
       const maxIndex: number = this.menuItems.length - 1;
       for (let i = changeItemIndex + 1; i <= maxIndex; ++i) {
         const item: Element = this.menuItems[i];
-        const role: string | null = item.getAttribute('role');
+        const role: string | null = (item as HTMLElement).role;
         if (role === MenuItemRole.menuitemradio) {
           (item as MenuItem).checked = false;
         }
@@ -263,9 +263,7 @@ export class MenuList extends FASTElement {
    * check if the item is a menu item
    */
   protected isMenuItemElement = (el: Element): el is HTMLElement => {
-    return (
-      isMenuItem(el) || (isHTMLElement(el) && (el.getAttribute('role') as string) in MenuList.focusableElementRoles)
-    );
+    return isMenuItem(el) || (isHTMLElement(el) && !!el.role && el.role in MenuList.focusableElementRoles);
   };
 
   /**