shoelace-style / shoelace

A collection of professionally designed, every day UI components built on Web standards. SHOELACE IS BECOMING WEB AWESOME 👇👇👇
https://webawesome.com
MIT License
12.9k stars 823 forks source link

sl-drawer issues when using keyboard navigation #2018

Open schilchSICKAG opened 5 months ago

schilchSICKAG commented 5 months ago

Describe the bug

When using keyboard navigation in <sl-drawer>, you will get one extra tabbable element because drawer__panel has a tabindex set to 0.

To Reproduce

Steps to reproduce the behavior:

Open this codepen. Navigate the pen via keyboard. Do not click the Danger Button. During navigation, you will have a highlight of:

  1. Close-Icon
  2. Button 1
  3. Button 2
  4. Close Button
  5. NOTHING (WRONG) (document.activeElement points to sl-drawer)
  6. Close-Button

The issue here is the applied tabindex="0" for the drawer__panel element.

Suggested fix: Remove the tabindex value. Demo:

As above, but click the danger button before tabbing. Expected highlight is now:

  1. Close-Icon
  2. Button 1
  3. Button 2
  4. Close Button
  5. Close-Icon

Demo

Codepen available at https://codepen.io/schilchSICKAG/pen/OJYygjK?editors=1001

Screenshots

If applicable, add screenshots to help explain the bug.

Browser / OS

Additional information

I do not know why the tabindex="0" is set on the element at all. The drawer__overlay already has an index of -1, so this should not be an issue? Would be happy to provide a PR for this :)

claviska commented 5 months ago

The zero tab index gives us a guaranteed element to focus on when the drawer opens. We're switching to the native <dialog> element for dialogs and drawers in Web Awesome. This will let us remove a ton of focus trap logic that's been difficult to maintain.

Unfortunately, <dialog> wasn't well-supported when these components were originally created, but with the 3.0 release in the works we're finally able to get things like this fixed.