Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select fixes 0.6.5 #1042

Merged
merged 7 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading