diff --git a/change/@fluentui-web-components-80c47489-fa3b-4929-8f16-f8ff8d79b89f.json b/change/@fluentui-web-components-80c47489-fa3b-4929-8f16-f8ff8d79b89f.json new file mode 100644 index 00000000000000..b6ae232147899a --- /dev/null +++ b/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" +} diff --git a/packages/web-components/src/menu-list/menu-list.base.ts b/packages/web-components/src/menu-list/menu-list.base.ts index d08aaa46d3b2ae..6ebcd314be5c0d 100644 --- a/packages/web-components/src/menu-list/menu-list.base.ts +++ b/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')); diff --git a/packages/web-components/src/menu-list/menu-list.spec.ts b/packages/web-components/src/menu-list/menu-list.spec.ts index 25372a141be4b9..68511a4c83f8a7 100644 --- a/packages/web-components/src/menu-list/menu-list.spec.ts +++ b/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 */ ` + Menu item 1 + + Menu item 3 + Menu item 4 + `, + }); + + 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, }) => { diff --git a/packages/web-components/src/tree/tree.base.ts b/packages/web-components/src/tree/tree.base.ts index aa8114ce468e98..b836dc312a3a8a 100644 --- a/packages/web-components/src/tree/tree.base.ts +++ b/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; diff --git a/packages/web-components/src/tree/tree.spec.ts b/packages/web-components/src/tree/tree.spec.ts index 72df168c459ea8..03b2c13c6b70c4 100644 --- a/packages/web-components/src/tree/tree.spec.ts +++ b/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 */ ` + Item 1 + Item 2 + Item 3 + `, + }); + + 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 */ ` + Item 1 + Item 2 + Item 3 + `, + }); + + 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 */ ` + + Item 1 + Nested Item A + + `, + }); + + 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 */ ` + + Item 1 + Nested Item A + + `, + }); + + 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 */ ` + + Item 1 + Nested Item A + + `, + }); + + 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 */ ` + + Item 1 + Nested Item A + + `, + }); + + 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 */ ` + Item 1 + Item 2 + Item 3 + `, + }); + + 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 */ ` + Item 1 + Item 2 + Item 3 + `, + }); + + await treeItems.nth(0).focus(); + await expect(treeItems.nth(0)).toBeFocused(); + + await page.keyboard.press('End'); + await expect(treeItems.nth(2)).toBeFocused(); + }); }); diff --git a/packages/web-components/src/tree/tree.template.ts b/packages/web-components/src/tree/tree.template.ts index 04f6392feefa4c..05b8e8073e1473 100644 --- a/packages/web-components/src/tree/tree.template.ts +++ b/packages/web-components/src/tree/tree.template.ts @@ -3,7 +3,7 @@ import type { Tree } from './tree.js'; export const template = html`