Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sl-select is not looping sl-options on keydown #2173

Open
arunks5 opened this issue Sep 18, 2024 · 7 comments
Open

sl-select is not looping sl-options on keydown #2173

arunks5 opened this issue Sep 18, 2024 · 7 comments
Labels
bug Things that aren't working right in the library.

Comments

@arunks5
Copy link

arunks5 commented Sep 18, 2024

Describe the bug

sl-select component is not looping sl-options on keydown. It is focusing only the first sl-option. This behavior is implemented in w3.org https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-select-only/ .

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://codepen.io/Arun-Kumar-S/pen/LYKKgbm
  2. Click on sl-select. Panel will be opened
  3. Press S. Shoelace option will be focused.
  4. Again press S. No changes.

On again pressing S, Summer option should have been focused.

Demo

Screenshots

If applicable, add screenshots to help explain the bug.

Browser / OS

  • OS: [e.g. Mac, Windows]
  • Browser: [e.g. Chrome, Firefox, Safari]
  • Browser version: [e.g. 22]

Additional information

Provide any additional information about the bug here.

@arunks5 arunks5 added the bug Things that aren't working right in the library. label Sep 18, 2024
@CetinSert
Copy link

CetinSert commented Sep 18, 2024

For reference1, on RTCode.io, we have implemented this 'first letter focus' behavior for <sl-menu> and <sl-menu-item>s:

image

with :focus-visible, :focus-within, :has(), ::first-letter to Underline the keys of the active <sl-menu> <sl-menu-item>s:

image


  1. open the RTCode.io playground
  2. right click → View page source
  3. search for
    1. first-letter-focus for CSS
      excerpt
            sl-menu:has(:focus-within:focus-visible):not(:has(sl-menu:focus-within)) {
              [f-l]::first-letter,
              & > sl-menu-item:not([disabled]):not(:has(#ubt))::part(label)::first-letter {
                text-decoration: underline 1px solid color-mix(in oklch, currentColor, 75% transparent);
                text-underline-offset: 1.5px;
              }
            }
    2. firstLetterFocus for JS
      excerpt
            SlMenu.firstLetterFocus = (function() { // works with RTCode.io overrides
              let enabled = false;
              let lastKeyPressed = '';
              let lastKeyTime = 0;
              function toggle(enable) { enable = !!enable; document.querySelectorAll('sl-menu').forEach(menu => menu[`${enable ? 'add' : 'remove'}EventListener`]('keydown', menuKeyHandler)); enabled = enable; }
              function menuKeyHandler(e) { if (!enabled || e.modKey) return;
                const key = e.key.toLowerCase();
                const currentTime = Date.now();
      
                // Reset if different key is pressed or if too much time has passed since last key press
                if (key !== lastKeyPressed || currentTime - lastKeyTime > 1000) {
                  lastKeyPressed = key;
                  lastKeyTime = currentTime;
                }
      
                // Filter menu items based on first letter and focus accordingly
                const items = Array.from(document.activeElement.closest('sl-menu')?.querySelectorAll?.(':scope > sl-menu-item:not([disabled])') ?? [])?.filter?.(item =>
                  item.textContent.trim().toLowerCase().startsWith(key) && getComputedStyle(item)?.display !== 'none'
                ); console.warn('firstLetterFocus', { items });
      
                if (items.length > 0) {
                  // Find the next item to focus
                  e.stopImmediatePropagation();
                  const currentIndex = items.indexOf(document.activeElement);
                  const nextIndex = (currentIndex + 1) % items.length; console.warn('firstLetterFocus', { nextIndex });
                  items[nextIndex].focus();
                }
              }
              return {
                get enabled()  { return enabled;  },
                set enabled(v) {        toggle(v) }
              };
            })();

Footnotes

  1. 👆🏻 Source code references.

@claviska
Copy link
Member

This component is designed to work like a standard <select>, where you can type an entire term and it will focus the correct one. The logic for this is located here:

// All other "printable" keys trigger type to select
if ((event.key && event.key.length === 1) || event.key === 'Backspace') {
const allOptions = this.getAllOptions();
// Don't block important key combos like CMD+R
if (event.metaKey || event.ctrlKey || event.altKey) {
return;
}
// Open, unless the key that triggered is backspace
if (!this.open) {
if (event.key === 'Backspace') {
return;
}
this.show();
}
event.stopPropagation();
event.preventDefault();
clearTimeout(this.typeToSelectTimeout);
this.typeToSelectTimeout = window.setTimeout(() => (this.typeToSelectString = ''), 1000);
if (event.key === 'Backspace') {
this.typeToSelectString = this.typeToSelectString.slice(0, -1);
} else {
this.typeToSelectString += event.key.toLowerCase();
}
for (const option of allOptions) {
const label = option.getTextLabel().toLowerCase();
if (label.startsWith(this.typeToSelectString)) {
this.setCurrentOption(option);
break;
}
}
}

Our upcoming combobox will follow ARIA APG as closely as possible, but this component intends to behave like a <select> and that seems to be working.

@KonnorRogers
Copy link
Collaborator

KonnorRogers commented Oct 29, 2024

@claviska Unfortunately <select> behaves as they describe.

If I have:

<select>
  <option>Adam</option>
  <option>Addison</option>
  <option>Amanda</option>
</select>

And I have the select focused + open and type "aaa" my focus will shift to "Amanda"

Now, this behavior is actually problematic.

Try this codepen in Chrome:

https://codepen.io/paramagicdev/pen/ZEgrPvv

My findings were it got stuck on the first two options and kept looping through them and never made it to the 3rd option. TBH, I never even knew this was the expected behavior until today, and the behavior is clearly broken in Safari and Chrome.

Its hard to tell from the video, but all I'm doing is opening the <select> and hitting the "a" key.

2024-10-28.23-52-10.remuxed.mp4

@arunks5
Copy link
Author

arunks5 commented Oct 29, 2024

@claviska @KonnorRogers yes, the native select works with the same behavior. I missed to document at the beginning. As this is closed now, do we have any plan to implement this in <sl-select> ?

@claviska
Copy link
Member

claviska commented Oct 30, 2024

Unfortunately <select> behaves as they describe. If I have ... And I have the select focused + open and type "aaa" my focus will shift to "Amanda"

Depends on the browser…cycling through the same letter is a Firefox-only thing :)

@KonnorRogers
Copy link
Collaborator

@claviska Chrome cycles too...but only when the <select> is closed 🙃

@claviska
Copy link
Member

Hmm. That makes a bit more sense.

@claviska claviska reopened this Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

No branches or pull requests

4 participants