Skip to content

Commit 5ed29ee

Browse files
thejacksheltonthejackshelton
committed
feat(combobox): selected behavior working according to spec
1 parent 6dd00a7 commit 5ed29ee

File tree

5 files changed

+61
-89
lines changed

5 files changed

+61
-89
lines changed

‎apps/website/src/routes/docs/headless/combobox/examples/hero.tsx

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@ import {
1111
ComboboxPopover,
1212
} from '@qwik-ui/headless';
1313

14-
import { component$ } from '@builder.io/qwik';
14+
import { component$, useSignal } from '@builder.io/qwik';
1515

1616
export default component$(() => {
17+
const selectedOptionIndexSig = useSignal<number>(-1);
18+
1719
const objectExample = [
1820
{ testValue: 'alice', testLabel: 'Alice', disabled: true },
1921
{ testValue: 'joana', testLabel: 'Joana', disabled: true },
@@ -40,32 +42,32 @@ export default component$(() => {
4042
optionLabelKey="testLabel"
4143
optionDisabledKey="disabled"
4244
class="relative"
45+
bind:selectedIndex={selectedOptionIndexSig}
4346
>
4447
<ComboboxLabel class="font-semibold">Personal Trainers ⚡</ComboboxLabel>
4548
<ComboboxControl class="relative flex items-center rounded-sm border">
4649
<ComboboxInput
4750
placeholder="Jim"
48-
class="px-d2 bg-background placeholder:text-muted-foreground w-44 rounded-sm px-2 pr-6"
51+
class="px-d2 rounded-sm bg-slate-900 px-2 pr-6 text-white placeholder:text-slate-400"
4952
/>
5053
<ComboboxTrigger class="group absolute right-0 h-6 w-6">
51-
<ComboboxIcon class="stroke-foreground transition-transform duration-[450ms] group-aria-expanded:-rotate-180" />
54+
<ComboboxIcon class="stroke-white transition-transform duration-[450ms] group-aria-expanded:-rotate-180" />
5255
</ComboboxTrigger>
5356
</ComboboxControl>
5457
<ComboboxPopover gutter={8}>
5558
<ComboboxListbox
56-
class="w-44 rounded-sm border-[1px] border-slate-400 bg-slate-900 px-4 py-2"
59+
class="rounded-sm border-[1px] border-slate-400 bg-slate-900 px-4 py-2"
5760
optionRenderer$={(option: ResolvedOption, index: number) => {
5861
const myData = option.option as MyData;
5962
return (
6063
<ComboboxOption
6164
key={option.key}
6265
resolved={option}
6366
index={index}
64-
class="hover:bg-accent aria-disabled:text-muted-foreground aria-disabled:hover:bg-muted aria-selected:border-border aria-selected:bg-accent group flex justify-between rounded-sm border border-transparent px-2 aria-disabled:font-light aria-selected:cursor-pointer"
67+
class="aria-disabled:text-muted-foreground data-[highlighted]:border-border group flex justify-between gap-4 rounded-sm border border-transparent px-2 text-white aria-disabled:font-light aria-disabled:hover:border-slate-500 data-[highlighted]:cursor-pointer data-[highlighted]:bg-slate-800"
6568
>
66-
<span class="duration-350 block transition-transform group-aria-selected:translate-x-[3px]">
67-
{myData.testLabel}
68-
</span>
69+
<span>{myData.testLabel}</span>
70+
<span>{selectedOptionIndexSig.value === index ? 'Selected' : ''} </span>
6971
</ComboboxOption>
7072
);
7173
}}

‎packages/kit-headless/src/components/combobox/combobox-input.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
useContext,
55
useSignal,
66
useTask$,
7-
type QwikKeyboardEvent,
87
type ContextId,
98
PropsOf,
109
} from '@builder.io/qwik';
@@ -43,7 +42,7 @@ export const ComboboxInput = component$(
4342
context.inputValueSig.value = inputElement.value;
4443
});
4544

46-
const onKeydownBehavior$ = $((e: QwikKeyboardEvent) => {
45+
const onKeydownBehavior$ = $((e: KeyboardEvent) => {
4746
if (e.key === 'ArrowDown') {
4847
if (context.isListboxOpenSig.value) {
4948
const nextEnabledOptionIndex = getNextEnabledOptionIndex(
@@ -73,6 +72,7 @@ export const ComboboxInput = component$(
7372

7473
if (e.key === 'Enter') {
7574
context.isListboxOpenSig.value = false;
75+
context.selectedOptionIndexSig.value = context.highlightedIndexSig.value;
7676

7777
// if they somehow manage to highlight a disabled option (bug)
7878
if (

‎packages/kit-headless/src/components/combobox/combobox-option.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@ export const ComboboxOption = component$(
2323
const optionId = `${context.localId}-${resolved.key}`;
2424

2525
const isHighlightedSig = useComputed$(
26-
// eslint-disable-next-line qwik/valid-lexical-scope
2726
() => !resolved.disabled && context.highlightedIndexSig.value === index,
2827
);
2928

3029
const onClickBehavior$ = $(() => {
31-
// eslint-disable-next-line qwik/valid-lexical-scope
3230
if (!context.inputRef.value || resolved.disabled) {
3331
return;
3432
}
3533

3634
(context.inputRef.value.value = context.filteredOptionsSig.value[index]?.label),
3735
(context.isListboxOpenSig.value = false);
36+
37+
context.selectedOptionIndexSig.value = index;
3838
});
3939

4040
const optionRef = useSignal<HTMLLIElement>();
@@ -46,7 +46,9 @@ export const ComboboxOption = component$(
4646
ref={optionRef}
4747
tabIndex={-1}
4848
role="option"
49-
aria-selected={isHighlightedSig.value}
49+
data-highlighted={isHighlightedSig.value}
50+
aria-selected={index === context.selectedOptionIndexSig.value}
51+
data-selected={index === context.selectedOptionIndexSig.value}
5052
aria-disabled={resolved.disabled}
5153
data-disabled={resolved.disabled}
5254
onClick$={[onClickBehavior$, props.onClick$]}

‎packages/kit-headless/src/components/combobox/combobox.spec.tsx

Lines changed: 32 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,19 @@ describe('Critical Functionality', () => {
9999
cy.get('button').click().should('have.attr', 'aria-expanded', 'true');
100100
});
101101

102+
it.only(`GIVEN a Combobox component with a trigger
103+
WHEN a trigger is clicked, the listbox is open, and the first open clicked
104+
THEN the first option should be selected
105+
`, () => {
106+
cy.mount(<StringCombobox />);
107+
108+
cy.get('button').click();
109+
110+
cy.get('li').first().click();
111+
112+
cy.get('li').first().should('have.attr', 'aria-selected');
113+
});
114+
102115
it(`GIVEN a Combobox component with an open listbox and trigger
103116
WHEN the trigger is clicked,
104117
THEN the listbox should close`, () => {
@@ -235,11 +248,7 @@ describe('Default Label', () => {
235248

236249
cy.get('input').should('have.value', 'Jabuticaba').type(`{downarrow}`);
237250

238-
cy.findByRole('option', { name: 'Jabuticaba' }).should(
239-
'have.attr',
240-
'aria-selected',
241-
'true',
242-
);
251+
cy.get('[data-highlighted]').should('have.text', 'Jabuticaba');
243252
});
244253
});
245254

@@ -265,7 +274,7 @@ describe('Keyboard Navigation', () => {
265274

266275
cy.get('input').type(`{downarrow}`).type(`{home}`);
267276

268-
cy.get('li').first().should('have.attr', 'aria-selected', 'true');
277+
cy.get('li').first().should('have.attr', 'data-highlighted');
269278
});
270279

271280
it(`GIVEN a Combobox component with a focused option inside a listbox,
@@ -275,7 +284,7 @@ describe('Keyboard Navigation', () => {
275284

276285
cy.get('input').type(`{downarrow}`).type(`{end}`);
277286

278-
cy.get('li').last().should('have.attr', 'aria-selected', 'true');
287+
cy.get('li').last().should('have.attr', 'data-highlighted');
279288
});
280289

281290
it(`GIVEN a Combobox component and selected text in an input field,
@@ -302,7 +311,7 @@ describe('Keyboard Navigation', () => {
302311
cy.get('input').type(`{downarrow}`).type(`{downarrow}`);
303312

304313
// grabs the 2nd element because the index is 1
305-
cy.get('li').filter(':visible').eq(1).should('have.attr', 'aria-selected', 'true');
314+
cy.get('li').filter(':visible').eq(1).should('have.attr', 'data-highlighted');
306315
});
307316

308317
it(`GIVEN a Combobox component with an open listbox and multiple filtered options
@@ -314,7 +323,7 @@ describe('Keyboard Navigation', () => {
314323

315324
cy.findByRole('listbox');
316325

317-
cy.get('li').filter(':visible').first().should('have.attr', 'aria-selected', 'true');
326+
cy.get('li').filter(':visible').first().should('have.attr', 'data-highlighted');
318327
});
319328

320329
it(`GIVEN a Combobox component with an open listbox and multiple filtered options
@@ -326,7 +335,7 @@ describe('Keyboard Navigation', () => {
326335

327336
cy.findByRole('listbox');
328337

329-
cy.get('li').filter(':visible').last().should('have.attr', 'aria-selected', 'true');
338+
cy.get('li').filter(':visible').last().should('have.attr', 'data-highlighted');
330339
});
331340

332341
it(`GIVEN a Combobox component with an open listbox and multiple filtered options
@@ -338,7 +347,7 @@ describe('Keyboard Navigation', () => {
338347

339348
cy.findByRole('listbox');
340349

341-
cy.get('li').filter(':visible').first().should('have.attr', 'aria-selected', 'true');
350+
cy.get('li').filter(':visible').first().should('have.attr', 'data-highlighted');
342351
});
343352

344353
it(`GIVEN a Combobox component with an open listbox and multiple filtered options
@@ -352,7 +361,7 @@ describe('Keyboard Navigation', () => {
352361

353362
cy.get('input').type(`{downarrow}`);
354363

355-
cy.get('li').filter(':visible').first().should('have.attr', 'aria-selected', 'true');
364+
cy.get('li').filter(':visible').first().should('have.attr', 'data-highlighted');
356365
});
357366

358367
it(`GIVEN a Combobox component with an open listbox and an option is in focus,
@@ -493,11 +502,7 @@ describe('Disabled & Object Combobox', () => {
493502

494503
cy.get('input').type(`{downarrow}`);
495504

496-
cy.findByRole('option', { name: `Malcolm` }).should(
497-
'have.attr',
498-
'aria-selected',
499-
'true',
500-
);
505+
cy.findByRole('option', { name: `Malcolm` }).should('have.attr', 'data-highlighted');
501506
});
502507

503508
it(`GIVEN a Combobox component with an open listbox and disabled options,
@@ -507,11 +512,7 @@ describe('Disabled & Object Combobox', () => {
507512

508513
cy.get('input').type(`{downarrow}`).type(`{uparrow}`);
509514

510-
cy.findByRole('option', { name: `Mark` }).should(
511-
'have.attr',
512-
'aria-selected',
513-
'true',
514-
);
515+
cy.findByRole('option', { name: `Mark` }).should('have.attr', 'data-highlighted');
515516
});
516517

517518
it(`GIVEN a Combobox component with an open listbox and disabled options,
@@ -523,11 +524,7 @@ describe('Disabled & Object Combobox', () => {
523524

524525
cy.get('input').type(`{home}`);
525526

526-
cy.findByRole('option', { name: `Malcolm` }).should(
527-
'have.attr',
528-
'aria-selected',
529-
'true',
530-
);
527+
cy.findByRole('option', { name: `Malcolm` }).should('have.attr', 'data-highlighted');
531528
});
532529

533530
it(`GIVEN a Combobox component with an open listbox and disabled options,
@@ -539,11 +536,7 @@ describe('Disabled & Object Combobox', () => {
539536

540537
cy.get('input').type(`{end}`);
541538

542-
cy.findByRole('option', { name: `Mark` }).should(
543-
'have.attr',
544-
'aria-selected',
545-
'true',
546-
);
539+
cy.findByRole('option', { name: `Mark` }).should('have.attr', 'data-highlighted');
547540
});
548541

549542
it(`GIVEN a Combobox component with an open listbox and disabled options,
@@ -556,11 +549,7 @@ describe('Disabled & Object Combobox', () => {
556549

557550
cy.get('input').type(`{downarrow}`);
558551

559-
cy.findByRole('option', { name: `Brian` }).should(
560-
'have.attr',
561-
'aria-selected',
562-
'true',
563-
);
552+
cy.findByRole('option', { name: `Brian` }).should('have.attr', 'data-highlighted');
564553
});
565554

566555
it(`GIVEN a Combobox component with an open listbox and disabled options,
@@ -575,11 +564,7 @@ describe('Disabled & Object Combobox', () => {
575564

576565
cy.get('input').type(`{uparrow}`);
577566

578-
cy.findByRole('option', { name: `Malcolm` }).should(
579-
'have.attr',
580-
'aria-selected',
581-
'true',
582-
);
567+
cy.findByRole('option', { name: `Malcolm` }).should('have.attr', 'data-highlighted');
583568
});
584569

585570
it(`GIVEN a Combobox component with an open listbox and disabled options,
@@ -595,19 +580,11 @@ describe('Disabled & Object Combobox', () => {
595580
.type(`{downarrow}`)
596581
.type(`{downarrow}`);
597582

598-
cy.findByRole('option', { name: `Randy` }).should(
599-
'have.attr',
600-
'aria-selected',
601-
'true',
602-
);
583+
cy.findByRole('option', { name: `Randy` }).should('have.attr', 'data-highlighted');
603584

604585
cy.get('input').type(`{downarrow}`);
605586

606-
cy.findByRole('option', { name: `Mark` }).should(
607-
'have.attr',
608-
'aria-selected',
609-
'true',
610-
);
587+
cy.findByRole('option', { name: `Mark` }).should('have.attr', 'data-highlighted');
611588
});
612589

613590
it(`GIVEN a Combobox component with an open listbox and disabled options,
@@ -623,26 +600,14 @@ describe('Disabled & Object Combobox', () => {
623600
.type(`{downarrow}`)
624601
.type(`{downarrow}`);
625602

626-
cy.findByRole('option', { name: `Randy` }).should(
627-
'have.attr',
628-
'aria-selected',
629-
'true',
630-
);
603+
cy.findByRole('option', { name: `Randy` }).should('have.attr', 'data-highlighted');
631604

632605
cy.get('input').type(`{downarrow}`);
633606

634-
cy.findByRole('option', { name: `Mark` }).should(
635-
'have.attr',
636-
'aria-selected',
637-
'true',
638-
);
607+
cy.findByRole('option', { name: `Mark` }).should('have.attr', 'data-highlighted');
639608

640609
cy.get('input').type(`{uparrow}`);
641610

642-
cy.findByRole('option', { name: `Randy` }).should(
643-
'have.attr',
644-
'aria-selected',
645-
'true',
646-
);
611+
cy.findByRole('option', { name: `Randy` }).should('have.attr', 'data-highlighted');
647612
});
648613
});

‎packages/kit-headless/src/components/combobox/combobox.tsx

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,19 +44,21 @@ export type ComboboxProps<O extends Option = Option> = PropsOf<'div'> & {
4444
defaultLabel?: string;
4545

4646
// signal binds
47-
'bind:isListboxOpenSig'?: Signal<boolean | undefined>;
48-
'bind:isInputFocusedSig'?: Signal<boolean | undefined>;
49-
'bind:inputValueSig'?: Signal<string>;
50-
'bind:highlightedIndexSig'?: Signal<number>;
47+
'bind:isListboxOpen'?: Signal<boolean | undefined>;
48+
'bind:isInputFocused'?: Signal<boolean | undefined>;
49+
'bind:inputValue'?: Signal<string>;
50+
'bind:highlightedIndex'?: Signal<number>;
51+
'bind:selectedIndex'?: Signal<number>;
5152
};
5253

5354
export const Combobox = component$(
5455
<O extends Option = Option>(props: ComboboxProps<O>) => {
5556
const {
56-
'bind:isListboxOpenSig': givenListboxOpenSig,
57-
'bind:isInputFocusedSig': givenInputFocusedSig,
58-
'bind:inputValueSig': givenInputValueSig,
59-
'bind:highlightedIndexSig': givenHighlightedIndexSig,
57+
'bind:isListboxOpen': givenListboxOpenSig,
58+
'bind:isInputFocused': givenInputFocusedSig,
59+
'bind:inputValue': givenInputValueSig,
60+
'bind:highlightedIndex': givenHighlightedIndexSig,
61+
'bind:selectedIndex': givenSelectedIndexSig,
6062
options,
6163
defaultLabel = '',
6264
optionValueKey = 'value',
@@ -130,7 +132,8 @@ export const Combobox = component$(
130132

131133
const triggerRef = useSignal<HTMLButtonElement>();
132134

133-
const selectedOptionIndexSig = useSignal<number>(-1);
135+
const defaultSelectedOptionIndexSig = useSignal<number>(-1);
136+
const selectedOptionIndexSig = givenSelectedIndexSig || defaultSelectedOptionIndexSig;
134137

135138
const defaultListboxOpenSig = useSignal<boolean | undefined>(false);
136139
const isListboxOpenSig = givenListboxOpenSig || defaultListboxOpenSig;

0 commit comments

Comments
 (0)