Skip to content

Commit

Permalink
fix(web-components): fix enterkey interaction on menu (microsoft#31894)
Browse files Browse the repository at this point in the history
  • Loading branch information
davatron5000 committed Jul 2, 2024
1 parent 16b43e3 commit 2ccb9dd
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "fix(web-components): fix menu enterkey interaction ",
"packageName": "@fluentui/web-components",
"email": "rupertdavid@microsoft.com",
"dependentChangeType": "patch"
}
2 changes: 1 addition & 1 deletion packages/web-components/docs/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -2292,7 +2292,7 @@ export type MediaQueryListListener = (this: MediaQueryList, ev?: MediaQueryListE

// @public
export class Menu extends FASTElement {
closeMenu: () => void;
closeMenu: (event?: Event) => void;
closeOnScroll?: boolean;
closeOnScrollChanged(oldValue: boolean, newValue: boolean): void;
connectedCallback(): void;
Expand Down
165 changes: 165 additions & 0 deletions packages/web-components/src/menu/menu.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
import { expect, test } from '@playwright/test';
import type { Locator, Page } from '@playwright/test';
import { fixtureURL } from '../helpers.tests.js';
import type { Menu } from './menu.js';

const menuFixture = `
<fluent-menu>
<fluent-menu-button appearance="primary" slot="trigger">Toggle Menu</fluent-menu-button>
<fluent-menu-list>
<fluent-menu-item>Menu item 1</fluent-menu-item>
<fluent-menu-item>Menu item 2</fluent-menu-item>
<fluent-menu-item>Menu item 3</fluent-menu-item>
<fluent-menu-item>Menu item 4</fluent-menu-item>
</fluent-menu-list>
</fluent-menu>
`;

test.describe('Menu', () => {
let page: Page;
let element: Locator;
let menuButton: Locator;
let menuList: Locator;
let menuItems: Locator;

test.beforeAll(async ({ browser }) => {
page = await browser.newPage();

element = page.locator('fluent-menu');

menuButton = element.locator('fluent-menu-button');

menuList = element.locator('fluent-menu-list');

menuItems = menuList.locator('fluent-menu-item');

await page.goto(fixtureURL('components-menu--default'));

page.setContent(menuFixture);
});

test.afterAll(async () => {
await page.close();
});

test('should have menu-list be initially hidden', async () => {
await expect(menuList).toBeHidden();
});

test('should be visible when the button is clicked', async () => {
await menuButton.click();

await expect(menuList).toBeVisible();
});

test('should be hidden when the button is clicked again', async () => {
page.setContent(menuFixture);

await menuButton.click();

await expect(menuList).toBeVisible();

await menuButton.click();

await expect(menuList).toBeHidden();
});

test('should be hidden when an item is clicked', async () => {
page.setContent(menuFixture);

await menuButton.click();

await expect(menuList).toBeVisible();

await menuItems.first().click();

await expect(menuList).toBeHidden();
});

test('should close when an item is keyboard clicked', async () => {
page.setContent(menuFixture);

await menuButton.click();

await expect(menuList).toBeVisible();

await menuItems.first().focus();

await page.keyboard.press('Enter');

await expect(menuList).toBeHidden();
});

test('should be hidden when clicked outside', async () => {
page.setContent(menuFixture);

await menuButton.click();

await expect(menuList).toBeVisible();

await page.mouse.click(0, 0);

await expect(menuList).toBeHidden();
});

test('should be hidden when the escape key is pressed', async () => {
page.setContent(menuFixture);

await menuButton.click();

await expect(menuList).toBeVisible();

await page.keyboard.press('Escape');

await expect(menuList).toBeHidden();
});

test('should be hidden when the escape key is pressed on an item', async () => {
page.setContent(menuFixture);

await menuButton.click();

await expect(menuList).toBeVisible();

await menuItems.first().focus();

await page.keyboard.press('Escape');

await expect(menuList).toBeHidden();
});

test('should open on hover when openOnHover is true', async () => {
page.setContent(menuFixture);

await expect(menuList).toBeHidden();
await menuButton.hover();
await expect(menuList).toBeHidden();
await page.mouse.move(0, 0);

await element.evaluate((x: Menu) => {
x.openOnHover = true;
});

await expect(menuList).toBeHidden();

await menuButton.hover();

await expect(menuList).toBeVisible();
});

test('should open on context when openOnContext is true', async () => {
page.setContent(menuFixture);

await expect(menuList).toBeHidden();
await menuButton.click({ button: 'right' });
await expect(menuList).toBeHidden();

await element.evaluate((x: Menu) => {
x.openOnContext = true;
});

await expect(menuList).toBeHidden();
await menuButton.dispatchEvent('contextmenu');
await expect(menuList).toBeVisible();
});
});
29 changes: 29 additions & 0 deletions packages/web-components/src/menu/menu.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,32 @@ export const MenuWithMaxHeight = renderComponent(html<MenuStoryArgs>`
</fluent-menu>
</div>
`);

export const MenuWithInteractiveItems = renderComponent(html<MenuStoryArgs>`
<div class="container">
<fluent-menu>
<fluent-menu-button appearance="primary" slot="trigger">Toggle Menu</fluent-menu-button>
<fluent-menu-list>
<fluent-menu-item>
Item 1
<fluent-menu-list slot="submenu">
<fluent-menu-item> Subitem 1 </fluent-menu-item>
<fluent-menu-item> Subitem 2 </fluent-menu-item>
</fluent-menu-list>
</fluent-menu-item>
<fluent-menu-item role="menuitemcheckbox"> Item 2 </fluent-menu-item>
<fluent-menu-item role="menuitemcheckbox"> Item 3 </fluent-menu-item>
<fluent-divider role="separator" aria-orientation="horizontal" orientation="horizontal"></fluent-divider>
<fluent-menu-item>Menu item 4</fluent-menu-item>
<fluent-menu-item>Menu item 5</fluent-menu-item>
<fluent-menu-item>Menu item 6</fluent-menu-item>
<fluent-menu-item>Menu item 7</fluent-menu-item>
<fluent-menu-item>Menu item 8</fluent-menu-item>
</fluent-menu-list>
</fluent-menu>
</div>
`);
21 changes: 16 additions & 5 deletions packages/web-components/src/menu/menu.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { attr, FASTElement, observable, Updates } from '@microsoft/fast-element';
import { keyEnter, keyEscape, keySpace, keyTab } from '@microsoft/fast-web-utilities';
import { MenuList } from '../menu-list/menu-list.js';
import { MenuItem } from '../menu-item/menu-item.js';
import { MenuItemRole } from '../menu-item/menu-item.options.js';

/**
* A Menu component that provides a customizable menu element.
Expand Down Expand Up @@ -154,7 +156,16 @@ export class Menu extends FASTElement {
* Closes the menu.
* @public
*/
public closeMenu = () => {
public closeMenu = (event?: Event) => {
// Keep menu open if the event target is a menu item checkbox or radio
if (
event?.target instanceof MenuItem &&
(event.target.getAttribute('role') === MenuItemRole.menuitemcheckbox ||
event.target.getAttribute('role') === MenuItemRole.menuitemradio)
) {
return;
}

this._menuList?.togglePopover(false);

if (this.closeOnScroll) {
Expand Down Expand Up @@ -238,9 +249,9 @@ export class Menu extends FASTElement {
*/
public persistOnItemClickChanged(oldValue: boolean, newValue: boolean): void {
if (!newValue) {
this._menuList?.addEventListener('click', this.closeMenu);
this._menuList?.addEventListener('change', this.closeMenu);
} else {
this._menuList?.removeEventListener('click', this.closeMenu);
this._menuList?.removeEventListener('change', this.closeMenu);
}
}

Expand Down Expand Up @@ -288,7 +299,7 @@ export class Menu extends FASTElement {
this._trigger?.addEventListener('keydown', this.triggerKeydownHandler);

if (!this.persistOnItemClick) {
this._menuList?.addEventListener('click', this.closeMenu);
this._menuList?.addEventListener('change', this.closeMenu);
}
if (this.openOnHover) {
this._trigger?.addEventListener('mouseover', this.openMenu);
Expand All @@ -313,7 +324,7 @@ export class Menu extends FASTElement {

this._trigger?.removeEventListener('keydown', this.triggerKeydownHandler);
if (!this.persistOnItemClick) {
this._menuList?.removeEventListener('click', this.closeMenu);
this._menuList?.removeEventListener('change', this.closeMenu);
}
if (this.openOnHover) {
this._trigger?.removeEventListener('mouseover', this.openMenu);
Expand Down

0 comments on commit 2ccb9dd

Please sign in to comment.