From b7bf0864ed7fd9716b5d5e1272de292ddbc20bce Mon Sep 17 00:00:00 2001 From: jack shelton Date: Fri, 20 Sep 2024 11:05:50 -0500 Subject: [PATCH 01/31] initial tests --- .../src/components/combobox/combobox.test.ts | 48 ++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/combobox/combobox.test.ts b/packages/kit-headless/src/components/combobox/combobox.test.ts index f76d8a8fc..80cee0898 100644 --- a/packages/kit-headless/src/components/combobox/combobox.test.ts +++ b/packages/kit-headless/src/components/combobox/combobox.test.ts @@ -61,7 +61,7 @@ test.describe('Mouse Behavior', () => { await expect(d.getItemAt(1)).toHaveAttribute('aria-selected', 'true'); }); - test(`GIVEN a combobox with an open listbox + test(`GIVEN a combobox with an open listbox WHEN the 3rd option is clicked THEN the 3rd option should be the selected value`, async ({ page }) => { const { driver: d } = await setup(page, 'hero'); @@ -587,6 +587,52 @@ test.describe('Props', () => { await expect(d.getListbox()).toBeVisible(); }); + + test(`GIVEN a reactive combobox + WHEN the user empties the input with selecting all text and backspace + THEN there should be no selected value`, async ({ page }) => { + const { driver: d } = await setup(page, 'reactive'); + + const apricot = (await d.getItemAt(1).textContent()) ?? ''; + + await expect(d.getInput()).toHaveValue(apricot); + + await d.getInput().clear(); + + await expect(d.getInput()).toHaveValue(''); + + await expect( + page.getByRole('paragraph', { name: 'Your favorite fruit is:' }), + ).toBeVisible(); + }); + + test(`GIVEN a reactive combobox + WHEN the user empties the input and selects a new value + THEN that should update the given signal`, async ({ page }) => { + const { driver: d } = await setup(page, 'reactive'); + + const apricot = (await d.getItemAt(1).textContent()) ?? ''; + + await expect(d.getInput()).toHaveValue(apricot); + + await d.getInput().clear(); + + await expect(d.getInput()).toHaveValue(''); + + await expect( + page.getByRole('paragraph', { name: 'Your favorite fruit is:' }), + ).toBeVisible(); + + await d.openListbox('click'); + + await d.getItemAt(1).click(); + + await expect(d.getInput()).toHaveValue('Apricot'); + + await expect( + page.getByRole('paragraph', { name: 'Your favorite fruit is: Apricot' }), + ).toBeVisible(); + }); }); test.describe('option value', () => { From 56613a9dcb769a014c2cc561f3b7fc50fde56572 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 08:08:29 -0500 Subject: [PATCH 02/31] get values with arrays instead --- .../components/combobox/combobox-context.tsx | 1 + .../src/components/combobox/combobox-root.tsx | 45 ++++++------------- .../src/components/combobox/use-combobox.tsx | 45 +++++++++---------- 3 files changed, 36 insertions(+), 55 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-context.tsx b/packages/kit-headless/src/components/combobox/combobox-context.tsx index aff81076c..dcc206c36 100644 --- a/packages/kit-headless/src/components/combobox/combobox-context.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-context.tsx @@ -18,6 +18,7 @@ export type ComboboxContext = { labelRef: Signal; controlRef: Signal; selectedValueSetSig: Signal>; + selectedValuesSig: Signal; highlightedIndexSig: Signal; currDisplayValueSig: Signal; isMouseOverPopupSig: Signal; diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 6e8a03477..79007d3d1 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -14,6 +14,7 @@ import { ComboboxContext, comboboxContextId } from './combobox-context'; import { InternalComboboxProps } from './combobox-inline'; import { useCombobox } from './use-combobox'; import { useCombinedRef } from '../../hooks/combined-refs'; +import { useBoundSignal } from '../../utils/bound-signal'; export type TMultiple = M extends true ? string[] : string; @@ -123,6 +124,7 @@ export const HComboboxRootImpl = component$< onInput$, onChange$, onOpenChange$, + 'bind:value': givenValueSig, _valuePropIndex: givenValuePropIndex, _value, loop: givenLoop, @@ -155,6 +157,10 @@ export const HComboboxRootImpl = component$< const localId = useId(); /** We use selected values to preserve the item state when the consumer filters the items. */ const selectedValueSetSig = useSignal>(new Set(_value ? [_value] : [])); + const selectedValuesSig = useBoundSignal( + givenValueSig, + multiple ? (_value ? [_value] : []) : _value || '', + ); const disabledIndexSetSig = useSignal>(new Set()); const isMouseOverPopupSig = useSignal(false); const isDisabledSig = useSignal(disabled ?? false); @@ -210,6 +216,7 @@ export const HComboboxRootImpl = component$< controlRef, localId, highlightedIndexSig, + selectedValuesSig, selectedValueSetSig, disabledIndexSetSig, currDisplayValueSig, @@ -234,12 +241,11 @@ export const HComboboxRootImpl = component$< const { selectionManager$ } = useCombobox(); useTask$(async function reactiveUserValue({ track }) { - const bindValueSig = props['bind:value']; - if (!bindValueSig) return; - track(() => bindValueSig.value); + track(() => selectedValuesSig.value); for (const [index, item] of itemsMapSig.value) { - if (bindValueSig.value?.includes(item.value)) { + if (selectedValuesSig.value.includes(item.value)) { + console.log('run reactive user value'); await selectionManager$(index, 'add'); if (initialLoadSig.value) { @@ -283,11 +289,9 @@ export const HComboboxRootImpl = component$< }); useTask$(async function updateConsumerProps({ track }) { - const bindValueSig = props['bind:value']; - const bindDisplayTextSig = props['bind:displayValue']; - track(() => selectedValueSetSig.value); + track(() => selectedValuesSig.value); - const values = Array.from(selectedValueSetSig.value); + const values = selectedValuesSig.value; const displayValues = []; for (const value of values) { @@ -299,32 +303,9 @@ export const HComboboxRootImpl = component$< } } - if (onChange$ && selectedValueSetSig.value.size > 0) { + if (onChange$ && selectedValuesSig.value.length > 0) { await onChange$(context.multiple ? values : values[0]); } - - // sync the user's given signal when an option is selected - if (bindValueSig && bindValueSig.value) { - const currUserSigValues = JSON.stringify(bindValueSig.value); - const newUserSigValues = JSON.stringify(values); - - if (currUserSigValues !== newUserSigValues) { - if (context.multiple) { - bindValueSig.value = values; - } else { - bindValueSig.value = values[0]; - } - } - } - - context.currDisplayValueSig.value = displayValues; - - // sync the user's given signal for the display value - if (bindDisplayTextSig && context.currDisplayValueSig.value) { - bindDisplayTextSig.value = context.multiple - ? context.currDisplayValueSig.value - : context.currDisplayValueSig.value[0]; - } }); useTask$(({ track }) => { diff --git a/packages/kit-headless/src/components/combobox/use-combobox.tsx b/packages/kit-headless/src/components/combobox/use-combobox.tsx index 3a970b7eb..f8f85cac2 100644 --- a/packages/kit-headless/src/components/combobox/use-combobox.tsx +++ b/packages/kit-headless/src/components/combobox/use-combobox.tsx @@ -16,47 +16,46 @@ export function useCombobox() { ); } + const currentValue = context.selectedValuesSig.value; + const isArray = Array.isArray(currentValue); + if (action === 'add') { if (context.multiple) { - context.selectedValueSetSig.value = new Set([ - ...context.selectedValueSetSig.value, - value, - ]); + context.selectedValuesSig.value = isArray ? [...currentValue, value] : [value]; } else { - context.selectedValueSetSig.value = new Set([value]); + context.selectedValuesSig.value = value; } } + if (action === 'toggle') { - if (context.selectedValueSetSig.value.has(value)) { - context.selectedValueSetSig.value = new Set( - [...context.selectedValueSetSig.value].filter( - (selectedValue) => selectedValue !== value, - ), - ); - } else { - if (context.multiple) { - context.selectedValueSetSig.value = new Set([ - ...context.selectedValueSetSig.value, - value, - ]); + if (context.multiple) { + if (isArray) { + context.selectedValuesSig.value = currentValue.includes(value) + ? currentValue.filter((selectedValue) => selectedValue !== value) + : [...currentValue, value]; } else { - context.selectedValueSetSig.value = new Set([value]); + context.selectedValuesSig.value = [value]; } + } else { + context.selectedValuesSig.value = currentValue === value ? '' : value; } } + if (action === 'remove') { - context.selectedValueSetSig.value = new Set( - [...context.selectedValueSetSig.value].filter( + if (context.multiple && isArray) { + context.selectedValuesSig.value = currentValue.filter( (selectedValue) => selectedValue !== value, - ), - ); + ); + } else { + context.selectedValuesSig.value = ''; + } } if (action === 'add' || action === 'toggle') { if (!context.inputRef.value) return; if (!selectedDisplayValue) return; - if (!context.multiple && context.selectedValueSetSig.value.has(value)) { + if (!context.multiple && context.selectedValuesSig.value === value) { context.inputRef.value.value = selectedDisplayValue; } } From f3f9d0e34831b020ce19c6611dc768bbeea43153 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 08:26:59 -0500 Subject: [PATCH 03/31] reactively update --- .../docs/headless/combobox/examples/reactive.tsx | 8 +++++++- .../src/components/combobox/combobox-item.tsx | 8 +++++++- .../src/components/combobox/combobox-root.tsx | 11 ++++++----- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/apps/website/src/routes/docs/headless/combobox/examples/reactive.tsx b/apps/website/src/routes/docs/headless/combobox/examples/reactive.tsx index 46e9b245d..2e53c4702 100644 --- a/apps/website/src/routes/docs/headless/combobox/examples/reactive.tsx +++ b/apps/website/src/routes/docs/headless/combobox/examples/reactive.tsx @@ -1,4 +1,4 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +import { component$, useSignal, useStyles$, useTask$ } from '@builder.io/qwik'; import { Combobox } from '@qwik-ui/headless'; import { LuCheck, LuChevronDown } from '@qwikest/icons/lucide'; @@ -18,6 +18,12 @@ export default component$(() => { 'Coconut', ]; + useTask$(({ track }) => { + track(() => selectedFruit.value); + + console.log(selectedFruit.value); + }); + return ( <> diff --git a/packages/kit-headless/src/components/combobox/combobox-item.tsx b/packages/kit-headless/src/components/combobox/combobox-item.tsx index ddb4f2a0d..c97078209 100644 --- a/packages/kit-headless/src/components/combobox/combobox-item.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-item.tsx @@ -44,7 +44,13 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { const isSelectedSig = useComputed$(() => { const index = props._index ?? -1; const selectedValue = context.itemsMapSig.value.get(index)?.value; - return !isDisabledSig.value && context.selectedValueSetSig.value.has(selectedValue!); + if (!selectedValue || isDisabledSig.value) return false; + + if (Array.isArray(context.selectedValuesSig.value)) { + return context.selectedValuesSig.value.includes(selectedValue); + } else { + return context.selectedValuesSig.value === selectedValue; + } }); const isHighlightedSig = useComputed$(() => { diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 79007d3d1..76527042b 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -161,6 +161,7 @@ export const HComboboxRootImpl = component$< givenValueSig, multiple ? (_value ? [_value] : []) : _value || '', ); + const disabledIndexSetSig = useSignal>(new Set()); const isMouseOverPopupSig = useSignal(false); const isDisabledSig = useSignal(disabled ?? false); @@ -244,16 +245,16 @@ export const HComboboxRootImpl = component$< track(() => selectedValuesSig.value); for (const [index, item] of itemsMapSig.value) { - if (selectedValuesSig.value.includes(item.value)) { - console.log('run reactive user value'); + if ( + selectedValuesSig.value.includes(item.value) || + selectedValuesSig.value === item.value + ) { await selectionManager$(index, 'add'); if (initialLoadSig.value) { // for both SSR and CSR, we need to set the initial index context.highlightedIndexSig.value = index; } - } else { - await selectionManager$(index, 'remove'); } } }); @@ -304,7 +305,7 @@ export const HComboboxRootImpl = component$< } if (onChange$ && selectedValuesSig.value.length > 0) { - await onChange$(context.multiple ? values : values[0]); + await onChange$(values); } }); From 6b22b90a2b5514de25605df85fe2c7eb376043a0 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 08:57:15 -0500 Subject: [PATCH 04/31] make selection manager cleaner --- .../src/components/combobox/use-combobox.tsx | 93 +++++++++++-------- 1 file changed, 52 insertions(+), 41 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/use-combobox.tsx b/packages/kit-headless/src/components/combobox/use-combobox.tsx index f8f85cac2..8799cb98f 100644 --- a/packages/kit-headless/src/components/combobox/use-combobox.tsx +++ b/packages/kit-headless/src/components/combobox/use-combobox.tsx @@ -1,13 +1,46 @@ import { useContext, $, Signal } from '@builder.io/qwik'; import { comboboxContextId } from './combobox-context'; +class ValueManager { + constructor( + private isMultiple: boolean, + private initialValue: T | T[], + ) {} + + add(value: T): T | T[] { + if (this.isMultiple) { + const currentArray = Array.isArray(this.initialValue) ? [...this.initialValue] : []; + return [...currentArray, value]; + } + return value; + } + + remove(value: T): T | T[] { + if (this.isMultiple && Array.isArray(this.initialValue)) { + return this.initialValue.filter((v) => v !== value); + } + return '' as T; + } + + toggle(value: T): T | T[] { + if (this.isMultiple) { + const currentArray = Array.isArray(this.initialValue) ? [...this.initialValue] : []; + return currentArray.includes(value) + ? currentArray.filter((v) => v !== value) + : [...currentArray, value]; + } + return this.initialValue === value ? ('' as T) : value; + } +} + export function useCombobox() { const context = useContext(comboboxContextId); + const selectionManager$ = $( async (index: number | null, action: 'add' | 'toggle' | 'remove') => { if (index === null) return; - const selectedDisplayValue = context.itemsMapSig.value.get(index)?.displayValue; + const selectedDisplayValue = context.itemsMapSig.value.get(index)?.displayValue; const value = context.itemsMapSig.value.get(index)?.value; if (!value) { @@ -16,48 +49,26 @@ export function useCombobox() { ); } - const currentValue = context.selectedValuesSig.value; - const isArray = Array.isArray(currentValue); - - if (action === 'add') { - if (context.multiple) { - context.selectedValuesSig.value = isArray ? [...currentValue, value] : [value]; - } else { - context.selectedValuesSig.value = value; - } - } - - if (action === 'toggle') { - if (context.multiple) { - if (isArray) { - context.selectedValuesSig.value = currentValue.includes(value) - ? currentValue.filter((selectedValue) => selectedValue !== value) - : [...currentValue, value]; - } else { - context.selectedValuesSig.value = [value]; - } - } else { - context.selectedValuesSig.value = currentValue === value ? '' : value; - } + const valueManager = new ValueManager( + context.multiple ?? false, + context.selectedValuesSig.value, + ); + + switch (action) { + case 'add': + context.selectedValuesSig.value = valueManager.add(value); + break; + case 'remove': + context.selectedValuesSig.value = valueManager.remove(value); + return; // Early return for 'remove' action + case 'toggle': + context.selectedValuesSig.value = valueManager.toggle(value); + break; } - if (action === 'remove') { - if (context.multiple && isArray) { - context.selectedValuesSig.value = currentValue.filter( - (selectedValue) => selectedValue !== value, - ); - } else { - context.selectedValuesSig.value = ''; - } - } - - if (action === 'add' || action === 'toggle') { - if (!context.inputRef.value) return; - if (!selectedDisplayValue) return; - - if (!context.multiple && context.selectedValuesSig.value === value) { - context.inputRef.value.value = selectedDisplayValue; - } + // Update input value for single-select combobox + if (!context.multiple && context.inputRef.value && selectedDisplayValue) { + context.inputRef.value.value = selectedDisplayValue; } }, ); From 160886d49987ebb8148ee5cb9d77c280029d5c1f Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 09:35:02 -0500 Subject: [PATCH 05/31] remove unnecessary addition --- .../src/components/combobox/combobox-root.tsx | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 76527042b..7b2a35fe4 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -12,7 +12,6 @@ import { } from '@builder.io/qwik'; import { ComboboxContext, comboboxContextId } from './combobox-context'; import { InternalComboboxProps } from './combobox-inline'; -import { useCombobox } from './use-combobox'; import { useCombinedRef } from '../../hooks/combined-refs'; import { useBoundSignal } from '../../utils/bound-signal'; @@ -239,26 +238,6 @@ export const HComboboxRootImpl = component$< useContextProvider(comboboxContextId, context); - const { selectionManager$ } = useCombobox(); - - useTask$(async function reactiveUserValue({ track }) { - track(() => selectedValuesSig.value); - - for (const [index, item] of itemsMapSig.value) { - if ( - selectedValuesSig.value.includes(item.value) || - selectedValuesSig.value === item.value - ) { - await selectionManager$(index, 'add'); - - if (initialLoadSig.value) { - // for both SSR and CSR, we need to set the initial index - context.highlightedIndexSig.value = index; - } - } - } - }); - useTask$(function reactiveUserOpen({ track }) { const bindOpenSig = props['bind:open']; if (!bindOpenSig) return; From d244a18b4490c2af93e5e2ff640e1aef8ec5bc19 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 09:45:49 -0500 Subject: [PATCH 06/31] fix: onChange$ only executes on client & simpler disabled state --- .../src/components/combobox/combobox-root.tsx | 55 +++++-------------- 1 file changed, 14 insertions(+), 41 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 7b2a35fe4..16dee10cf 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -118,12 +118,12 @@ export type HComboboxRootImplProps = Omit< export const HComboboxRootImpl = component$< HComboboxRootImplProps & InternalComboboxProps >((props: HComboboxRootImplProps & InternalComboboxProps) => { - const isListboxOpenSig = useSignal(false); const { onInput$, onChange$, onOpenChange$, 'bind:value': givenValueSig, + 'bind:open': givenOpenSig, _valuePropIndex: givenValuePropIndex, _value, loop: givenLoop, @@ -137,11 +137,16 @@ export const HComboboxRootImpl = component$< removeOnBackspace = false, name, required, - disabled, ...rest } = props; // source of truth + const isListboxOpenSig = useBoundSignal(givenOpenSig, false); + const selectedValuesSig = useBoundSignal( + givenValueSig, + multiple ? (_value ? [_value] : []) : _value || '', + ); + const itemsMapSig = useComputed$(() => { return props._itemsMap ?? new Map(); }); @@ -156,14 +161,9 @@ export const HComboboxRootImpl = component$< const localId = useId(); /** We use selected values to preserve the item state when the consumer filters the items. */ const selectedValueSetSig = useSignal>(new Set(_value ? [_value] : [])); - const selectedValuesSig = useBoundSignal( - givenValueSig, - multiple ? (_value ? [_value] : []) : _value || '', - ); const disabledIndexSetSig = useSignal>(new Set()); const isMouseOverPopupSig = useSignal(false); - const isDisabledSig = useSignal(disabled ?? false); const highlightedIndexSig = useSignal(givenValuePropIndex ?? null); const initialLoadSig = useSignal(true); const currDisplayValueSig = useSignal(); @@ -174,6 +174,7 @@ export const HComboboxRootImpl = component$< }; const inputValueSig = useSignal(inputRef.value?.value ?? ''); const isInvalidSig = useSignal(hasErrorComp ?? false); + const isDisabledSig = useComputed$(() => props.disabled ?? false); // check any initial disabled items before the computed read below useTask$(() => { @@ -238,29 +239,15 @@ export const HComboboxRootImpl = component$< useContextProvider(comboboxContextId, context); - useTask$(function reactiveUserOpen({ track }) { - const bindOpenSig = props['bind:open']; - if (!bindOpenSig) return; - track(() => bindOpenSig.value); - - isListboxOpenSig.value = bindOpenSig.value ?? isListboxOpenSig.value; - }); - - useTask$(function onOpenChangeTask({ track }) { - const bindOpenSig = props['bind:open']; + useTask$(function handleOpenChange({ track }) { track(() => isListboxOpenSig.value); if (!initialLoadSig.value) { onOpenChange$?.(isListboxOpenSig.value); } - - // sync the user's given signal for the open state - if (bindOpenSig && bindOpenSig.value !== isListboxOpenSig.value) { - bindOpenSig.value = isListboxOpenSig.value; - } }); - useTask$(function onInputTask({ track }) { + useTask$(function handleInput({ track }) { track(() => context.inputValueSig.value); if (!initialLoadSig.value) { @@ -268,30 +255,16 @@ export const HComboboxRootImpl = component$< } }); - useTask$(async function updateConsumerProps({ track }) { + useTask$(async function handleChange({ track }) { track(() => selectedValuesSig.value); - const values = selectedValuesSig.value; - const displayValues = []; - - for (const value of values) { - for (const item of context.itemsMapSig.value.values()) { - if (item.value === value) { - displayValues.push(item.displayValue); - break; - } - } - } + if (!onChange$) return; - if (onChange$ && selectedValuesSig.value.length > 0) { - await onChange$(values); + if (!initialLoadSig.value && selectedValuesSig.value.length > 0) { + await onChange$(selectedValuesSig.value); } }); - useTask$(({ track }) => { - isDisabledSig.value = track(() => disabled ?? false); - }); - useTask$(() => { initialLoadSig.value = false; }); From e321b55ebc1fade8dae7745ef403ef993c254c8b Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 09:47:39 -0500 Subject: [PATCH 07/31] feat: simpler invalid check --- .../kit-headless/src/components/combobox/combobox-root.tsx | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 16dee10cf..4b791d3ca 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -133,7 +133,6 @@ export const HComboboxRootImpl = component$< filter = true, _itemsMap, hasEmptyComp, - hasErrorComp, removeOnBackspace = false, name, required, @@ -173,8 +172,8 @@ export const HComboboxRootImpl = component$< inline: 'nearest', }; const inputValueSig = useSignal(inputRef.value?.value ?? ''); - const isInvalidSig = useSignal(hasErrorComp ?? false); const isDisabledSig = useComputed$(() => props.disabled ?? false); + const isInvalidSig = useComputed$(() => props.hasErrorComp ?? false); // check any initial disabled items before the computed read below useTask$(() => { @@ -188,10 +187,6 @@ export const HComboboxRootImpl = component$< disabledIndexSetSig.value = disabledIndices; }); - useTask$(({ track }) => { - isInvalidSig.value = track(() => props.hasErrorComp ?? false); - }); - const hasVisibleItemsSig = useComputed$(() => { return itemsMapSig.value.size !== disabledIndexSetSig.value.size; }); From d1be585cde8277e8f4334a337df3c4065296dd7a Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 09:48:29 -0500 Subject: [PATCH 08/31] remove early return --- .../kit-headless/src/components/combobox/combobox-root.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 4b791d3ca..b504f0361 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -253,10 +253,8 @@ export const HComboboxRootImpl = component$< useTask$(async function handleChange({ track }) { track(() => selectedValuesSig.value); - if (!onChange$) return; - if (!initialLoadSig.value && selectedValuesSig.value.length > 0) { - await onChange$(selectedValuesSig.value); + await onChange$?.(selectedValuesSig.value); } }); From 5bffa49397ff0d36b098c60c39e2b2df5da9f8c5 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 10:15:42 -0500 Subject: [PATCH 09/31] feat: simplify inline component --- .../components/combobox/combobox-inline.tsx | 148 +++++++----------- 1 file changed, 55 insertions(+), 93 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-inline.tsx b/packages/kit-headless/src/components/combobox/combobox-inline.tsx index ac8e604d4..38a055ea4 100644 --- a/packages/kit-headless/src/components/combobox/combobox-inline.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-inline.tsx @@ -1,9 +1,10 @@ -import { type JSXNode, Component } from '@builder.io/qwik'; +import { Component, JSXChildren } from '@builder.io/qwik'; import { HComboboxRootImpl, HComboboxRootImplProps } from './combobox-root'; import { HComboboxItem as InternalComboboxItem } from './combobox-item'; import { HComboboxItemLabel as InternalComboboxItemLabel } from './combobox-item-label'; import { HComboboxEmpty as InternalComboboxEmpty } from './combobox-empty'; import { HComboboxErrorMessage } from './combobox-error-message'; +import { findComponent, processChildren } from '../../utils/inline-component'; export type TItemsMap = Map< number, @@ -35,7 +36,7 @@ export const HComboboxRoot: Component { const { - children: myChildren, + children, comboboxItemComponent: UserItem, comboboxItemLabelComponent: UserItemLabel, ...rest @@ -49,108 +50,69 @@ export const HComboboxRoot: Component; + findComponent(HComboboxItem, (itemProps) => { + itemProps._index = currItemIndex; - while (childrenToProcess.length) { - const child = childrenToProcess.shift(); + isItemDisabled = itemProps.disabled === true; - if (!child) { - continue; + if (itemProps.value) { + givenItemValue = itemProps.value as string; } - if (Array.isArray(child)) { - childrenToProcess.unshift(...child); - continue; + // the default case isn't handled here, so we need to process the children to get to the label component + if (itemProps.children) { + return processChildren(itemProps.children as JSXChildren); } + }); - switch (child.type) { - case HComboboxItem: { - // get the index of the current option - child.props._index = currItemIndex; - - isItemDisabled = child.props.disabled === true; - - if (child.props.value) { - givenItemValue = child.props.value; - } - - // the default case isn't handled here, so we need to process the children to get to the label component - if (child.props.children) { - const childChildren = Array.isArray(child.props.children) - ? [...child.props.children] - : [child.props.children]; - childrenToProcess.unshift(...childChildren); - } - - break; - } - - case HComboboxItemLabel: { - const displayValue = child.props.children as string; - - // distinct value, or the display value is the same as the value - const value = (givenItemValue !== null ? givenItemValue : displayValue) as string; - - itemsMap.set(currItemIndex, { value, displayValue, disabled: isItemDisabled }); - - if (props.value && props.multiple) { - throw new Error( - `Qwik UI: When in multiple selection mode, the value prop is disabled. Use the bind:value prop's initial signal value instead.`, - ); - } - - // if the current option value is equal to the initial value - if (value === props.value) { - // minus one because it is incremented already in SelectOption - valuePropIndex = currItemIndex; - _value = value; - } - - const isString = typeof child.props.children === 'string'; - - if (!isString) { - throw new Error( - `Qwik UI: select item label passed was not a string. It was a ${typeof child - .props.children}.`, - ); - } - - // increment after processing children - currItemIndex++; - - break; - } - - case HComboboxEmpty: { - hasEmptyComp = true; - break; - } - - case HComboboxErrorMessage: { - hasErrorComp = true; - break; - } - - default: { - if (child) { - const anyChildren = Array.isArray(child.children) - ? [...child.children] - : [child.children]; - childrenToProcess.unshift(...(anyChildren as JSXNode[])); - } - - break; - } + findComponent(HComboboxItemLabel, (labelProps) => { + const displayValue = labelProps.children as string; + + // distinct value, or the display value is the same as the value + const value = (givenItemValue !== null ? givenItemValue : displayValue) as string; + + itemsMap.set(currItemIndex, { value, displayValue, disabled: isItemDisabled }); + + if (props.value && props.multiple) { + throw new Error( + `Qwik UI: When in multiple selection mode, the value prop is disabled. Use the bind:value prop's initial signal value instead.`, + ); + } + + // if the current option value is equal to the initial value + if (value === props.value) { + // minus one because it is incremented already in SelectOption + valuePropIndex = currItemIndex; + _value = value; + } + + const isString = typeof labelProps.children === 'string'; + + if (!isString) { + throw new Error( + `Qwik UI: select item label passed was not a string. It was a ${typeof labelProps.children}.`, + ); } - } + + // increment after processing children + currItemIndex++; + }); + + findComponent(HComboboxEmpty, () => { + hasEmptyComp = true; + }); + + findComponent(HComboboxErrorMessage, () => { + hasErrorComp = true; + }); + + processChildren(children); return ( - {props.children} + {children} ); }; From ed7b86f9bcdcaced981e6e9ecfee40bec1082601 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 10:21:06 -0500 Subject: [PATCH 10/31] simpler initial value naming --- .../components/combobox/combobox-inline.tsx | 21 ++++++++++++------- .../src/components/combobox/combobox-root.tsx | 14 +++++++------ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-inline.tsx b/packages/kit-headless/src/components/combobox/combobox-inline.tsx index 38a055ea4..2ca5a5c7f 100644 --- a/packages/kit-headless/src/components/combobox/combobox-inline.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-inline.tsx @@ -14,8 +14,8 @@ export type TItemsMap = Map< export type InternalComboboxProps = { /** When a value is passed, we check if it's an actual item value, and get its index at pre-render time. **/ - _valuePropIndex?: number | null; - _value?: string; + initialIndex?: number | null; + initialValue?: string; /** Checks if the consumer added the label in their JSX */ _label?: boolean; @@ -48,11 +48,16 @@ export const HComboboxRoot: Component { @@ -159,11 +159,13 @@ export const HComboboxRootImpl = component$< const loop = givenLoop ?? false; const localId = useId(); /** We use selected values to preserve the item state when the consumer filters the items. */ - const selectedValueSetSig = useSignal>(new Set(_value ? [_value] : [])); + const selectedValueSetSig = useSignal>( + new Set(initialValue ? [initialValue] : []), + ); const disabledIndexSetSig = useSignal>(new Set()); const isMouseOverPopupSig = useSignal(false); - const highlightedIndexSig = useSignal(givenValuePropIndex ?? null); + const highlightedIndexSig = useSignal(initialIndex ?? null); const initialLoadSig = useSignal(true); const currDisplayValueSig = useSignal(); const scrollOptions = givenScrollOptions ?? { @@ -222,7 +224,7 @@ export const HComboboxRootImpl = component$< filter, loop, multiple, - _value, + _value: initialValue, scrollOptions, placeholder, hasVisibleItemsSig, From 83258f1ddc1b241ee66b59e88c247741a13d5931 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 10:28:38 -0500 Subject: [PATCH 11/31] refactor: better context names --- .../src/components/combobox/combobox-context.tsx | 2 +- .../src/components/combobox/combobox-root.tsx | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-context.tsx b/packages/kit-headless/src/components/combobox/combobox-context.tsx index dcc206c36..c6096a790 100644 --- a/packages/kit-headless/src/components/combobox/combobox-context.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-context.tsx @@ -26,7 +26,7 @@ export type ComboboxContext = { removeOnBackspace: boolean; hasVisibleItemsSig: Signal; initialLoadSig: Signal; - _value: string | undefined; + initialValue: string | undefined; loop: boolean; multiple: boolean | undefined; diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index d398bec3d..967c3fe4d 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -224,7 +224,7 @@ export const HComboboxRootImpl = component$< filter, loop, multiple, - _value: initialValue, + initialValue, scrollOptions, placeholder, hasVisibleItemsSig, @@ -236,19 +236,19 @@ export const HComboboxRootImpl = component$< useContextProvider(comboboxContextId, context); - useTask$(function handleOpenChange({ track }) { + useTask$(async function handleOpenChange({ track }) { track(() => isListboxOpenSig.value); if (!initialLoadSig.value) { - onOpenChange$?.(isListboxOpenSig.value); + await onOpenChange$?.(isListboxOpenSig.value); } }); - useTask$(function handleInput({ track }) { + useTask$(async function handleInput({ track }) { track(() => context.inputValueSig.value); if (!initialLoadSig.value) { - onInput$?.(context.inputValueSig.value); + await onInput$?.(context.inputValueSig.value); } }); From 4c67462793b4da869de40c17fc3fa0ac77a8f850 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 19:10:00 -0500 Subject: [PATCH 12/31] fix: combobox closes on enter key --- .../kit-headless/src/components/combobox/combobox-input.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index e4bb482f7..5c5d8fd9c 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -107,7 +107,7 @@ export const HComboboxInput = component$( if (!context.isListboxOpenSig.value) break; await selectionManager$(context.highlightedIndexSig.value, 'toggle'); - if (context.selectedValueSetSig.value.size <= 0) break; + if (context.selectedValuesSig.value.length <= 0) break; if (!context.multiple) { context.isListboxOpenSig.value = false; From 7a77229214728f23edc066fd06ddc414e0142bd5 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 19:28:08 -0500 Subject: [PATCH 13/31] fix: combobox properly resets when empty --- packages/kit-headless/src/components/combobox/combobox-item.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-item.tsx b/packages/kit-headless/src/components/combobox/combobox-item.tsx index c97078209..afba9d41c 100644 --- a/packages/kit-headless/src/components/combobox/combobox-item.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-item.tsx @@ -144,7 +144,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { if (isServer || !itemRef.value) return; if (context.inputValueSig.value === '' && !context.multiple) { - context.selectedValueSetSig.value = new Set(); + context.selectedValuesSig.value = ''; } else { context.isListboxOpenSig.value = true; } From 4ba2d9d74ac489597d10257adde34f84886f703c Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 19:59:52 -0500 Subject: [PATCH 14/31] get correct initial value reactively --- .../components/combobox/combobox-input.tsx | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index 5c5d8fd9c..21f5010b6 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -25,7 +25,7 @@ export const HComboboxInput = component$( const labelId = `${context.localId}-label`; const descriptionId = `${context.localId}-description`; const errorMessageId = `${context.localId}-error-message`; - const initialValueSig = useSignal(); + const initialValueSig = useSignal(); const wasEmptyBeforeBackspaceSig = useSignal(false); const isInputResetSig = useSignal(false); @@ -168,32 +168,27 @@ export const HComboboxInput = component$( }); /** Users may pass an initial value to bind:value on the input, use the value, or bind:value props on the root. */ - useTask$(function initialState() { - const selectedValue = - context.selectedValueSetSig.value.size > 0 - ? context.selectedValueSetSig.value.values().next().value - : ''; - - let initialValue = ''; - let matchingItemValue = null; - let matchingItemIndex = -1; - - context.itemsMapSig.value.forEach((item, index) => { - if (item.value === selectedValue) { - initialValue = item.displayValue; - } - if (inputValueSig?.value && item.displayValue === inputValueSig.value) { - matchingItemValue = item.value; - matchingItemIndex = index; + useTask$(function getInitialValues() { + const { selectedValuesSig, multiple, itemsMapSig, highlightedIndexSig } = context; + const selectedValues = selectedValuesSig.value; + const initialValue: string[] = []; + + for (const [index, item] of itemsMapSig.value.entries()) { + const isSelected = multiple + ? Array.isArray(selectedValues) && selectedValues.includes(item.value) + : item.value === selectedValues; + + if (isSelected) { + initialValue.push(item.displayValue); + if (highlightedIndexSig.value === -1) { + highlightedIndexSig.value = index; + } + // end the loop when we've found our selected value in single mode + if (!multiple) break; } - }); - - initialValueSig.value = initialValue; - - if (matchingItemValue !== null) { - context.selectedValueSetSig.value.add(matchingItemValue); - context.highlightedIndexSig.value = matchingItemIndex; } + + initialValueSig.value = multiple ? initialValue : initialValue[0] || ''; }); const computedInputValueSig = useComputed$(() => { From 6a8efd00ed7add1da3d3178381557f11064c49ce Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 20:50:06 -0500 Subject: [PATCH 15/31] fix: highlight jumping --- .../components/combobox/combobox-input.tsx | 25 ++++++------------- .../src/components/combobox/combobox-root.tsx | 17 +++++++++++++ 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index 21f5010b6..b0ec88a5b 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -15,7 +15,7 @@ import { useCombinedRef } from '../../hooks/combined-refs'; type HComboboxInputProps = PropsOf<'input'>; export const HComboboxInput = component$( - ({ 'bind:value': inputValueSig, ...props }: HComboboxInputProps) => { + ({ 'bind:value': givenInputValueSig, ...props }: HComboboxInputProps) => { const context = useContext(comboboxContextId); const contextRefOpts = { context, givenContextRef: context.inputRef }; const inputRef = useCombinedRef(props.ref, contextRefOpts); @@ -143,8 +143,8 @@ export const HComboboxInput = component$( isInputResetSig.value = false; // bind:value on the input - if (inputValueSig) { - inputValueSig.value = el.value; + if (givenInputValueSig) { + givenInputValueSig.value = el.value; context.inputValueSig.value = el.value; } }); @@ -180,9 +180,7 @@ export const HComboboxInput = component$( if (isSelected) { initialValue.push(item.displayValue); - if (highlightedIndexSig.value === -1) { - highlightedIndexSig.value = index; - } + highlightedIndexSig.value = index; // end the loop when we've found our selected value in single mode if (!multiple) break; } @@ -191,21 +189,14 @@ export const HComboboxInput = component$( initialValueSig.value = multiple ? initialValue : initialValue[0] || ''; }); - const computedInputValueSig = useComputed$(() => { - if (initialValueSig.value) { - return initialValueSig.value; - } else { - if (inputValueSig?.value) { - return inputValueSig.value; - } - return ''; - } - }); + const inputValueSig = useComputed$( + () => initialValueSig.value ?? givenInputValueSig?.value ?? '', + ); return ( 0) { await onChange$?.(selectedValuesSig.value); } + + const selectedValue = Array.isArray(selectedValuesSig.value) + ? selectedValuesSig.value[selectedValuesSig.value.length - 1] + : selectedValuesSig.value; + + for (const [index, item] of itemsMapSig.value.entries()) { + if (item.value === selectedValue) { + /* avoid "highlight jumping" on multiple selections */ + if (context.isListboxOpenSig.value === false) { + context.highlightedIndexSig.value = index; + } + + if (!context.inputRef.value) return; + context.inputRef.value.value = item.displayValue; + break; + } + } }); useTask$(() => { From ccc789901b2f1d7bda55e0d54d639d6a73972404 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 21:03:39 -0500 Subject: [PATCH 16/31] multiple shows correct display --- .../components/combobox/combobox-context.tsx | 2 +- .../src/components/combobox/combobox-root.tsx | 30 +++++++++++-------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-context.tsx b/packages/kit-headless/src/components/combobox/combobox-context.tsx index c6096a790..a267bca27 100644 --- a/packages/kit-headless/src/components/combobox/combobox-context.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-context.tsx @@ -20,7 +20,7 @@ export type ComboboxContext = { selectedValueSetSig: Signal>; selectedValuesSig: Signal; highlightedIndexSig: Signal; - currDisplayValueSig: Signal; + displayValuesSig: Signal; isMouseOverPopupSig: Signal; disabledIndexSetSig: Signal>; removeOnBackspace: boolean; diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index ae2590a98..2d63c329d 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -122,8 +122,6 @@ export const HComboboxRootImpl = component$< onInput$, onChange$, onOpenChange$, - 'bind:value': givenValueSig, - 'bind:open': givenOpenSig, initialIndex, initialValue, loop: givenLoop, @@ -136,11 +134,15 @@ export const HComboboxRootImpl = component$< removeOnBackspace = false, name, required, + 'bind:value': givenValueSig, + 'bind:open': givenOpenSig, + 'bind:displayValue': givenDisplayValueSig, ...rest } = props; // source of truth const isListboxOpenSig = useBoundSignal(givenOpenSig, false); + const displayValuesSig = useBoundSignal(givenDisplayValueSig, []); const selectedValuesSig = useBoundSignal( givenValueSig, multiple ? (initialValue ? [initialValue] : []) : initialValue || '', @@ -167,7 +169,6 @@ export const HComboboxRootImpl = component$< const isMouseOverPopupSig = useSignal(false); const highlightedIndexSig = useSignal(initialIndex ?? null); const initialLoadSig = useSignal(true); - const currDisplayValueSig = useSignal(); const scrollOptions = givenScrollOptions ?? { behavior: 'smooth', block: 'center', @@ -217,7 +218,7 @@ export const HComboboxRootImpl = component$< selectedValuesSig, selectedValueSetSig, disabledIndexSetSig, - currDisplayValueSig, + displayValuesSig, isMouseOverPopupSig, initialLoadSig, removeOnBackspace, @@ -259,22 +260,27 @@ export const HComboboxRootImpl = component$< await onChange$?.(selectedValuesSig.value); } - const selectedValue = Array.isArray(selectedValuesSig.value) - ? selectedValuesSig.value[selectedValuesSig.value.length - 1] - : selectedValuesSig.value; + const selectedValues = Array.isArray(selectedValuesSig.value) + ? selectedValuesSig.value + : [selectedValuesSig.value]; + + const displayValues: string[] = []; for (const [index, item] of itemsMapSig.value.entries()) { - if (item.value === selectedValue) { + if (selectedValues.includes(item.value)) { + displayValues.push(item.displayValue); + /* avoid "highlight jumping" on multiple selections */ if (context.isListboxOpenSig.value === false) { context.highlightedIndexSig.value = index; } - - if (!context.inputRef.value) return; - context.inputRef.value.value = item.displayValue; - break; } } + + displayValuesSig.value = displayValues; + + if (multiple || !context.inputRef.value) return; + context.inputRef.value.value = displayValues[0]; }); useTask$(() => { From a2c67d74887f2ad66ea3ba09b3bfdb7e73c093c6 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 21:05:36 -0500 Subject: [PATCH 17/31] fix: undefined does not become part of the input --- packages/kit-headless/src/components/combobox/combobox-root.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 2d63c329d..a0f8e27fa 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -279,7 +279,7 @@ export const HComboboxRootImpl = component$< displayValuesSig.value = displayValues; - if (multiple || !context.inputRef.value) return; + if (multiple || !context.inputRef.value || !displayValues[0]) return; context.inputRef.value.value = displayValues[0]; }); From ae99c8b4d3f01eab8eb335bf7e8a64edc046fad8 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 21:33:39 -0500 Subject: [PATCH 18/31] fix: handle proper empty input --- .../src/components/combobox/combobox-input.tsx | 6 ++++++ .../src/components/combobox/combobox-item.tsx | 11 +++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index b0ec88a5b..7d25625b9 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -142,6 +142,12 @@ export const HComboboxInput = component$( context.highlightedIndexSig.value = null; isInputResetSig.value = false; + if (target.value === '' && !context.multiple) { + context.selectedValuesSig.value = ''; + } else { + context.isListboxOpenSig.value = true; + } + // bind:value on the input if (givenInputValueSig) { givenInputValueSig.value = el.value; diff --git a/packages/kit-headless/src/components/combobox/combobox-item.tsx b/packages/kit-headless/src/components/combobox/combobox-item.tsx index afba9d41c..0c3c13ac4 100644 --- a/packages/kit-headless/src/components/combobox/combobox-item.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-item.tsx @@ -138,19 +138,14 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { } }); - useTask$(async ({ track }) => { + useTask$(async function defaultFilter({ track }) { track(() => context.inputValueSig.value); if (isServer || !itemRef.value) return; - if (context.inputValueSig.value === '' && !context.multiple) { - context.selectedValuesSig.value = ''; - } else { - context.isListboxOpenSig.value = true; - } + const displayValue = context.itemsMapSig.value.get(props._index ?? -1)?.displayValue; let isVisible; - const displayValue = context.itemsMapSig.value.get(props._index ?? -1)?.displayValue; if (!displayValue) return; if (context.filter) { const lowerCaseDisplayValue = displayValue?.toLowerCase(); @@ -183,7 +178,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { onFocus$={[handleFocus$, props.onFocus$]} aria-labelledby={itemLabelId} data-item - onClick$={handleClick$} + onClick$={[handleClick$, props.onClick$]} {...props} > From c9969d114595c5163fadd31a3c993ae0e994daca Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sat, 21 Sep 2024 21:38:57 -0500 Subject: [PATCH 19/31] feat: use local index instead --- .../src/components/combobox/combobox-item.tsx | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-item.tsx b/packages/kit-headless/src/components/combobox/combobox-item.tsx index 0c3c13ac4..741e30f2f 100644 --- a/packages/kit-headless/src/components/combobox/combobox-item.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-item.tsx @@ -29,20 +29,24 @@ export type HComboboxItemProps = PropsOf<'div'> & { }; export const HComboboxItem = component$((props: HComboboxItemProps) => { + const context = useContext(comboboxContextId); + if (props._index === undefined) { throw new Error('Qwik UI: Combobox component item cannot find its proper index.'); } - const context = useContext(comboboxContextId); + const localIndexSig = useComputed$(() => props._index ?? -1); const itemRef = useCombinedRef(props.ref); - const itemLabelId = `${context.localId}-${props._index ?? -1}-item-label`; const { selectionManager$, filterManager$ } = useCombobox(); + + const itemLabelId = `${context.localId}-${localIndexSig.value}-item-label`; + const isDisabledSig = useComputed$(() => - context.disabledIndexSetSig.value.has(props._index ?? -1), + context.disabledIndexSetSig.value.has(localIndexSig.value), ); const isSelectedSig = useComputed$(() => { - const index = props._index ?? -1; + const index = localIndexSig.value; const selectedValue = context.itemsMapSig.value.get(index)?.value; if (!selectedValue || isDisabledSig.value) return false; @@ -56,7 +60,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { const isHighlightedSig = useComputed$(() => { if (isDisabledSig.value) return; - if (context.highlightedIndexSig.value === props._index ?? -1) { + if (context.highlightedIndexSig.value === localIndexSig.value) { return true; } else { return false; @@ -84,9 +88,9 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { }); const handleClick$ = $(async () => { - if (isDisabledSig.value || (props._index ?? -1) === null) return; + if (isDisabledSig.value || localIndexSig.value === null) return; - await selectionManager$(props._index ?? -1, 'toggle'); + await selectionManager$(localIndexSig.value, 'toggle'); if (!isSelectedSig.value && !context.multiple) { context.isListboxOpenSig.value = false; @@ -97,7 +101,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { if (isDisabledSig.value) return; if (props._index !== undefined && context.isMouseOverPopupSig.value) { - context.highlightedIndexSig.value = props._index ?? -1; + context.highlightedIndexSig.value = localIndexSig.value; } }); @@ -143,7 +147,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { if (isServer || !itemRef.value) return; - const displayValue = context.itemsMapSig.value.get(props._index ?? -1)?.displayValue; + const displayValue = context.itemsMapSig.value.get(localIndexSig.value)?.displayValue; let isVisible; if (!displayValue) return; @@ -151,7 +155,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { const lowerCaseDisplayValue = displayValue?.toLowerCase(); const lowerCaseInputValue = context.inputValueSig.value.toLowerCase(); isVisible = lowerCaseDisplayValue?.includes(lowerCaseInputValue); - filterManager$(!!isVisible, itemRef, props._index ?? -1); + filterManager$(!!isVisible, itemRef, localIndexSig.value); } }); @@ -168,7 +172,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { role="option" ref={itemRef} tabIndex={-1} - id={`${context.localId}-${props._index ?? -1}`} + id={`${context.localId}-${localIndexSig.value}`} aria-selected={isSelectedSig.value} aria-disabled={isDisabledSig.value === true ? 'true' : 'false'} data-disabled={isDisabledSig.value ? '' : undefined} From f55d297e1d66aa756e03d6a9ec6647eb2828c94c Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 07:02:56 -0500 Subject: [PATCH 20/31] fix: last filtered item now gets highlighted on down key --- .../components/combobox/combobox-input.tsx | 3 ++ .../src/components/combobox/use-combobox.tsx | 33 +++++++++---------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index 7d25625b9..d30e311ea 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -65,9 +65,12 @@ export const HComboboxInput = component$( context.isListboxOpenSig.value && context.highlightedIndexSig.value !== null ) { + console.log('BEFORE: ', context.highlightedIndexSig.value); + context.highlightedIndexSig.value = await getNextEnabledItemIndex$( context.highlightedIndexSig.value, ); + console.log(context.highlightedIndexSig.value); } else { context.isListboxOpenSig.value = true; } diff --git a/packages/kit-headless/src/components/combobox/use-combobox.tsx b/packages/kit-headless/src/components/combobox/use-combobox.tsx index 8799cb98f..c0c70d4a9 100644 --- a/packages/kit-headless/src/components/combobox/use-combobox.tsx +++ b/packages/kit-headless/src/components/combobox/use-combobox.tsx @@ -88,25 +88,24 @@ export function useCombobox() { const getNextEnabledItemIndex$ = $((index: number) => { const len = context.itemsMapSig.value.size; - if (len === 1) { - return context.disabledIndexSetSig.value.has(0) ? -1 : 0; - } - - let offset = 1; - if (!context.loop && index + 1 >= len) { - return index; - } - while (offset < len) { - const nextIndex = (index + offset) % len; - if (!context.disabledIndexSetSig.value.has(nextIndex)) { - return nextIndex; - } - offset++; - if (!context.loop && index + offset >= len) { - break; + if (len === 0) return -1; + + const findNextEnabled = (start: number) => { + for (let i = 0; i < len; i++) { + const nextIndex = (start + i) % len; + if (!context.disabledIndexSetSig.value.has(nextIndex)) { + return nextIndex; + } } + return -1; + }; + + if (index === -1 || len === 1) { + return findNextEnabled(0); } - return index; + + const nextIndex = findNextEnabled(index + 1); + return context.loop || nextIndex > index ? nextIndex : index; }); const getPrevEnabledItemIndex$ = $((index: number) => { From d8e8c21f114e38fe9e21e1e2098d685bcb177ddc Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 10:10:35 -0500 Subject: [PATCH 21/31] fix: preserve item disabled state --- .../components/combobox/combobox-context.tsx | 1 + .../src/components/combobox/combobox-root.tsx | 2 + .../src/components/combobox/use-combobox.tsx | 62 +++++++++++++------ 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-context.tsx b/packages/kit-headless/src/components/combobox/combobox-context.tsx index a267bca27..2c3e1ee8a 100644 --- a/packages/kit-headless/src/components/combobox/combobox-context.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-context.tsx @@ -19,6 +19,7 @@ export type ComboboxContext = { controlRef: Signal; selectedValueSetSig: Signal>; selectedValuesSig: Signal; + filteredIndexSetSig: Signal>; highlightedIndexSig: Signal; displayValuesSig: Signal; isMouseOverPopupSig: Signal; diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index a0f8e27fa..635d8a266 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -166,6 +166,7 @@ export const HComboboxRootImpl = component$< ); const disabledIndexSetSig = useSignal>(new Set()); + const filteredIndexSetSig = useSignal>(new Set()); const isMouseOverPopupSig = useSignal(false); const highlightedIndexSig = useSignal(initialIndex ?? null); const initialLoadSig = useSignal(true); @@ -218,6 +219,7 @@ export const HComboboxRootImpl = component$< selectedValuesSig, selectedValueSetSig, disabledIndexSetSig, + filteredIndexSetSig, displayValuesSig, isMouseOverPopupSig, initialLoadSig, diff --git a/packages/kit-headless/src/components/combobox/use-combobox.tsx b/packages/kit-headless/src/components/combobox/use-combobox.tsx index c0c70d4a9..a1462a1e3 100644 --- a/packages/kit-headless/src/components/combobox/use-combobox.tsx +++ b/packages/kit-headless/src/components/combobox/use-combobox.tsx @@ -76,14 +76,31 @@ export function useCombobox() { const filterManager$ = $((isVisible: boolean, itemRef: Signal, index: number) => { if (!itemRef.value) return; + const isDisabled = context.itemsMapSig.value.get(index)?.disabled; + itemRef.value.style.display = isVisible ? '' : 'none'; - context.disabledIndexSetSig.value = new Set( + + context.filteredIndexSetSig.value = new Set( isVisible - ? [...context.disabledIndexSetSig.value].filter( - (selectedIndex) => selectedIndex !== index, + ? [...context.filteredIndexSetSig.value].filter( + (filteredIndex) => filteredIndex !== index, ) - : [...context.disabledIndexSetSig.value, index], + : [...context.filteredIndexSetSig.value, index], ); + + // Update disabledIndexSetSig + if (isDisabled) { + context.disabledIndexSetSig.value = new Set([ + ...context.disabledIndexSetSig.value, + index, + ]); + } else { + context.disabledIndexSetSig.value = new Set( + [...context.disabledIndexSetSig.value].filter( + (disabledIndex) => disabledIndex !== index, + ), + ); + } }); const getNextEnabledItemIndex$ = $((index: number) => { @@ -93,7 +110,10 @@ export function useCombobox() { const findNextEnabled = (start: number) => { for (let i = 0; i < len; i++) { const nextIndex = (start + i) % len; - if (!context.disabledIndexSetSig.value.has(nextIndex)) { + if ( + !context.disabledIndexSetSig.value.has(nextIndex) && + !context.filteredIndexSetSig.value.has(nextIndex) + ) { return nextIndex; } } @@ -109,22 +129,28 @@ export function useCombobox() { }); const getPrevEnabledItemIndex$ = $((index: number) => { - let offset = 1; const len = context.itemsMapSig.value.size; - if (!context.loop && index - 1 < 0) { - return index; - } - while (offset <= len) { - const prevIndex = (index - offset + len) % len; - if (!context.disabledIndexSetSig.value.has(prevIndex)) { - return prevIndex; - } - offset++; - if (!context.loop && index - offset < 0) { - break; + if (len === 0) return -1; + + const findPrevEnabled = (start: number) => { + for (let i = 0; i < len; i++) { + const prevIndex = (start - i + len) % len; + if ( + !context.disabledIndexSetSig.value.has(prevIndex) && + !context.filteredIndexSetSig.value.has(prevIndex) + ) { + return prevIndex; + } } + return -1; + }; + + if (index === -1 || len === 1) { + return findPrevEnabled(len - 1); } - return index; + + const prevIndex = findPrevEnabled(index - 1); + return context.loop || prevIndex < index ? prevIndex : index; }); const getActiveDescendant$ = $((index: number) => { From a88b6b5367275f02158d8be3e1290528592d2ee9 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 10:25:03 -0500 Subject: [PATCH 22/31] fix: combobox empty now properly shows for both filter and non-filtered comboboxes --- .../components/combobox/combobox-context.tsx | 2 +- .../components/combobox/combobox-empty.tsx | 2 +- .../src/components/combobox/combobox-root.tsx | 21 ++++++++++++++----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-context.tsx b/packages/kit-headless/src/components/combobox/combobox-context.tsx index 2c3e1ee8a..7eead1b16 100644 --- a/packages/kit-headless/src/components/combobox/combobox-context.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-context.tsx @@ -25,7 +25,7 @@ export type ComboboxContext = { isMouseOverPopupSig: Signal; disabledIndexSetSig: Signal>; removeOnBackspace: boolean; - hasVisibleItemsSig: Signal; + isNoItemsSig: Signal; initialLoadSig: Signal; initialValue: string | undefined; diff --git a/packages/kit-headless/src/components/combobox/combobox-empty.tsx b/packages/kit-headless/src/components/combobox/combobox-empty.tsx index d2eac1d7b..6c9833d12 100644 --- a/packages/kit-headless/src/components/combobox/combobox-empty.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-empty.tsx @@ -7,7 +7,7 @@ export const HComboboxEmpty = component$((props: PropsOf<'div'>) => { return (
diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 635d8a266..5ea59c536 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -191,15 +191,26 @@ export const HComboboxRootImpl = component$< disabledIndexSetSig.value = disabledIndices; }); - const hasVisibleItemsSig = useComputed$(() => { - return itemsMapSig.value.size !== disabledIndexSetSig.value.size; + const isNoItemsSig = useComputed$(() => { + return ( + itemsMapSig.value.size === filteredIndexSetSig.value.size || + itemsMapSig.value.size === 0 + ); + }); + + useTask$(({ track }) => { + track(() => itemsMapSig.value); + + console.log(itemsMapSig.value.size); + console.log(isNoItemsSig.value); }); useTask$(function closeIfEmptyComp({ track }) { - track(() => disabledIndexSetSig.value); + track(() => itemsMapSig.value); + track(() => filteredIndexSetSig.value); // automatically closes the listbox if there are no visible items AND the combobox does not have an empty component - if (!hasEmptyComp && !hasVisibleItemsSig.value) { + if (!hasEmptyComp && isNoItemsSig.value) { isListboxOpenSig.value = false; } }); @@ -230,7 +241,7 @@ export const HComboboxRootImpl = component$< initialValue, scrollOptions, placeholder, - hasVisibleItemsSig, + isNoItemsSig, name, required, isDisabledSig, From 01503a03c03a8f416819785d5cf654ec95afcfa2 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 10:58:58 -0500 Subject: [PATCH 23/31] fix: removing items on backspace --- .../src/components/combobox/combobox-input.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index d30e311ea..93f7c3157 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -162,16 +162,17 @@ export const HComboboxInput = component$( if (e.key === 'Backspace') { // removeOnBackspace if (!context.multiple) return; - if (context.selectedValueSetSig.value.size === 0) return; + if (context.selectedValuesSig.value.length === 0) return; if (!context.removeOnBackspace) return; if ( (wasEmptyBeforeBackspaceSig.value || isInputResetSig.value) && context.inputValueSig.value.length === 0 ) { - const selectedValuesArray = [...context.selectedValueSetSig.value]; - selectedValuesArray.pop(); // Remove the last element - context.selectedValueSetSig.value = new Set(selectedValuesArray); + const selectedValuesArray = [...context.selectedValuesSig.value]; + selectedValuesArray.pop(); + + context.selectedValuesSig.value = selectedValuesArray; } } }); From 908b0b2b910f9a6b549e0cec328d161c805653e0 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 11:14:39 -0500 Subject: [PATCH 24/31] respect input's given signal first --- .../kit-headless/src/components/combobox/combobox-input.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index 93f7c3157..50afc8f17 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -200,7 +200,7 @@ export const HComboboxInput = component$( }); const inputValueSig = useComputed$( - () => initialValueSig.value ?? givenInputValueSig?.value ?? '', + () => givenInputValueSig?.value ?? initialValueSig.value ?? '', ); return ( From bc397f99ed72aa0a579631a67d48018db03e5a2d Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 11:16:12 -0500 Subject: [PATCH 25/31] refactor: remove all the logs --- .../routes/docs/headless/combobox/examples/reactive.tsx | 8 +------- .../src/components/combobox/combobox-input.tsx | 3 --- .../src/components/combobox/combobox-root.tsx | 7 ------- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/apps/website/src/routes/docs/headless/combobox/examples/reactive.tsx b/apps/website/src/routes/docs/headless/combobox/examples/reactive.tsx index 2e53c4702..46e9b245d 100644 --- a/apps/website/src/routes/docs/headless/combobox/examples/reactive.tsx +++ b/apps/website/src/routes/docs/headless/combobox/examples/reactive.tsx @@ -1,4 +1,4 @@ -import { component$, useSignal, useStyles$, useTask$ } from '@builder.io/qwik'; +import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; import { Combobox } from '@qwik-ui/headless'; import { LuCheck, LuChevronDown } from '@qwikest/icons/lucide'; @@ -18,12 +18,6 @@ export default component$(() => { 'Coconut', ]; - useTask$(({ track }) => { - track(() => selectedFruit.value); - - console.log(selectedFruit.value); - }); - return ( <> diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index 50afc8f17..fe0d8eb80 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -65,12 +65,9 @@ export const HComboboxInput = component$( context.isListboxOpenSig.value && context.highlightedIndexSig.value !== null ) { - console.log('BEFORE: ', context.highlightedIndexSig.value); - context.highlightedIndexSig.value = await getNextEnabledItemIndex$( context.highlightedIndexSig.value, ); - console.log(context.highlightedIndexSig.value); } else { context.isListboxOpenSig.value = true; } diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 5ea59c536..8b5b41dbe 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -198,13 +198,6 @@ export const HComboboxRootImpl = component$< ); }); - useTask$(({ track }) => { - track(() => itemsMapSig.value); - - console.log(itemsMapSig.value.size); - console.log(isNoItemsSig.value); - }); - useTask$(function closeIfEmptyComp({ track }) { track(() => itemsMapSig.value); track(() => filteredIndexSetSig.value); From 6605f9ef565d172d1dc0eb3fb04d95d83b066d64 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 12:29:52 -0500 Subject: [PATCH 26/31] fix: combobox scrolling --- .../components/combobox/combobox-context.tsx | 1 + .../components/combobox/combobox-input.tsx | 2 + .../src/components/combobox/combobox-item.tsx | 39 ++++++++++++------- .../components/combobox/combobox-popover.tsx | 19 +++------ .../src/components/combobox/combobox-root.tsx | 2 + 5 files changed, 36 insertions(+), 27 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-context.tsx b/packages/kit-headless/src/components/combobox/combobox-context.tsx index 7eead1b16..a22bce646 100644 --- a/packages/kit-headless/src/components/combobox/combobox-context.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-context.tsx @@ -23,6 +23,7 @@ export type ComboboxContext = { highlightedIndexSig: Signal; displayValuesSig: Signal; isMouseOverPopupSig: Signal; + isKeyboardFocusSig: Signal; disabledIndexSetSig: Signal>; removeOnBackspace: boolean; isNoItemsSig: Signal; diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index fe0d8eb80..c0c5bd7e5 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -54,6 +54,8 @@ export const HComboboxInput = component$( const handleKeyDown$ = $(async (e: KeyboardEvent) => { if (!context.itemsMapSig.value) return; + context.isKeyboardFocusSig.value = true; + if (e.key === 'Backspace') { // check if input was empty before backspace wasEmptyBeforeBackspaceSig.value = context.inputValueSig.value.length === 0; diff --git a/packages/kit-headless/src/components/combobox/combobox-item.tsx b/packages/kit-headless/src/components/combobox/combobox-item.tsx index 741e30f2f..2e4275446 100644 --- a/packages/kit-headless/src/components/combobox/combobox-item.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-item.tsx @@ -7,6 +7,7 @@ import { useTask$, useComputed$, useContextProvider, + useSignal, } from '@builder.io/qwik'; import { ComboboxItemContext, @@ -30,6 +31,7 @@ export type HComboboxItemProps = PropsOf<'div'> & { export const HComboboxItem = component$((props: HComboboxItemProps) => { const context = useContext(comboboxContextId); + const debounceTimeoutSig = useSignal(); if (props._index === undefined) { throw new Error('Qwik UI: Combobox component item cannot find its proper index.'); @@ -74,7 +76,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { const containerRect = context.panelRef.value?.getBoundingClientRect(); const itemRect = itemRef.value?.getBoundingClientRect(); - if (!containerRect || !itemRect || context.isMouseOverPopupSig.value) return; + if (!containerRect || !itemRect || !context.isKeyboardFocusSig.value) return; // Calculates the offset to center the item within the container const offset = @@ -117,29 +119,40 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { useContextProvider(comboboxItemContextId, itemContext); - useTask$(async function navigationTask({ track, cleanup }) { + useTask$(function handleScrolling({ track, cleanup }) { track(() => context.highlightedIndexSig.value); if (isServer || !context.panelRef.value) return; - if (props._index !== context.highlightedIndexSig.value) return; const hasScrollbar = context.panelRef.value.scrollHeight > context.panelRef.value.clientHeight; - if (!hasScrollbar) { - return; + if (!hasScrollbar) return; + + if (debounceTimeoutSig.value !== undefined) { + clearTimeout(debounceTimeoutSig.value); } - const observer = new IntersectionObserver(checkVisibility$, { - root: context.panelRef.value, - threshold: 1.0, - }); + debounceTimeoutSig.value = setTimeout(() => { + if (props._index !== context.highlightedIndexSig.value) return; - cleanup(() => observer?.disconnect()); + const observer = new IntersectionObserver(checkVisibility$, { + root: context.panelRef.value, + threshold: 1.0, + }); - if (itemRef.value) { - observer.observe(itemRef.value); - } + cleanup(() => observer?.disconnect()); + + if (itemRef.value) { + observer.observe(itemRef.value); + } + }, 100); + + cleanup(() => { + if (debounceTimeoutSig.value !== undefined) { + clearTimeout(debounceTimeoutSig.value); + } + }); }); useTask$(async function defaultFilter({ track }) { diff --git a/packages/kit-headless/src/components/combobox/combobox-popover.tsx b/packages/kit-headless/src/components/combobox/combobox-popover.tsx index b935f42f2..aafd115ca 100644 --- a/packages/kit-headless/src/components/combobox/combobox-popover.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-popover.tsx @@ -14,7 +14,6 @@ import { comboboxContextId } from './combobox-context'; import { HPopoverRoot } from '../popover/popover-root'; import { isServer } from '@builder.io/qwik/build'; import { useCombinedRef } from '../../hooks/combined-refs'; -import { useDebouncer } from '../../hooks/use-debouncer'; export const HComboboxPopover = component$>((props) => { const context = useContext(comboboxContextId); @@ -85,12 +84,10 @@ export const HComboboxPopover = component$>((props) initialLoadSig.value = false; }); - const resetScrollMove = useDebouncer( - $(() => { - context.isMouseOverPopupSig.value = false; - }), - 650, - ); + const handleMouseEnter$ = $(() => { + context.isKeyboardFocusSig.value = false; + context.isMouseOverPopupSig.value = true; + }); return ( >((props) role="listbox" aria-expanded={context.isListboxOpenSig.value ? 'true' : 'false'} aria-multiselectable={context.multiple ? 'true' : undefined} - onMouseMove$={async () => { - context.isMouseOverPopupSig.value = true; - - await resetScrollMove(); - }} - onMouseOut$={() => (context.isMouseOverPopupSig.value = false)} - onKeyDown$={() => (context.isMouseOverPopupSig.value = true)} + onMouseEnter$={[handleMouseEnter$, props.onMouseEnter$]} {...rest} > diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 8b5b41dbe..b901f662f 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -178,6 +178,7 @@ export const HComboboxRootImpl = component$< const inputValueSig = useSignal(inputRef.value?.value ?? ''); const isDisabledSig = useComputed$(() => props.disabled ?? false); const isInvalidSig = useComputed$(() => props.hasErrorComp ?? false); + const isKeyboardFocusSig = useSignal(false); // check any initial disabled items before the computed read below useTask$(() => { @@ -226,6 +227,7 @@ export const HComboboxRootImpl = component$< filteredIndexSetSig, displayValuesSig, isMouseOverPopupSig, + isKeyboardFocusSig, initialLoadSig, removeOnBackspace, filter, From 18eefb985f87eb5e3dec25c843b9cff73cc7b055 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 12:32:37 -0500 Subject: [PATCH 27/31] fix: form submissions --- .../src/components/combobox/combobox-hidden-option.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-hidden-option.tsx b/packages/kit-headless/src/components/combobox/combobox-hidden-option.tsx index 1ee0bc7a9..017cc2e5f 100644 --- a/packages/kit-headless/src/components/combobox/combobox-hidden-option.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-hidden-option.tsx @@ -22,7 +22,7 @@ export const ComboboxHiddenSelectOption = component$( const context = useContext(comboboxContextId); useTask$(async function modularFormsValidation({ track }) { - track(() => context.selectedValueSetSig.value); + track(() => context.selectedValuesSig.value); if (isServer || !nativeSelectRef.value || !optionRef.value) return; @@ -37,7 +37,11 @@ export const ComboboxHiddenSelectOption = component$( 'Qwik UI: value not found when trying to select or unselect an item.', ); } - optionRef.value.selected = context.selectedValueSetSig.value.has(value); + + const selectedValues = context.selectedValuesSig.value; + optionRef.value.selected = Array.isArray(selectedValues) + ? selectedValues.includes(value) + : selectedValues === value; }); return ( From 40a3d9bbd7f28327e578268a51ce08674b536cf5 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 12:41:27 -0500 Subject: [PATCH 28/31] fix: pw tests --- .../src/components/combobox/combobox.test.ts | 36 +++++++------------ 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox.test.ts b/packages/kit-headless/src/components/combobox/combobox.test.ts index 80cee0898..7b8d64bc1 100644 --- a/packages/kit-headless/src/components/combobox/combobox.test.ts +++ b/packages/kit-headless/src/components/combobox/combobox.test.ts @@ -589,49 +589,39 @@ test.describe('Props', () => { }); test(`GIVEN a reactive combobox - WHEN the user empties the input with selecting all text and backspace - THEN there should be no selected value`, async ({ page }) => { + WHEN the user empties the input with selecting all text and backspace + THEN there should be no selected value`, async ({ page }) => { const { driver: d } = await setup(page, 'reactive'); - const apricot = (await d.getItemAt(1).textContent()) ?? ''; - - await expect(d.getInput()).toHaveValue(apricot); + await expect(d.getInput()).toHaveValue('Apricot'); + await expect(page.getByRole('paragraph')).toContainText('Apricot'); await d.getInput().clear(); await expect(d.getInput()).toHaveValue(''); - await expect( - page.getByRole('paragraph', { name: 'Your favorite fruit is:' }), - ).toBeVisible(); + await expect(page.getByRole('paragraph')).not.toContainText('Apricot'); }); test(`GIVEN a reactive combobox - WHEN the user empties the input and selects a new value - THEN that should update the given signal`, async ({ page }) => { + WHEN the user empties the input and selects a new value + THEN that should update the given signal`, async ({ page }) => { const { driver: d } = await setup(page, 'reactive'); - const apricot = (await d.getItemAt(1).textContent()) ?? ''; - - await expect(d.getInput()).toHaveValue(apricot); + await expect(d.getInput()).toHaveValue('Apricot'); + await expect(page.getByRole('paragraph')).toContainText('Apricot'); await d.getInput().clear(); await expect(d.getInput()).toHaveValue(''); - await expect( - page.getByRole('paragraph', { name: 'Your favorite fruit is:' }), - ).toBeVisible(); + await expect(page.getByRole('paragraph')).not.toContainText('Apricot'); await d.openListbox('click'); + await d.getItemAt(0).click(); - await d.getItemAt(1).click(); - - await expect(d.getInput()).toHaveValue('Apricot'); - - await expect( - page.getByRole('paragraph', { name: 'Your favorite fruit is: Apricot' }), - ).toBeVisible(); + await expect(d.getInput()).toHaveValue('Apple'); + await expect(page.getByRole('paragraph')).toContainText('Apple'); }); }); From 7d30b0ccce8f8a7d405e34811e628569ec068e3d Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 14:15:15 -0500 Subject: [PATCH 29/31] refactor: remove test since behavior is no longer warranted --- .../src/components/combobox/combobox.test.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox.test.ts b/packages/kit-headless/src/components/combobox/combobox.test.ts index 7b8d64bc1..48bb9f82d 100644 --- a/packages/kit-headless/src/components/combobox/combobox.test.ts +++ b/packages/kit-headless/src/components/combobox/combobox.test.ts @@ -882,19 +882,6 @@ test.describe('Filtering options', () => { expect(await d.getVisibleItemsLength()).toBe(8); }); - test(`GIVEN a combobox - WHEN an option is selected - AND the typed string does not match the filter function - THEN the option should be unselected`, async ({ page }) => { - const { driver: d } = await setup(page, 'hero'); - await d.openListbox('ArrowDown'); - await d.getInput().press('Enter'); - await expect(d.getItemAt(0)).toHaveAttribute('aria-selected', 'true'); - - await d.getInput().press('z'); - await expect(d.getItemAt(0)).toHaveAttribute('aria-selected', 'false'); - }); - test(`GIVEN a combobox WHEN a custom filter is set THEN it should respect the filter function`, async ({ page }) => { From 0eab200de42c4105b8b746d24dccc60607a45173 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 17:03:58 -0500 Subject: [PATCH 30/31] fix: mouse should not affect initial keyboard highlight --- .../src/components/combobox/combobox-item.tsx | 2 +- .../src/components/combobox/combobox.test.ts | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-item.tsx b/packages/kit-headless/src/components/combobox/combobox-item.tsx index 2e4275446..ff816e72d 100644 --- a/packages/kit-headless/src/components/combobox/combobox-item.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-item.tsx @@ -100,7 +100,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { }); const handlePointerOver$ = $(() => { - if (isDisabledSig.value) return; + if (isDisabledSig.value || context.isKeyboardFocusSig.value) return; if (props._index !== undefined && context.isMouseOverPopupSig.value) { context.highlightedIndexSig.value = localIndexSig.value; diff --git a/packages/kit-headless/src/components/combobox/combobox.test.ts b/packages/kit-headless/src/components/combobox/combobox.test.ts index 48bb9f82d..012bc4a1a 100644 --- a/packages/kit-headless/src/components/combobox/combobox.test.ts +++ b/packages/kit-headless/src/components/combobox/combobox.test.ts @@ -238,6 +238,32 @@ test.describe('Keyboard Behavior', () => { await d.getInput().press('ArrowUp'); await expect(d.getItemAt(1)).toHaveAttribute('data-highlighted'); }); + + test(`GIVEN a combobox with a selected value via mouse + WHEN using the arrow keys to select another option + THEN the latest selected option should be highlighted when the menu is opened again`, async ({ + page, + }) => { + const { driver: d } = await setup(page, 'hero'); + + // initial mouse selection + await d.openListbox('click'); + await d.getItemAt(2).click(); + + await d.openListbox('ArrowDown'); + await expect(d.getItemAt(2)).toHaveAttribute('data-highlighted'); + + // selection via arrow keys + await d.getInput().press('ArrowDown'); + await expect(d.getItemAt(3)).toHaveAttribute('data-highlighted'); + + await d.getInput().press('Enter'); + await expect(d.getItemAt(3)).toHaveAttribute('data-selected'); + + // latest selected option should be highlighted when the menu is opened again + await d.openListbox('ArrowDown'); + await expect(d.getItemAt(3)).toHaveAttribute('data-highlighted'); + }); }); test.describe('selecting options', () => { From 31f4a0ad0eed61e65e586aa4eb6b5bd4385d2688 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Sun, 22 Sep 2024 17:11:46 -0500 Subject: [PATCH 31/31] feat: changeset --- .changeset/funny-pens-vanish.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .changeset/funny-pens-vanish.md diff --git a/.changeset/funny-pens-vanish.md b/.changeset/funny-pens-vanish.md new file mode 100644 index 000000000..bd99ffd15 --- /dev/null +++ b/.changeset/funny-pens-vanish.md @@ -0,0 +1,31 @@ +--- +'@qwik-ui/headless': patch +--- + +# Combobox Improvements + +## ๐Ÿ”„ Reactive Improvements + +- Better handling of array-based values +- Improved handling of initial and reactive values + +## ๐Ÿ› Key Bug Fixes + +- Fixed highlight jumping issues +- Enhanced empty input handling +- Better filtered item highlighting + +## ๐Ÿ–ฑ๏ธ Interaction Enhancements + +- Smoother scrolling experience +- Improved keyboard and mouse coordination + +## ๐Ÿš€ Performance Optimizations + +- More efficient item filtering + +## ๐Ÿงช Reliability + +- Added tests for reactivity handling, item unselection, and mouse-to-pointer interaction switching + +- Improved handling of edge cases in user interactions