Fix required double ArrowDown requirement (#1281)

* fix double arrow down requirement

If the `activeOptionIndex` is set to `null`, then we default to the very
first non-disabled option. This data is _not_ stored in state because if
you as the user go to a specific option, then start searching then we
will maintain the active option. This means that we have to **update**
the `activeOptionIndex` when options are moving around.

While making the first option the active one, we can't store that in
state directly otherwise the very first option becomes the active one.
If we then inject combobox options _before_ the current one then all of
a sudden your active option would jump around a bit.

We don't want this jumping to happen, we want the very first option to
be the one that's active no matter which option it is.

Since this is not stored in state, our keydown handler was a bit borked.
Internally it thinks we are still at `activeOptionIndex === null`
therefore pressing arrow down would move us to `activeOptionIndex ===
0`. To go to the second option, we can press down again which would move
us to `activeOptionIndex === 1`. The only issue is that visually we were
already at `0`.

This fixes that by making sure that if we have `activeOptionIndex ===
null` that we fallback to the very first non disabled option _before_ we
execute the `goToOption()` code.

 ### Before:

**Open combobox**, `activeOptionIndex === null`

| Combobox                |
| ----------------------- |
| **Option A** _(active)_ |
| Option B                |
| Option C                |

**Arrow Down**, `activeOptionIndex === 0`

| Combobox                |
| ----------------------- |
| **Option A** _(active)_ |
| Option B                |
| Option C                |

**Arrow Down**, `activeOptionIndex === 1`

| Combobox                |
| ----------------------- |
| Option A                |
| **Option B** _(active)_ |
| Option C                |

 ### After:

**Open combobox**, `activeOptionIndex === null`

| Combobox                |
| ----------------------- |
| **Option A** _(active)_ |
| Option B                |
| Option C                |

**Arrow Down**, `activeOptionIndex === 1`

| Combobox                |
| ----------------------- |
| Option A                |
| **Option B** _(active)_ |
| Option C                |

* update changelog

CHANGELOG.md

@@ -33,7 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Properly merge incoming props ([#1265](https://github.com/tailwindlabs/headlessui/pull/1265))
 - Fix incorrect closing while interacting with third party libraries in `Dialog` component ([#1268](https://github.com/tailwindlabs/headlessui/pull/1268))
 - Mimic browser select on focus when navigating via `Tab` ([#1272](https://github.com/tailwindlabs/headlessui/pull/1272))
-- Ensure that there is always an active option in the `Combobox` ([#1279](https://github.com/tailwindlabs/headlessui/pull/1279))
+- Ensure that there is always an active option in the `Combobox` ([#1279](https://github.com/tailwindlabs/headlessui/pull/1279), [#1281](https://github.com/tailwindlabs/headlessui/pull/1281))
 
 ### Added
 
@@ -69,7 +69,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix incorrect closing while interacting with third party libraries in `Dialog` component ([#1268](https://github.com/tailwindlabs/headlessui/pull/1268))
 - Mimic browser select on focus when navigating via `Tab` ([#1272](https://github.com/tailwindlabs/headlessui/pull/1272))
 - Resolve `initialFocusRef` correctly ([#1276](https://github.com/tailwindlabs/headlessui/pull/1276))
-- Ensure that there is always an active option in the `Combobox` ([#1279](https://github.com/tailwindlabs/headlessui/pull/1279))
+- Ensure that there is always an active option in the `Combobox` ([#1279](https://github.com/tailwindlabs/headlessui/pull/1279), [#1281](https://github.com/tailwindlabs/headlessui/pull/1281))
 
 ### Added
 

packages/@headlessui-react/src/components/combobox/combobox.test.tsx

@@ -615,9 +615,6 @@ describe('Rendering', () => {
 
     let options = getComboboxOptions()
 
-    // Focus the first item
-    await press(Keys.ArrowDown)
-
     // Verify that the first combobox option is active
     assertActiveComboboxOption(options[0])
 
@@ -666,6 +663,9 @@ describe('Rendering composition', () => {
 
       let options = getComboboxOptions()
 
+      // Verify that the first combobox option is active
+      assertActiveComboboxOption(options[0])
+
       // Verify correct classNames
       expect('' + options[0].classList).toEqual(
         JSON.stringify({ active: true, selected: false, disabled: false })
@@ -675,21 +675,6 @@ describe('Rendering composition', () => {
       )
       expect('' + options[2].classList).toEqual('no-special-treatment')
 
-      // Make the first option active
-      await press(Keys.ArrowDown)
-
-      // Verify the classNames
-      expect('' + options[0].classList).toEqual(
-        JSON.stringify({ active: true, selected: false, disabled: false })
-      )
-      expect('' + options[1].classList).toEqual(
-        JSON.stringify({ active: false, selected: false, disabled: true })
-      )
-      expect('' + options[2].classList).toEqual('no-special-treatment')
-
-      // Double check that the first option is the active one
-      assertActiveComboboxOption(options[0])
-
       // Let's go down, this should go to the third option since the second option is disabled!
       await press(Keys.ArrowDown)
 
@@ -1846,7 +1831,6 @@ describe('Keyboard interactions', () => {
 
           // Select the 2nd option
           await press(Keys.ArrowDown)
-          await press(Keys.ArrowDown)
 
           // Tab to the next DOM node
           await press(Keys.Tab)
@@ -1899,7 +1883,6 @@ describe('Keyboard interactions', () => {
 
           // Select the 2nd option
           await press(Keys.ArrowDown)
-          await press(Keys.ArrowDown)
 
           // Tab to the next DOM node
           await press(shift(Keys.Tab))
@@ -2274,17 +2257,14 @@ describe('Keyboard interactions', () => {
 
           // We should be able to go down once
           await press(Keys.ArrowDown)
-          assertActiveComboboxOption(options[0])
-
-          // We should be able to go down again
-          await press(Keys.ArrowDown)
           assertActiveComboboxOption(options[1])
 
           // We should be able to go down again
           await press(Keys.ArrowDown)
           assertActiveComboboxOption(options[2])
 
-          // We should NOT be able to go down again (because last option). Current implementation won't go around.
+          // We should NOT be able to go down again (because last option).
+          // Current implementation won't go around.
           await press(Keys.ArrowDown)
           assertActiveComboboxOption(options[2])
         })
@@ -2324,7 +2304,7 @@ describe('Keyboard interactions', () => {
 
           // We should be able to go down once
           await press(Keys.ArrowDown)
-          assertActiveComboboxOption(options[1])
+          assertActiveComboboxOption(options[2])
         })
       )
 
@@ -2367,6 +2347,43 @@ describe('Keyboard interactions', () => {
           assertActiveComboboxOption(options[2])
         })
       )
+
+      it(
+        'should be possible to go to the next item if no value is set',
+        suppressConsoleLogs(async () => {
+          render(
+            <Combobox value={null} onChange={console.log}>
+              <Combobox.Input onChange={NOOP} />
+              <Combobox.Button>Trigger</Combobox.Button>
+              <Combobox.Options>
+                <Combobox.Option value="a">Option A</Combobox.Option>
+                <Combobox.Option value="b">Option B</Combobox.Option>
+                <Combobox.Option value="c">Option C</Combobox.Option>
+              </Combobox.Options>
+            </Combobox>
+          )
+
+          assertComboboxButton({
+            state: ComboboxState.InvisibleUnmounted,
+            attributes: { id: 'headlessui-combobox-button-2' },
+          })
+          assertComboboxList({ state: ComboboxState.InvisibleUnmounted })
+
+          // Open combobox
+          await click(getComboboxButton())
+
+          let options = getComboboxOptions()
+
+          // Verify that we are on the first option
+          assertActiveComboboxOption(options[0])
+
+          // Go down once
+          await press(Keys.ArrowDown)
+
+          // We should be on the next item
+          assertActiveComboboxOption(options[1])
+        })
+      )
     })
 
     describe('`ArrowUp` key', () => {
@@ -3440,7 +3457,6 @@ describe('Keyboard interactions', () => {
 
           let options: ReturnType<typeof getComboboxOptions>
 
-          await press(Keys.ArrowDown)
           await press(Keys.ArrowDown)
 
           // Person B should be active

packages/@headlessui-react/src/components/combobox/combobox.tsx

@@ -191,6 +191,20 @@ let reducers: {
     }
 
     let adjustedState = adjustOrderedState(state)
+
+    // It's possible that the activeOptionIndex is set to `null` internally, but
+    // this means that we will fallback to the first non-disabled option by default.
+    // We have to take this into account.
+    if (adjustedState.activeOptionIndex === null) {
+      let localActiveOptionIndex = adjustedState.options.findIndex(
+        (option) => !option.dataRef.current.disabled
+      )
+
+      if (localActiveOptionIndex !== -1) {
+        adjustedState.activeOptionIndex = localActiveOptionIndex
+      }
+    }
+
     let activeOptionIndex = calculateActiveIndex(action, {
       resolveItems: () => adjustedState.options,
       resolveActiveIndex: () => adjustedState.activeOptionIndex,

packages/@headlessui-vue/src/components/combobox/combobox.test.ts

@@ -732,9 +732,6 @@ describe('Rendering', () => {
 
     let options = getComboboxOptions()
 
-    // Focus the first item
-    await press(Keys.ArrowDown)
-
     // Verify that the first combobox option is active
     assertActiveComboboxOption(options[0])
 
@@ -1987,7 +1984,6 @@ describe('Keyboard interactions', () => {
 
           // Select the 2nd option
           await press(Keys.ArrowDown)
-          await press(Keys.ArrowDown)
 
           // Tab to the next DOM node
           await press(Keys.Tab)
@@ -2035,7 +2031,6 @@ describe('Keyboard interactions', () => {
 
           // Select the 2nd option
           await press(Keys.ArrowDown)
-          await press(Keys.ArrowDown)
 
           // Tab to the next DOM node
           await press(shift(Keys.Tab))
@@ -2437,17 +2432,14 @@ describe('Keyboard interactions', () => {
 
           // We should be able to go down once
           await press(Keys.ArrowDown)
-          assertActiveComboboxOption(options[0])
-
-          // We should be able to go down again
-          await press(Keys.ArrowDown)
           assertActiveComboboxOption(options[1])
 
           // We should be able to go down again
           await press(Keys.ArrowDown)
           assertActiveComboboxOption(options[2])
 
-          // We should NOT be able to go down again (because last option). Current implementation won't go around.
+          // We should NOT be able to go down again (because last option).
+          // Current implementation won't go around.
           await press(Keys.ArrowDown)
           assertActiveComboboxOption(options[2])
         })
@@ -2488,7 +2480,7 @@ describe('Keyboard interactions', () => {
 
           // We should be able to go down once
           await press(Keys.ArrowDown)
-          assertActiveComboboxOption(options[1])
+          assertActiveComboboxOption(options[2])
         })
       )
 
@@ -2530,6 +2522,46 @@ describe('Keyboard interactions', () => {
           assertActiveComboboxOption(options[2])
         })
       )
+
+      it(
+        'should be possible to go to the next item if no value is set',
+        suppressConsoleLogs(async () => {
+          renderTemplate({
+            template: html`
+              <Combobox v-model="value">
+                <ComboboxInput />
+                <ComboboxButton>Trigger</ComboboxButton>
+                <ComboboxOptions>
+                  <ComboboxOption value="a">Option A</ComboboxOption>
+                  <ComboboxOption value="b">Option B</ComboboxOption>
+                  <ComboboxOption value="c">Option C</ComboboxOption>
+                </ComboboxOptions>
+              </Combobox>
+            `,
+            setup: () => ({ value: ref(null) }),
+          })
+
+          assertComboboxButton({
+            state: ComboboxState.InvisibleUnmounted,
+            attributes: { id: 'headlessui-combobox-button-2' },
+          })
+          assertComboboxList({ state: ComboboxState.InvisibleUnmounted })
+
+          // Open combobox
+          await click(getComboboxButton())
+
+          let options = getComboboxOptions()
+
+          // Verify that we are on the first option
+          assertActiveComboboxOption(options[0])
+
+          // Go down once
+          await press(Keys.ArrowDown)
+
+          // We should be on the next item
+          assertActiveComboboxOption(options[1])
+        })
+      )
     })
 
     describe('`ArrowUp` key', () => {
@@ -3631,7 +3663,6 @@ describe('Keyboard interactions', () => {
 
           let options: ReturnType<typeof getComboboxOptions>
 
-          await press(Keys.ArrowDown)
           await press(Keys.ArrowDown)
 
           // Person B should be active

packages/@headlessui-vue/src/components/combobox/combobox.ts

@@ -234,6 +234,20 @@ export let Combobox = defineComponent({
         }
 
         let adjustedState = adjustOrderedState()
+
+        // It's possible that the activeOptionIndex is set to `null` internally, but
+        // this means that we will fallback to the first non-disabled option by default.
+        // We have to take this into account.
+        if (adjustedState.activeOptionIndex === null) {
+          let localActiveOptionIndex = adjustedState.options.findIndex(
+            (option) => !option.dataRef.disabled
+          )
+
+          if (localActiveOptionIndex !== -1) {
+            adjustedState.activeOptionIndex = localActiveOptionIndex
+          }
+        }
+
         let nextActiveOptionIndex = calculateActiveIndex(
           focus === Focus.Specific
             ? { focus: Focus.Specific, id: id! }