[Bug]: MenuGroup selects="multiple" throws an error when used inside ActionMenu

[godanny86]

trafficstars

Code of conduct

• [X] I agree to follow this project's code of conduct.

Impacted component(s)

ActionMenu, MenuGroup

Expected behavior

When using a sp-menu-group inside an sp-action-menu that is configured with selects="multiple" a user should be able to select multiple menu items and de-select all items so that none of the items are selected.

Actual behavior

When using a sp-menu-group inside an sp-action-menu that is configured with selects="multiple" we observe an error thrown:

Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'value')
    at ActionMenu.setValueFromItem (a4b5a5065fc4a5aaea94f0ac868f443104a0a424:125:6648)
    at ActionMenu.handleChange (a4b5a5065fc4a5aaea94f0ac868f443104a0a424:125:6489)
    at I.handleEvent (a4b5a5065fc4a5aaea94f0ac868f443104a0a424:15:7186)
    at MenuGroup.selectOrToggleItem (a4b5a5065fc4a5aaea94f0ac868f443104a0a424:119:7976)
    at MenuGroup.handlePointerBasedSelection (a4b5a5065fc4a5aaea94f0ac868f443104a0a424:119:6046)
    at MenuGroup.handlePointerup (a4b5a5065fc4a5aaea94f0ac868f443104a0a424:119:5477)

I believe this may be related to ActionMenu not supporting "multiple" at the top level? Possibly this issue: https://github.com/adobe/spectrum-web-components/issues/1665

Screenshots

https://github.com/adobe/spectrum-web-components/assets/8974514/19c0d385-3bd1-4686-b6e1-5bccfa621417

What browsers are you seeing the problem in?

Chrome, Safari

How can we reproduce this issue?

1. Navigate to this sandbox: https://studio.webcomponents.dev/edit/JciavEjTy2VT17fMD1Rp/src/index.ts?p=stories
2. Open the ActionMenu
3. Select "Reverse Order"
4. re-open the ActionMenu
5. Toggle "Reverse Order" to de-select
6. Observe error thrown.

Sample code that illustrates the problem

import { LitElement, html, TemplateResult } from "lit";
import { customElement } from 'lit/decorators.js';
import "@spectrum-web-components/action-menu/sp-action-menu.js";
import "@spectrum-web-components/menu/sp-menu.js";
import "@spectrum-web-components/menu/sp-menu-item.js";
import "@spectrum-web-components/menu/sp-menu-divider.js";
import "@spectrum-web-components/menu/sp-menu-group.js";

@customElement("my-counter")
export default class MyCounter extends LitElement {
  protected render(): TemplateResult {
    return html`
      <sp-action-menu @change=${e => console.log(e.target.value)}>
        <sp-menu-group selects="single" id="group-1">
          <span slot="header">Sort By</span>
          <sp-menu-item>
            Name
          </sp-menu-item>
          <sp-menu-item>
            Created
          </sp-menu-item>
          <sp-menu-item>
            Modified
          </sp-menu-item>
        </sp-menu-group>
        <sp-menu-divider></sp-menu-divider>
        <sp-menu-group selects="multiple" id="group-2">
        <sp-menu-item>
          Reverse Order
        </sp-menu-item>
        </sp-menu-group>
      </sp-action-menu>
      
    `;
  }
}

Logs taken while reproducing problem

No response

[Westbrook]

We're looking at this as the "correct" updated functionality here. Can you take a look at confirm we're moving in a direction that supports your goals here?

[godanny86]

@Westbrook yes, this looks great! A major reason we want to use ActionMenu is for the built in tray support for mobile. I tested there as well and so far looks good!

[Westbrook]

Awesome! We've gotta get in some tests around this change to make sure its stable, and possibly add some item resolution recursion, then we'll be able to open a PR for you on this.