Support an "sp-action-menu" w/o value

[Westbrook]

I'm working with an sp-action-menu instance that doesn't want for a specific sp-menu-item to be selected or for the sp-action-menu to have a value. This is different than the current version which implements an extension of sp-dropdown and always has a value and selected menu item. It's unclear whether this element should just not have a value, ever, or whether we should add an attribute that can prevent it from managing this data.

Thoughts on whether this pattern should ever have a value/manage the selected attribute on its children? This is lightly implied by the lack of designs to support these states: https://opensource.adobe.com/spectrum-css/components/actionmenu/

On the attribute based flag site, it might might sense to use an action-only attribute, which would leave the following implementation as not managing a value property or setting selected on it's children:

                <sp-action-menu action-only>
                    <span slot="icon">More</span>
                    <sp-menu slot="options">
                        <sp-menu-item @click=${doFlipHorizontal}>
                            Flip Horizontally
                        </sp-menu-item>
                        <sp-menu-item @click=${doFlipVertical}>
                            Flip Vertically
                        </sp-menu-item>
                    </sp-menu>
                </sp-action-menu>

[adixon-adobe]

@Westbrook have you given any more thought to this? I hadn't noticed the selected states until just now.

I think the "action" in "action-menu" implies that every option should lead to an action (and that shouldn't result in an option in the menu being selected). So I'd strongly lean towards having the default not be selectable. I also think we should check in with the spectrum team to see if the selectable pattern here is ever desired.

[Westbrook]

I think that makes sense, too.

Is LRWeb looking to leverage something like this soon? Can you bring it up with Spectrum? I'd be happy to support getting this change into the library via which ever pattern makes sense from their feedback.

[adixon-adobe]

Yeah -- we're using this and it's an ugly aesthetic issue right now, so a fix would be nice. Sounds like the Spectrum team agrees that there shouldn't be a need for a selected state (or value). "Use Dropdown if you need that".

Also, this feels like it should be a "breaking change" for this component.

[adixon-adobe]

When fixing this we should add some example usage.

Also, if a client sets a menu item as selected on their own, would that still be valid & show up right? (there's some interesting menus in the Lightroom editor with some items that are selectable more like an internal toggle)

[adixon-adobe]

One thing to keep in mind with the fix here is that right now on-change fires after the menu closes so you can safely re-focus a new element from a handler. I'm not sure on-change is the right event with this fix, but I know we'll want something similar.

[Westbrook]

Interesting point to work out. We use the change event as that's indicative of a "value" being stored. In a world where we respond individually to interactions on the items would you want a similar (maybe with a different name) event or would you be better served by managing that on the items a la the initial post that uses:

<sp-menu-item @click=${doFlipVertical}>
    Flip Vertically
</sp-menu-item>

A implementing app would then have the flexibility to customize this either as individual listeners or as a form of delegation:

<sp-action-menu
    action-only
    @click=${(event) => {
        const actionId = event.target.getAttribute('my-action-id');
        doAnAction(actionId);
    }}
>
    <span slot="icon">More</span>
    <sp-menu slot="options">
        <sp-menu-item my-action-id="flipH">
            Flip Horizontally
        </sp-menu-item>
        <sp-menu-item my-action-id="flipV">
            Flip Vertically
        </sp-menu-item>
    </sp-menu>
</sp-action-menu>

[adixon-adobe]

I could see a click event on the menu being a little confusing (wait -- I just wanted to handle clicks on the button there -- not the menu it opens!). Certainly handling click events on the menu items should (and already does) work, and if that works with the close animation timing that might be good. I could also see having an item-click event on the menu itself.

[Westbrook]

@charlessuh Thanks for following up on the question of advanced Action Menu patterns with the Spectrum team. I'm still not seeing the type of pattern you've referenced in their design documentation, but I definitely see your point on the possibility of that pattern. To the end of actually delivering that, do you have thoughts on an API that would feel comfortable when applying such a pattern? You make a great point about not wanting to require a breaking change down the road, and agree we should discuss it a little deeper (even if we don't fully implement yet) before we move forward on #912

Some things that I see deserving discussion here:

• Currently the change event API only registers a single "selected" element (regardless of visual display). To have something "visually selected" and another things "not visually selected" it would seem we'd need a much more complex value property.
  • Would that only be a single value for "visually" and a single for "not", or do you expect we'd want to directly support multiple "selections" for each type in a single menu?
  • Could we rely on the change => "read value" pattern that mimics the <select> element, or would this require a more complex notification system?
• If we can assume some lists only have "visually selectable" items and some only have "non-visually selectable" items, would we want to be able to manage that centrally like the proposal in fix(action-menu): default to not taking a selection #912 does currently or would ALL lists need to be managed at the item level?
  • With a centrally managed list element, would lists with multiple types of selections be one or the other PLUS custom items, or should it be a unique list unto itself?
• Does <sp-action-menu> need to support this complexity out of the box, or would the element (and users) be better served by extended documentation on the parts that come together to make an Action Menu with the goal of making complex Action Menu patterns more attainable?

[adixon-adobe]

I'll preface this by saying that outside of having the design & spectrum design teams start thinking about this, I don't think solving this completely in SWC is something urgent.

I do think it might be nice to think about how hard it would be to have client-managed selection in sp-action-menu & sp-menu. E.g. maybe when selectable is off, sp-action-menu doesn't manage the selected state at all, and fires on item-click event instead of a change event -- clients could then manage which items are selected directly.

[Westbrook]

Thinking a little more about this I wonder how an API like this feels:

    <sp-action-menu
        label="Test menu"
        @change=${(e) =>
          console.log(`do something about a value of: ${e.target.value}. maybe it's a STRING or an ARRAY? see two groups with [selectable]`)}
        @action=${(e) =>
          console.log(`do something about an action of: ${e.target.action}. probably just the LAST action string?`)}
      >
        <sp-menu>
          <sp-menu-group selectable>
            <div slot="header">Selectable Things</div>
            <sp-menu-item>First Selectable</sp-menu-item>
            <sp-menu-item>Second Selectable</sp-menu-item>
            <sp-menu-item>Third Selectable</sp-menu-item>
            <sp-menu-item>Fourth Selectable</sp-menu-item>
          </sp-menu-group>
          <sp-menu-divider></sp-menu-divider>
          <sp-menu-group>
            <div slot="header">Actionable Things</div>
            <sp-menu-item>First Actionable</sp-menu-item>
            <sp-menu-item>Second Actionable</sp-menu-item>
            <sp-menu-item>Third Actionable</sp-menu-item>
            <sp-menu-item>Fourth Actionable</sp-menu-item>
          </sp-menu-group>
          <sp-menu-divider></sp-menu-divider>
          <sp-menu-group selectable>
            <div slot="header">Other Selectable Things</div>
            <sp-menu-item>First Selectable</sp-menu-item>
            <sp-menu-item>Second Selectable</sp-menu-item>
            <sp-menu-item>Third Selectable</sp-menu-item>
            <sp-menu-item>Fourth Selectable</sp-menu-item>
          </sp-menu-group>
        </sp-menu>
      </sp-action-menu>

In theory this positions an Action Menu to take the [selectable] attribute for it's entirety while layering intersections at which to manage the groups of items (as I think managing items singularly feels a little heavy for "built-in" functionality) that either "select" or "make action". I'm going back an forth as to how much of this the Action Menu should manage as a "built-in" vs things that are possible when mixing overlay-trigger, action-button and sp-menu elements yourself.

The action event above would need to be custom anyways, so appending data to detail is also an options, however I like the symmetry of it to the change event right now. In the end though, it's just cached data or not, so maybe it's not worth it.

I want to think some more about how this lines up with #715 and https://github.com/adobe/spectrum-web-components/pull/912/files/5bf12b3f649f64b031cfa0be88180ef976637fc9#r499001494 as far as short term API changes (which seem needed regardless) that prepare us for not needing breaking changes down the way. There is also a lot of internal aria attributing that would need to be addressed for this and finding the right level at which to manage that is important.

if you have any other thoughts on API structure and what it should support/how the workflow should traverse it, please share here!

[adixon-adobe]

@charlessuh please weigh in when you get a chance

Overall I think I like where this is going, though I'd want to make sure you're thinking through the complexity. This feels like something we might want to tackle at the level you're describing a little later (while making sure the support we add now doesn't get in the way of that).

I also want to make sure we're on the same page about the simpler cases. Here's how I'm reading them based on what you're described above:

• We'd still want a selectable attribute on sp-action-menu like in your PR.
• By default, sp-action-menu is not selectable (unlike sp-dropdown)
• e.target.action is always set when a menu item is clicked/activated, and the action event is triggered. I don't have a preference about the detail bit you mentioned.
• The change event only fire if it's selectable, and in that case e.target.value is set.

[Westbrook]

Reviewing this, the current PR available in #912, and the currently available features in the public release, I'd like to greatly simplify this API at the ecosystem level so that this sort of functionality isn't bolted (tracked in changes to selection) on so much as a configuration that is actively available for leveraging (so that selection could have more robust and cacheable values over the lifecycle of a UI).

I think instead we can move away from raw click events in the entirety of the sp-menu-item support structure and prepare these elements to dispatch an activate event with a details that contains as: 'select'. Any element in the ecosystem could interrupt that event in flight and switch the action to as: 'action' or the like. The activate event would be trapped by the parent element of a "group" and converted into a change or action event as outlined above, waiting for a later date to expand the API there to being able to support more than one value or action when listening/responding to the event.

In this way:

<sp-menu>
    <sp-menu-item>Example</sp-menu-item>
</sp-menu>

Would be able to exist as is.

<sp-dropdown>
    <sp-menu>
        <sp-menu-item>Example</sp-menu-item>
    </sp-menu>
</sp-dropdown>

Would be able to exist as is.

<sp-action-menu>
    <sp-menu>
        <sp-menu-item>Example</sp-menu-item>
    </sp-menu>
</sp-action-menu>

Would be able to exist as is, but we could make the choice is selectable the modifier that causes <sp-action-menu> to select a child on activate, or is actionable the modifier the prevents it from selecting a child on activate.

<sp-split-button type="more">
    Split Button
    <sp-popover slot="more" open>
        <sp-menu>
            <sp-menu-item>Option 1</sp-menu-item>
            <sp-menu-item>Option Extended</sp-menu-item>
        </sp-menu>
    </sp-popover>
</sp-split-button>

Would be able to do things in response to clicking an option without selecting it (type="more"), while:

<sp-split-button variant="secondary">
    Split Button
    <sp-popover slot="more" open>
        <sp-menu>
            <sp-menu-item>Option 1</sp-menu-item>
            <sp-menu-item>Option Extended</sp-menu-item>
        </sp-menu>
    </sp-popover>
</sp-split-button>

Would be able to do the same while managing the presence of the item that is switched into the button text.

All the while, I think this would open some good options for #715, #254, and the possibility of mixed selectability menus by diversifying the elements that have the selectable/actionable attributes and can modifier the as: 'action/selection' values in the event.