Skip to content

Commit da4d05c

Browse files
authored
[fix]: remove unnecessary filtering in menu-item (#33795)
1 parent 959a43f commit da4d05c

File tree

4 files changed

+48
-11
lines changed

4 files changed

+48
-11
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "remove unnecessary filtering in menu-item",
4+
"packageName": "@fluentui/web-components",
5+
"email": "jes@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

packages/web-components/src/menu-item/menu-item.template.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { ElementViewTemplate } from '@microsoft/fast-element';
22
import { html, slotted } from '@microsoft/fast-element';
33
import { staticallyCompose } from '../utils/template-helpers.js';
44
import { endSlotTemplate, startSlotTemplate } from '../patterns/index.js';
5-
import { menuFilter, type MenuItem, type MenuItemOptions } from './menu-item.js';
5+
import type { MenuItem, MenuItemOptions } from './menu-item.js';
66

77
const Checkmark16Filled = html.partial(
88
`<svg class="indicator" fill="currentColor" aria-hidden="true" width="16" height="16" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="M14.05 3.49c.28.3.27.77-.04 1.06l-7.93 7.47A.85.85 0 014.9 12L2.22 9.28a.75.75 0 111.06-1.06l2.24 2.27 7.47-7.04a.75.75 0 011.06.04z" fill="currentColor"></path></svg>`,
@@ -27,7 +27,7 @@ export function menuItemTemplate<T extends MenuItem>(options: MenuItemOptions =
2727
</div>
2828
${endSlotTemplate(options)}
2929
<slot name="submenu-glyph"> ${staticallyCompose(options.submenuGlyph)} </slot>
30-
<slot name="submenu" ${slotted({ property: 'slottedSubmenu', filter: menuFilter() })}></slot>
30+
<slot name="submenu" ${slotted({ property: 'slottedSubmenu' })}></slot>
3131
</template>
3232
`;
3333
}

packages/web-components/src/menu-item/menu-item.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { attr, type ElementsFilter, FASTElement, observable } from '@microsoft/fast-element';
1+
import { attr, FASTElement, observable } from '@microsoft/fast-element';
22
import { keyArrowLeft, keyArrowRight, keyEnter, keySpace } from '@microsoft/fast-web-utilities';
3-
import type { MenuList } from '../menu-list/menu-list.js';
43
import type { StartEndOptions } from '../patterns/start-end.js';
54
import { StartEnd } from '../patterns/start-end.js';
65
import { applyMixins } from '../utils/apply-mixins.js';
@@ -21,13 +20,6 @@ export type MenuItemOptions = StartEndOptions<MenuItem> & {
2120
submenuGlyph?: StaticallyComposableHTML<MenuItem>;
2221
};
2322

24-
/**
25-
* Creates a function that can be used to filter a Node array, selecting only elements with elementInternals role of "menu".
26-
* @public
27-
*/
28-
export const menuFilter = (): ElementsFilter => value =>
29-
value.nodeType === 1 && (value as MenuList).elementInternals.role === 'menu';
30-
3123
/**
3224
* A Switch Custom HTML Element.
3325
* Implements {@link https://www.w3.org/TR/wai-aria-1.1/#menuitem | ARIA menuitem }, {@link https://www.w3.org/TR/wai-aria-1.1/#menuitemcheckbox | ARIA menuitemcheckbox}, or {@link https://www.w3.org/TR/wai-aria-1.1/#menuitemradio | ARIA menuitemradio }.

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,44 @@ test.describe('Menu', () => {
174174
await expect(menuList).toBeVisible();
175175
});
176176

177+
test('should set popover attribute on slotted submenu', async ({ fastPage }) => {
178+
const { element } = fastPage;
179+
180+
const menuButton = element.locator('fluent-menu-button');
181+
const menuList = element.locator('fluent-menu-list:not([slot])');
182+
const menuItems = menuList.locator('fluent-menu-item');
183+
184+
const submenuList = element.locator('fluent-menu-list[slot="submenu"]');
185+
186+
await fastPage.setTemplate({
187+
innerHTML: /* html */ `
188+
<fluent-menu-button appearance="primary" slot="trigger">Toggle Menu</fluent-menu-button>
189+
<fluent-menu-list>
190+
<fluent-menu-item>
191+
Menu item 1
192+
<fluent-menu-list slot="submenu">
193+
<fluent-menu-item> Subitem 1 </fluent-menu-item>
194+
<fluent-menu-item> Subitem 2 </fluent-menu-item>
195+
<fluent-menu-item> Subitem 3 </fluent-menu-item>
196+
</fluent-menu-list>
197+
</fluent-menu-item>
198+
<fluent-menu-item>Menu item 2</fluent-menu-item>
199+
<fluent-menu-item>Menu item 3</fluent-menu-item>
200+
<fluent-menu-item>Menu item 4</fluent-menu-item>
201+
</fluent-menu-list>
202+
`,
203+
});
204+
205+
await menuButton.click();
206+
207+
await menuItems.first().focus();
208+
209+
await element.press('ArrowRight');
210+
211+
await expect(submenuList).toBeVisible();
212+
await expect(submenuList).toHaveAttribute('popover');
213+
});
214+
177215
test('should focus the first item when a submenu is closed', async ({ fastPage }) => {
178216
const { element } = fastPage;
179217

0 commit comments

Comments
 (0)