<sl-menu-item disabled> triggers click/tap events

[CetinSert]

Describe the bug

<sl-menu-item disabled> is ignored. Tested in versions .88→.61 (likely goes further past).

To Reproduce

Steps to reproduce the behavior:

1. Go to sl.rt.ht/1113/?
2. Click/tap on Option 2 in the output panel
   • ❌ <sl-menu-item disabled> is ignored!
3. Click/tap on Built-in or Shoelace buttons
   • ✔️ <⋯ disabled> prevents event.

Demo

See above.

Screenshots

N/A.

Browser / OS

• OS: all
• Browser: all
• Browser version: all

Additional information

Discovered while adding support for <sl-menu-item inert> in #1107 (comment)

[CetinSert]

Fixed playground link in

• <sl-menu-item disabled> triggers click/tap events #1113 (comment)

[CetinSert]

@claviska – still an issue with 2.0.0.

[claviska]

Fixed in b8695b7.

I also updated <sl-button> so clicks on the host element will be suppressed when disabled because I noticed it was technically possible for the button to emit a click if there was padding between the host element and the internal <button>.

[yao-motif]

@claviska I'm not sure if I'm testing the wrong thing, but I took the above example, updated to 2.4.0 and the alert still triggers while sl-menu-item is disabled

example

[CetinSert]

@yao-motif -
2.5.2 also triggers the click handler!
See sl.rt.ht/? for the updated include.

[yao-motif]

Ah, I realized this might be intended, what's the reason for doing so?

[claviska]

I investigated this further and realized that even attaching the listener in the constructor isn't a reliable way to prevent retargeted clicks on the host element. The reason it "works" for <sl-button> is because the underlying HTML button is also disabled, so there's no click to retarget. The listener logic wasn't even executing.

As for <sl-menu-item>, the click was retargeted and the logic was executing, but only after the inline onclick listener. I don't really see a way around this behavior except perhaps appending an inline listener on the host element itself, but then again users would be able to overload it with their own.

I suggest checking for the disabled property in your handlers instead of relying on the disabled attribute. It gets messy trying to fight the order of event listeners, and I don't know that we can reliably do this with retargeted events.

[yao-motif]

I suggest checking for the disabled property in your handlers instead of relying on the disabled attribute. It gets messy trying to fight the order of event listeners, and I don't know that we can reliably do this with retargeted events.

that's what i ended up doing, and it works well, thank you for investigating!