fix(web-components): fix keyboard navigation regressions for tree and menu-list

change/@fluentui-web-components-80c47489-fa3b-4929-8f16-f8ff8d79b89f.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "fix keyboard navigation regressions for tree and menu-list",
+  "packageName": "@fluentui/web-components",
+  "email": "machi@microsoft.com",
+  "dependentChangeType": "patch"
+}

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

@@ -1,4 +1,4 @@
-import { FASTElement, observable, Updates } from '@microsoft/fast-element';
+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';
@@ -63,6 +63,9 @@ export class BaseMenuList extends FASTElement {
    */
   public disconnectedCallback(): void {
     super.disconnectedCallback();
+    Array.from(this.children).forEach(child => {
+      Observable.getNotifier(child).unsubscribe(this, 'hidden');
+    });
     this.menuChildren = undefined;
     this.removeEventListener('change', this.changedMenuItemHandler);
   }
@@ -100,6 +103,9 @@ export class BaseMenuList extends FASTElement {
 
   protected setItems(): void {
     const children: HTMLElement[] = Array.from(this.children) as HTMLElement[];
+    children.forEach((child: Element) => {
+      Observable.getNotifier(child).subscribe(this, 'hidden');
+    });
 
     this.menuChildren = children.filter(child => !child.hasAttribute('hidden'));
 

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

@@ -400,6 +400,44 @@ test.describe('MenuList', () => {
     await expect(menuItems.nth(0)).toBeFocused();
   });
 
+  test('should navigate to previously hidden items when visibility restored', async ({ fastPage }) => {
+    const { element } = fastPage;
+    const menuItems = element.locator('fluent-menu-item');
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+        <fluent-menu-item>Menu item 1</fluent-menu-item>
+        <fluent-menu-item hidden="hidden">Menu item 2</fluent-menu-item>
+        <fluent-menu-item>Menu item 3</fluent-menu-item>
+        <fluent-menu-item>Menu item 4</fluent-menu-item>
+      `,
+    });
+
+    await element.evaluate(node => {
+      node.focus();
+    });
+
+    await expect(menuItems.nth(0)).toBeFocused();
+
+    await element.press('ArrowDown');
+
+    await expect(menuItems.nth(2)).toBeFocused();
+
+    await menuItems.nth(1).evaluate(node => {
+      node.removeAttribute('hidden');
+    });
+
+    await element.evaluate(node => {
+      node.focus();
+    });
+
+    await expect(menuItems.nth(0)).toBeFocused();
+
+    await element.press('ArrowDown');
+
+    await expect(menuItems.nth(1)).toBeFocused();
+  });
+
   test('should set the data-indent attribute to 0 correctly on all MenuItem elements when role of menuitem and not content in start slot', async ({
     fastPage,
   }) => {

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

@@ -83,6 +83,8 @@ export class BaseTree extends FASTElement {
         if (item?.childTreeItems?.length) {
           if (!item.expanded) {
             item.expanded = true;
+          } else {
+            return true;
           }
         }
         return;

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

@@ -216,6 +216,7 @@ test.describe('Tree', () => {
     await expect(treeItem1).toHaveAttribute('selected');
     expect(await elementHandle).toBe(false);
   });
+
   test('keyboard navigation should work when the tree-item contains focusable elements', async ({
     fastPage,
     page,
@@ -241,4 +242,185 @@ test.describe('Tree', () => {
     await page.keyboard.press(browserName === 'webkit' ? 'Alt+Tab' : 'Tab');
     await expect(anchor).toBeFocused();
   });
+  test('should move focus down with ArrowDown', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+    const treeItems = element.locator(`:scope > fluent-tree-item`);
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+                <fluent-tree-item>Item 1</fluent-tree-item>
+                <fluent-tree-item>Item 2</fluent-tree-item>
+                <fluent-tree-item>Item 3</fluent-tree-item>
+            `,
+    });
+
+    await treeItems.nth(0).focus();
+    await expect(treeItems.nth(0)).toBeFocused();
+
+    await page.keyboard.press('ArrowDown');
+    await expect(treeItems.nth(1)).toBeFocused();
+
+    await page.keyboard.press('ArrowDown');
+    await expect(treeItems.nth(2)).toBeFocused();
+  });
+
+  test('should move focus up with ArrowUp', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+    const treeItems = element.locator(`:scope > fluent-tree-item`);
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+                <fluent-tree-item>Item 1</fluent-tree-item>
+                <fluent-tree-item>Item 2</fluent-tree-item>
+                <fluent-tree-item>Item 3</fluent-tree-item>
+            `,
+    });
+
+    await treeItems.nth(0).focus();
+    await page.keyboard.press('ArrowDown');
+    await page.keyboard.press('ArrowDown');
+    await expect(treeItems.nth(2)).toBeFocused();
+
+    await page.keyboard.press('ArrowUp');
+    await expect(treeItems.nth(1)).toBeFocused();
+
+    await page.keyboard.press('ArrowUp');
+    await expect(treeItems.nth(0)).toBeFocused();
+  });
+
+  test('should expand a collapsed item with ArrowRight', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+    const parentItem = element.locator(`:scope > fluent-tree-item`);
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+                <fluent-tree-item>
+                    Item 1
+                    <fluent-tree-item slot="item">Nested Item A</fluent-tree-item>
+                </fluent-tree-item>
+            `,
+    });
+
+    await parentItem.focus();
+    await expect(parentItem).toBeFocused();
+    await expect(parentItem).not.toHaveAttribute('expanded');
+
+    await page.keyboard.press('ArrowRight');
+    await expect(parentItem).toHaveAttribute('expanded');
+  });
+
+  test('should focus child after ArrowRight on an expanded item', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+    const parentItem = element.locator(`:scope > fluent-tree-item`);
+    const nestedItem = parentItem.locator('fluent-tree-item');
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+                <fluent-tree-item>
+                    Item 1
+                    <fluent-tree-item slot="item">Nested Item A</fluent-tree-item>
+                </fluent-tree-item>
+            `,
+    });
+
+    await parentItem.focus();
+    // expand first
+    await page.keyboard.press('ArrowRight');
+    await expect(parentItem).toHaveAttribute('expanded');
+    await expect(nestedItem).toBeVisible();
+
+    // arrow right again should focus the nested item
+    await page.keyboard.press('ArrowRight');
+    await expect(nestedItem).toBeFocused();
+  });
+
+  test('should collapse an expanded item with ArrowLeft', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+    const parentItem = element.locator(`:scope > fluent-tree-item`);
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+                <fluent-tree-item>
+                    Item 1
+                    <fluent-tree-item slot="item">Nested Item A</fluent-tree-item>
+                </fluent-tree-item>
+            `,
+    });
+
+    await parentItem.focus();
+    await page.keyboard.press('ArrowRight');
+    await expect(parentItem).toHaveAttribute('expanded');
+
+    await page.keyboard.press('ArrowLeft');
+    await expect(parentItem).not.toHaveAttribute('expanded');
+  });
+
+  test('should focus parent item with ArrowLeft on a nested item', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+    const parentItem = element.locator(`:scope > fluent-tree-item`);
+    const nestedItem = parentItem.locator('fluent-tree-item');
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+                <fluent-tree-item>
+                    Item 1
+                    <fluent-tree-item slot="item">Nested Item A</fluent-tree-item>
+                </fluent-tree-item>
+            `,
+    });
+
+    await parentItem.focus();
+    // expand and wait for nested item to be visible
+    await page.keyboard.press('ArrowRight');
+    await expect(parentItem).toHaveAttribute('expanded');
+    await expect(nestedItem).toBeVisible();
+
+    // focus nested item
+    await page.keyboard.press('ArrowRight');
+    await expect(nestedItem).toBeFocused();
+
+    // arrow left on leaf should focus parent
+    await page.keyboard.press('ArrowLeft');
+    await expect(parentItem).toBeFocused();
+  });
+
+  test('should focus first item with Home key', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+    const treeItems = element.locator(`:scope > fluent-tree-item`);
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+                <fluent-tree-item>Item 1</fluent-tree-item>
+                <fluent-tree-item>Item 2</fluent-tree-item>
+                <fluent-tree-item>Item 3</fluent-tree-item>
+            `,
+    });
+
+    await treeItems.nth(0).focus();
+    await page.keyboard.press('ArrowDown');
+    await page.keyboard.press('ArrowDown');
+    await expect(treeItems.nth(2)).toBeFocused();
+
+    await page.keyboard.press('Home');
+    await expect(treeItems.nth(0)).toBeFocused();
+  });
+
+  test('should focus last item with End key', async ({ fastPage, page }) => {
+    const { element } = fastPage;
+    const treeItems = element.locator(`:scope > fluent-tree-item`);
+
+    await fastPage.setTemplate({
+      innerHTML: /* html */ `
+                <fluent-tree-item>Item 1</fluent-tree-item>
+                <fluent-tree-item>Item 2</fluent-tree-item>
+                <fluent-tree-item>Item 3</fluent-tree-item>
+            `,
+    });
+
+    await treeItems.nth(0).focus();
+    await expect(treeItems.nth(0)).toBeFocused();
+
+    await page.keyboard.press('End');
+    await expect(treeItems.nth(2)).toBeFocused();
+  });
 });

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

@@ -3,7 +3,7 @@ import type { Tree } from './tree.js';
 
 export const template = html<Tree>`
   <template
-    focusgroup="menu nowrap nomemory"
+    focusgroup="menu inline block nowrap nomemory"
     @click="${(x, c) => x.clickHandler(c.event)}"
     @keydown="${(x, c) => x.keydownHandler(c.event as KeyboardEvent)}"
     @change="${(x, c) => x.changeHandler(c.event)}"

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

@@ -61,7 +61,7 @@ export class Tree extends BaseTree {
       this.fg = new FocusGroup(this, this.fgItems, {
         definition: {
           behavior: 'menu',
-          axis: 'block',
+          axis: undefined,
           memory: false,
         },
       });