refactor: move helper functions inside of useSelect hook

Author: thejackshelton

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

@@ -1,11 +1,11 @@
 import { $, Slot, component$, sync$, useContext, type PropsOf } from '@builder.io/qwik';
 import SelectContextId from './select-context';
-import { useTypeahead } from './use-select';
-import { getNextEnabledOptionIndex, getPrevEnabledOptionIndex } from './utils';
+import { useSelect, useTypeahead } from './use-select';
 
 type SelectTriggerProps = PropsOf<'button'>;
 export const SelectTrigger = component$<SelectTriggerProps>((props) => {
   const context = useContext(SelectContextId);
+  const { getNextEnabledOptionIndex, getPrevEnabledOptionIndex } = useSelect();
   const openKeys = ['ArrowUp', 'ArrowDown'];
   const closedKeys = [`Escape`];
 
@@ -32,7 +32,7 @@ export const SelectTrigger = component$<SelectTriggerProps>((props) => {
     }
   });
 
-  const handleKeyDown$ = $((e: KeyboardEvent) => {
+  const handleKeyDown$ = $(async (e: KeyboardEvent) => {
     typeahead$(e.key);
     const shouldOpen = !context.isListboxOpenSig.value && openKeys.includes(e.key);
     const shouldClose = context.isListboxOpenSig.value && closedKeys.includes(e.key);
@@ -47,15 +47,15 @@ export const SelectTrigger = component$<SelectTriggerProps>((props) => {
     }
 
     if (e.key === 'Home') {
-      context.highlightedIndexSig.value = getNextEnabledOptionIndex(
+      context.highlightedIndexSig.value = await getNextEnabledOptionIndex(
         -1,
         context.optionsSig.value,
         context.loop,
       );
     }
 
     if (e.key === 'End') {
-      const lastEnabledOptionIndex = getPrevEnabledOptionIndex(
+      const lastEnabledOptionIndex = await getPrevEnabledOptionIndex(
         context.optionsSig.value.length,
         context.optionsSig.value,
         context.loop,
@@ -65,7 +65,7 @@ export const SelectTrigger = component$<SelectTriggerProps>((props) => {
 
     if (!context.isListboxOpenSig.value) {
       if (e.key === 'ArrowRight' && context.highlightedIndexSig.value === null) {
-        context.selectedIndexSig.value = getNextEnabledOptionIndex(
+        context.selectedIndexSig.value = await getNextEnabledOptionIndex(
           -1,
           context.optionsSig.value,
           context.loop,
@@ -76,17 +76,19 @@ export const SelectTrigger = component$<SelectTriggerProps>((props) => {
       }
 
       if (e.key === 'ArrowRight' && context.highlightedIndexSig.value !== null) {
-        context.selectedIndexSig.value = getNextEnabledOptionIndex(
+        context.selectedIndexSig.value = await getNextEnabledOptionIndex(
           context.selectedIndexSig.value!,
           context.optionsSig.value,
           context.loop,
         );
 
+        console.log('selectedIndex', context.selectedIndexSig.value);
+
         context.highlightedIndexSig.value = context.selectedIndexSig.value;
       }
 
       if (e.key === 'ArrowLeft' && context.highlightedIndexSig.value === null) {
-        context.selectedIndexSig.value = getPrevEnabledOptionIndex(
+        context.selectedIndexSig.value = await getPrevEnabledOptionIndex(
           context.optionsSig.value.length,
           context.optionsSig.value,
           context.loop,
@@ -97,7 +99,7 @@ export const SelectTrigger = component$<SelectTriggerProps>((props) => {
       }
 
       if (e.key === 'ArrowLeft' && context.highlightedIndexSig.value !== null) {
-        context.selectedIndexSig.value = getPrevEnabledOptionIndex(
+        context.selectedIndexSig.value = await getPrevEnabledOptionIndex(
           context.highlightedIndexSig.value,
           context.optionsSig.value,
           context.loop,
@@ -109,7 +111,7 @@ export const SelectTrigger = component$<SelectTriggerProps>((props) => {
 
     /** When initially opening the listbox, we want to grab the first enabled option index */
     if (context.highlightedIndexSig.value === null) {
-      context.highlightedIndexSig.value = getNextEnabledOptionIndex(
+      context.highlightedIndexSig.value = await getNextEnabledOptionIndex(
         -1,
         context.optionsSig.value,
         context.loop,
@@ -128,15 +130,15 @@ export const SelectTrigger = component$<SelectTriggerProps>((props) => {
       }
 
       if (e.key === 'ArrowDown') {
-        context.highlightedIndexSig.value = getNextEnabledOptionIndex(
+        context.highlightedIndexSig.value = await getNextEnabledOptionIndex(
           context.highlightedIndexSig.value,
           context.optionsSig.value,
           context.loop,
         );
       }
 
       if (e.key === 'ArrowUp') {
-        context.highlightedIndexSig.value = getPrevEnabledOptionIndex(
+        context.highlightedIndexSig.value = await getPrevEnabledOptionIndex(
           context.highlightedIndexSig.value,
           context.optionsSig.value,
           context.loop,

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

@@ -444,7 +444,7 @@ test.describe('Keyboard Behavior', () => {
       await expect(getValueElement()).toHaveText(expectedValue!);
     });
 
-    test(`GIVEN an already selected item
+    test(`GIVEN no selected item and a placeholder
           WHEN pressing the right arrow key once
           THEN the first enabled option should be selected and have aria-selected`, async ({
       page,
@@ -463,24 +463,23 @@ test.describe('Keyboard Behavior', () => {
       await expect(getHiddenOptionAt(0)).toHaveAttribute('data-highlighted');
     });
 
-    test(`GIVEN an already selected item
+    test(`GIVEN no selected item and a placeholder
           WHEN pressing the right arrow key twice
           THEN the first enabled option should be selected and have aria-selected`, async ({
       page,
     }) => {
-      const { getTrigger, getHiddenOptionAt, getValueElement } = await setup(
-        page,
-        'hero',
-      );
+      const { driver: d } = await setup(page, 'hero');
 
-      const secondItemValue = await getHiddenOptionAt(1).textContent();
-      await getTrigger().focus();
-      await getTrigger().press('ArrowRight');
-      await getTrigger().press('ArrowRight');
+      const firstItemValue = await d.getHiddenOptionAt(0).textContent();
+      const secondItemValue = await d.getHiddenOptionAt(1).textContent();
 
-      expect(getValueElement()).toHaveText(secondItemValue!);
-      await expect(getHiddenOptionAt(1)).toHaveAttribute('aria-selected', 'true');
-      await expect(getHiddenOptionAt(1)).toHaveAttribute('data-highlighted');
+      await d.getTrigger().press('ArrowRight');
+      await expect(d.getValueElement()).toHaveText(firstItemValue!);
+      await d.getTrigger().press('ArrowRight');
+
+      await expect(d.getValueElement()).toHaveText(secondItemValue!);
+      await expect(d.getHiddenOptionAt(1)).toHaveAttribute('aria-selected', 'true');
+      await expect(d.getHiddenOptionAt(1)).toHaveAttribute('data-highlighted');
     });
 
     test(`GIVEN the second item is selected

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

@@ -1,5 +1,58 @@
 import { useContext, useSignal, $, useComputed$ } from '@builder.io/qwik';
 import SelectContextId from './select-context';
+import { Opt } from './select-inline';
+
+/**
+ * Helper functions go inside of hooks.
+ * This is because outside of the component$ boundary Qwik core wakes up.
+ */
+export function useSelect() {
+  const getNextEnabledOptionIndex = $((index: number, options: Opt[], loop: boolean) => {
+    let offset = 1;
+    const len = options.length;
+
+    if (!loop && index + 1 >= len) {
+      return index;
+    }
+
+    while (offset < len) {
+      const nextIndex = (index + offset) % len;
+      if (!options[nextIndex].isDisabled) {
+        return nextIndex;
+      }
+      offset++;
+      if (!loop && index + offset >= len) {
+        break;
+      }
+    }
+
+    return index;
+  });
+
+  const getPrevEnabledOptionIndex = $((index: number, options: Opt[], loop: boolean) => {
+    let offset = 1;
+    const len = options.length;
+
+    if (!loop && index - 1 < 0) {
+      return index;
+    }
+
+    while (offset <= len) {
+      const prevIndex = (index - offset + len) % len;
+      if (!options[prevIndex].isDisabled) {
+        return prevIndex;
+      }
+      offset++;
+      if (!loop && index - offset < 0) {
+        break;
+      }
+    }
+
+    return index;
+  });
+
+  return { getNextEnabledOptionIndex, getPrevEnabledOptionIndex };
+}
 
 export function useTypeahead() {
   const context = useContext(SelectContextId);

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

@@ -1,57 +1,5 @@
 import { type Opt } from './select-inline';
 
-export const getNextEnabledOptionIndex = (
-  index: number,
-  options: Opt[],
-  loop: boolean,
-) => {
-  let offset = 1;
-  const len = options.length;
-
-  if (!loop && index + 1 >= len) {
-    return index;
-  }
-
-  while (offset < len) {
-    const nextIndex = (index + offset) % len;
-    if (!options[nextIndex].isDisabled) {
-      return nextIndex;
-    }
-    offset++;
-    if (!loop && index + offset >= len) {
-      break;
-    }
-  }
-
-  return index;
-};
-
-export const getPrevEnabledOptionIndex = (
-  index: number,
-  options: Opt[],
-  loop: boolean,
-) => {
-  let offset = 1;
-  const len = options.length;
-
-  if (!loop && index - 1 < 0) {
-    return index;
-  }
-
-  while (offset <= len) {
-    const prevIndex = (index - offset + len) % len;
-    if (!options[prevIndex].isDisabled) {
-      return prevIndex;
-    }
-    offset++;
-    if (!loop && index - offset < 0) {
-      break;
-    }
-  }
-
-  return index;
-};
-
 export const getActiveDescendant = (index: number, options: Opt[], localId: string) => {
   const option = options[index];