From 91a748dd72d37e8092048e8ba6961d6e456b32c5 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 10:49:10 -0600 Subject: [PATCH 01/17] add beforetoggle event --- .../src/menu-button/index.ts | 28 +++++-- .../src/menu-button/specs/README.md | 3 + .../src/menu-button/tests/menu-button.spec.ts | 76 ++++++++++++++++++- .../menu-button/tests/menu-button.stories.ts | 2 +- .../src/menu-button/types.ts | 9 +++ 5 files changed, 108 insertions(+), 10 deletions(-) diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index 0beb314b73..a7af564143 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 { MenuButtonBeforeToggleEventDetail, MenuButtonPosition } from './types'; import type { ButtonPattern } from '../patterns/button/types'; import type { AnchoredRegion } from '../anchored-region'; @@ -130,7 +130,7 @@ export class MenuButton extends FoundationElement implements ButtonPattern { const focusTarget = e.relatedTarget as HTMLElement; if (!this.contains(focusTarget)) { - this.open = false; + this.setOpen(false); return false; } @@ -138,7 +138,7 @@ 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. e.stopPropagation(); @@ -149,10 +149,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 +162,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,6 +170,20 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } } + private setOpen(newValue: boolean): void { + if (this.open === newValue) { + return; + } + + const eventDetail: MenuButtonBeforeToggleEventDetail = { + oldState: this.open, + newState: newValue + }; + this.$emit('beforetoggle', eventDetail); + + this.open = newValue; + } + private get menu(): HTMLElement | undefined { return this.slottedMenus?.length ? this.slottedMenus[0] : undefined; } @@ -187,7 +201,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..c6ec4bd790 100644 --- a/packages/nimble-components/src/menu-button/specs/README.md +++ b/packages/nimble-components/src/menu-button/specs/README.md @@ -84,6 +84,9 @@ _Methods_ _Events_ +- `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. - `open-change` (event) - event for when the opened state has changed _CSS Classes and CSS Custom Properties that affect the component_ 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..cd7ae6a5ec 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 @@ -10,7 +10,7 @@ import { import type { Menu, MenuItem } from '@microsoft/fast-foundation'; import { fixture, Fixture } from '../../utilities/tests/fixture'; import { MenuButton } from '..'; -import { MenuButtonPosition } from '../types'; +import { MenuButtonBeforeToggleEventDetail, MenuButtonPosition } from '../types'; async function setup(): Promise> { return fixture(html``); @@ -26,7 +26,7 @@ describe('MenuButton', () => { let menuItem2: MenuItem; let menuItem3: MenuItem; - /** A helper function to abstract adding an 'open-change' event listener, spying + /** 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. */ @@ -48,6 +48,35 @@ describe('MenuButton', () => { }; } + /** 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 + * `openChangedSpy` has not been called. + */ + function createBeforeToggleListener(expectedOpenState: boolean, openChangedSpy: jasmine.Spy): { + promise: Promise, + spy: jasmine.Spy + } { + const spy = jasmine.createSpy(); + return { + promise: new Promise(resolve => { + const handler = (...args: unknown[]): void => { + expect(element.open).toEqual(expectedOpenState); + expect(openChangedSpy).not.toHaveBeenCalled(); + + element.removeEventListener('beforetoggle', handler); + spy(...args); + resolve(); + }; + element.addEventListener('beforetoggle', handler); + }), + spy + }; + } + beforeEach(async () => { ({ element, connect, disconnect, parent } = await setup()); @@ -344,6 +373,49 @@ describe('MenuButton', () => { expect(openChangeListener.spy).toHaveBeenCalledTimes(1); }); + it("should fire 'beforetoggle' event before the menu opens", async () => { + await connect(); + const openChangeListener = createOpenChangeListener(); + const beforeToggleListener = createBeforeToggleListener(false, openChangeListener.spy); + const expectedDetails: MenuButtonBeforeToggleEventDetail = { + 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 openChangeListener.promise; + expect(beforeToggleListener.spy).not.toHaveBeenCalled(); + expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + }); + + it("should fire 'beforetoggle' event before the menu is closed", async () => { + element.open = true; + await connect(); + const openChangeListener = createOpenChangeListener(); + const beforeToggleListener = createBeforeToggleListener(true, openChangeListener.spy); + const expectedDetails: MenuButtonBeforeToggleEventDetail = { + 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 openChangeListener.promise; + expect(beforeToggleListener.spy).not.toHaveBeenCalled(); + expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + }); + it('should not interact with form', async () => { element.setAttribute('name', 'test'); element.open = true; 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..d57f646af6 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: ['open-change', 'beforetoggle'] } }, argTypes: { diff --git a/packages/nimble-components/src/menu-button/types.ts b/packages/nimble-components/src/menu-button/types.ts index 71f63241d7..43c28ddbb8 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 `beforetoggle` CustomEvent + * on the menu button. + */ +export interface MenuButtonBeforeToggleEventDetail { + newState: boolean; + oldState: boolean; +} From 23360c5ec2ade924bbd56207493154720750054f Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 11:58:56 -0600 Subject: [PATCH 02/17] partial - nested slotted menus --- .../src/menu-button/index.ts | 24 ++- .../src/menu-button/template.ts | 4 +- .../src/menu-button/tests/menu-button.spec.ts | 200 ++++++++++++------ 3 files changed, 162 insertions(+), 66 deletions(-) diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index a7af564143..48b6c90e96 100644 --- a/packages/nimble-components/src/menu-button/index.ts +++ b/packages/nimble-components/src/menu-button/index.ts @@ -129,7 +129,7 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } const focusTarget = e.relatedTarget as HTMLElement; - if (!this.contains(focusTarget)) { + if (!this.contains(focusTarget) && !this.menu?.contains(focusTarget)) { this.setOpen(false); return false; } @@ -185,7 +185,27 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } private get menu(): HTMLElement | undefined { - return this.slottedMenus?.length ? this.slottedMenus[0] : 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 = this.slottedMenus[0]; + while (currentItem) { + if (currentItem.getAttribute('role') === 'menu') { + return currentItem; + } + + if (currentItem?.nodeName === 'SLOT') { + currentItem = (currentItem as HTMLSlotElement).assignedNodes()[0] as HTMLElement; + } else { + return undefined; + } + } + + return undefined; } private focusMenu(): void { 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 cd7ae6a5ec..f80bbb3c1b 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,18 +7,83 @@ 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 { MenuButtonBeforeToggleEventDetail, MenuButtonPosition } from '../types'; +class TestSlottedElement extends FoundationElement {} +const foo = TestSlottedElement.compose({ + baseName: 'test-slotted-element', + template: html` + + + + ` +}); + async function setup(): Promise> { return fixture(html``); } -describe('MenuButton', () => { +async function slottedSetup(): Promise> { + return fixture(foo()); +} + +/** 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(menuButton: MenuButton): { + promise: Promise, + spy: jasmine.Spy +} { + const spy = jasmine.createSpy(); + return { + promise: new Promise(resolve => { + const handler = (...args: unknown[]): void => { + menuButton.removeEventListener('open-change', handler); + spy(...args); + resolve(); + }; + menuButton.addEventListener('open-change', 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 + * `openChangedSpy` has not been called. + */ +function createBeforeToggleListener(menuButton: MenuButton, expectedOpenState: boolean, openChangedSpy: 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(openChangedSpy).not.toHaveBeenCalled(); + + menuButton.removeEventListener('beforetoggle', handler); + spy(...args); + resolve(); + }; + menuButton.addEventListener('beforetoggle', handler); + }), + spy + }; +} + +fdescribe('MenuButton2', () => { let parent: HTMLElement; - let element: MenuButton; + let element: TestSlottedElement; let connect: () => Promise; let disconnect: () => Promise; let menu: Menu; @@ -26,56 +91,67 @@ describe('MenuButton', () => { 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 - }; + function getMenuButton(): MenuButton { + return element.shadowRoot!.querySelector('nimble-menu-button')!; } - /** 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 - * `openChangedSpy` has not been called. - */ - function createBeforeToggleListener(expectedOpenState: boolean, openChangedSpy: jasmine.Spy): { - promise: Promise, - spy: jasmine.Spy - } { - const spy = jasmine.createSpy(); - return { - promise: new Promise(resolve => { - const handler = (...args: unknown[]): void => { - expect(element.open).toEqual(expectedOpenState); - expect(openChangedSpy).not.toHaveBeenCalled(); - - element.removeEventListener('beforetoggle', handler); - spy(...args); - resolve(); - }; - element.addEventListener('beforetoggle', handler); - }), - spy - }; - } + beforeEach(async () => { + ({ element, connect, disconnect, parent } = await slottedSetup()); + + menu = document.createElement('nimble-menu'); + menu.slot = 'test-menu'; + + menuItem1 = document.createElement('nimble-menu-item'); + menuItem1.textContent = 'menu item 1'; + menu.appendChild(menuItem1); + + menuItem2 = document.createElement('nimble-menu-item'); + menuItem2.textContent = 'menu item 2'; + menu.appendChild(menuItem2); + + menuItem3 = document.createElement('nimble-menu-item'); + menuItem3.textContent = 'menu item 3'; + menu.appendChild(menuItem3); + + element.appendChild(menu); + }); + + 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 = getMenuButton(); + const openChangeListener = createOpenChangeListener(menuButton); + menuButton.toggleButton!.control.click(); + expect(menuButton.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(element); + 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); + }); +}); + +fdescribe('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; beforeEach(async () => { ({ element, connect, disconnect, parent } = await setup()); @@ -175,7 +251,7 @@ describe('MenuButton', () => { it('should open the menu and focus first menu item when the toggle button is clicked', async () => { await connect(); - const openChangeListener = createOpenChangeListener(); + const openChangeListener = createOpenChangeListener(element); element.toggleButton!.control.click(); expect(element.open).toBeTrue(); await openChangeListener.promise; @@ -184,7 +260,7 @@ describe('MenuButton', () => { 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 openChangeListener = createOpenChangeListener(element); const event = new KeyboardEvent('keypress', { key: keyEnter } as KeyboardEventInit); @@ -196,7 +272,7 @@ describe('MenuButton', () => { 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 openChangeListener = createOpenChangeListener(element); const event = new KeyboardEvent('keypress', { key: keySpace } as KeyboardEventInit); @@ -208,7 +284,7 @@ describe('MenuButton', () => { 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 openChangeListener = createOpenChangeListener(element); const event = new KeyboardEvent('keydown', { key: keyArrowDown } as KeyboardEventInit); @@ -220,7 +296,7 @@ describe('MenuButton', () => { 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 openChangeListener = createOpenChangeListener(element); const event = new KeyboardEvent('keydown', { key: keyArrowUp } as KeyboardEventInit); @@ -358,7 +434,7 @@ describe('MenuButton', () => { it("should fire 'openChanged' event when the menu is opened", async () => { await connect(); - const openChangeListener = createOpenChangeListener(); + const openChangeListener = createOpenChangeListener(element); element.open = true; await openChangeListener.promise; expect(openChangeListener.spy).toHaveBeenCalledTimes(1); @@ -367,7 +443,7 @@ describe('MenuButton', () => { it("should fire 'openChanged' event when the menu is closed", async () => { element.open = true; await connect(); - const openChangeListener = createOpenChangeListener(); + const openChangeListener = createOpenChangeListener(element); element.open = false; await openChangeListener.promise; expect(openChangeListener.spy).toHaveBeenCalledTimes(1); @@ -375,8 +451,8 @@ describe('MenuButton', () => { it("should fire 'beforetoggle' event before the menu opens", async () => { await connect(); - const openChangeListener = createOpenChangeListener(); - const beforeToggleListener = createBeforeToggleListener(false, openChangeListener.spy); + const openChangeListener = createOpenChangeListener(element); + const beforeToggleListener = createBeforeToggleListener(element, false, openChangeListener.spy); const expectedDetails: MenuButtonBeforeToggleEventDetail = { newState: true, oldState: false @@ -397,8 +473,8 @@ describe('MenuButton', () => { it("should fire 'beforetoggle' event before the menu is closed", async () => { element.open = true; await connect(); - const openChangeListener = createOpenChangeListener(); - const beforeToggleListener = createBeforeToggleListener(true, openChangeListener.spy); + const openChangeListener = createOpenChangeListener(element); + const beforeToggleListener = createBeforeToggleListener(element, true, openChangeListener.spy); const expectedDetails: MenuButtonBeforeToggleEventDetail = { newState: false, oldState: true From 25fac7424c0ccae88b7f37034fcc9efeac65cfa1 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 13:07:39 -0600 Subject: [PATCH 03/17] tests for nested slotted menus --- .../src/menu-button/tests/menu-button.spec.ts | 804 +++++++++--------- 1 file changed, 408 insertions(+), 396 deletions(-) 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 f80bbb3c1b..f0c17859de 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 @@ -17,7 +17,7 @@ const foo = TestSlottedElement.compose({ baseName: 'test-slotted-element', template: html` - + ` }); @@ -81,81 +81,14 @@ function createBeforeToggleListener(menuButton: MenuButton, expectedOpenState: b }; } -fdescribe('MenuButton2', () => { +describe('MenuButton', () => { let parent: HTMLElement; - let element: TestSlottedElement; - let connect: () => Promise; - let disconnect: () => Promise; let menu: Menu; let menuItem1: MenuItem; let menuItem2: MenuItem; let menuItem3: MenuItem; - function getMenuButton(): MenuButton { - return element.shadowRoot!.querySelector('nimble-menu-button')!; - } - - beforeEach(async () => { - ({ element, connect, disconnect, parent } = await slottedSetup()); - - menu = document.createElement('nimble-menu'); - menu.slot = 'test-menu'; - - menuItem1 = document.createElement('nimble-menu-item'); - menuItem1.textContent = 'menu item 1'; - menu.appendChild(menuItem1); - - menuItem2 = document.createElement('nimble-menu-item'); - menuItem2.textContent = 'menu item 2'; - menu.appendChild(menuItem2); - - menuItem3 = document.createElement('nimble-menu-item'); - menuItem3.textContent = 'menu item 3'; - menu.appendChild(menuItem3); - - element.appendChild(menu); - }); - - 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 = getMenuButton(); - const openChangeListener = createOpenChangeListener(menuButton); - menuButton.toggleButton!.control.click(); - expect(menuButton.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(element); - 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); - }); -}); - -fdescribe('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; - - beforeEach(async () => { - ({ element, connect, disconnect, parent } = await setup()); - + function createAndSlotMenu(parentElement: HTMLElement): void { menu = document.createElement('nimble-menu'); menu.slot = 'menu'; @@ -171,337 +104,416 @@ fdescribe('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); - 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(element); - 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(element); - 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(element); - 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(element); - 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'); - }); + parentElement.appendChild(menu); + } - it("should fire 'openChanged' event when the menu is opened", async () => { - await connect(); - const openChangeListener = createOpenChangeListener(element); - element.open = true; - 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 'openChanged' event when the menu is opened", async () => { + await connect(); + const openChangeListener = createOpenChangeListener(element); + element.open = true; + await openChangeListener.promise; + expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + }); + + it("should fire 'openChanged' event when the menu is closed", async () => { + element.open = true; + await connect(); + const openChangeListener = createOpenChangeListener(element); + element.open = false; + await openChangeListener.promise; + expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + }); + + it("should fire 'beforetoggle' event before the menu opens", async () => { + await connect(); + const openChangeListener = createOpenChangeListener(element); + const beforeToggleListener = createBeforeToggleListener(element, false, openChangeListener.spy); + const expectedDetails: MenuButtonBeforeToggleEventDetail = { + newState: true, + oldState: false + }; - it("should fire 'openChanged' event when the menu is closed", async () => { - element.open = true; - await connect(); - const openChangeListener = createOpenChangeListener(element); - element.open = false; - await openChangeListener.promise; - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); - }); + 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 openChangeListener.promise; + expect(beforeToggleListener.spy).not.toHaveBeenCalled(); + expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + }); + + it("should fire 'beforetoggle' event before the menu is closed", async () => { + element.open = true; + await connect(); + const openChangeListener = createOpenChangeListener(element); + const beforeToggleListener = createBeforeToggleListener(element, true, openChangeListener.spy); + const expectedDetails: MenuButtonBeforeToggleEventDetail = { + newState: false, + oldState: true + }; - it("should fire 'beforetoggle' event before the menu opens", async () => { - await connect(); - const openChangeListener = createOpenChangeListener(element); - const beforeToggleListener = createBeforeToggleListener(element, false, openChangeListener.spy); - const expectedDetails: MenuButtonBeforeToggleEventDetail = { - 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 openChangeListener.promise; - expect(beforeToggleListener.spy).not.toHaveBeenCalled(); - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); - }); + 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(); - it("should fire 'beforetoggle' event before the menu is closed", async () => { - element.open = true; - await connect(); - const openChangeListener = createOpenChangeListener(element); - const beforeToggleListener = createBeforeToggleListener(element, true, openChangeListener.spy); - const expectedDetails: MenuButtonBeforeToggleEventDetail = { - 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 openChangeListener.promise; - expect(beforeToggleListener.spy).not.toHaveBeenCalled(); - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + await openChangeListener.promise; + expect(beforeToggleListener.spy).not.toHaveBeenCalled(); + expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + }); }); - 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(); + interface MenuSlotConfiguration { + description: string; + setupFunction: () => Promise>; + getMenuButton: (element: HTMLElement) => MenuButton; + } - const formData = new FormData(form); - expect(formData.has('test')).toBeFalse(); - }); + const menuSlotConfigurations: MenuSlotConfiguration[] = [ + { + description: 'directly in menu-button', + setupFunction: setup, + getMenuButton: (element: HTMLElement) => element as MenuButton + }, + { + description: '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 menu ${configuration.description}`, () => { + let element: HTMLElement; + let connect: () => Promise; + let disconnect: () => Promise; + + async function openMenu(menuButton: MenuButton): Promise { + if (menuButton.open) { + return; + } + + const openChangeListener = createOpenChangeListener(menuButton); + menuButton.open = true; + await openChangeListener.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 openChangeListener = createOpenChangeListener(menuButton); + menuButton.toggleButton!.control.click(); + expect(menuButton.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 menuButton = configuration.getMenuButton(element); + const openChangeListener = createOpenChangeListener(menuButton); + const event = new KeyboardEvent('keypress', { + key: keyEnter + } as KeyboardEventInit); + menuButton.toggleButton!.control.dispatchEvent(event); + expect(menuButton.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 menuButton = configuration.getMenuButton(element); + const openChangeListener = createOpenChangeListener(menuButton); + const event = new KeyboardEvent('keypress', { + key: keySpace + } as KeyboardEventInit); + menuButton.toggleButton!.control.dispatchEvent(event); + expect(menuButton.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 menuButton = configuration.getMenuButton(element); + const openChangeListener = createOpenChangeListener(menuButton); + const event = new KeyboardEvent('keydown', { + key: keyArrowDown + } as KeyboardEventInit); + menuButton.toggleButton!.dispatchEvent(event); + expect(menuButton.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 menuButton = configuration.getMenuButton(element); + const openChangeListener = createOpenChangeListener(menuButton); + const event = new KeyboardEvent('keydown', { + key: keyArrowUp + } as KeyboardEventInit); + menuButton.toggleButton!.dispatchEvent(event); + expect(menuButton.open).toBeTrue(); + await openChangeListener.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(); + 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(); + }); + }); + } }); From 0fbdac634175a75b5ade7ca574969ea5c176f9c0 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 13:09:52 -0600 Subject: [PATCH 04/17] format --- .../src/menu-button/index.ts | 4 +- .../src/menu-button/tests/menu-button.spec.ts | 43 ++++++++++++++----- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index 48b6c90e96..0edfdb3b0e 100644 --- a/packages/nimble-components/src/menu-button/index.ts +++ b/packages/nimble-components/src/menu-button/index.ts @@ -199,7 +199,9 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } if (currentItem?.nodeName === 'SLOT') { - currentItem = (currentItem as HTMLSlotElement).assignedNodes()[0] as HTMLElement; + currentItem = ( + currentItem as HTMLSlotElement + ).assignedNodes()[0] as HTMLElement; } else { return undefined; } 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 f0c17859de..e8b02060e0 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 @@ -10,7 +10,10 @@ import { import { FoundationElement, Menu, MenuItem } from '@microsoft/fast-foundation'; import { fixture, Fixture } from '../../utilities/tests/fixture'; import { MenuButton } from '..'; -import { MenuButtonBeforeToggleEventDetail, MenuButtonPosition } from '../types'; +import { + MenuButtonBeforeToggleEventDetail, + MenuButtonPosition +} from '../types'; class TestSlottedElement extends FoundationElement {} const foo = TestSlottedElement.compose({ @@ -60,10 +63,14 @@ function createOpenChangeListener(menuButton: MenuButton): { * `beforetoggle` is fired and that when the `beforetoggle` event is fired, the * `openChangedSpy` has not been called. */ -function createBeforeToggleListener(menuButton: MenuButton, expectedOpenState: boolean, openChangedSpy: jasmine.Spy): { - promise: Promise, - spy: jasmine.Spy -} { +function createBeforeToggleListener( + menuButton: MenuButton, + expectedOpenState: boolean, + openChangedSpy: jasmine.Spy +): { + promise: Promise, + spy: jasmine.Spy + } { const spy = jasmine.createSpy(); return { promise: new Promise(resolve => { @@ -224,7 +231,9 @@ describe('MenuButton', () => { element.open = true; element.position = MenuButtonPosition.above; await connect(); - expect(element.region!.verticalPositioningMode).toBe('locktodefault'); + expect(element.region!.verticalPositioningMode).toBe( + 'locktodefault' + ); expect(element.region!.verticalDefaultPosition).toBe('top'); }); @@ -232,7 +241,9 @@ describe('MenuButton', () => { element.open = true; element.position = MenuButtonPosition.below; await connect(); - expect(element.region!.verticalPositioningMode).toBe('locktodefault'); + expect(element.region!.verticalPositioningMode).toBe( + 'locktodefault' + ); expect(element.region!.verticalDefaultPosition).toBe('bottom'); }); @@ -263,7 +274,11 @@ describe('MenuButton', () => { it("should fire 'beforetoggle' event before the menu opens", async () => { await connect(); const openChangeListener = createOpenChangeListener(element); - const beforeToggleListener = createBeforeToggleListener(element, false, openChangeListener.spy); + const beforeToggleListener = createBeforeToggleListener( + element, + false, + openChangeListener.spy + ); const expectedDetails: MenuButtonBeforeToggleEventDetail = { newState: true, oldState: false @@ -272,7 +287,8 @@ describe('MenuButton', () => { element.toggleButton!.control.click(); await beforeToggleListener.promise; expect(beforeToggleListener.spy).toHaveBeenCalledTimes(1); - const event = beforeToggleListener.spy.calls.first().args[0] as CustomEvent; + const event = beforeToggleListener.spy.calls.first() + .args[0] as CustomEvent; expect(event.detail).toEqual(expectedDetails); beforeToggleListener.spy.calls.reset(); @@ -285,7 +301,11 @@ describe('MenuButton', () => { element.open = true; await connect(); const openChangeListener = createOpenChangeListener(element); - const beforeToggleListener = createBeforeToggleListener(element, true, openChangeListener.spy); + const beforeToggleListener = createBeforeToggleListener( + element, + true, + openChangeListener.spy + ); const expectedDetails: MenuButtonBeforeToggleEventDetail = { newState: false, oldState: true @@ -294,7 +314,8 @@ describe('MenuButton', () => { element.toggleButton!.control.click(); await beforeToggleListener.promise; expect(beforeToggleListener.spy).toHaveBeenCalledTimes(1); - const event = beforeToggleListener.spy.calls.first().args[0] as CustomEvent; + const event = beforeToggleListener.spy.calls.first() + .args[0] as CustomEvent; expect(event.detail).toEqual(expectedDetails); beforeToggleListener.spy.calls.reset(); From 443e4896d2ed76a6346ef42368caf30af93d44b0 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 14:45:26 -0600 Subject: [PATCH 05/17] Angular support for beforetoggle event --- .../menu-button/nimble-menu-button.directive.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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..e8128c5250 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 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, MenuButtonBeforeToggleEventDetail } from '@ni/nimble-components/dist/esm/menu-button/types'; import { BooleanValueOrAttribute, toBooleanProperty } from '../utilities/template-value-helpers'; export type { MenuButton }; +export type { MenuButtonBeforeToggleEventDetail }; /** * Directive to provide Angular integration for the menu button. @@ -48,6 +49,8 @@ export class NimbleMenuButtonDirective { @Output() public openChange = new EventEmitter(); + @Output() public beforeToggle = new EventEmitter(); + public constructor(private readonly renderer: Renderer2, private readonly elementRef: ElementRef) {} @HostListener('open-change', ['$event']) @@ -56,4 +59,9 @@ export class NimbleMenuButtonDirective { this.openChange.emit(this.open); } } + + @HostListener('beforetoggle', ['$event']) + public onBeforeToggle($event: CustomEvent): void { + this.beforeToggle.emit($event.detail as MenuButtonBeforeToggleEventDetail); + } } From a64706f8459df64c4561ed2c99e1db3b96d60003 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 15:04:55 -0600 Subject: [PATCH 06/17] Blazor support for beforetoggle event --- .../Components/NimbleMenuButton.razor | 1 + .../Components/NimbleMenuButton.razor.cs | 15 +++++++++++++++ .../nimble-blazor/NimbleBlazor/EventHandlers.cs | 9 ++++++++- .../wwwroot/NimbleBlazor.lib.module.js | 10 ++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor index 5c93b1d2d9..99a3fc3987 100644 --- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor +++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor @@ -3,6 +3,7 @@ OpenChanged { 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. /// @@ -62,6 +68,15 @@ protected async void UpdateOpen(bool? value) await OpenChanged.InvokeAsync(value); } + /// + /// Called when the 'beforetoggle' event is fired on the web component + /// + /// The state of the menu button + protected async void HandleBeforeToggle(MenuButtonBeforeToggleEventArgs eventArgs) + { + await BeforeToggle.InvokeAsync(eventArgs); + } + /// /// Gets or sets the additional attributes on the . /// diff --git a/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs b/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs index b77c5fe636..af68b43b3d 100644 --- a/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs +++ b/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs @@ -18,9 +18,16 @@ public class MenuButtonOpenChangeEventArgs : EventArgs public bool Open { get; set; } } +public class MenuButtonBeforeToggleEventArgs : EventArgs +{ + 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("onnimblemenubuttonopenchange", typeof(MenuButtonOpenChangeEventArgs), enableStopPropagation: true, enablePreventDefault: false)] +[EventHandler("onnimblemenubuttonbeforetoggle", typeof(MenuButtonBeforeToggleEventArgs), 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..6b9e82ae71 100644 --- a/packages/nimble-blazor/NimbleBlazor/wwwroot/NimbleBlazor.lib.module.js +++ b/packages/nimble-blazor/NimbleBlazor/wwwroot/NimbleBlazor.lib.module.js @@ -39,6 +39,16 @@ export function afterStarted(Blazor) { }; } }); + // Used by NimbleMenuButton.razor + Blazor.registerCustomEventType('nimblemenubuttonbeforetoggle', { + browserEventName: 'beforetoggle', + createEventArgs: event => { + return { + newState: event.detail.newState, + oldState: event.detail.oldState + }; + } + }); } window.NimbleBlazor = { From 53123ed25be536ac87224141615fe79cfe86ad06 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 15:10:10 -0600 Subject: [PATCH 07/17] Change files --- ...imble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json | 7 +++++++ ...nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json | 7 +++++++ ...le-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json | 7 +++++++ 3 files changed, 21 insertions(+) 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/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..88c1a6c2ae --- /dev/null +++ b/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add beforeToggle event on menu button", + "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..9b18263a1f --- /dev/null +++ b/change/@ni-nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add beforetoggle event on menu button", + "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..51606ffa63 --- /dev/null +++ b/change/@ni-nimble-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add beforetoggle event on menu button and update 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" +} From 91697862a8882b0424bbfd2ae50c2aa72e1ad7ee Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 15:12:32 -0600 Subject: [PATCH 08/17] Update @ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json --- ...@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json b/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json index 88c1a6c2ae..5a8278303b 100644 --- a/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json +++ b/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json @@ -1,6 +1,6 @@ { "type": "minor", - "comment": "Add beforeToggle event on menu button", + "comment": "Add beforetoggle event on menu button", "packageName": "@ni/nimble-angular", "email": "20542556+mollykreis@users.noreply.github.com", "dependentChangeType": "patch" From 36bd9c53846b8f025c11b36205ed0e9377cace2e Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 20 Jan 2023 15:25:23 -0600 Subject: [PATCH 09/17] Update menu-button.spec.ts --- .../src/menu-button/tests/menu-button.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 e8b02060e0..c44ebca1dc 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 @@ -16,7 +16,7 @@ import { } from '../types'; class TestSlottedElement extends FoundationElement {} -const foo = TestSlottedElement.compose({ +const composedTestSlottedElement = TestSlottedElement.compose({ baseName: 'test-slotted-element', template: html` @@ -30,7 +30,7 @@ async function setup(): Promise> { } async function slottedSetup(): Promise> { - return fixture(foo()); + return fixture(composedTestSlottedElement()); } /** A helper function to abstract adding an `open-change` event listener, spying From d7ab1e318be0599d080b72b86cd74f3580af3294 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 24 Jan 2023 08:53:04 -0600 Subject: [PATCH 10/17] rename 'open-change' event to 'toggle' --- .../nimble-menu-button.directive.ts | 18 ++-- .../Components/NimbleMenuButton.razor | 2 +- .../Components/NimbleMenuButton.razor.cs | 12 +-- .../NimbleBlazor/EventHandlers.cs | 11 +-- .../wwwroot/NimbleBlazor.lib.module.js | 7 +- .../src/menu-button/index.ts | 18 ++-- .../src/menu-button/specs/README.md | 4 +- .../src/menu-button/tests/menu-button.spec.ts | 90 +++++++++++-------- .../menu-button/tests/menu-button.stories.ts | 2 +- .../src/menu-button/types.ts | 6 +- 10 files changed, 94 insertions(+), 76 deletions(-) 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 e8128c5250..a87421e341 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,10 +1,10 @@ import { Directive, ElementRef, EventEmitter, HostListener, Input, Output, Renderer2 } from '@angular/core'; import type { MenuButton } from '@ni/nimble-components/dist/esm/menu-button'; -import type { ButtonAppearance, MenuButtonBeforeToggleEventDetail } 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 { MenuButtonBeforeToggleEventDetail }; +export type { MenuButtonToggleEventDetail }; /** * Directive to provide Angular integration for the menu button. @@ -47,21 +47,19 @@ export class NimbleMenuButtonDirective { this.renderer.setProperty(this.elementRef.nativeElement, 'open', toBooleanProperty(value)); } - @Output() public openChange = new EventEmitter(); + @Output() public toggle = new EventEmitter(); - @Output() public beforeToggle = new EventEmitter(); + @Output() public beforeToggle = 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); - } + @HostListener('toggle', ['$event']) + public onToggle($event: CustomEvent): void { + this.toggle.emit($event.detail as MenuButtonToggleEventDetail); } @HostListener('beforetoggle', ['$event']) public onBeforeToggle($event: CustomEvent): void { - this.beforeToggle.emit($event.detail as MenuButtonBeforeToggleEventDetail); + this.beforeToggle.emit($event.detail as MenuButtonToggleEventDetail); } } diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor index 99a3fc3987..05fe882a8d 100644 --- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor +++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor @@ -2,7 +2,7 @@ @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; } + public EventCallback BeforeToggle { get; set; } /// /// Called when 'open' changes on the web component. /// /// New value of open - protected async void UpdateOpen(bool? value) + protected async void UpdateOpen(MenuButtonToggleEventArgs eventArgs) { - Open = value; - await OpenChanged.InvokeAsync(value); + 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(MenuButtonBeforeToggleEventArgs eventArgs) + protected async void HandleBeforeToggle(MenuButtonToggleEventArgs eventArgs) { await BeforeToggle.InvokeAsync(eventArgs); } diff --git a/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs b/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs index af68b43b3d..f675d2f70d 100644 --- a/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs +++ b/packages/nimble-blazor/NimbleBlazor/EventHandlers.cs @@ -13,12 +13,7 @@ public class CheckboxChangeEventArgs : EventArgs public bool Checked { get; set; } } -public class MenuButtonOpenChangeEventArgs : EventArgs -{ - public bool Open { get; set; } -} - -public class MenuButtonBeforeToggleEventArgs : EventArgs +public class MenuButtonToggleEventArgs : EventArgs { public bool NewState { get; set; } public bool OldState { get; set; } @@ -26,8 +21,8 @@ public class MenuButtonBeforeToggleEventArgs : EventArgs [EventHandler("onnimbletabsactiveidchange", typeof(TabsChangeEventArgs), enableStopPropagation: true, enablePreventDefault: true)] [EventHandler("onnimblecheckedchange", typeof(CheckboxChangeEventArgs), enableStopPropagation: true, enablePreventDefault: true)] -[EventHandler("onnimblemenubuttonopenchange", typeof(MenuButtonOpenChangeEventArgs), enableStopPropagation: true, enablePreventDefault: false)] -[EventHandler("onnimblemenubuttonbeforetoggle", typeof(MenuButtonBeforeToggleEventArgs), enableStopPropagation: true, enablePreventDefault: false)] +[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 6b9e82ae71..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,12 @@ 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 }; } }); diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index 0edfdb3b0e..8c950728c7 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 { MenuButtonBeforeToggleEventDetail, 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 { @@ -140,7 +148,7 @@ export class MenuButton extends FoundationElement implements ButtonPattern { public toggleButtonCheckedChangeHandler(e: Event): boolean { 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; } @@ -175,7 +183,7 @@ export class MenuButton extends FoundationElement implements ButtonPattern { return; } - const eventDetail: MenuButtonBeforeToggleEventDetail = { + const eventDetail: MenuButtonToggleEventDetail = { oldState: this.open, newState: newValue }; diff --git a/packages/nimble-components/src/menu-button/specs/README.md b/packages/nimble-components/src/menu-button/specs/README.md index c6ec4bd790..8af7ddbaa3 100644 --- a/packages/nimble-components/src/menu-button/specs/README.md +++ b/packages/nimble-components/src/menu-button/specs/README.md @@ -87,7 +87,9 @@ _Events_ - `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. -- `open-change` (event) - event for when the opened state has changed +- `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/tests/menu-button.spec.ts b/packages/nimble-components/src/menu-button/tests/menu-button.spec.ts index c44ebca1dc..93e08b9032 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 @@ -11,7 +11,7 @@ import { FoundationElement, Menu, MenuItem } from '@microsoft/fast-foundation'; import { fixture, Fixture } from '../../utilities/tests/fixture'; import { MenuButton } from '..'; import { - MenuButtonBeforeToggleEventDetail, + MenuButtonToggleEventDetail, MenuButtonPosition } from '../types'; @@ -33,11 +33,11 @@ async function slottedSetup(): Promise> { return fixture(composedTestSlottedElement()); } -/** A helper function to abstract adding an `open-change` event listener, spying +/** 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 createOpenChangeListener(menuButton: MenuButton): { +function createToggleListener(menuButton: MenuButton): { promise: Promise, spy: jasmine.Spy } { @@ -45,11 +45,11 @@ function createOpenChangeListener(menuButton: MenuButton): { return { promise: new Promise(resolve => { const handler = (...args: unknown[]): void => { - menuButton.removeEventListener('open-change', handler); + menuButton.removeEventListener('toggle', handler); spy(...args); resolve(); }; - menuButton.addEventListener('open-change', handler); + menuButton.addEventListener('toggle', handler); }), spy }; @@ -61,12 +61,12 @@ function createOpenChangeListener(menuButton: MenuButton): { * * 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 - * `openChangedSpy` has not been called. + * `toggleSpy` has not been called. */ function createBeforeToggleListener( menuButton: MenuButton, expectedOpenState: boolean, - openChangedSpy: jasmine.Spy + toggleSpy: jasmine.Spy ): { promise: Promise, spy: jasmine.Spy @@ -76,7 +76,7 @@ function createBeforeToggleListener( promise: new Promise(resolve => { const handler = (...args: unknown[]): void => { expect(menuButton.open).toEqual(expectedOpenState); - expect(openChangedSpy).not.toHaveBeenCalled(); + expect(toggleSpy).not.toHaveBeenCalled(); menuButton.removeEventListener('beforetoggle', handler); spy(...args); @@ -254,32 +254,46 @@ describe('MenuButton', () => { expect(element.region!.verticalPositioningMode).toBe('dynamic'); }); - it("should fire 'openChanged' event when the menu is opened", async () => { + it("should fire 'toggle' event when the menu is opened", async () => { await connect(); - const openChangeListener = createOpenChangeListener(element); + const toggleListener = createToggleListener(element); element.open = true; - await openChangeListener.promise; - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + 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 'openChanged' event when the menu is closed", async () => { + it("should fire 'toggle' event when the menu is closed", async () => { element.open = true; await connect(); - const openChangeListener = createOpenChangeListener(element); + const toggleListener = createToggleListener(element); element.open = false; - await openChangeListener.promise; - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + 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 openChangeListener = createOpenChangeListener(element); + const toggleListener = createToggleListener(element); const beforeToggleListener = createBeforeToggleListener( element, false, - openChangeListener.spy + toggleListener.spy ); - const expectedDetails: MenuButtonBeforeToggleEventDetail = { + const expectedDetails: MenuButtonToggleEventDetail = { newState: true, oldState: false }; @@ -292,21 +306,21 @@ describe('MenuButton', () => { expect(event.detail).toEqual(expectedDetails); beforeToggleListener.spy.calls.reset(); - await openChangeListener.promise; + await toggleListener.promise; expect(beforeToggleListener.spy).not.toHaveBeenCalled(); - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + expect(toggleListener.spy).toHaveBeenCalledTimes(1); }); it("should fire 'beforetoggle' event before the menu is closed", async () => { element.open = true; await connect(); - const openChangeListener = createOpenChangeListener(element); + const toggleListener = createToggleListener(element); const beforeToggleListener = createBeforeToggleListener( element, true, - openChangeListener.spy + toggleListener.spy ); - const expectedDetails: MenuButtonBeforeToggleEventDetail = { + const expectedDetails: MenuButtonToggleEventDetail = { newState: false, oldState: true }; @@ -319,9 +333,9 @@ describe('MenuButton', () => { expect(event.detail).toEqual(expectedDetails); beforeToggleListener.spy.calls.reset(); - await openChangeListener.promise; + await toggleListener.promise; expect(beforeToggleListener.spy).not.toHaveBeenCalled(); - expect(openChangeListener.spy).toHaveBeenCalledTimes(1); + expect(toggleListener.spy).toHaveBeenCalledTimes(1); }); }); @@ -355,9 +369,9 @@ describe('MenuButton', () => { return; } - const openChangeListener = createOpenChangeListener(menuButton); + const toggleListener = createToggleListener(menuButton); menuButton.open = true; - await openChangeListener.promise; + await toggleListener.promise; } beforeEach(async () => { @@ -372,62 +386,62 @@ describe('MenuButton', () => { 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 openChangeListener = createOpenChangeListener(menuButton); + const toggleListener = createToggleListener(menuButton); menuButton.toggleButton!.control.click(); expect(menuButton.open).toBeTrue(); - await openChangeListener.promise; + 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 openChangeListener = createOpenChangeListener(menuButton); + const toggleListener = createToggleListener(menuButton); const event = new KeyboardEvent('keypress', { key: keyEnter } as KeyboardEventInit); menuButton.toggleButton!.control.dispatchEvent(event); expect(menuButton.open).toBeTrue(); - await openChangeListener.promise; + 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 openChangeListener = createOpenChangeListener(menuButton); + const toggleListener = createToggleListener(menuButton); const event = new KeyboardEvent('keypress', { key: keySpace } as KeyboardEventInit); menuButton.toggleButton!.control.dispatchEvent(event); expect(menuButton.open).toBeTrue(); - await openChangeListener.promise; + 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 openChangeListener = createOpenChangeListener(menuButton); + const toggleListener = createToggleListener(menuButton); const event = new KeyboardEvent('keydown', { key: keyArrowDown } as KeyboardEventInit); menuButton.toggleButton!.dispatchEvent(event); expect(menuButton.open).toBeTrue(); - await openChangeListener.promise; + 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 openChangeListener = createOpenChangeListener(menuButton); + const toggleListener = createToggleListener(menuButton); const event = new KeyboardEvent('keydown', { key: keyArrowUp } as KeyboardEventInit); menuButton.toggleButton!.dispatchEvent(event); expect(menuButton.open).toBeTrue(); - await openChangeListener.promise; + await toggleListener.promise; expect(document.activeElement).toEqual(menuItem3); }); 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 d57f646af6..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', 'beforetoggle'] + 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 43c28ddbb8..be96594258 100644 --- a/packages/nimble-components/src/menu-button/types.ts +++ b/packages/nimble-components/src/menu-button/types.ts @@ -16,10 +16,10 @@ export type MenuButtonPosition = typeof MenuButtonPosition[keyof typeof MenuButtonPosition]; /** - * The type of the detail associated with the `beforetoggle` CustomEvent - * on the menu button. + * The type of the detail associated with the `toggle` and `beforetoggle` + * events on the menu button. */ -export interface MenuButtonBeforeToggleEventDetail { +export interface MenuButtonToggleEventDetail { newState: boolean; oldState: boolean; } From 7341414478d9805b521da56de40203c9f2a052fd Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 24 Jan 2023 10:22:51 -0600 Subject: [PATCH 11/17] clean up slot traversal code + a few more tests --- .../src/menu-button/index.ts | 17 +- .../src/menu-button/tests/menu-button.spec.ts | 155 +++++++++++++++++- 2 files changed, 164 insertions(+), 8 deletions(-) diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index 8c950728c7..47a34bb6ff 100644 --- a/packages/nimble-components/src/menu-button/index.ts +++ b/packages/nimble-components/src/menu-button/index.ts @@ -200,16 +200,19 @@ export class MenuButton extends FoundationElement implements ButtonPattern { return undefined; } - let currentItem = this.slottedMenus[0]; + let currentItem: HTMLElement | undefined = this.slottedMenus[0]; while (currentItem) { if (currentItem.getAttribute('role') === 'menu') { return currentItem; } - if (currentItem?.nodeName === 'SLOT') { - currentItem = ( - currentItem as HTMLSlotElement - ).assignedNodes()[0] as HTMLElement; + if (this.isSlotElement(currentItem)) { + const firstNode = currentItem.assignedNodes()[0]; + if (firstNode instanceof HTMLElement) { + currentItem = firstNode; + } else { + currentItem = undefined; + } } else { return undefined; } @@ -218,6 +221,10 @@ export class MenuButton extends FoundationElement implements ButtonPattern { return undefined; } + private isSlotElement(element: HTMLElement | undefined): element is HTMLSlotElement { + return element?.nodeName === 'SLOT'; + } + private focusMenu(): void { this.menu?.focus(); } 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 93e08b9032..3a603f9314 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 @@ -347,19 +347,19 @@ describe('MenuButton', () => { const menuSlotConfigurations: MenuSlotConfiguration[] = [ { - description: 'directly in menu-button', + description: 'menu slotted directly in menu-button', setupFunction: setup, getMenuButton: (element: HTMLElement) => element as MenuButton }, { - description: 'passed through slot of additional element', + 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 menu ${configuration.description}`, () => { + describe(`menu interaction with ${configuration.description}`, () => { let element: HTMLElement; let connect: () => Promise; let disconnect: () => Promise; @@ -551,4 +551,153 @@ describe('MenuButton', () => { }); }); } + + 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); + }); + }); + } }); From 26196318e2be7984cc354699e700b812c7851cec Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 24 Jan 2023 10:25:01 -0600 Subject: [PATCH 12/17] update change files --- ...i-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json | 4 ++-- ...ni-nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json | 4 ++-- ...imble-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json b/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json index 5a8278303b..4dd5527e74 100644 --- a/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json +++ b/change/@ni-nimble-angular-81d4a2bd-2ca0-47ba-a7bb-c47be917d8ae.json @@ -1,6 +1,6 @@ { - "type": "minor", - "comment": "Add beforetoggle event on menu button", + "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 index 9b18263a1f..6bfd807ce1 100644 --- a/change/@ni-nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json +++ b/change/@ni-nimble-blazor-51e63b35-2ce0-44af-9dda-744c7234e857.json @@ -1,6 +1,6 @@ { - "type": "minor", - "comment": "Add beforetoggle event on menu button", + "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 index 51606ffa63..dc285e9805 100644 --- a/change/@ni-nimble-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json +++ b/change/@ni-nimble-components-0c3b5f38-1366-416c-8b35-20029a8d3eba.json @@ -1,6 +1,6 @@ { - "type": "minor", - "comment": "Add beforetoggle event on menu button and update menu button to work when the slotted menu is nested within additional slots", + "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" From f95249cc392fe2422850aa9c70e3608b8786183a Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 24 Jan 2023 10:50:37 -0600 Subject: [PATCH 13/17] Angular fixes --- .../menu-button/nimble-menu-button.directive.ts | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) 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 a87421e341..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,4 +1,4 @@ -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, MenuButtonToggleEventDetail } from '@ni/nimble-components/dist/esm/menu-button/types'; import { BooleanValueOrAttribute, toBooleanProperty } from '../utilities/template-value-helpers'; @@ -47,19 +47,5 @@ export class NimbleMenuButtonDirective { this.renderer.setProperty(this.elementRef.nativeElement, 'open', toBooleanProperty(value)); } - @Output() public toggle = new EventEmitter(); - - @Output() public beforeToggle = new EventEmitter(); - public constructor(private readonly renderer: Renderer2, private readonly elementRef: ElementRef) {} - - @HostListener('toggle', ['$event']) - public onToggle($event: CustomEvent): void { - this.toggle.emit($event.detail as MenuButtonToggleEventDetail); - } - - @HostListener('beforetoggle', ['$event']) - public onBeforeToggle($event: CustomEvent): void { - this.beforeToggle.emit($event.detail as MenuButtonToggleEventDetail); - } } From 2f8f5b5eb7c3450188566cb72a8ce610bbd90ff8 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 24 Jan 2023 10:57:54 -0600 Subject: [PATCH 14/17] Fix Blazor --- .../NimbleBlazor/Components/NimbleMenuButton.razor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor.cs b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor.cs index 2840064ad9..f76aa2a29c 100644 --- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor.cs +++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor.cs @@ -62,7 +62,7 @@ public partial class NimbleMenuButton : ComponentBase /// Called when 'open' changes on the web component. /// /// New value of open - protected async void UpdateOpen(MenuButtonToggleEventArgs eventArgs) + protected async void HandleToggle(MenuButtonToggleEventArgs eventArgs) { Open = eventArgs.NewState; await Toggle.InvokeAsync(eventArgs); From cda6c8aa35558c1ce572b8f21796dd7592e2d0d7 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 24 Jan 2023 11:03:01 -0600 Subject: [PATCH 15/17] lint --- .../NimbleBlazor/Components/NimbleMenuButton.razor.cs | 2 +- packages/nimble-components/src/menu-button/index.ts | 4 +++- .../src/menu-button/tests/menu-button.spec.ts | 5 +---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor.cs b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor.cs index f76aa2a29c..92ef881666 100644 --- a/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor.cs +++ b/packages/nimble-blazor/NimbleBlazor/Components/NimbleMenuButton.razor.cs @@ -61,7 +61,7 @@ public partial class NimbleMenuButton : ComponentBase /// /// Called when 'open' changes on the web component. /// - /// New value of open + /// The state of the menu button protected async void HandleToggle(MenuButtonToggleEventArgs eventArgs) { Open = eventArgs.NewState; diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index 47a34bb6ff..f78678d0f2 100644 --- a/packages/nimble-components/src/menu-button/index.ts +++ b/packages/nimble-components/src/menu-button/index.ts @@ -221,7 +221,9 @@ export class MenuButton extends FoundationElement implements ButtonPattern { return undefined; } - private isSlotElement(element: HTMLElement | undefined): element is HTMLSlotElement { + private isSlotElement( + element: HTMLElement | undefined + ): element is HTMLSlotElement { return element?.nodeName === 'SLOT'; } 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 3a603f9314..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 @@ -10,10 +10,7 @@ import { import { FoundationElement, Menu, MenuItem } from '@microsoft/fast-foundation'; import { fixture, Fixture } from '../../utilities/tests/fixture'; import { MenuButton } from '..'; -import { - MenuButtonToggleEventDetail, - MenuButtonPosition -} from '../types'; +import { MenuButtonToggleEventDetail, MenuButtonPosition } from '../types'; class TestSlottedElement extends FoundationElement {} const composedTestSlottedElement = TestSlottedElement.compose({ From d9c0299603596ed3f8b3e06488a1a5f7c2b43826 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 26 Jan 2023 08:48:52 -0600 Subject: [PATCH 16/17] menu getter to getMenu() function --- packages/nimble-components/src/menu-button/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index f78678d0f2..1ef4d1a3b7 100644 --- a/packages/nimble-components/src/menu-button/index.ts +++ b/packages/nimble-components/src/menu-button/index.ts @@ -137,7 +137,7 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } const focusTarget = e.relatedTarget as HTMLElement; - if (!this.contains(focusTarget) && !this.menu?.contains(focusTarget)) { + if (!this.contains(focusTarget) && !this.getMenu()?.contains(focusTarget)) { this.setOpen(false); return false; } @@ -192,7 +192,7 @@ export class MenuButton extends FoundationElement implements ButtonPattern { this.open = newValue; } - private get menu(): HTMLElement | undefined { + 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. @@ -228,11 +228,11 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } 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(); From f75502ce8192a32f41fc7e61bb5a6df4441f1fec Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 26 Jan 2023 11:27:50 -0600 Subject: [PATCH 17/17] format --- packages/nimble-components/src/menu-button/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/nimble-components/src/menu-button/index.ts b/packages/nimble-components/src/menu-button/index.ts index 1ef4d1a3b7..f98f297005 100644 --- a/packages/nimble-components/src/menu-button/index.ts +++ b/packages/nimble-components/src/menu-button/index.ts @@ -137,7 +137,10 @@ export class MenuButton extends FoundationElement implements ButtonPattern { } const focusTarget = e.relatedTarget as HTMLElement; - if (!this.contains(focusTarget) && !this.getMenu()?.contains(focusTarget)) { + if ( + !this.contains(focusTarget) + && !this.getMenu()?.contains(focusTarget) + ) { this.setOpen(false); return false; }