Skip to content

Commit abc767a

Browse files
refactor(select): research of necessary select comp ideas, props, features
1 parent 173e226 commit abc767a

File tree

6 files changed

+147
-310
lines changed

6 files changed

+147
-310
lines changed

apps/website/src/routes/docs/headless/contributing/index.mdx

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,80 @@ Tests ensure we can sleep sound at night and know that our component behavior is
104104
- get the test passing by adding said feature!
105105
- enjoy life when refactoring 🏝️
106106

107-
We currently use [playwright](https://playwright.dev/docs/intro) for testing.
107+
We strongly recommend TDD development for the headless library, and we are currently in the process of a playwright integration.
108108

109-
> We also currently have a few cypress tests that we are migrating over to playwright.
109+
Currently, the component testing integration for Qwik & Playwright is in development, and we are using e2e tests for the time being. That said, most tests should be very easy to migrate later on.
110110

111-
Shai also has a testing course that Qwik UI contributors get access to for the price of **FREE**! Don't hesitate to reach out.
111+
#### Getting started w/ testing
112+
113+
Here's an example way of getting a testid of the `Hero` select docs example, without affecting any visible markup.
114+
115+
```tsx
116+
<div data-testid="select-hero-test">
117+
<Showcase name="hero" />
118+
</div>
119+
```
120+
121+
Then, we get our testDriver, you can think of it as reusable code we use throughout the test. For example, in the Select component we constantly grab the listbox, trigger, etc.
122+
123+
```tsx
124+
export function selectTestDriver<T extends DriverLocator>(locator: T) {
125+
const getRoot = () => {
126+
return locator.getByRole('combobox');
127+
};
128+
129+
return {
130+
...locator,
131+
locator,
132+
getRoot,
133+
getListbox() {
134+
return getRoot().getByRole('listbox');
135+
},
136+
getTrigger() {
137+
return getRoot().getByRole('button');
138+
},
139+
};
140+
}
141+
```
142+
143+
Now we can write some tests:
144+
145+
```tsx
146+
test(`GIVEN a basic select
147+
WHEN clicking on the trigger
148+
THEN open up the listbox`, async ({ page }) => {
149+
await page.goto('/docs/headless/select');
150+
151+
const testDriver = selectTestDriver(page.getByTestId('select-hero-test'));
152+
153+
const { getListbox, getTrigger } = testDriver;
154+
155+
await getTrigger().click();
156+
157+
await expect(getListbox()).toBeVisible();
158+
});
159+
160+
test(`GIVEN a basic select
161+
WHEN focusing the trigger and hitting enter
162+
THEN open up the listbox`, async ({ page }) => {
163+
await page.goto('/docs/headless/select');
164+
165+
const testDriver = selectTestDriver(page.getByTestId('select-hero-test'));
166+
167+
const { getListbox, getTrigger } = testDriver;
168+
169+
await getTrigger().focus();
170+
await page.keyboard.press('Enter');
171+
172+
await expect(getListbox()).toBeVisible();
173+
});
174+
```
175+
176+
To run the tests, use the `nx e2e website` command. You can also do `--ui` to open the UI mode (which is pretty awesome!)
177+
178+
> This example is in `spec.select.tsx` in the e2e folder of the website.
179+
180+
Once we've added a failing test with what we expect, we can now go ahead and implement the feature for that!
112181

113182
## Docs
114183

apps/website/src/routes/docs/headless/select/examples/hero.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,8 @@ export default component$(() => {
66

77
return (
88
<Select>
9-
<SelectTrigger>Trigger</SelectTrigger>
10-
<SelectListbox
11-
class="hidden"
12-
style={{ padding: '0px', margin: '0px', listStyle: 'none' }}
13-
>
9+
<SelectTrigger class="min-w-40 bg-slate-700 py-2" />
10+
<SelectListbox class="absolute min-w-40 bg-slate-800 px-3 py-2">
1411
{usersSig.value.map((user) => (
1512
<SelectOption key={user}>{user}</SelectOption>
1613
))}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,3 @@ export * from './select-listbox';
44
export * from './select-option';
55
export * from './select-popover';
66
export * from './select-trigger';
7-
export * from './use-select';

packages/kit-headless/src/components/select/notes.md

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
What do we absolutely need? I am talking about the bare minimum, but powerful functionality we need for a select component.
22

3+
Inspiration:
4+
5+
- Radix UI
6+
- Kobalte UI
7+
- Melt UI
8+
9+
What do they all have in common? How do people use them? What are the most important features?
10+
311
# Select Research
412

513
## Anatomy:
@@ -8,13 +16,30 @@ What do we absolutely need? I am talking about the bare minimum, but powerful fu
816
<SelectTrigger />
917
<SelectPopover>
1018
<SelectListbox>
11-
<SelectOption>
19+
<SelectOption />
20+
<SelectGroup>
21+
<SelectLabel />
22+
<SelectOption />
23+
</SelectGroup>
1224
</SelectListbox>
1325
</SelectPopover>
1426
</Select>
1527

28+
## Features:
29+
30+
- Single Select
31+
- Multi Select
32+
- Controlled or uncontrolled
33+
- Keyboard Interactions
34+
- Grouped options
35+
- Typeahead support (user typing / filter)
36+
- RTL support
37+
- Scrollable
38+
1639
## Props:
1740

41+
### State
42+
1843
name: bind:selected
1944
type: Signal
2045
description: controlled selected value, manages the selected option.
@@ -25,10 +50,53 @@ What do we absolutely need? I am talking about the bare minimum, but powerful fu
2550

2651
name: onSelectedChange$
2752
type: PropFunction
28-
description: Prop function called when the value changes.
53+
description: function called when the selected value changes.
54+
55+
name: onOpenChange$
56+
type: PropFunction
57+
description: function called when the listbox opens or closes.
58+
59+
---
60+
61+
### Behavior
62+
63+
name: disabled
64+
type: boolean
65+
description: When true, the option is disabled.
2966

3067
name: multiple
3168
type: boolean
3269
description: used for multi-select
3370

71+
name: loop
72+
type: boolean
73+
description: Determines if focus cycles from the last option back to the first, or vice versa.
74+
75+
---
76+
77+
### Forms
78+
79+
name: name
80+
type: string
81+
description: The name attribute identifies the select element when submitting the form.
82+
83+
name: required
84+
type: boolean
85+
description: When true, the user must select a value to submit the form.
86+
3487
## Keyboard Interactions:
88+
89+
key: Space;
90+
description: Pressing space opens the select menu and highlights the chosen option. If an option is highlighted, pressing space selects it.
91+
92+
key: Enter
93+
description: Pressing Enter opens the select menu and highlights the selected option. If an option is highlighted, pressing Enter selects it.
94+
95+
key: ArrowDown
96+
description: Pressing ArrowDown opens the select menu if it's closed. If an option is focused, it moves the focus to the next option.
97+
98+
key: ArrowUp
99+
description: Pressing ArrowUp opens the select menu if it's closed. If an option is focused, it moves the focus to the previous option.
100+
101+
key: Escape
102+
description: Pressing Escape closes the select menu. The trigger is then focused.
Lines changed: 3 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -1,157 +1,15 @@
1-
import { component$, Slot, type PropsOf, useContext, sync$, $ } from '@builder.io/qwik';
2-
import { useSelect } from './use-select';
1+
import { component$, type PropsOf, useContext, sync$, $ } from '@builder.io/qwik';
32
import SelectContextId from './select-context';
43

54
type SelectTriggerProps = PropsOf<'button'>;
65
export type DisabledArr = Array<{ disabled: boolean }>;
76
export const SelectTrigger = component$<SelectTriggerProps>((props) => {
8-
const {
9-
getNextEnabledOptionIndex,
10-
// getPrevEnabledOptionIndex,
11-
// getIntialIndexOnKey,
12-
// manageToggle,
13-
// setTriggerText,
14-
// singleCharSearch,
15-
} = useSelect();
16-
177
const context = useContext(SelectContextId);
18-
// const typedLettersSig = useSignal("");
19-
// const deltaIndexSig = useSignal(-1);
208

219
const handleClick$ = $(() => {
2210
context.isListboxOpenSig.value = !context.isListboxOpenSig.value;
2311
});
2412

25-
const handleKeyDown$ = $(async (e: KeyboardEvent) => {
26-
// we migth want to consider making a ts type just for the strgs we care about
27-
// in the future, multiple keys need to open the popup
28-
// const openPopupKeys = ["ArrowDown", "ArrowUp"];
29-
const options = context.optionRefsArray.value.map((e) => e.value) as HTMLElement[];
30-
31-
// const singleCharRegex = /^[a-z]$/;
32-
// const isSingleChar = singleCharRegex.test(e.key);
33-
34-
if (e.key === 'ArrowDown') {
35-
context.isListboxOpenSig.value = true;
36-
37-
const nextIndex = await getNextEnabledOptionIndex(
38-
context.highlightedIndexSig.value,
39-
options,
40-
);
41-
42-
console.log(nextIndex);
43-
44-
// const nextIndex = getNextEnabledOptionIndexFromDisabledArr(
45-
// context.highlightedIndexSig.value,
46-
// disabledOptions,
47-
// );
48-
// manageToggle(await nextIndex, context.highlightedIndexSig, options);
49-
// return;
50-
}
51-
52-
// TODO: refactor each if statement with a function inside of it instead of the current pattern of:
53-
// if(true){lines of code}
54-
//to
55-
//if(true){fun(...)}
56-
57-
// this whole section in the if statement could be refactored into a "closed behavior" fn
58-
// if (!context.isListboxOpenSig.value) {
59-
// if (isSingleChar) {
60-
// context.isListboxOpenSig.value = true;
61-
// const posIndex = await singleCharSearch(e.key, deltaIndexSig, options);
62-
// if (posIndex !== -1) {
63-
// manageToggle(posIndex, context.highlightedIndexSig, options);
64-
// }
65-
// return;
66-
// }
67-
68-
// if (e.key === "Home") {
69-
// context.isListboxOpenSig.value = true;
70-
// // const firstOpt = disabledOptions.findIndex((e) => e.disabled === false);
71-
// // manageToggle(firstOpt, context.highlightedIndexSig, options);
72-
// return;
73-
// }
74-
// if (e.key === "End") {
75-
// context.isListboxOpenSig.value = true;
76-
// // for (let index = disabledOptions.length - 1; index > -1; index--) {
77-
// // const elementStatus = disabledOptions[index];
78-
// // if (!elementStatus.disabled) {
79-
// // manageToggle(index, context.highlightedIndexSig, options);
80-
// // context.highlightedIndexSig.value = index;
81-
// // break;
82-
// // }
83-
// // }
84-
// return;
85-
// }
86-
// }
87-
// if (!context.isListboxOpenSig.value && openPopupKeys.includes(e.key)) {
88-
// context.isListboxOpenSig.value = true;
89-
// if (context.highlightedIndexSig.value !== -1) {
90-
// return;
91-
// }
92-
// const initalIndex = getIntialIndexOnKey(e.key);
93-
// manageToggle(await initalIndex, context.highlightedIndexSig, options);
94-
// return;
95-
// }
96-
// if (context.isListboxOpenSig.value) {
97-
// // typedLettersSig.value = typedLettersSig.value + e.key;
98-
// if (isSingleChar) {
99-
// // const posIndex = await singleCharSearch(e.key, deltaIndexSig, options);
100-
// // if (posIndex !== -1) {
101-
// // manageToggle(posIndex, context.highlightedIndexSig, options);
102-
// // return;
103-
// // }
104-
// // return;
105-
// }
106-
// if (e.key === "ArrowUp") {
107-
// // if (context.highlightedIndexSig.value === -1) {
108-
// // const initalIndex = getIntialIndexOnKey(e.key);
109-
// // manageToggle(await initalIndex, context.highlightedIndexSig, options);
110-
// // return;
111-
// // }
112-
// // const nextIndex = getPrevEnabledOptionIndexFromDisabledArr(
113-
// // context.highlightedIndexSig.value,
114-
// // disabledOptions,
115-
// // );
116-
// // manageToggle(await nextIndex, context.highlightedIndexSig, options);
117-
// // return;
118-
// }
119-
// if (e.key === "Enter") {
120-
// // setTriggerText(context.highlightedIndexSig, options);
121-
// // return;
122-
// }
123-
// if (e.key === "Home") {
124-
// // const firstOpt = disabledOptions.findIndex((e) => e.disabled === false);
125-
// // manageToggle(firstOpt, context.highlightedIndexSig, options);
126-
// // return;
127-
// }
128-
// if (e.key === "End") {
129-
// // the things we do when no lastIndex :(
130-
// // for (let index = disabledOptions.length - 1; index > -1; index--) {
131-
// // const elementStatus = disabledOptions[index];
132-
// // if (!elementStatus.disabled) {
133-
// // manageToggle(index, context.highlightedIndexSig, options);
134-
// // context.highlightedIndexSig.value = index;
135-
// // break;
136-
// // }
137-
// // }
138-
// // return;
139-
// }
140-
// // if (e.key === "Tab") {
141-
// // const tabIndex =
142-
// // context.highlightedIndexSig.value === -1
143-
// // ? 0
144-
// // : context.highlightedIndexSig.value;
145-
// // setTriggerText({ value: tabIndex }, options);
146-
// // context.isListboxOpenSig.value = false;
147-
// // return;
148-
// // }
149-
// // if (e.key === "Escape") {
150-
// // context.isListboxOpenSig.value = false;
151-
// // }
152-
// }
153-
});
154-
15513
const handleKeyDownSync$ = sync$((e: KeyboardEvent) => {
15614
const keys = ['ArrowUp', 'ArrowDown', 'Home', 'End', 'PageDown', 'PageUp'];
15715
if (keys.includes(e.key)) {
@@ -164,14 +22,12 @@ export const SelectTrigger = component$<SelectTriggerProps>((props) => {
16422
{...props}
16523
ref={context.triggerRef}
16624
onClick$={[handleClick$, props.onClick$]}
167-
onKeyDown$={[handleKeyDownSync$, handleKeyDown$, props.onKeyDown$]}
25+
onKeyDown$={[handleKeyDownSync$, props.onKeyDown$]}
16826
data-open={context.isListboxOpenSig.value ? '' : undefined}
16927
data-closed={!context.isListboxOpenSig.value ? '' : undefined}
170-
class="bg-slate-800 p-2 text-white"
17128
aria-expanded={context.isListboxOpenSig.value}
17229
>
173-
<Slot />
174-
{context.selectedOptionRef.value?.textContent}
30+
{context.selectedOptionRef.value?.textContent ?? 'Select an option'}
17531
</button>
17632
);
17733
});

0 commit comments

Comments
 (0)