fix(menu): allow menu-item to support arbitrary element as the submen…

…u root (#4720) * fix(menu): allow using non-menu-item element as submenu root * chore(menu): fixed broken test * chore(menu): added a custom root submenu example in storybook * chore(menu): updated docs --------- Co-authored-by: Rúben Carvalho <rubcar@sapo.pt>
adobe · Sep 11, 2024 · 4c6a0dc · 4c6a0dc
1 parent edf8ee2
commit 4c6a0dc
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 2 deletions.
diff --git a/packages/menu/menu-item.md b/packages/menu/menu-item.md
@@ -106,6 +106,24 @@ An `<sp-menu-item>` can also accept content addressed to its `submenu` slot. Usi
 </sp-menu>
 ```
 
+Note: While `sp-menu-item` can accommodate any custom content in the `submenu` slot, it will not handle selection or keyboard navigation for such content. To ensure proper management of selection and keyboard navigation, it is recommended to use `sp-menu` within the `submenu` slot```
+
+```html
+<sp-menu style="width: 200px;">
+    <sp-menu-item>
+        Item with arbitrary content in submenu
+        <div role="menuitem" slot="submenu" style="padding: 12px">
+            <img
+                src="https://placekitten.com/200/200"
+                alt="Kitten"
+                style="width: 100%; height: auto; border-radius: 4px"
+            />
+            <p>I am an arbitrary content in submenu</p>
+        </div>
+    </sp-menu-item>
+</sp-menu>
+```
+
 ### Value attribute
 
 When displayed as a descendent of an element that manages selection (e.g. `<sp-action-menu>`, `<sp-picker>`, `<sp-split-button>`, etc.), an `<sp-menu-item>` will represent the "selected" value of that ancestor when its `value` attribute or the trimmed `textContent` (represeted by `el.itemText`) matches the `value` of the ancestor element.

diff --git a/packages/menu/src/Menu.ts b/packages/menu/src/Menu.ts
@@ -375,7 +375,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
         }) as MenuItem;
         if (event.defaultPrevented) {
             const index = this.childItems.indexOf(target);
-            if (target?.menuData.focusRoot === this && index > -1) {
+            if (target?.menuData?.focusRoot === this && index > -1) {
                 this.focusedItemIndex = index;
             }
             return;
@@ -391,7 +391,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
             );
             return;
         } else if (
-            target?.menuData.selectionRoot === this &&
+            target?.menuData?.selectionRoot === this &&
             this.childItems.length
         ) {
             event.preventDefault();

diff --git a/packages/menu/stories/submenu.stories.ts b/packages/menu/stories/submenu.stories.ts
@@ -368,3 +368,26 @@ export const contextMenu = (): TemplateResult => {
         </sp-overlay>
     `;
 };
+
+export const customRootSubmenu = (): TemplateResult => {
+    return html`
+        <sp-action-menu label="More Actions">
+            <sp-menu-item>Bronx</sp-menu-item>
+            <sp-menu-item id="submenu-item-1">
+                Brooklyn
+                <div role="menuitem" slot="submenu" style="padding: 12px">
+                    <img
+                        src="https://placekitten.com/200/200"
+                        alt="Kitten"
+                        style="width: 100%; height: auto; border-radius: 4px"
+                    />
+                    <p>I am an arbitrary content in submenu</p>
+                </div>
+            </sp-menu-item>
+        </sp-action-menu>
+    `;
+};
+
+customRootSubmenu.swc_vrt = {
+    skip: true,
+};
diff --git a/packages/menu/test/submenu.test.ts b/packages/menu/test/submenu.test.ts
@@ -1068,4 +1068,65 @@ describe('Submenu', () => {
         expect(rootItem1.open, 'finally closed 1').to.be.false;
         expect(rootItem2.open, 'finally closed 2').to.be.false;
     });
+    it('allows using non-menu-item elements as the root of a submenu', async () => {
+        const el = await fixture<Menu>(html`
+            <sp-menu>
+                <sp-menu-item class="root">
+                    Has submenu
+                    <div role="menuitem" slot="submenu">
+                        <sp-menu-item class="submenu-1">One</sp-menu-item>
+                        <sp-menu-item>Two</sp-menu-item>
+                        <sp-menu-item>Three</sp-menu-item>
+                    </div
+                ></div>
+                </sp-menu-item>
+            </sp-menu>
+        `);
+        await elementUpdated(el);
+        const rootItem = el.querySelector('.root') as MenuItem;
+        const rootItemBoundingRect = rootItem.getBoundingClientRect();
+
+        // Open the first submenu
+        await sendMouse({
+            steps: [
+                {
+                    type: 'move',
+                    position: [
+                        rootItemBoundingRect.left +
+                            rootItemBoundingRect.width / 2,
+                        rootItemBoundingRect.top +
+                            rootItemBoundingRect.height / 2,
+                    ],
+                },
+            ],
+        });
+
+        expect(rootItem.open).to.be.true;
+
+        const firstSubMenuItemRect = el
+            .querySelector('.submenu-1')
+            ?.getBoundingClientRect();
+
+        if (!firstSubMenuItemRect) {
+            throw new Error('Submenu item not found');
+        }
+
+        // click to select
+        await sendMouse({
+            steps: [
+                {
+                    type: 'click',
+                    position: [
+                        firstSubMenuItemRect.left +
+                            firstSubMenuItemRect.width / 2,
+                        firstSubMenuItemRect.top +
+                            firstSubMenuItemRect.height / 2,
+                    ],
+                },
+            ],
+        });
+
+        // This test will fail if the click event throws an error
+        // because the submenu root is not a menu-item
+    });
 });