Close Event force closing other element [ Dialog, dropdown, drawer] about shoelace HOT 6 CLOSED

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

from shoelace.

@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 ↓.

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

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

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

from shoelace.

@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

from shoelace.

We have reproduced 2 independent issues:

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

from shoelace.

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/

from shoelace.

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)

from shoelace.