Select focus navigation refactor

.changeset/olive-sheep-run.md

@@ -0,0 +1,5 @@
+---
+'@qwik-ui/headless': patch
+---
+
+refactor: improved select focus navigation

apps/website/src/routes/docs/headless/select/snippets/select.css

@@ -17,12 +17,18 @@
   display: flex;
   justify-content: center;
   align-items: center;
+  margin-top: 0.25rem;
 }
 
 .select-trigger:hover {
   background-color: hsla(var(--primary) / 0.08);
 }
 
+.select-trigger:focus-visible {
+  outline: 2px solid hsla(var(--primary) / 1);
+  outline-offset: 2px;
+}
+
 .select-popover {
   width: 100%;
   max-width: var(--select-width);

cla-signs/v1/cla.json

@@ -479,14 +479,6 @@
       "created_at": "2024-05-31T08:39:43Z",
       "repoId": 545159943,
       "pullRequestNo": 810
-    },
-    {
-      "name": "harshmangalam",
-      "id": 57381638,
-      "comment_id": 2145776407,
-      "created_at": "2024-06-03T17:40:03Z",
-      "repoId": 545159943,
-      "pullRequestNo": 820
     }
   ]
 }

packages/kit-headless/src/components/select/select-context.ts

@@ -14,6 +14,7 @@ export type SelectContext = {
   listboxRef: Signal<HTMLUListElement | undefined>;
   groupRef: Signal<HTMLDivElement | undefined>;
   labelRef: Signal<HTMLDivElement | undefined>;
+  highlightedItemRef: Signal<HTMLLIElement | undefined>;
 
   // core state
   itemsMapSig: Readonly<Signal<TItemsMap>>;

packages/kit-headless/src/components/select/select-item.tsx

@@ -8,6 +8,9 @@ import {
   useTask$,
   type PropsOf,
   useContextProvider,
+  sync$,
+  useOnWindow,
+  QRL,
 } from '@builder.io/qwik';
 import { isServer, isBrowser } from '@builder.io/qwik/build';
 import SelectContextId, {
@@ -34,28 +37,50 @@ export const HSelectItem = component$<SelectItemProps>((props) => {
   const itemRef = useSignal<HTMLLIElement>();
   const localIndexSig = useSignal<number | null>(null);
   const itemId = `${context.localId}-${_index}`;
+  const typeaheadFnSig = useSignal<QRL<(key: string) => Promise<void>>>();
 
-  const { selectionManager$ } = useSelect();
+  const { selectionManager$, getNextEnabledItemIndex$, getPrevEnabledItemIndex$ } =
+    useSelect();
+
+  // we're getting the same function instance from the trigger, without needing to restructure context
+  useOnWindow(
+    'typeaheadFn',
+    $((e: CustomEvent) => {
+      typeaheadFnSig.value = e.detail;
+    }),
+  );
 
   const isSelectedSig = useComputed$(() => {
     const index = _index ?? null;
     return !disabled && context.selectedIndexSetSig.value.has(index!);
   });
 
   const isHighlightedSig = useComputed$(() => {
-    return !disabled && context.highlightedIndexSig.value === _index;
+    if (disabled) return;
+
+    if (context.highlightedIndexSig.value === _index) {
+      itemRef.value?.focus();
+      return true;
+    } else {
+      return false;
+    }
   });
 
-  useTask$(function getIndexTask() {
+  useTask$(async function getIndexTask() {
     if (_index === undefined)
       throw Error('Qwik UI: Select component item cannot find its proper index.');
 
     localIndexSig.value = _index;
   });
 
-  useTask$(function scrollableTask({ track, cleanup }) {
+  useTask$(async function navigationTask({ track, cleanup }) {
     track(() => context.highlightedIndexSig.value);
 
+    // update the context with the highlighted item ref
+    if (localIndexSig.value === context.highlightedIndexSig.value) {
+      context.highlightedItemRef = itemRef;
+    }
+
     if (isServer) return;
 
     let observer: IntersectionObserver;
@@ -109,13 +134,109 @@ export const HSelectItem = component$<SelectItemProps>((props) => {
     isSelectedSig,
   };
 
+  const handleKeyDownSync$ = sync$((e: KeyboardEvent) => {
+    const keys = [
+      'ArrowUp',
+      'ArrowDown',
+      'ArrowRight',
+      'ArrowLeft',
+      'Home',
+      'End',
+      'PageDown',
+      'PageUp',
+      'Enter',
+      ' ',
+    ];
+    if (keys.includes(e.key)) {
+      e.preventDefault();
+    }
+  });
+
+  const handleKeyDown$ = $(async (e: KeyboardEvent) => {
+    typeaheadFnSig.value?.(e.key);
+
+    switch (e.key) {
+      case 'ArrowDown':
+        if (context.isListboxOpenSig.value) {
+          context.highlightedIndexSig.value = await getNextEnabledItemIndex$(
+            context.highlightedIndexSig.value!,
+          );
+          if (context.multiple && e.shiftKey) {
+            await selectionManager$(context.highlightedIndexSig.value, 'toggle');
+          }
+        }
+        break;
+
+      case 'ArrowUp':
+        if (context.isListboxOpenSig.value) {
+          context.highlightedIndexSig.value = await getPrevEnabledItemIndex$(
+            context.highlightedIndexSig.value!,
+          );
+          if (context.multiple && e.shiftKey) {
+            await selectionManager$(context.highlightedIndexSig.value, 'toggle');
+          }
+        }
+        break;
+
+      case 'Home':
+        if (context.isListboxOpenSig.value) {
+          context.highlightedIndexSig.value = await getNextEnabledItemIndex$(-1);
+        }
+        break;
+
+      case 'End':
+        if (context.isListboxOpenSig.value) {
+          const lastEnabledOptionIndex = await getPrevEnabledItemIndex$(
+            context.itemsMapSig.value.size,
+          );
+          context.highlightedIndexSig.value = lastEnabledOptionIndex;
+        }
+        break;
+
+      case 'Escape':
+        context.triggerRef.value?.focus();
+        context.isListboxOpenSig.value = false;
+        break;
+
+      case 'Tab':
+        context.isListboxOpenSig.value = false;
+        break;
+
+      case 'Enter':
+      case ' ':
+        if (context.isListboxOpenSig.value) {
+          const action = context.multiple ? 'toggle' : 'add';
+          await selectionManager$(context.highlightedIndexSig.value, action);
+
+          if (!context.multiple) {
+            context.triggerRef.value?.focus();
+          }
+        }
+        context.isListboxOpenSig.value = context.multiple
+          ? true
+          : !context.isListboxOpenSig.value;
+        break;
+
+      case 'a':
+        if (e.ctrlKey && context.multiple) {
+          for (const [index, item] of context.itemsMapSig.value) {
+            if (!item.disabled) {
+              await selectionManager$(index, 'add');
+            }
+          }
+        }
+        break;
+    }
+  });
+
   useContextProvider(selectItemContextId, selectContext);
 
   return (
     <li
       {...rest}
       id={itemId}
       onClick$={[handleClick$, props.onClick$]}
+      onKeyDown$={[handleKeyDownSync$, handleKeyDown$, props.onKeyDown$]}
       onPointerOver$={[handlePointerOver$, props.onPointerOver$]}
       ref={itemRef}
       tabIndex={-1}

packages/kit-headless/src/components/select/select-root.tsx

@@ -161,6 +161,7 @@ export const HSelectImpl = component$<SelectProps<boolean> & InternalSelectProps
     const currDisplayValueSig = useSignal<string | string[]>();
 
     const initialLoadSig = useSignal<boolean>(true);
+    const highlightedItemRef = useSignal<HTMLLIElement>();
 
     const context: SelectContext = {
       itemsMapSig,
@@ -170,6 +171,7 @@ export const HSelectImpl = component$<SelectProps<boolean> & InternalSelectProps
       listboxRef,
       labelRef,
       groupRef,
+      highlightedItemRef,
       localId,
       highlightedIndexSig,
       selectedIndexSetSig,

packages/kit-headless/src/components/select/select-trigger.tsx

@@ -1,4 +1,12 @@
-import { $, Slot, component$, sync$, useContext, type PropsOf } from '@builder.io/qwik';
+import {
+  $,
+  Slot,
+  component$,
+  sync$,
+  useContext,
+  useSignal,
+  type PropsOf,
+} from '@builder.io/qwik';
 import SelectContextId from './select-context';
 import { useSelect, useTypeahead } from './use-select';
 
@@ -9,7 +17,7 @@ export const HSelectTrigger = component$<SelectTriggerProps>((props) => {
     useSelect();
   const labelId = `${context.localId}-label`;
   const descriptionId = `${context.localId}-description`;
-
+  const initialKeyDownSig = useSignal(true);
   const { typeahead$ } = useTypeahead();
 
   const handleClickSync$ = sync$((e: MouseEvent) => {
@@ -42,66 +50,23 @@ export const HSelectTrigger = component$<SelectTriggerProps>((props) => {
   const handleKeyDown$ = $(async (e: KeyboardEvent) => {
     if (!context.itemsMapSig.value) return;
 
-    typeahead$(e.key);
+    if (!context.isListboxOpenSig.value) {
+      typeahead$(e.key);
+    }
 
     switch (e.key) {
-      case 'Enter':
-      case ' ':
-        if (context.isListboxOpenSig.value) {
-          const action = context.multiple ? 'toggle' : 'add';
-          await selectionManager$(context.highlightedIndexSig.value, action);
-        }
-        context.isListboxOpenSig.value = context.multiple
-          ? true
-          : !context.isListboxOpenSig.value;
+      case 'Tab':
+      case 'Escape':
+        context.isListboxOpenSig.value = false;
         break;
 
       case 'ArrowDown':
-        if (context.isListboxOpenSig.value) {
-          context.highlightedIndexSig.value = await getNextEnabledItemIndex$(
-            context.highlightedIndexSig.value!,
-          );
-          if (context.multiple && e.shiftKey) {
-            await selectionManager$(context.highlightedIndexSig.value, 'toggle');
-          }
-        } else {
-          context.isListboxOpenSig.value = true;
-        }
-        break;
-
       case 'ArrowUp':
-        if (context.isListboxOpenSig.value) {
-          context.highlightedIndexSig.value = await getPrevEnabledItemIndex$(
-            context.highlightedIndexSig.value!,
-          );
-          if (context.multiple && e.shiftKey) {
-            await selectionManager$(context.highlightedIndexSig.value, 'toggle');
-          }
-        } else {
+        if (!context.isListboxOpenSig.value) {
           context.isListboxOpenSig.value = true;
         }
         break;
 
-      case 'Home':
-        if (context.isListboxOpenSig.value) {
-          context.highlightedIndexSig.value = await getNextEnabledItemIndex$(-1);
-        }
-        break;
-
-      case 'End':
-        if (context.isListboxOpenSig.value) {
-          const lastEnabledOptionIndex = await getPrevEnabledItemIndex$(
-            context.itemsMapSig.value.size,
-          );
-          context.highlightedIndexSig.value = lastEnabledOptionIndex;
-        }
-        break;
-
-      case 'Tab':
-      case 'Escape':
-        context.isListboxOpenSig.value = false;
-        break;
-
       case 'ArrowRight':
         if (!context.multiple) {
           const currentIndex = context.highlightedIndexSig.value ?? -1;
@@ -121,22 +86,27 @@ export const HSelectTrigger = component$<SelectTriggerProps>((props) => {
         }
         break;
 
-      case 'a':
-        if (e.ctrlKey && context.multiple) {
-          for (const [index, item] of context.itemsMapSig.value) {
-            if (!item.disabled) {
-              await selectionManager$(index, 'add');
-            }
-          }
-        }
+      case 'Enter':
+      case ' ':
+        context.isListboxOpenSig.value = context.multiple
+          ? true
+          : !context.isListboxOpenSig.value;
         break;
     }
 
     /** When initially opening the listbox, we want to grab the first enabled option index */
     if (context.highlightedIndexSig.value === null) {
       context.highlightedIndexSig.value = await getNextEnabledItemIndex$(-1);
-      return;
     }
+
+    // Wait for the popover code to be executed
+    while (context.highlightedItemRef.value !== document.activeElement) {
+      await new Promise((resolve) => setTimeout(resolve, 5));
+      context.highlightedItemRef.value?.focus();
+    }
+
+    if (!initialKeyDownSig.value) return;
+    document.dispatchEvent(new CustomEvent('typeaheadFn', { detail: typeahead$ }));
   });
 
   return (

packages/kit-headless/src/components/select/select.driver.ts

@@ -35,6 +35,10 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
     return getTrigger().locator('[data-value]');
   };
 
+  const getHighlightedItem = () => {
+    return getRoot().locator('[data-highlighted]');
+  };
+
   const openListbox = async (key: OpenKeys | 'click') => {
     await getTrigger().focus();
 
@@ -59,5 +63,6 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
     getItemAt,
     getValueElement,
     openListbox,
+    getHighlightedItem,
   };
 }