diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d2601a..209d7f6 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index b7a3920..b5a2d70 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/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( + + + Trigger + + Option A + Option B + Option C + + + ) + + 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 - await press(Keys.ArrowDown) await press(Keys.ArrowDown) // Person B should be active diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index aac2bf7..d277298 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/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, diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 55d6e11..427fcae 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/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` + + + Trigger + + Option A + Option B + Option C + + + `, + 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 - await press(Keys.ArrowDown) await press(Keys.ArrowDown) // Person B should be active diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index a90b3a3..d807388 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/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! }