Close Event force closing other element [ Dialog, dropdown, drawer]

[IshanJaiswal99]

Currently, I was trying to implement a UI where, inside a drawer there was a dropdown on a button click, When I click outside the dropdown ( still inside the drawer) or press escape key, Ideally I would have wanted the dropdown to be closed and the drawer to remain opened, however, the close event for the dropdown is also closing the drawer in the background as well. I am unable to stop this from happening

As long as the dropdown is opened I am able to interact with the content inside the dropdown without any issues, but as soon as I am performing an action that is suppose to close the dropdown, then the drawer and/dialog is also getting closed.

I suspect, that Dialog, dropdown and drawer components utilize the same close event and for some reasons they are conflicting with each other.

• OS: Windows
• Browser: Chrome
• Browser version: 120.0.6099.130

[claviska]

Thanks for the report. Care to post a repro? Thanks!

[CetinSert]

@claviska

Press Esc

1. Open sl.rt.ht/1801? that contains an <sl-dropdown> within an <sl-drawer>.
2. Click on the <sl-button> or focus on it and press Enter or Space.
   1. This opens the <sl-dropdown> with document.activeElement being the <sl-button> ⚠️.
3. Press Esc, without selecting any <sl-menu-item>.
   1. This closes not only the <sl-dropdown>, but also the <sl-drawer> containing it ❌.

Observation

If you open <sl-dropdown> by pressing ↓/↑ instead, document.activeElement is an <sl-menu-item>. <sl-dropdown> (or perhaps the <sl-menu> itself) captures the Esc event according to expectations in that case.

Suggestion

<sl-dropdown><sl-button slot=trigger> can be made to treat Enter and Space the same way as it does ↓.

[!IMPORTANT]
<sl-dropdown> can be used with non-<sl-menu> panel content! The ↓(/↑) is a special treatment for <sl-menu><sl-menu-item>s.

[!NOTE]
Enter/Space and click focuses the first element in other platforms/libraries. It can be difficult to determine what to focus on when arbitrary panel content elements are involved.

Click outside <sl-dropdown> inside <sl-drawer>

@IshanJaiswal99 – I cannot reproduce this (with 2.12.0).

---

sl.rt.ht/1801? 👈🏻 minimal playground

[IshanJaiswal99]

@claviska @CetinSert possibly this issue with the react version of shoelace, I am attaching a codesandbox react project demonstrating the issues and I can confirm that I am facing this same issue in my local projects as well so it's not something specific to online IDEs.

Steps to reproduce: 1) Click on the Open Drawer Button 2) Click on Color Picker / Open Dialog Button / Dropdown Button 3) Close the opened Color Picker / Dialog / Dropdown by clicking on the dedicated close buttons or on the overlay of the opened component in order to close it.

Expected: The opened component should close while the drawer should stay opened

Actual The opened component is getting closed as well as the drawer in the background is getting closed automatically.

Code Sandbox

[CetinSert]

We have reproduced 2 independent issues:

1. 👆🏻 Press Esc
2. 👆🏻 Click outside <sl-dropdown> inside <sl-drawer>

[claviska]

As you can see from this example, the issue isn't coming from the library itself.

https://codepen.io/claviska/pen/OJqmXqO?editors=1010

The problem is here:

Indeed, the other components emit sl-hide and sl-after-hide just like every HTML element emits a click event when clicked. The events bubbles, so you need to check the target to make sure you're closing only when the drawer is emitting the event.

I've written about this more here, including a number of ways to do this check.

https://www.abeautifulsite.net/posts/custom-event-names-and-the-bubbling-problem/

[trusktr]

I believe that these types of events bubbling is not an ideal pattern for the library. Not all events need to bubble. Native click events bubble because a tree branch is being clicked, and it makes sense that if you click content inside of a <button> element (for example) you will want to know if the button was clicked. Native load events do not bubble, as load events do not have to do with a whole tree branch: if an <img> element nexted inside of a <button> (for sake of example) loads, that does not have to do with the <button> (the button did not load).

In this sense, the issue is coming from the library, because it when something is working and then it breaks due to changing the composition of the tree, unexpected defensive code (checking event.target) is required when most likely it should not be needed and better pattern exist (for example we can use the Lit Context API to explicitly create communication channels between higher levels and lower levels of the DOM tree)