spectrum-web-components icon indicating copy to clipboard operation
spectrum-web-components copied to clipboard

[Bug]: Sub-menus within a on overlay trigger sp-closed for the outer overlay-trigger element

Open adixon-adobe opened this issue 2 years ago • 4 comments

Code of conduct

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

Impacted component(s)

overlay-trigger

Expected behavior

I can listen to sp-closed on overlay-trigger and only get close events for that trigger

Actual behavior

If I use sp-action-menu or sp-picker in the overlay rendered by overlay-trigger, closing these menus also triggers sp-closed for the outer menu. It does look like I can match the event.target to the original trigger button, but I could see this really throwing people for a loop.

How can we reproduce this issue?

I can look at getting a simpler reproduction when I have a bit more time.

adixon-adobe avatar Apr 12 '24 14:04 adixon-adobe

I'm not really sure if another event or better documentation is the right approach here, but it was tricky to figure this out, and I'm curious how this would work for manually triggered overlays as well.

adixon-adobe avatar Apr 12 '24 15:04 adixon-adobe

As I think about this more, I think what I'm really asking for is an event that doesn't bubble. I just want to know if this overlay opened or closed. Using the same (bubbling) event that the overlay system gets confusing because it's less common, and surprising when you run into it.

So ideally there's be another event that doesn't bubble, which when documented should make the difference in behavior very clear. And all that said, I won't be too offended if you decide it's just simpler to improve the documentation. Just pointing out that this can be a real surprising gotcha.

adixon-adobe avatar Apr 12 '24 18:04 adixon-adobe

Some quick documentation points:

  • managing the events in the merged DOM tree https://opensource.adobe.com/spectrum-web-components/migrations/2023-8-18/#prepare-for-descendant-overlays-to-exist-in-the-same-dom-tree
  • the overlay property on these events
    type OverlayStateEvent = Event & {
      overlay: Overlay;
    };
    
    The section before this (“Events” is a section twice 🤦‍♂️) https://opensource.adobe.com/spectrum-web-components/components/overlay/#when-it-is-%22fully%22-open-or-closed%3F
  • there are also some native-like events available: https://developer.mozilla.org/en-US/docs/Web/API/Popover_API#events

Westbrook avatar Apr 12 '24 21:04 Westbrook

@adixon-adobe Does the above comment from Westbrook give you a path forward?

najikahalsema avatar May 22 '24 14:05 najikahalsema