Skip to content

Commit

Permalink
Select fixes 0.6.5 (#1042)
Browse files Browse the repository at this point in the history
* fix: select now shows first item instead of placeholder when initial value is set

* fix: prevent native scrolling on focus in select

* feat: initial scroll handling logic

* feat: correct key navigation

* fix: infinite scroll

* comment out broken ci check

* changeset
  • Loading branch information
thejackshelton authored Jan 23, 2025
1 parent ac52aa1 commit e3b8dd2
Show file tree
Hide file tree
Showing 13 changed files with 175 additions and 66 deletions.
10 changes: 10 additions & 0 deletions .changeset/old-ladybugs-pump.md
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 6 additions & 5 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<Combobox.Root class="combobox-root">
Expand All @@ -17,23 +16,11 @@ export default component$(() => {
</Combobox.Trigger>
</Combobox.Control>
<Combobox.Popover class="combobox-popover combobox-max-height" gutter={8}>
<Combobox.Group>
<Combobox.GroupLabel class="combobox-group-label">Active</Combobox.GroupLabel>
{activeUsers.map((user) => (
<Combobox.Item key={user}>
<Combobox.ItemLabel>{user}</Combobox.ItemLabel>
</Combobox.Item>
))}
</Combobox.Group>

<Combobox.Group>
<Combobox.GroupLabel class="combobox-group-label">Offline</Combobox.GroupLabel>
{offlineUsers.map((user) => (
<Combobox.Item key={user}>
<Combobox.ItemLabel>{user}</Combobox.ItemLabel>
</Combobox.Item>
))}
</Combobox.Group>
{items.map((item) => (
<Combobox.Item key={item}>
<Combobox.ItemLabel>{item}</Combobox.ItemLabel>
</Combobox.Item>
))}
</Combobox.Popover>
</Combobox.Root>
);
Expand Down
55 changes: 55 additions & 0 deletions apps/website/src/routes/docs/headless/select/auto-api/api.ts
Original file line number Diff line number Diff line change
@@ -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': [],
},
],
};
Original file line number Diff line number Diff line change
@@ -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 (
<Select.Root value="Tim" class="select">
<Select.Label>Logged in users</Select.Label>
<Select.Trigger class="select-trigger">
<Select.DisplayValue placeholder="Select an option" />
</Select.Trigger>
<Select.Popover class="select-popover">
{users.map((user) => (
<Select.Item key={user}>
<Select.ItemLabel>{user}</Select.ItemLabel>
</Select.Item>
))}
</Select.Popover>
</Select.Root>
);
});

// internal
import styles from '../snippets/select.css?inline';
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,19 @@ 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 (
<Select.Root class="select">
<Select.Trigger class="select-trigger">
<Select.DisplayValue placeholder="Select an option" />
</Select.Trigger>
<Select.Popover class="select-popover select-max-height">
<Select.Group>
<Select.GroupLabel class="select-label">People</Select.GroupLabel>
{users.map((user) => (
<Select.Item key={user}>
<Select.ItemLabel>{user}</Select.ItemLabel>
</Select.Item>
))}
</Select.Group>
<Select.Group>
<Select.GroupLabel class="select-label">Animals</Select.GroupLabel>
{animals.map((animal) => (
<Select.Item key={animal}>
<Select.ItemLabel>{animal}</Select.ItemLabel>
</Select.Item>
))}
</Select.Group>
{items.map((item) => (
<Select.Item key={item} class="highlight-hover">
<Select.ItemLabel>{item}</Select.ItemLabel>
</Select.Item>
))}
</Select.Popover>
</Select.Root>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions packages/kit-headless/src/components/select/select-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export type SelectContext = {
selectedIndexSetSig: Signal<Set<number>>;
highlightedIndexSig: Signal<number | null>;
currDisplayValueSig: Signal<string | string[] | undefined>;
isKeyboardFocusSig: Signal<boolean>;
isMouseOverPopupSig: Signal<boolean>;
isListboxOpenSig: Signal<boolean>;
isDisabledSig: Signal<boolean>;
localId: string;
Expand Down
48 changes: 32 additions & 16 deletions packages/kit-headless/src/components/select/select-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import {
useOnWindow,
QRL,
} from '@builder.io/qwik';
import { isServer } from '@builder.io/qwik/build';
import SelectContextId, {
SelectItemContext,
selectItemContextId,
} 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 */
Expand All @@ -39,6 +39,7 @@ export const HSelectItem = component$<SelectItemProps>((props) => {
const localIndexSig = useSignal<number | null>(null);
const itemId = `${context.localId}-${_index}`;
const typeaheadFnSig = useSignal<QRL<(key: string) => Promise<void>>>();
const debounceTimeoutSig = useSignal<NodeJS.Timeout>();

const { selectionManager$, getNextEnabledItemIndex$, getPrevEnabledItemIndex$ } =
useSelect();
Expand All @@ -60,7 +61,7 @@ export const HSelectItem = component$<SelectItemProps>((props) => {
if (disabled) return;

if (context.highlightedIndexSig.value === _index) {
itemRef.value?.focus();
itemRef.value?.focus({ preventScroll: true });
return true;
} else {
return false;
Expand All @@ -81,17 +82,20 @@ export const HSelectItem = component$<SelectItemProps>((props) => {
const containerRect = context.popoverRef.value?.getBoundingClientRect();
const itemRect = itemRef.value?.getBoundingClientRect();

if (!containerRect || !itemRect) return;
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: offset, ...context.scrollOptions });
context.popoverRef.value?.scrollBy({
top: document.hasFocus() ? offset : undefined,
...context.scrollOptions,
});
}
});

useTask$(async function navigationTask({ track, cleanup }) {
useTask$(function handleScrolling({ track, cleanup }) {
track(() => context.highlightedIndexSig.value);

// update the context with the highlighted item ref
Expand All @@ -100,25 +104,36 @@ export const HSelectItem = component$<SelectItemProps>((props) => {
}

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;
if (!hasScrollbar) return;

if (debounceTimeoutSig.value !== undefined) {
clearTimeout(debounceTimeoutSig.value);
}

const observer = new IntersectionObserver(checkVisibility$, {
root: context.popoverRef.value,
threshold: 1.0,
});
debounceTimeoutSig.value = setTimeout(() => {
if (props._index !== context.highlightedIndexSig.value) return;

cleanup(() => observer?.disconnect());
const observer = new IntersectionObserver(checkVisibility$, {
root: context.popoverRef.value,
threshold: 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);
}
});
});

const handleClick$ = $(async () => {
Expand Down Expand Up @@ -167,6 +182,7 @@ export const HSelectItem = component$<SelectItemProps>((props) => {

const handleKeyDown$ = $(async (e: KeyboardEvent) => {
typeaheadFnSig.value?.(e.key);
context.isKeyboardFocusSig.value = true;

switch (e.key) {
case 'ArrowDown':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ export const HSelectPopover = component$<PropsOf<typeof HPopoverRoot>>((props) =
initialLoadSig.value = false;
});

const handleMouseEnter$ = $(() => {
context.isKeyboardFocusSig.value = false;
context.isMouseOverPopupSig.value = true;
});

return (
<HPopoverRoot
floating={floating}
Expand All @@ -105,6 +110,7 @@ export const HSelectPopover = component$<PropsOf<typeof HPopoverRoot>>((props) =
aria-expanded={context.isListboxOpenSig.value ? 'true' : undefined}
aria-multiselectable={context.multiple ? 'true' : undefined}
aria-labelledby={triggerId}
onMouseEnter$={[handleMouseEnter$, props.onMouseEnter$]}
{...rest}
>
<Slot />
Expand Down
18 changes: 11 additions & 7 deletions packages/kit-headless/src/components/select/select-root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<M extends boolean = boolean> = Omit<
PropsOf<'div'>,
Expand Down Expand Up @@ -150,7 +150,7 @@ export const HSelectImpl = component$<SelectProps<boolean> & InternalSelectProps
});

const selectedIndexSetSig = useSignal<Set<number>>(
new Set(givenValuePropIndex ? [givenValuePropIndex] : []),
new Set([givenValuePropIndex ?? []].flat()),
);

const highlightedIndexSig = useSignal<number | null>(givenValuePropIndex ?? null);
Expand All @@ -168,6 +168,8 @@ export const HSelectImpl = component$<SelectProps<boolean> & InternalSelectProps
const highlightedItemRef = useSignal<HTMLLIElement>();
const isDisabledSig = useSignal<boolean>(disabled ?? false);
const isInvalidSig = useSignal<boolean>(props.invalid ?? false);
const isKeyboardFocusSig = useSignal<boolean>(false);
const isMouseOverPopupSig = useSignal<boolean>(false);

useTask$(({ track }) => {
/**
Expand All @@ -192,6 +194,8 @@ export const HSelectImpl = component$<SelectProps<boolean> & InternalSelectProps
localId,
highlightedIndexSig,
selectedIndexSetSig,
isKeyboardFocusSig,
isMouseOverPopupSig,
isListboxOpenSig,
scrollOptions,
loop,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const HSelectTrigger = component$<SelectTriggerProps>((props) => {

const handleKeyDown$ = $(async (e: KeyboardEvent) => {
if (!context.itemsMapSig.value) return;
context.isKeyboardFocusSig.value = true;

if (!context.isListboxOpenSig.value) {
typeahead$(e.key);
Expand Down
Loading

0 comments on commit e3b8dd2

Please sign in to comment.