From 854ccf3bc614c31cf2b36babb20646b50417639b Mon Sep 17 00:00:00 2001 From: thejackshelton Date: Wed, 22 Jan 2025 19:21:30 -0600 Subject: [PATCH 1/7] fix: select now shows first item instead of placeholder when initial value is set --- .../docs/headless/select/auto-api/api.ts | 55 +++++++++++++++++++ .../select/examples/first-value-selected.tsx | 26 +++++++++ .../src/components/select/select-root.tsx | 14 ++--- .../src/components/select/select.test.ts | 13 +++++ 4 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 apps/website/src/routes/docs/headless/select/auto-api/api.ts create mode 100644 apps/website/src/routes/docs/headless/select/examples/first-value-selected.tsx diff --git a/apps/website/src/routes/docs/headless/select/auto-api/api.ts b/apps/website/src/routes/docs/headless/select/auto-api/api.ts new file mode 100644 index 000000000..f0f4a49de --- /dev/null +++ b/apps/website/src/routes/docs/headless/select/auto-api/api.ts @@ -0,0 +1,55 @@ +export const api = { + select: [ + { + 'hidden-select-option': [], + }, + { + 'hidden-select': [], + }, + { + 'select-description': [], + }, + { + 'select-display-value': [], + }, + { + 'select-error-message': [], + }, + { + 'select-group-label': [], + }, + { + 'select-group': [], + }, + { + 'select-inline': [], + }, + { + 'select-item-indicator': [], + }, + { + 'select-item-label': [], + }, + { + 'select-item': [], + }, + { + 'select-label': [], + }, + { + 'select-listbox': [], + }, + { + 'select-popover': [], + }, + { + 'select-root': [], + }, + { + 'select-trigger': [], + }, + { + 'use-select': [], + }, + ], +}; diff --git a/apps/website/src/routes/docs/headless/select/examples/first-value-selected.tsx b/apps/website/src/routes/docs/headless/select/examples/first-value-selected.tsx new file mode 100644 index 000000000..dd803b21e --- /dev/null +++ b/apps/website/src/routes/docs/headless/select/examples/first-value-selected.tsx @@ -0,0 +1,26 @@ +import { component$, useStyles$ } from '@builder.io/qwik'; +import { Select } from '@qwik-ui/headless'; + +export default component$(() => { + useStyles$(styles); + const users = ['Tim', 'Ryan', 'Jim', 'Jessie', 'Abby']; + + return ( + + Logged in users + + + + + {users.map((user) => ( + + {user} + + ))} + + + ); +}); + +// internal +import styles from '../snippets/select.css?inline'; diff --git a/packages/kit-headless/src/components/select/select-root.tsx b/packages/kit-headless/src/components/select/select-root.tsx index 449c9d812..3573aff20 100644 --- a/packages/kit-headless/src/components/select/select-root.tsx +++ b/packages/kit-headless/src/components/select/select-root.tsx @@ -44,13 +44,13 @@ type TMultiValue = type TStringOrArray = | { - multiple?: true; - onChange$?: QRL<(value: string[]) => void>; - } + multiple?: true; + onChange$?: QRL<(value: string[]) => void>; + } | { - multiple?: false; - onChange$?: QRL<(value: string) => void>; - }; + multiple?: false; + onChange$?: QRL<(value: string) => void>; + }; export type SelectProps = Omit< PropsOf<'div'>, @@ -150,7 +150,7 @@ export const HSelectImpl = component$ & InternalSelectProps }); const selectedIndexSetSig = useSignal>( - new Set(givenValuePropIndex ? [givenValuePropIndex] : []), + new Set([givenValuePropIndex ?? []].flat()), ); const highlightedIndexSig = useSignal(givenValuePropIndex ?? null); diff --git a/packages/kit-headless/src/components/select/select.test.ts b/packages/kit-headless/src/components/select/select.test.ts index a56a10e66..deacb32f4 100644 --- a/packages/kit-headless/src/components/select/select.test.ts +++ b/packages/kit-headless/src/components/select/select.test.ts @@ -839,6 +839,19 @@ test.describe('Props', () => { await expect(d.getItemAt(3)).toHaveAttribute('data-highlighted'); await expect(d.getItemAt(3)).toHaveAttribute('aria-selected', 'true'); }); + + test(` + GIVEN a select + WHEN the first option is selected on render + THEN the first option should be displayed`, async ({ page }) => { + const { driver: d } = await setup(page, 'first-value-selected'); + + const expectedValue = await d.getItemAt(0).textContent(); + + await expect(d.getValueElement()).toHaveText(expectedValue!); + await expect(d.getItemAt(0)).toHaveAttribute('data-highlighted'); + await expect(d.getItemAt(0)).toHaveAttribute('aria-selected', 'true'); + }); }); test.describe('controlled', () => { From 3d718905117ec03369ce943c4c08030491478e6c Mon Sep 17 00:00:00 2001 From: thejackshelton Date: Wed, 22 Jan 2025 20:27:05 -0600 Subject: [PATCH 2/7] fix: prevent native scrolling on focus in select --- .../headless/combobox/examples/scrollable.tsx | 25 +++-------- .../headless/select/examples/scrollable.tsx | 24 +++-------- .../src/components/select/select-item.tsx | 43 +------------------ 3 files changed, 14 insertions(+), 78 deletions(-) diff --git a/apps/website/src/routes/docs/headless/combobox/examples/scrollable.tsx b/apps/website/src/routes/docs/headless/combobox/examples/scrollable.tsx index 60ba309f6..9f6bf2ac7 100644 --- a/apps/website/src/routes/docs/headless/combobox/examples/scrollable.tsx +++ b/apps/website/src/routes/docs/headless/combobox/examples/scrollable.tsx @@ -4,8 +4,7 @@ import { LuChevronDown } from '@qwikest/icons/lucide'; export default component$(() => { useStyles$(styles); - const activeUsers = ['Tim', 'Ryan', 'Jim', 'Abby']; - const offlineUsers = ['Joey', 'Bob', 'Jack', 'John']; + const items = Array.from({ length: 100 }, (_, i) => (i + 1).toString()); return ( @@ -17,23 +16,11 @@ export default component$(() => { - - Active - {activeUsers.map((user) => ( - - {user} - - ))} - - - - Offline - {offlineUsers.map((user) => ( - - {user} - - ))} - + {items.map((item) => ( + + {item} + + ))} ); diff --git a/apps/website/src/routes/docs/headless/select/examples/scrollable.tsx b/apps/website/src/routes/docs/headless/select/examples/scrollable.tsx index 0f9fa3bbe..38e9fa4f8 100644 --- a/apps/website/src/routes/docs/headless/select/examples/scrollable.tsx +++ b/apps/website/src/routes/docs/headless/select/examples/scrollable.tsx @@ -3,8 +3,7 @@ import { Select } from '@qwik-ui/headless'; export default component$(() => { useStyles$(styles); - const users = ['Tim', 'Ryan', 'Jim', 'Jessie', 'Abby']; - const animals = ['Dog', 'Cat', 'Bird', 'Fish', 'Snake']; + const items = Array.from({ length: 100 }, (_, i) => (i + 1).toString()); return ( @@ -12,22 +11,11 @@ export default component$(() => { - - People - {users.map((user) => ( - - {user} - - ))} - - - Animals - {animals.map((animal) => ( - - {animal} - - ))} - + {items.map((item) => ( + + {item} + + ))} ); diff --git a/packages/kit-headless/src/components/select/select-item.tsx b/packages/kit-headless/src/components/select/select-item.tsx index ba6dd4b69..85f2fa6b7 100644 --- a/packages/kit-headless/src/components/select/select-item.tsx +++ b/packages/kit-headless/src/components/select/select-item.tsx @@ -12,7 +12,6 @@ import { useOnWindow, QRL, } from '@builder.io/qwik'; -import { isServer } from '@builder.io/qwik/build'; import SelectContextId, { SelectItemContext, selectItemContextId, @@ -60,7 +59,7 @@ export const HSelectItem = component$((props) => { if (disabled) return; if (context.highlightedIndexSig.value === _index) { - itemRef.value?.focus(); + itemRef.value?.focus({ preventScroll: true }); return true; } else { return false; @@ -74,51 +73,13 @@ export const HSelectItem = component$((props) => { localIndexSig.value = _index; }); - const checkVisibility$ = $(async (entries: IntersectionObserverEntry[]) => { - const [entry] = entries; - - if (isHighlightedSig.value && !entry.isIntersecting) { - const containerRect = context.popoverRef.value?.getBoundingClientRect(); - const itemRect = itemRef.value?.getBoundingClientRect(); - - if (!containerRect || !itemRect) return; - - // Calculates the offset to center the item within the container - const offset = - itemRect.top - containerRect.top - containerRect.height / 2 + itemRect.height / 2; - - context.popoverRef.value?.scrollBy({ top: offset, ...context.scrollOptions }); - } - }); - - useTask$(async function navigationTask({ track, cleanup }) { + useTask$(async function navigationTask({ track }) { track(() => context.highlightedIndexSig.value); // update the context with the highlighted item ref if (localIndexSig.value === context.highlightedIndexSig.value) { context.highlightedItemRef = itemRef; } - - if (isServer || !context.popoverRef.value) return; - if (_index !== context.highlightedIndexSig.value) return; - - const hasScrollbar = - context.popoverRef.value.scrollHeight > context.popoverRef.value.clientHeight; - - if (!hasScrollbar) { - return; - } - - const observer = new IntersectionObserver(checkVisibility$, { - root: context.popoverRef.value, - threshold: 1.0, - }); - - cleanup(() => observer?.disconnect()); - - if (itemRef.value) { - observer.observe(itemRef.value); - } }); const handleClick$ = $(async () => { From a17eb9164c641eea2ba8ab9276f21e76817697a3 Mon Sep 17 00:00:00 2001 From: thejackshelton Date: Wed, 22 Jan 2025 20:37:26 -0600 Subject: [PATCH 3/7] feat: initial scroll handling logic --- .../src/components/select/select-context.ts | 1 + .../src/components/select/select-item.tsx | 56 ++++++++++++++++++- .../src/components/select/select-root.tsx | 2 + 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/select/select-context.ts b/packages/kit-headless/src/components/select/select-context.ts index a4187c50b..053c3bb75 100644 --- a/packages/kit-headless/src/components/select/select-context.ts +++ b/packages/kit-headless/src/components/select/select-context.ts @@ -21,6 +21,7 @@ export type SelectContext = { selectedIndexSetSig: Signal>; highlightedIndexSig: Signal; currDisplayValueSig: Signal; + isKeyboardFocusSig: Signal; isListboxOpenSig: Signal; isDisabledSig: Signal; localId: string; diff --git a/packages/kit-headless/src/components/select/select-item.tsx b/packages/kit-headless/src/components/select/select-item.tsx index 85f2fa6b7..30caf5834 100644 --- a/packages/kit-headless/src/components/select/select-item.tsx +++ b/packages/kit-headless/src/components/select/select-item.tsx @@ -18,6 +18,7 @@ import SelectContextId, { } from './select-context'; import { useSelect } from './use-select'; import { useCombinedRef } from '../../hooks/combined-refs'; +import { isServer } from '@builder.io/qwik/build'; export type SelectItemProps = PropsOf<'div'> & { /** Internal index we get from the inline component. Please see select-inline.tsx */ @@ -38,6 +39,7 @@ export const HSelectItem = component$((props) => { const localIndexSig = useSignal(null); const itemId = `${context.localId}-${_index}`; const typeaheadFnSig = useSignal Promise>>(); + const debounceTimeoutSig = useSignal(); const { selectionManager$, getNextEnabledItemIndex$, getPrevEnabledItemIndex$ } = useSelect(); @@ -73,13 +75,65 @@ export const HSelectItem = component$((props) => { localIndexSig.value = _index; }); - useTask$(async function navigationTask({ track }) { + const checkVisibility$ = $(async (entries: IntersectionObserverEntry[]) => { + const [entry] = entries; + + if (isHighlightedSig.value && !entry.isIntersecting) { + const containerRect = context.popoverRef.value?.getBoundingClientRect(); + const itemRect = itemRef.value?.getBoundingClientRect(); + + if (!containerRect || !itemRect || !context.isKeyboardFocusSig.value) return; + + // Calculates the offset to center the item within the container + const offset = + itemRect.top - containerRect.top - containerRect.height / 2 + itemRect.height / 2; + + context.popoverRef.value?.scrollBy({ + top: document.hasFocus() ? offset : undefined, + ...context.scrollOptions, + }); + } + }); + + useTask$(function handleScrolling({ track, cleanup }) { track(() => context.highlightedIndexSig.value); // update the context with the highlighted item ref if (localIndexSig.value === context.highlightedIndexSig.value) { context.highlightedItemRef = itemRef; } + + if (isServer || !context.popoverRef.value) return; + + const hasScrollbar = + context.popoverRef.value.scrollHeight > context.popoverRef.value.clientHeight; + + if (!hasScrollbar) return; + + if (debounceTimeoutSig.value !== undefined) { + clearTimeout(debounceTimeoutSig.value); + } + + debounceTimeoutSig.value = setTimeout(() => { + if (props._index !== context.highlightedIndexSig.value) return; + + const observer = new IntersectionObserver(checkVisibility$, { + root: context.popoverRef.value, + threshold: 1.0, + }); + + cleanup(() => observer?.disconnect()); + + if (itemRef.value) { + observer.observe(itemRef.value); + } + }, 100); + + cleanup(() => { + if (debounceTimeoutSig.value !== undefined) { + clearTimeout(debounceTimeoutSig.value); + } + }); }); const handleClick$ = $(async () => { diff --git a/packages/kit-headless/src/components/select/select-root.tsx b/packages/kit-headless/src/components/select/select-root.tsx index 3573aff20..4ff51f571 100644 --- a/packages/kit-headless/src/components/select/select-root.tsx +++ b/packages/kit-headless/src/components/select/select-root.tsx @@ -168,6 +168,7 @@ export const HSelectImpl = component$ & InternalSelectProps const highlightedItemRef = useSignal(); const isDisabledSig = useSignal(disabled ?? false); const isInvalidSig = useSignal(props.invalid ?? false); + const isKeyboardFocusSig = useSignal(false); useTask$(({ track }) => { /** @@ -192,6 +193,7 @@ export const HSelectImpl = component$ & InternalSelectProps localId, highlightedIndexSig, selectedIndexSetSig, + isKeyboardFocusSig, isListboxOpenSig, scrollOptions, loop, From fabb53958cf623d13b0b44e3066a06e19aa75e06 Mon Sep 17 00:00:00 2001 From: thejackshelton Date: Wed, 22 Jan 2025 20:41:21 -0600 Subject: [PATCH 4/7] feat: correct key navigation --- .../kit-headless/src/components/select/select-context.ts | 1 + packages/kit-headless/src/components/select/select-item.tsx | 1 + .../kit-headless/src/components/select/select-popover.tsx | 6 ++++++ packages/kit-headless/src/components/select/select-root.tsx | 2 ++ .../kit-headless/src/components/select/select-trigger.tsx | 1 + 5 files changed, 11 insertions(+) diff --git a/packages/kit-headless/src/components/select/select-context.ts b/packages/kit-headless/src/components/select/select-context.ts index 053c3bb75..0310338f7 100644 --- a/packages/kit-headless/src/components/select/select-context.ts +++ b/packages/kit-headless/src/components/select/select-context.ts @@ -22,6 +22,7 @@ export type SelectContext = { highlightedIndexSig: Signal; currDisplayValueSig: Signal; isKeyboardFocusSig: Signal; + isMouseOverPopupSig: Signal; isListboxOpenSig: Signal; isDisabledSig: Signal; localId: string; diff --git a/packages/kit-headless/src/components/select/select-item.tsx b/packages/kit-headless/src/components/select/select-item.tsx index 30caf5834..431064ad8 100644 --- a/packages/kit-headless/src/components/select/select-item.tsx +++ b/packages/kit-headless/src/components/select/select-item.tsx @@ -182,6 +182,7 @@ export const HSelectItem = component$((props) => { const handleKeyDown$ = $(async (e: KeyboardEvent) => { typeaheadFnSig.value?.(e.key); + context.isKeyboardFocusSig.value = true; switch (e.key) { case 'ArrowDown': diff --git a/packages/kit-headless/src/components/select/select-popover.tsx b/packages/kit-headless/src/components/select/select-popover.tsx index 0b8660494..6e1e9a8ba 100644 --- a/packages/kit-headless/src/components/select/select-popover.tsx +++ b/packages/kit-headless/src/components/select/select-popover.tsx @@ -85,6 +85,11 @@ export const HSelectPopover = component$>((props) = initialLoadSig.value = false; }); + const handleMouseEnter$ = $(() => { + context.isKeyboardFocusSig.value = false; + context.isMouseOverPopupSig.value = true; + }); + return ( >((props) = aria-expanded={context.isListboxOpenSig.value ? 'true' : undefined} aria-multiselectable={context.multiple ? 'true' : undefined} aria-labelledby={triggerId} + onMouseEnter$={[handleMouseEnter$, props.onMouseEnter$]} {...rest} > diff --git a/packages/kit-headless/src/components/select/select-root.tsx b/packages/kit-headless/src/components/select/select-root.tsx index 4ff51f571..27732df8f 100644 --- a/packages/kit-headless/src/components/select/select-root.tsx +++ b/packages/kit-headless/src/components/select/select-root.tsx @@ -169,6 +169,7 @@ export const HSelectImpl = component$ & InternalSelectProps const isDisabledSig = useSignal(disabled ?? false); const isInvalidSig = useSignal(props.invalid ?? false); const isKeyboardFocusSig = useSignal(false); + const isMouseOverPopupSig = useSignal(false); useTask$(({ track }) => { /** @@ -194,6 +195,7 @@ export const HSelectImpl = component$ & InternalSelectProps highlightedIndexSig, selectedIndexSetSig, isKeyboardFocusSig, + isMouseOverPopupSig, isListboxOpenSig, scrollOptions, loop, diff --git a/packages/kit-headless/src/components/select/select-trigger.tsx b/packages/kit-headless/src/components/select/select-trigger.tsx index 55e686591..f58391b43 100644 --- a/packages/kit-headless/src/components/select/select-trigger.tsx +++ b/packages/kit-headless/src/components/select/select-trigger.tsx @@ -56,6 +56,7 @@ export const HSelectTrigger = component$((props) => { const handleKeyDown$ = $(async (e: KeyboardEvent) => { if (!context.itemsMapSig.value) return; + context.isKeyboardFocusSig.value = true; if (!context.isListboxOpenSig.value) { typeahead$(e.key); From ee9a5f11cadd7737f010858ddbfea0a628d400fe Mon Sep 17 00:00:00 2001 From: thejackshelton Date: Wed, 22 Jan 2025 21:20:42 -0600 Subject: [PATCH 5/7] fix: infinite scroll --- packages/kit-headless/src/components/combobox/combobox-item.tsx | 2 +- packages/kit-headless/src/components/select/select-item.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit-headless/src/components/combobox/combobox-item.tsx b/packages/kit-headless/src/components/combobox/combobox-item.tsx index 7c5bad38b..90bfd71d4 100644 --- a/packages/kit-headless/src/components/combobox/combobox-item.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-item.tsx @@ -141,7 +141,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { const observer = new IntersectionObserver(checkVisibility$, { root: context.panelRef.value, - threshold: 1.0, + threshold: 0, }); cleanup(() => observer?.disconnect()); diff --git a/packages/kit-headless/src/components/select/select-item.tsx b/packages/kit-headless/src/components/select/select-item.tsx index 431064ad8..b880ae1f6 100644 --- a/packages/kit-headless/src/components/select/select-item.tsx +++ b/packages/kit-headless/src/components/select/select-item.tsx @@ -119,7 +119,7 @@ export const HSelectItem = component$((props) => { const observer = new IntersectionObserver(checkVisibility$, { root: context.popoverRef.value, - threshold: 1.0, + threshold: 0, }); cleanup(() => observer?.disconnect()); From 37fd69c0ce5bb414715a10a273054b4d98abf3cd Mon Sep 17 00:00:00 2001 From: thejackshelton Date: Wed, 22 Jan 2025 21:30:50 -0600 Subject: [PATCH 6/7] comment out broken ci check --- .github/workflows/test.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index da7d2c42f..9189d3d51 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -47,8 +47,9 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # GITHUB_TOKEN is provided automatically in any repository - pagefind: - runs-on: ubuntu-latest - steps: - - name: Generate - run: npx pagefind --site dist/apps/website/client && cp -r dist/apps/website/client/pagefind apps/website/public/pagefind + + # pagefind: + # runs-on: ubuntu-latest + # steps: + # - name: Generate + # run: npx pagefind --site dist/apps/website/client && cp -r dist/apps/website/client/pagefind apps/website/public/pagefind From e453978cd61f94f3a3c919499a9b2a75d6d185c8 Mon Sep 17 00:00:00 2001 From: thejackshelton Date: Wed, 22 Jan 2025 22:08:12 -0600 Subject: [PATCH 7/7] changeset --- .changeset/old-ladybugs-pump.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/old-ladybugs-pump.md diff --git a/.changeset/old-ladybugs-pump.md b/.changeset/old-ladybugs-pump.md new file mode 100644 index 000000000..3c5e8287d --- /dev/null +++ b/.changeset/old-ladybugs-pump.md @@ -0,0 +1,10 @@ +--- +'@qwik-ui/headless': patch +--- + +Select fixes for: + +- [#1001](https://github.com/qwikifiers/qwik-ui/issues/1001) +- [#979](https://github.com/qwikifiers/qwik-ui/issues/979) + +Also fixes for infinite scroll issues when both keyboard and mouse are used to navigate the listbox.