Skip to content

Commit cd934d8

Browse files
Merge pull request #822 from thejackshelton/select-focus
Select focus navigation refactor
2 parents b2d8dfe + 2aa748f commit cd934d8

File tree

9 files changed

+480
-572
lines changed

9 files changed

+480
-572
lines changed

.changeset/olive-sheep-run.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@qwik-ui/headless': patch
3+
---
4+
5+
refactor: improved select focus navigation

apps/website/src/routes/docs/headless/select/snippets/select.css

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,18 @@
1717
display: flex;
1818
justify-content: center;
1919
align-items: center;
20+
margin-top: 0.25rem;
2021
}
2122

2223
.select-trigger:hover {
2324
background-color: hsla(var(--primary) / 0.08);
2425
}
2526

27+
.select-trigger:focus-visible {
28+
outline: 2px solid hsla(var(--primary) / 1);
29+
outline-offset: 2px;
30+
}
31+
2632
.select-popover {
2733
width: 100%;
2834
max-width: var(--select-width);

cla-signs/v1/cla.json

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -479,14 +479,6 @@
479479
"created_at": "2024-05-31T08:39:43Z",
480480
"repoId": 545159943,
481481
"pullRequestNo": 810
482-
},
483-
{
484-
"name": "harshmangalam",
485-
"id": 57381638,
486-
"comment_id": 2145776407,
487-
"created_at": "2024-06-03T17:40:03Z",
488-
"repoId": 545159943,
489-
"pullRequestNo": 820
490482
}
491483
]
492484
}

packages/kit-headless/src/components/select/select-context.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export type SelectContext = {
1414
listboxRef: Signal<HTMLUListElement | undefined>;
1515
groupRef: Signal<HTMLDivElement | undefined>;
1616
labelRef: Signal<HTMLDivElement | undefined>;
17+
highlightedItemRef: Signal<HTMLLIElement | undefined>;
1718

1819
// core state
1920
itemsMapSig: Readonly<Signal<TItemsMap>>;

packages/kit-headless/src/components/select/select-item.tsx

Lines changed: 125 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import {
88
useTask$,
99
type PropsOf,
1010
useContextProvider,
11+
sync$,
12+
useOnWindow,
13+
QRL,
1114
} from '@builder.io/qwik';
1215
import { isServer, isBrowser } from '@builder.io/qwik/build';
1316
import SelectContextId, {
@@ -34,28 +37,50 @@ export const HSelectItem = component$<SelectItemProps>((props) => {
3437
const itemRef = useSignal<HTMLLIElement>();
3538
const localIndexSig = useSignal<number | null>(null);
3639
const itemId = `${context.localId}-${_index}`;
40+
const typeaheadFnSig = useSignal<QRL<(key: string) => Promise<void>>>();
3741

38-
const { selectionManager$ } = useSelect();
42+
const { selectionManager$, getNextEnabledItemIndex$, getPrevEnabledItemIndex$ } =
43+
useSelect();
44+
45+
// we're getting the same function instance from the trigger, without needing to restructure context
46+
useOnWindow(
47+
'typeaheadFn',
48+
$((e: CustomEvent) => {
49+
typeaheadFnSig.value = e.detail;
50+
}),
51+
);
3952

4053
const isSelectedSig = useComputed$(() => {
4154
const index = _index ?? null;
4255
return !disabled && context.selectedIndexSetSig.value.has(index!);
4356
});
4457

4558
const isHighlightedSig = useComputed$(() => {
46-
return !disabled && context.highlightedIndexSig.value === _index;
59+
if (disabled) return;
60+
61+
if (context.highlightedIndexSig.value === _index) {
62+
itemRef.value?.focus();
63+
return true;
64+
} else {
65+
return false;
66+
}
4767
});
4868

49-
useTask$(function getIndexTask() {
69+
useTask$(async function getIndexTask() {
5070
if (_index === undefined)
5171
throw Error('Qwik UI: Select component item cannot find its proper index.');
5272

5373
localIndexSig.value = _index;
5474
});
5575

56-
useTask$(function scrollableTask({ track, cleanup }) {
76+
useTask$(async function navigationTask({ track, cleanup }) {
5777
track(() => context.highlightedIndexSig.value);
5878

79+
// update the context with the highlighted item ref
80+
if (localIndexSig.value === context.highlightedIndexSig.value) {
81+
context.highlightedItemRef = itemRef;
82+
}
83+
5984
if (isServer) return;
6085

6186
let observer: IntersectionObserver;
@@ -109,13 +134,109 @@ export const HSelectItem = component$<SelectItemProps>((props) => {
109134
isSelectedSig,
110135
};
111136

137+
const handleKeyDownSync$ = sync$((e: KeyboardEvent) => {
138+
const keys = [
139+
'ArrowUp',
140+
'ArrowDown',
141+
'ArrowRight',
142+
'ArrowLeft',
143+
'Home',
144+
'End',
145+
'PageDown',
146+
'PageUp',
147+
'Enter',
148+
' ',
149+
];
150+
if (keys.includes(e.key)) {
151+
e.preventDefault();
152+
}
153+
});
154+
155+
const handleKeyDown$ = $(async (e: KeyboardEvent) => {
156+
typeaheadFnSig.value?.(e.key);
157+
158+
switch (e.key) {
159+
case 'ArrowDown':
160+
if (context.isListboxOpenSig.value) {
161+
context.highlightedIndexSig.value = await getNextEnabledItemIndex$(
162+
context.highlightedIndexSig.value!,
163+
);
164+
if (context.multiple && e.shiftKey) {
165+
await selectionManager$(context.highlightedIndexSig.value, 'toggle');
166+
}
167+
}
168+
break;
169+
170+
case 'ArrowUp':
171+
if (context.isListboxOpenSig.value) {
172+
context.highlightedIndexSig.value = await getPrevEnabledItemIndex$(
173+
context.highlightedIndexSig.value!,
174+
);
175+
if (context.multiple && e.shiftKey) {
176+
await selectionManager$(context.highlightedIndexSig.value, 'toggle');
177+
}
178+
}
179+
break;
180+
181+
case 'Home':
182+
if (context.isListboxOpenSig.value) {
183+
context.highlightedIndexSig.value = await getNextEnabledItemIndex$(-1);
184+
}
185+
break;
186+
187+
case 'End':
188+
if (context.isListboxOpenSig.value) {
189+
const lastEnabledOptionIndex = await getPrevEnabledItemIndex$(
190+
context.itemsMapSig.value.size,
191+
);
192+
context.highlightedIndexSig.value = lastEnabledOptionIndex;
193+
}
194+
break;
195+
196+
case 'Escape':
197+
context.triggerRef.value?.focus();
198+
context.isListboxOpenSig.value = false;
199+
break;
200+
201+
case 'Tab':
202+
context.isListboxOpenSig.value = false;
203+
break;
204+
205+
case 'Enter':
206+
case ' ':
207+
if (context.isListboxOpenSig.value) {
208+
const action = context.multiple ? 'toggle' : 'add';
209+
await selectionManager$(context.highlightedIndexSig.value, action);
210+
211+
if (!context.multiple) {
212+
context.triggerRef.value?.focus();
213+
}
214+
}
215+
context.isListboxOpenSig.value = context.multiple
216+
? true
217+
: !context.isListboxOpenSig.value;
218+
break;
219+
220+
case 'a':
221+
if (e.ctrlKey && context.multiple) {
222+
for (const [index, item] of context.itemsMapSig.value) {
223+
if (!item.disabled) {
224+
await selectionManager$(index, 'add');
225+
}
226+
}
227+
}
228+
break;
229+
}
230+
});
231+
112232
useContextProvider(selectItemContextId, selectContext);
113233

114234
return (
115235
<li
116236
{...rest}
117237
id={itemId}
118238
onClick$={[handleClick$, props.onClick$]}
239+
onKeyDown$={[handleKeyDownSync$, handleKeyDown$, props.onKeyDown$]}
119240
onPointerOver$={[handlePointerOver$, props.onPointerOver$]}
120241
ref={itemRef}
121242
tabIndex={-1}

packages/kit-headless/src/components/select/select-root.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ export const HSelectImpl = component$<SelectProps<boolean> & InternalSelectProps
161161
const currDisplayValueSig = useSignal<string | string[]>();
162162

163163
const initialLoadSig = useSignal<boolean>(true);
164+
const highlightedItemRef = useSignal<HTMLLIElement>();
164165

165166
const context: SelectContext = {
166167
itemsMapSig,
@@ -170,6 +171,7 @@ export const HSelectImpl = component$<SelectProps<boolean> & InternalSelectProps
170171
listboxRef,
171172
labelRef,
172173
groupRef,
174+
highlightedItemRef,
173175
localId,
174176
highlightedIndexSig,
175177
selectedIndexSetSig,

packages/kit-headless/src/components/select/select-trigger.tsx

Lines changed: 31 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import { $, Slot, component$, sync$, useContext, type PropsOf } from '@builder.io/qwik';
1+
import {
2+
$,
3+
Slot,
4+
component$,
5+
sync$,
6+
useContext,
7+
useSignal,
8+
type PropsOf,
9+
} from '@builder.io/qwik';
210
import SelectContextId from './select-context';
311
import { useSelect, useTypeahead } from './use-select';
412

@@ -9,7 +17,7 @@ export const HSelectTrigger = component$<SelectTriggerProps>((props) => {
917
useSelect();
1018
const labelId = `${context.localId}-label`;
1119
const descriptionId = `${context.localId}-description`;
12-
20+
const initialKeyDownSig = useSignal(true);
1321
const { typeahead$ } = useTypeahead();
1422

1523
const handleClickSync$ = sync$((e: MouseEvent) => {
@@ -42,66 +50,23 @@ export const HSelectTrigger = component$<SelectTriggerProps>((props) => {
4250
const handleKeyDown$ = $(async (e: KeyboardEvent) => {
4351
if (!context.itemsMapSig.value) return;
4452

45-
typeahead$(e.key);
53+
if (!context.isListboxOpenSig.value) {
54+
typeahead$(e.key);
55+
}
4656

4757
switch (e.key) {
48-
case 'Enter':
49-
case ' ':
50-
if (context.isListboxOpenSig.value) {
51-
const action = context.multiple ? 'toggle' : 'add';
52-
await selectionManager$(context.highlightedIndexSig.value, action);
53-
}
54-
context.isListboxOpenSig.value = context.multiple
55-
? true
56-
: !context.isListboxOpenSig.value;
58+
case 'Tab':
59+
case 'Escape':
60+
context.isListboxOpenSig.value = false;
5761
break;
5862

5963
case 'ArrowDown':
60-
if (context.isListboxOpenSig.value) {
61-
context.highlightedIndexSig.value = await getNextEnabledItemIndex$(
62-
context.highlightedIndexSig.value!,
63-
);
64-
if (context.multiple && e.shiftKey) {
65-
await selectionManager$(context.highlightedIndexSig.value, 'toggle');
66-
}
67-
} else {
68-
context.isListboxOpenSig.value = true;
69-
}
70-
break;
71-
7264
case 'ArrowUp':
73-
if (context.isListboxOpenSig.value) {
74-
context.highlightedIndexSig.value = await getPrevEnabledItemIndex$(
75-
context.highlightedIndexSig.value!,
76-
);
77-
if (context.multiple && e.shiftKey) {
78-
await selectionManager$(context.highlightedIndexSig.value, 'toggle');
79-
}
80-
} else {
65+
if (!context.isListboxOpenSig.value) {
8166
context.isListboxOpenSig.value = true;
8267
}
8368
break;
8469

85-
case 'Home':
86-
if (context.isListboxOpenSig.value) {
87-
context.highlightedIndexSig.value = await getNextEnabledItemIndex$(-1);
88-
}
89-
break;
90-
91-
case 'End':
92-
if (context.isListboxOpenSig.value) {
93-
const lastEnabledOptionIndex = await getPrevEnabledItemIndex$(
94-
context.itemsMapSig.value.size,
95-
);
96-
context.highlightedIndexSig.value = lastEnabledOptionIndex;
97-
}
98-
break;
99-
100-
case 'Tab':
101-
case 'Escape':
102-
context.isListboxOpenSig.value = false;
103-
break;
104-
10570
case 'ArrowRight':
10671
if (!context.multiple) {
10772
const currentIndex = context.highlightedIndexSig.value ?? -1;
@@ -121,22 +86,27 @@ export const HSelectTrigger = component$<SelectTriggerProps>((props) => {
12186
}
12287
break;
12388

124-
case 'a':
125-
if (e.ctrlKey && context.multiple) {
126-
for (const [index, item] of context.itemsMapSig.value) {
127-
if (!item.disabled) {
128-
await selectionManager$(index, 'add');
129-
}
130-
}
131-
}
89+
case 'Enter':
90+
case ' ':
91+
context.isListboxOpenSig.value = context.multiple
92+
? true
93+
: !context.isListboxOpenSig.value;
13294
break;
13395
}
13496

13597
/** When initially opening the listbox, we want to grab the first enabled option index */
13698
if (context.highlightedIndexSig.value === null) {
13799
context.highlightedIndexSig.value = await getNextEnabledItemIndex$(-1);
138-
return;
139100
}
101+
102+
// Wait for the popover code to be executed
103+
while (context.highlightedItemRef.value !== document.activeElement) {
104+
await new Promise((resolve) => setTimeout(resolve, 5));
105+
context.highlightedItemRef.value?.focus();
106+
}
107+
108+
if (!initialKeyDownSig.value) return;
109+
document.dispatchEvent(new CustomEvent('typeaheadFn', { detail: typeahead$ }));
140110
});
141111

142112
return (

packages/kit-headless/src/components/select/select.driver.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
3535
return getTrigger().locator('[data-value]');
3636
};
3737

38+
const getHighlightedItem = () => {
39+
return getRoot().locator('[data-highlighted]');
40+
};
41+
3842
const openListbox = async (key: OpenKeys | 'click') => {
3943
await getTrigger().focus();
4044

@@ -59,5 +63,6 @@ export function createTestDriver<T extends DriverLocator>(rootLocator: T) {
5963
getItemAt,
6064
getValueElement,
6165
openListbox,
66+
getHighlightedItem,
6267
};
6368
}

0 commit comments

Comments
 (0)