From c39e8c80af79ada2a696372c93161355187944af Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 26 Jan 2023 12:38:23 -0600 Subject: [PATCH] Menu button updates for table (#986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request ## ๐Ÿคจ Rationale Some changes need to be made to the menu button before it can be used within the table for the action menu. Specifically, those changes are: - adding a `beforetoggle` event - having the menu button find the slotted menu when it is nested within multiple slots See #956 for more details. ## ๐Ÿ‘ฉโ€๐Ÿ’ป Implementation - Add `beforetoggle` event to the menu button that is fired before the menu opens or closes - Add Angular & Blazor support for the `beforetoggle` event - Change the logic for finding the slotted menu. We can no longer use a filter in the template to find slotted elements with a role of `menu` because this won't match `slot` elements. Instead, the template assigns all slotted elements to the `slottedMenus` array, and the getter for `menu` finds the menu, which may be nested within `slot` elements. ## ๐Ÿงช Testing - Manually tested that the `beforetoggle` event handler works correctly in Blazor & Angular - Added auto tests for the `beforetoggle` event works correctly for the web components - Basic manual testing in Storybook that the component still behaves as expected - Added tests that the focus & menu opening behavior works correctly when the menu is nested in an additional slot ## โœ… Checklist - [ ] I have updated the project documentation to reflect my changes or determined no changes are needed. --- .../nimble-menu-button.directive.ts | 14 +- ...-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json | 7 + ...-51e63b35-2ce0-44af-9dda-744c7234e857.json | 7 + ...-0c3b5f38-1366-416c-8b35-20029a8d3eba.json | 7 + .../Components/NimbleMenuButton.razor | 3 +- .../Components/NimbleMenuButton.razor.cs | 25 +- .../NimbleBlazor/EventHandlers.cs | 8 +- .../wwwroot/NimbleBlazor.lib.module.js | 17 +- .../src/menu-button/index.ts | 86 +- .../src/menu-button/specs/README.md | 7 +- .../src/menu-button/template.ts | 4 +- .../src/menu-button/tests/menu-button.spec.ts | 973 ++++++++++++------ .../menu-button/tests/menu-button.stories.ts | 2 +- .../src/menu-button/types.ts | 9 + 14 files changed, 811 insertions(+), 358 deletions(-) create mode 100644 change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json create mode 100644 change/@ni-nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json create mode 100644 change/@ni-nimble-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json diff --git a/angular-workspace/projects/ni/nimble-angular/src/directives/menu-button/nimble-menu-button.directive.ts b/angular-workspace/projects/ni/nimble-angular/src/directives/menu-button/nimble-menu-button.directive.ts index e0c2984a4e..d4763fc069 100644 --- a/angular-workspace/projects/ni/nimble-angular/src/directives/menu-button/nimble-menu-button.directive.ts +++ b/angular-workspace/projects/ni/nimble-angular/src/directives/menu-button/nimble-menu-button.directive.ts @@ -1,9 +1,10 @@ -import { Directive, ElementRef, EventEmitter, HostListener, Input, Output, Renderer2 } from '@angular/core'; +import { Directive, ElementRef, Input, Renderer2 } from '@angular/core'; import type { MenuButton } from '@ni/nimble-components/dist/esm/menu-button'; -import type { ButtonAppearance } from '@ni/nimble-components/dist/esm/menu-button/types'; +import type { ButtonAppearance, MenuButtonToggleEventDetail } from '@ni/nimble-components/dist/esm/menu-button/types'; import { BooleanValueOrAttribute, toBooleanProperty } from '../utilities/template-value-helpers'; export type { MenuButton }; +export type { MenuButtonToggleEventDetail }; /** * Directive to provide Angular integration for the menu button. @@ -46,14 +47,5 @@ export class NimbleMenuButtonDirective { this.renderer.setProperty(this.elementRef.nativeElement, 'open', toBooleanProperty(value)); } - @Output() public openChange = new EventEmitter(); - public constructor(private readonly renderer: Renderer2, private readonly elementRef: ElementRef) {} - - @HostListener('open-change', ['$event']) - public onOpenChange($event: Event): void { - if ($event.target === this.elementRef.nativeElement) { - this.openChange.emit(this.open); - } - } } diff --git a/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json b/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json new file mode 100644 index 0000000000..4dd5527e74 --- /dev/null +++ b/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json @@ -0,0 +1,7 @@ +{ + "type": "major", + "comment": "Add 'beforetoggle' event on menu button and rename 'open-change' event to 'toggle'", + "packageName": "@ni/nimble-angular", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/change/@ni-nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json b/change/@ni-nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json new file mode 100644 index 0000000000..6bfd807ce1 --- /dev/null +++ b/change/@ni-nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json @@ -0,0 +1,7 @@ +{ + "type": "major", + "comment": "Add 'beforetoggle' event on menu button and rename 'open-change' event to 'toggle'", + "packageName": "@ni/nimble-blazor", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/change/@ni-nimble-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json b/change/@ni-nimble-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json new file mode 100644 index 0000000000..dc285e9805 --- /dev/null +++ b/change/@ni-nimble-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json @@ -0,0 +1,7 @@ +{ + "type": "major", + "comment": "Add 'beforetoggle' event on menu button and rename 'open-change' event to 'toggle'.\nUpdate menu button to work when the slotted menu is nested within additional slots.", + "packageName": "@ni/nimble-components", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor index 5c93b1d2d9..05fe882a8d 100644 --- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor +++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor @@ -2,7 +2,8 @@ @inherits ComponentBase [Parameter] - public EventCallback OpenChanged { get; set; } + public EventCallback Toggle { get; set; } + + /// + /// Gets or sets a callback that's invoked before the 'open' state of the menu button changes + /// + [Parameter] + public EventCallback BeforeToggle { get; set; } /// /// Called when 'open' changes on the web component. /// - /// New value of open - protected async void UpdateOpen(bool? value) + /// The state of the menu button + protected async void HandleToggle(MenuButtonToggleEventArgs eventArgs) + { + Open = eventArgs.NewState; + await Toggle.InvokeAsync(eventArgs); + } + + /// + /// Called when the 'beforetoggle' event is fired on the web component + /// + /// The state of the menu button + protected async void HandleBeforeToggle(MenuButtonToggleEventArgs eventArgs) { - Open = value; - await OpenChanged.InvokeAsync(value); + await BeforeToggle.InvokeAsync(eventArgs); } /// diff --git a/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs b/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs index b77c5fe636..f675d2f70d 100644 --- a/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs +++ b/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs @@ -13,14 +13,16 @@ public class CheckboxChangeEventArgs : EventArgs public bool Checked { get; set; } } -public class MenuButtonOpenChangeEventArgs : EventArgs +public class MenuButtonToggleEventArgs : EventArgs { - public bool Open { get; set; } + public bool NewState { get; set; } + public bool OldState { get; set; } } [EventHandler("onnimbletabsactiveidchange", typeof(TabsChangeEventArgs), enableStopPropagation: true, enablePreventDefault: true)] [EventHandler("onnimblecheckedchange", typeof(CheckboxChangeEventArgs), enableStopPropagation: true, enablePreventDefault: true)] -[EventHandler("onnimblemenubuttonopenchange", typeof(MenuButtonOpenChangeEventArgs), enableStopPropagation: true, enablePreventDefault: true)] +[EventHandler("onnimblemenubuttontoggle", typeof(MenuButtonToggleEventArgs), enableStopPropagation: true, enablePreventDefault: false)] +[EventHandler("onnimblemenubuttonbeforetoggle", typeof(MenuButtonToggleEventArgs), enableStopPropagation: true, enablePreventDefault: false)] public static class EventHandlers { } diff --git a/packages/nimble-blazor/NimbleBlazor/wwwroot/NimbleBlazor.lib.module.js b/packages/nimble-blazor/NimbleBlazor/wwwroot/NimbleBlazor.lib.module.js index fdce6471d5..38caef70f3 100644 --- a/packages/nimble-blazor/NimbleBlazor/wwwroot/NimbleBlazor.lib.module.js +++ b/packages/nimble-blazor/NimbleBlazor/wwwroot/NimbleBlazor.lib.module.js @@ -31,11 +31,22 @@ export function afterStarted(Blazor) { } }); // Used by NimbleMenuButton.razor - Blazor.registerCustomEventType('nimblemenubuttonopenchange', { - browserEventName: 'open-change', + Blazor.registerCustomEventType('nimblemenubuttontoggle', { + browserEventName: 'toggle', createEventArgs: event => { return { - open: event.target.open + newState: event.detail.newState, + oldState: event.detail.oldState + }; + } + }); + // Used by NimbleMenuButton.razor + Blazor.registerCustomEventType('nimblemenubuttonbeforetoggle', { + browserEventName: 'beforetoggle', + createEventArgs: event => { + return { + newState: event.detail.newState, + oldState: event.detail.oldState }; } }); diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index 0beb314b73..f98f297005 100644 --- a/packages/nimble-components/src/menu-button/index.ts +++ b/packages/nimble-components/src/menu-button/index.ts @@ -10,7 +10,7 @@ import { ButtonAppearance } from '../button/types'; import type { ToggleButton } from '../toggle-button'; import { styles } from './styles'; import { template } from './template'; -import { MenuButtonPosition } from './types'; +import { MenuButtonToggleEventDetail, MenuButtonPosition } from './types'; import type { ButtonPattern } from '../patterns/button/types'; import type { AnchoredRegion } from '../anchored-region'; @@ -108,7 +108,11 @@ export class MenuButton extends FoundationElement implements ButtonPattern { if (!this.open) { // Only fire an event here if the menu is changing to being closed. Otherwise, // wait until the menu is actually opened before firing the event. - this.$emit('open-change'); + const eventDetail: MenuButtonToggleEventDetail = { + oldState: true, + newState: false + }; + this.$emit('toggle', eventDetail); } } @@ -120,7 +124,11 @@ export class MenuButton extends FoundationElement implements ButtonPattern { this.focusMenu(); } - this.$emit('open-change'); + const eventDetail: MenuButtonToggleEventDetail = { + oldState: false, + newState: true + }; + this.$emit('toggle', eventDetail); } public focusoutHandler(e: FocusEvent): boolean { @@ -129,8 +137,11 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } const focusTarget = e.relatedTarget as HTMLElement; - if (!this.contains(focusTarget)) { - this.open = false; + if ( + !this.contains(focusTarget) + && !this.getMenu()?.contains(focusTarget) + ) { + this.setOpen(false); return false; } @@ -138,9 +149,9 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } public toggleButtonCheckedChangeHandler(e: Event): boolean { - this.open = this.toggleButton!.checked; + this.setOpen(this.toggleButton!.checked); // Don't bubble the 'change' event from the toggle button because - // the menu button has its own 'open-change' event. + // the menu button has its own 'toggle' event. e.stopPropagation(); return false; } @@ -149,10 +160,10 @@ export class MenuButton extends FoundationElement implements ButtonPattern { switch (e.key) { case keyArrowUp: this.focusLastItemWhenOpened = true; - this.open = true; + this.setOpen(true); return false; case keyArrowDown: - this.open = true; + this.setOpen(true); return false; default: return true; @@ -162,7 +173,7 @@ export class MenuButton extends FoundationElement implements ButtonPattern { public menuKeyDownHandler(e: KeyboardEvent): boolean { switch (e.key) { case keyEscape: - this.open = false; + this.setOpen(false); this.toggleButton!.focus(); return false; default: @@ -170,16 +181,61 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } } - private get menu(): HTMLElement | undefined { - return this.slottedMenus?.length ? this.slottedMenus[0] : undefined; + private setOpen(newValue: boolean): void { + if (this.open === newValue) { + return; + } + + const eventDetail: MenuButtonToggleEventDetail = { + oldState: this.open, + newState: newValue + }; + this.$emit('beforetoggle', eventDetail); + + this.open = newValue; + } + + private getMenu(): HTMLElement | undefined { + // Get the menu that is slotted within the menu-button, taking into account + // that it may be nested within multiple 'slot' elements, such as when used + // within a table. + if (!this.slottedMenus?.length) { + return undefined; + } + + let currentItem: HTMLElement | undefined = this.slottedMenus[0]; + while (currentItem) { + if (currentItem.getAttribute('role') === 'menu') { + return currentItem; + } + + if (this.isSlotElement(currentItem)) { + const firstNode = currentItem.assignedNodes()[0]; + if (firstNode instanceof HTMLElement) { + currentItem = firstNode; + } else { + currentItem = undefined; + } + } else { + return undefined; + } + } + + return undefined; + } + + private isSlotElement( + element: HTMLElement | undefined + ): element is HTMLSlotElement { + return element?.nodeName === 'SLOT'; } private focusMenu(): void { - this.menu?.focus(); + this.getMenu()?.focus(); } private focusLastMenuItem(): void { - const menuItems = this.menu?.querySelectorAll('[role=menuitem]'); + const menuItems = this.getMenu()?.querySelectorAll('[role=menuitem]'); if (menuItems?.length) { const lastMenuItem = menuItems[menuItems.length - 1] as HTMLElement; lastMenuItem.focus(); @@ -187,7 +243,7 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } private readonly menuChangeHandler = (): void => { - this.open = false; + this.setOpen(false); this.toggleButton!.focus(); }; } diff --git a/packages/nimble-components/src/menu-button/specs/README.md b/packages/nimble-components/src/menu-button/specs/README.md index 7ce9af06c1..8af7ddbaa3 100644 --- a/packages/nimble-components/src/menu-button/specs/README.md +++ b/packages/nimble-components/src/menu-button/specs/README.md @@ -84,7 +84,12 @@ _Methods_ _Events_ -- `open-change` (event) - event for when the opened state has changed +- `beforetoggle` (event) - event fired before the opened state has changed. The event detail contains: + - `newState` - boolean - The value of `open` on the menu button that the element is transitioning in to. + - `oldState` - boolean - The value of `open` on the menu button that the element is transitioning out of. +- `toggle` (event) - event for when the opened state has changed + - `newState` - boolean - The value of `open` on the menu button that the element transitioned in to. + - `oldState` - boolean - The value of `open` on the menu button that the element transitioned out of. _CSS Classes and CSS Custom Properties that affect the component_ diff --git a/packages/nimble-components/src/menu-button/template.ts b/packages/nimble-components/src/menu-button/template.ts index 752e57b85a..6411df3ab1 100644 --- a/packages/nimble-components/src/menu-button/template.ts +++ b/packages/nimble-components/src/menu-button/template.ts @@ -1,4 +1,4 @@ -import { elements, html, ref, slotted, when } from '@microsoft/fast-element'; +import { html, ref, slotted, when } from '@microsoft/fast-element'; import { DesignSystem } from '@microsoft/fast-foundation'; import type { MenuButton } from '.'; import { ToggleButton } from '../toggle-button'; @@ -43,7 +43,7 @@ export const template = html` ${ref('region')} > - + ` diff --git a/packages/nimble-components/src/menu-button/tests/menu-button.spec.ts b/packages/nimble-components/src/menu-button/tests/menu-button.spec.ts index 810dd9d3e4..2a2134e81b 100644 --- a/packages/nimble-components/src/menu-button/tests/menu-button.spec.ts +++ b/packages/nimble-components/src/menu-button/tests/menu-button.spec.ts @@ -7,50 +7,92 @@ import { keyEscape, keySpace } from '@microsoft/fast-web-utilities'; -import type { Menu, MenuItem } from '@microsoft/fast-foundation'; +import { FoundationElement, Menu, MenuItem } from '@microsoft/fast-foundation'; import { fixture, Fixture } from '../../utilities/tests/fixture'; import { MenuButton } from '..'; -import { MenuButtonPosition } from '../types'; +import { MenuButtonToggleEventDetail, MenuButtonPosition } from '../types'; + +class TestSlottedElement extends FoundationElement {} +const composedTestSlottedElement = TestSlottedElement.compose({ + baseName: 'test-slotted-element', + template: html` + + + + ` +}); async function setup(): Promise> { return fixture(html``); } +async function slottedSetup(): Promise> { + return fixture(composedTestSlottedElement()); +} + +/** A helper function to abstract adding a `toggle` event listener, spying + * on the event being called, and removing the event listener. The returned promise + * should be resolved prior to completing a test. + */ +function createToggleListener(menuButton: MenuButton): { + promise: Promise, + spy: jasmine.Spy +} { + const spy = jasmine.createSpy(); + return { + promise: new Promise(resolve => { + const handler = (...args: unknown[]): void => { + menuButton.removeEventListener('toggle', handler); + spy(...args); + resolve(); + }; + menuButton.addEventListener('toggle', handler); + }), + spy + }; +} + +/** A helper function to abstract adding a `beforetoggle` event listener, spying + * on the event being called, and removing the event listener. The returned promise + * should be resolved prior to completing a test. + * + * The function asserts that the menu button has the expected `open` value when the + * `beforetoggle` is fired and that when the `beforetoggle` event is fired, the + * `toggleSpy` has not been called. + */ +function createBeforeToggleListener( + menuButton: MenuButton, + expectedOpenState: boolean, + toggleSpy: jasmine.Spy +): { + promise: Promise, + spy: jasmine.Spy + } { + const spy = jasmine.createSpy(); + return { + promise: new Promise(resolve => { + const handler = (...args: unknown[]): void => { + expect(menuButton.open).toEqual(expectedOpenState); + expect(toggleSpy).not.toHaveBeenCalled(); + + menuButton.removeEventListener('beforetoggle', handler); + spy(...args); + resolve(); + }; + menuButton.addEventListener('beforetoggle', handler); + }), + spy + }; +} + describe('MenuButton', () => { let parent: HTMLElement; - let element: MenuButton; - let connect: () => Promise; - let disconnect: () => Promise; let menu: Menu; let menuItem1: MenuItem; let menuItem2: MenuItem; let menuItem3: MenuItem; - /** A helper function to abstract adding an 'open-change' event listener, spying - * on the event being called, and removing the event listener. The returned promise - * should be resolved prior to completing a test. - */ - function createOpenChangeListener(): { - promise: Promise, - spy: jasmine.Spy - } { - const spy = jasmine.createSpy(); - return { - promise: new Promise(resolve => { - const handler = (...args: unknown[]): void => { - element.removeEventListener('open-change', handler); - spy(...args); - resolve(); - }; - element.addEventListener('open-change', handler); - }), - spy - }; - } - - beforeEach(async () => { - ({ element, connect, disconnect, parent } = await setup()); - + function createAndSlotMenu(parentElement: HTMLElement): void { menu = document.createElement('nimble-menu'); menu.slot = 'menu'; @@ -66,294 +108,593 @@ describe('MenuButton', () => { menuItem3.textContent = 'menu item 3'; menu.appendChild(menuItem3); - element.appendChild(menu); - }); - - afterEach(async () => { - await disconnect(); - }); - - it('can construct an element instance', () => { - expect(document.createElement('nimble-menu-button')).toBeInstanceOf( - MenuButton - ); - }); - - it('should disable the toggle button when the disabled is `true`', async () => { - element.disabled = true; - await connect(); - expect(element.toggleButton!.disabled).toBeTrue(); - }); - - it('should set aria-haspopup on toggle button', async () => { - await connect(); - expect(element.toggleButton!.getAttribute('aria-haspopup')).toEqual( - 'true' - ); - }); - - it('should set aria-expanded to true on the toggle button when the menu is open', async () => { - element.open = true; - await connect(); - expect(element.toggleButton!.getAttribute('aria-expanded')).toEqual( - 'true' - ); - }); - - it('should set aria-expanded to false on the toggle button when the menu is closed', async () => { - await connect(); - expect(element.toggleButton!.getAttribute('aria-expanded')).toEqual( - 'false' - ); - }); - - it('should mark the toggle button as checked when the menu is opened before connect', async () => { - element.open = true; - await connect(); - expect(element.toggleButton!.checked).toBeTrue(); - }); - - it('should mark the toggle button as checked when the menu is opened after connect', async () => { - await connect(); - element.open = true; - DOM.processUpdates(); - expect(element.toggleButton!.checked).toBeTrue(); - }); - - it('should not mark the toggle button as checked when the menu is not open', async () => { - await connect(); - expect(element.toggleButton!.checked).toBeFalse(); - }); - - it("should default 'open' to false", async () => { - await connect(); - expect(element.open).toBeFalse(); - }); - - it('should not open menu when the toggle button is clicked if the element is disabled', async () => { - element.disabled = true; - await connect(); - element.toggleButton!.control.click(); - expect(element.open).toBeFalse(); - }); - - it('should close menu when toggle button is clicked while the menu is open', async () => { - element.open = true; - await connect(); - element.toggleButton!.control.click(); - expect(element.open).toBeFalse(); - }); - - it('should open the menu and focus first menu item when the toggle button is clicked', async () => { - await connect(); - const openChangeListener = createOpenChangeListener(); - element.toggleButton!.control.click(); - expect(element.open).toBeTrue(); - await openChangeListener.promise; - expect(document.activeElement).toEqual(menuItem1); - }); - - it("should open the menu and focus first menu item when 'Enter' is pressed while the toggle button is focused", async () => { - await connect(); - const openChangeListener = createOpenChangeListener(); - const event = new KeyboardEvent('keypress', { - key: keyEnter - } as KeyboardEventInit); - element.toggleButton!.control.dispatchEvent(event); - expect(element.open).toBeTrue(); - await openChangeListener.promise; - expect(document.activeElement).toEqual(menuItem1); - }); - - it("should open the menu and focus first menu item when 'Space' is pressed while the toggle button is focused", async () => { - await connect(); - const openChangeListener = createOpenChangeListener(); - const event = new KeyboardEvent('keypress', { - key: keySpace - } as KeyboardEventInit); - element.toggleButton!.control.dispatchEvent(event); - expect(element.open).toBeTrue(); - await openChangeListener.promise; - expect(document.activeElement).toEqual(menuItem1); - }); - - it('should open the menu and focus first menu item when the down arrow is pressed while the toggle button is focused', async () => { - await connect(); - const openChangeListener = createOpenChangeListener(); - const event = new KeyboardEvent('keydown', { - key: keyArrowDown - } as KeyboardEventInit); - element.toggleButton!.dispatchEvent(event); - expect(element.open).toBeTrue(); - await openChangeListener.promise; - expect(document.activeElement).toEqual(menuItem1); - }); - - it('should open the menu and focus last menu item when the up arrow is pressed while the toggle button is focused', async () => { - await connect(); - const openChangeListener = createOpenChangeListener(); - const event = new KeyboardEvent('keydown', { - key: keyArrowUp - } as KeyboardEventInit); - element.toggleButton!.dispatchEvent(event); - expect(element.open).toBeTrue(); - await openChangeListener.promise; - expect(document.activeElement).toEqual(menuItem3); - }); - - it("should close the menu when pressing 'Escape'", async () => { - element.open = true; - await connect(); - const event = new KeyboardEvent('keydown', { - key: keyEscape - } as KeyboardEventInit); - element.region!.dispatchEvent(event); - expect(element.open).toBeFalse(); - }); - - it("should focus the button when the menu is closed by pressing 'Escape'", async () => { - element.open = true; - await connect(); - const event = new KeyboardEvent('keydown', { - key: keyEscape - } as KeyboardEventInit); - element.region!.dispatchEvent(event); - expect(document.activeElement).toEqual(element); - }); - - it('should close the menu when selecting a menu item by clicking it', async () => { - element.open = true; - await connect(); - menuItem1.click(); - expect(element.open).toBeFalse(); - }); - - it('should focus the button when the menu is closed by selecting a menu item by clicking it', async () => { - element.open = true; - await connect(); - menuItem1.click(); - expect(document.activeElement).toEqual(element); - }); - - it("should close the menu when selecting a menu item using 'Enter'", async () => { - element.open = true; - await connect(); - const event = new KeyboardEvent('keydown', { - key: keyEnter - } as KeyboardEventInit); - menuItem1.dispatchEvent(event); - expect(element.open).toBeFalse(); - }); - - it("should focus the button when the menu is closed by selecting a menu item using 'Enter'", async () => { - element.open = true; - await connect(); - const event = new KeyboardEvent('keydown', { - key: keyEnter - } as KeyboardEventInit); - menuItem1.dispatchEvent(event); - expect(document.activeElement).toEqual(element); - }); - - it("should focus the button before bubbling 'change' event on a menu item", async () => { - let menuItemChangeEventHandled = false; - const onMenuItemChange = (): void => { - expect(document.activeElement).toEqual(element); - menuItemChangeEventHandled = true; - }; - - element.open = true; - await connect(); - menuItem1.addEventListener(eventChange, onMenuItemChange); - menuItem1.click(); - expect(menuItemChangeEventHandled).toBeTrue(); - menuItem1.removeEventListener(eventChange, onMenuItemChange); - }); - - it('should not close the menu when clicking on a disabled menu item', async () => { - element.open = true; - await connect(); - menuItem1.disabled = true; - menuItem1.click(); - expect(element.open).toBeTrue(); - }); - - it('should close the menu when the element loses focus', async () => { - const focusableElement = document.createElement('input'); - parent.appendChild(focusableElement); - await connect(); - // Start with the focus on the menu button so that it can lose focus later - element.focus(); - element.open = true; - focusableElement.focus(); - expect(element.open).toBeFalse(); - }); - - it('anchored-region should not exist in DOM when the menu is closed', async () => { - await connect(); - expect( - element.shadowRoot?.querySelector('nimble-anchored-region') - ).toBeNull(); - }); - - it('anchored-region should exist in DOM when the menu is open', async () => { - element.open = true; - await connect(); - expect( - element.shadowRoot?.querySelector('nimble-anchored-region') - ).not.toBeNull(); - }); - - it("anchored-region should be configured correctly when the menu button position is configured to 'above'", async () => { - element.open = true; - element.position = MenuButtonPosition.above; - await connect(); - expect(element.region!.verticalPositioningMode).toBe('locktodefault'); - expect(element.region!.verticalDefaultPosition).toBe('top'); - }); - - it("anchored-region should be configured correctly when the menu button position is configured to 'below'", async () => { - element.open = true; - element.position = MenuButtonPosition.below; - await connect(); - expect(element.region!.verticalPositioningMode).toBe('locktodefault'); - expect(element.region!.verticalDefaultPosition).toBe('bottom'); - }); - - it("anchored-region should be configured correctly when the menu button position is configured to 'auto'", async () => { - element.open = true; - element.position = MenuButtonPosition.auto; - await connect(); - expect(element.region!.verticalPositioningMode).toBe('dynamic'); - }); - - it("should fire 'openChanged' event when the menu is opened", async () => { - await connect(); - const openChangeListener = createOpenChangeListener(); - element.open = true; - await openChangeListener.promise; - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); - }); + parentElement.appendChild(menu); + } - it("should fire 'openChanged' event when the menu is closed", async () => { - element.open = true; - await connect(); - const openChangeListener = createOpenChangeListener(); - element.open = false; - await openChangeListener.promise; - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); - }); + describe('basic functionality', () => { + let element: MenuButton; + let connect: () => Promise; + let disconnect: () => Promise; + + beforeEach(async () => { + ({ element, connect, disconnect, parent } = await setup()); + createAndSlotMenu(element); + }); + + afterEach(async () => { + await disconnect(); + }); + + it('can construct an element instance', () => { + expect(document.createElement('nimble-menu-button')).toBeInstanceOf( + MenuButton + ); + }); + + it('should disable the toggle button when the disabled is `true`', async () => { + element.disabled = true; + await connect(); + expect(element.toggleButton!.disabled).toBeTrue(); + }); + + it('should set aria-haspopup on toggle button', async () => { + await connect(); + expect(element.toggleButton!.getAttribute('aria-haspopup')).toEqual( + 'true' + ); + }); + + it('should set aria-expanded to true on the toggle button when the menu is open', async () => { + element.open = true; + await connect(); + expect(element.toggleButton!.getAttribute('aria-expanded')).toEqual( + 'true' + ); + }); + + it('should set aria-expanded to false on the toggle button when the menu is closed', async () => { + await connect(); + expect(element.toggleButton!.getAttribute('aria-expanded')).toEqual( + 'false' + ); + }); + + it('should mark the toggle button as checked when the menu is opened before connect', async () => { + element.open = true; + await connect(); + expect(element.toggleButton!.checked).toBeTrue(); + }); + + it('should mark the toggle button as checked when the menu is opened after connect', async () => { + await connect(); + element.open = true; + DOM.processUpdates(); + expect(element.toggleButton!.checked).toBeTrue(); + }); + + it('should not mark the toggle button as checked when the menu is not open', async () => { + await connect(); + expect(element.toggleButton!.checked).toBeFalse(); + }); + + it("should default 'open' to false", async () => { + await connect(); + expect(element.open).toBeFalse(); + }); + + it('should not open menu when the toggle button is clicked if the element is disabled', async () => { + element.disabled = true; + await connect(); + element.toggleButton!.control.click(); + expect(element.open).toBeFalse(); + }); + + it('should close menu when toggle button is clicked while the menu is open', async () => { + element.open = true; + await connect(); + element.toggleButton!.control.click(); + expect(element.open).toBeFalse(); + }); + + it('should not interact with form', async () => { + element.setAttribute('name', 'test'); + element.open = true; + const form = document.createElement('form'); + form.appendChild(element); + parent.appendChild(form); + + await connect(); + + const formData = new FormData(form); + expect(formData.has('test')).toBeFalse(); + }); + + it('anchored-region should not exist in DOM when the menu is closed', async () => { + await connect(); + expect( + element.shadowRoot?.querySelector('nimble-anchored-region') + ).toBeNull(); + }); + + it('anchored-region should exist in DOM when the menu is open', async () => { + element.open = true; + await connect(); + expect( + element.shadowRoot?.querySelector('nimble-anchored-region') + ).not.toBeNull(); + }); + + it("anchored-region should be configured correctly when the menu button position is configured to 'above'", async () => { + element.open = true; + element.position = MenuButtonPosition.above; + await connect(); + expect(element.region!.verticalPositioningMode).toBe( + 'locktodefault' + ); + expect(element.region!.verticalDefaultPosition).toBe('top'); + }); + + it("anchored-region should be configured correctly when the menu button position is configured to 'below'", async () => { + element.open = true; + element.position = MenuButtonPosition.below; + await connect(); + expect(element.region!.verticalPositioningMode).toBe( + 'locktodefault' + ); + expect(element.region!.verticalDefaultPosition).toBe('bottom'); + }); + + it("anchored-region should be configured correctly when the menu button position is configured to 'auto'", async () => { + element.open = true; + element.position = MenuButtonPosition.auto; + await connect(); + expect(element.region!.verticalPositioningMode).toBe('dynamic'); + }); + + it("should fire 'toggle' event when the menu is opened", async () => { + await connect(); + const toggleListener = createToggleListener(element); + element.open = true; + await toggleListener.promise; + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: true, + oldState: false + }; + const event = toggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(event.detail).toEqual(expectedDetails); + }); + + it("should fire 'toggle' event when the menu is closed", async () => { + element.open = true; + await connect(); + const toggleListener = createToggleListener(element); + element.open = false; + await toggleListener.promise; + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: false, + oldState: true + }; + const event = toggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(event.detail).toEqual(expectedDetails); + }); + + it("should fire 'beforetoggle' event before the menu opens", async () => { + await connect(); + const toggleListener = createToggleListener(element); + const beforeToggleListener = createBeforeToggleListener( + element, + false, + toggleListener.spy + ); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: true, + oldState: false + }; + + element.toggleButton!.control.click(); + await beforeToggleListener.promise; + expect(beforeToggleListener.spy).toHaveBeenCalledTimes(1); + const event = beforeToggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(event.detail).toEqual(expectedDetails); + beforeToggleListener.spy.calls.reset(); + + await toggleListener.promise; + expect(beforeToggleListener.spy).not.toHaveBeenCalled(); + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + }); + + it("should fire 'beforetoggle' event before the menu is closed", async () => { + element.open = true; + await connect(); + const toggleListener = createToggleListener(element); + const beforeToggleListener = createBeforeToggleListener( + element, + true, + toggleListener.spy + ); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: false, + oldState: true + }; + + element.toggleButton!.control.click(); + await beforeToggleListener.promise; + expect(beforeToggleListener.spy).toHaveBeenCalledTimes(1); + const event = beforeToggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(event.detail).toEqual(expectedDetails); + beforeToggleListener.spy.calls.reset(); + + await toggleListener.promise; + expect(beforeToggleListener.spy).not.toHaveBeenCalled(); + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + }); + }); + + interface MenuSlotConfiguration { + description: string; + setupFunction: () => Promise>; + getMenuButton: (element: HTMLElement) => MenuButton; + } - it('should not interact with form', async () => { - element.setAttribute('name', 'test'); - element.open = true; - const form = document.createElement('form'); - form.appendChild(element); - parent.appendChild(form); + const menuSlotConfigurations: MenuSlotConfiguration[] = [ + { + description: 'menu slotted directly in menu-button', + setupFunction: setup, + getMenuButton: (element: HTMLElement) => element as MenuButton + }, + { + description: 'menu passed through slot of additional element', + setupFunction: slottedSetup, + getMenuButton: (element: HTMLElement) => element.shadowRoot!.querySelector('nimble-menu-button')! + } + ]; + for (const configuration of menuSlotConfigurations) { + // eslint-disable-next-line @typescript-eslint/no-loop-func + describe(`menu interaction with ${configuration.description}`, () => { + let element: HTMLElement; + let connect: () => Promise; + let disconnect: () => Promise; + + async function openMenu(menuButton: MenuButton): Promise { + if (menuButton.open) { + return; + } + + const toggleListener = createToggleListener(menuButton); + menuButton.open = true; + await toggleListener.promise; + } + + beforeEach(async () => { + ({ element, connect, disconnect, parent } = await configuration.setupFunction()); + createAndSlotMenu(element); + }); + + afterEach(async () => { + await disconnect(); + }); + + it('should open the menu and focus first menu item when the toggle button is clicked', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + menuButton.toggleButton!.control.click(); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(document.activeElement).toEqual(menuItem1); + }); + + it("should open the menu and focus first menu item when 'Enter' is pressed while the toggle button is focused", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + const event = new KeyboardEvent('keypress', { + key: keyEnter + } as KeyboardEventInit); + menuButton.toggleButton!.control.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(document.activeElement).toEqual(menuItem1); + }); + + it("should open the menu and focus first menu item when 'Space' is pressed while the toggle button is focused", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + const event = new KeyboardEvent('keypress', { + key: keySpace + } as KeyboardEventInit); + menuButton.toggleButton!.control.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(document.activeElement).toEqual(menuItem1); + }); + + it('should open the menu and focus first menu item when the down arrow is pressed while the toggle button is focused', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + const event = new KeyboardEvent('keydown', { + key: keyArrowDown + } as KeyboardEventInit); + menuButton.toggleButton!.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(document.activeElement).toEqual(menuItem1); + }); + + it('should open the menu and focus last menu item when the up arrow is pressed while the toggle button is focused', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + const event = new KeyboardEvent('keydown', { + key: keyArrowUp + } as KeyboardEventInit); + menuButton.toggleButton!.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(document.activeElement).toEqual(menuItem3); + }); + + it("should close the menu when pressing 'Escape'", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + const event = new KeyboardEvent('keydown', { + key: keyEscape + } as KeyboardEventInit); + menuButton.region!.dispatchEvent(event); + expect(menuButton.open).toBeFalse(); + }); + + it("should focus the button when the menu is closed by pressing 'Escape'", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + const event = new KeyboardEvent('keydown', { + key: keyEscape + } as KeyboardEventInit); + menuButton.region!.dispatchEvent(event); + expect(document.activeElement).toEqual(element); + }); + + it('should close the menu when selecting a menu item by clicking it', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + menuItem1.click(); + expect(menuButton.open).toBeFalse(); + }); + + it('should focus the button when the menu is closed by selecting a menu item by clicking it', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + menuItem1.click(); + expect(document.activeElement).toEqual(element); + }); + + it("should close the menu when selecting a menu item using 'Enter'", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + const event = new KeyboardEvent('keydown', { + key: keyEnter + } as KeyboardEventInit); + menuItem1.dispatchEvent(event); + expect(menuButton.open).toBeFalse(); + }); + + it("should focus the button when the menu is closed by selecting a menu item using 'Enter'", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + const event = new KeyboardEvent('keydown', { + key: keyEnter + } as KeyboardEventInit); + menuItem1.dispatchEvent(event); + expect(document.activeElement).toEqual(element); + }); + + it("should focus the button before bubbling 'change' event on a menu item", async () => { + let menuItemChangeEventHandled = false; + const onMenuItemChange = (): void => { + expect(document.activeElement).toEqual(element); + menuItemChangeEventHandled = true; + }; - await connect(); + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + menuItem1.addEventListener(eventChange, onMenuItemChange); + menuItem1.click(); + expect(menuItemChangeEventHandled).toBeTrue(); + menuItem1.removeEventListener(eventChange, onMenuItemChange); + }); + + it('should not close the menu when clicking on a disabled menu item', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + menuItem1.disabled = true; + menuItem1.click(); + expect(menuButton.open).toBeTrue(); + }); + + it('should close the menu when the element loses focus', async () => { + const focusableElement = document.createElement('input'); + parent.appendChild(focusableElement); + await connect(); + const menuButton = configuration.getMenuButton(element); + // Start with the focus on the menu button so that it can lose focus later + menuButton.focus(); + menuButton.open = true; + focusableElement.focus(); + expect(menuButton.open).toBeFalse(); + }); + }); + } - const formData = new FormData(form); - expect(formData.has('test')).toBeFalse(); - }); + for (const configuration of menuSlotConfigurations) { + // eslint-disable-next-line @typescript-eslint/no-loop-func + describe(`menu interaction without a ${configuration.description}`, () => { + let element: HTMLElement; + let connect: () => Promise; + let disconnect: () => Promise; + + async function openMenu(menuButton: MenuButton): Promise { + if (menuButton.open) { + return; + } + + const toggleListener = createToggleListener(menuButton); + menuButton.open = true; + await toggleListener.promise; + } + + beforeEach(async () => { + ({ element, connect, disconnect, parent } = await configuration.setupFunction()); + // Unlike other tests, explicitly do not slot a menu in the parent element + }); + + afterEach(async () => { + await disconnect(); + }); + + it('should transition to the open state when the toggle button is clicked', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + menuButton.toggleButton!.control.click(); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: true, + oldState: false + }; + const event = toggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(event.detail).toEqual(expectedDetails); + }); + + it("should transition to the open state when 'Enter' is pressed while the toggle button is focused", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + const event = new KeyboardEvent('keypress', { + key: keyEnter + } as KeyboardEventInit); + menuButton.toggleButton!.control.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: true, + oldState: false + }; + const toggleEvent = toggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(toggleEvent.detail).toEqual(expectedDetails); + }); + + it("should transition to the open state when 'Space' is pressed while the toggle button is focused", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + const event = new KeyboardEvent('keypress', { + key: keySpace + } as KeyboardEventInit); + menuButton.toggleButton!.control.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: true, + oldState: false + }; + const toggleEvent = toggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(toggleEvent.detail).toEqual(expectedDetails); + }); + + it('should transition to the open state when the down arrow is pressed while the toggle button is focused', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + const event = new KeyboardEvent('keydown', { + key: keyArrowDown + } as KeyboardEventInit); + menuButton.toggleButton!.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: true, + oldState: false + }; + const toggleEvent = toggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(toggleEvent.detail).toEqual(expectedDetails); + }); + + it('should transition to the open state when the up arrow is pressed while the toggle button is focused', async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + const toggleListener = createToggleListener(menuButton); + const event = new KeyboardEvent('keydown', { + key: keyArrowUp + } as KeyboardEventInit); + menuButton.toggleButton!.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await toggleListener.promise; + expect(toggleListener.spy).toHaveBeenCalledTimes(1); + const expectedDetails: MenuButtonToggleEventDetail = { + newState: true, + oldState: false + }; + const toggleEvent = toggleListener.spy.calls.first() + .args[0] as CustomEvent; + expect(toggleEvent.detail).toEqual(expectedDetails); + }); + + it("should transition to the closed state when pressing 'Escape'", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + const event = new KeyboardEvent('keydown', { + key: keyEscape + } as KeyboardEventInit); + menuButton.region!.dispatchEvent(event); + expect(menuButton.open).toBeFalse(); + }); + + it("should focus the button when moving to the closed state by pressing 'Escape'", async () => { + await connect(); + const menuButton = configuration.getMenuButton(element); + await openMenu(menuButton); + + const event = new KeyboardEvent('keydown', { + key: keyEscape + } as KeyboardEventInit); + menuButton.region!.dispatchEvent(event); + expect(document.activeElement).toEqual(element); + }); + }); + } }); diff --git a/packages/nimble-components/src/menu-button/tests/menu-button.stories.ts b/packages/nimble-components/src/menu-button/tests/menu-button.stories.ts index 98e0d14b21..abfb76549f 100644 --- a/packages/nimble-components/src/menu-button/tests/menu-button.stories.ts +++ b/packages/nimble-components/src/menu-button/tests/menu-button.stories.ts @@ -38,7 +38,7 @@ const metadata: Meta = { 'https://xd.adobe.com/view/33ffad4a-eb2c-4241-b8c5-ebfff1faf6f6-66ac/screen/d022d8af-22f4-4bf2-981c-1dc0c61afece/specs' }, actions: { - handles: ['open-change'] + handles: ['toggle', 'beforetoggle'] } }, argTypes: { diff --git a/packages/nimble-components/src/menu-button/types.ts b/packages/nimble-components/src/menu-button/types.ts index 71f63241d7..be96594258 100644 --- a/packages/nimble-components/src/menu-button/types.ts +++ b/packages/nimble-components/src/menu-button/types.ts @@ -14,3 +14,12 @@ export const MenuButtonPosition = { } as const; export type MenuButtonPosition = typeof MenuButtonPosition[keyof typeof MenuButtonPosition]; + +/** + * The type of the detail associated with the `toggle` and `beforetoggle` + * events on the menu button. + */ +export interface MenuButtonToggleEventDetail { + newState: boolean; + oldState: boolean; +}