From c818eec052b7479bbd559946fe72e95a6bd589ee Mon Sep 17 00:00:00 2001 From: Jack Shelton <104264123+thejackshelton@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:26:14 -0500 Subject: [PATCH 1/3] Updated checkbox (#969) * initial tests * updated checkbox --- .../headless/checkbox/examples/checklist.tsx | 28 + .../checkbox/examples/controlled-values.tsx | 2 +- .../docs/headless/checkbox/examples/hero.tsx | 22 +- .../docs/headless/checkbox/examples/pizza.tsx | 77 +-- .../headless/checkbox/examples/reactive.tsx | 27 + .../examples/test-controlled-list-false.tsx | 6 +- .../examples/test-controlled-list-falses.tsx | 54 +- .../examples/test-controlled-list-mixed.tsx | 85 +-- .../examples/test-controlled-list-true.tsx | 55 +- .../examples/test-controlled-list-trues.tsx | 6 +- .../examples/test-controlled-list.tsx | 6 +- .../checkbox/examples/test-default.tsx | 27 +- ...test-hero.tsx => test-initial-checked.tsx} | 8 +- .../headless/checkbox/examples/test-list.tsx | 60 +- .../checkbox/examples/test-props-ids-list.tsx | 6 +- .../routes/docs/headless/checkbox/index.mdx | 2 + .../components/checkbox/checkbox-context.tsx | 3 + .../checkbox/checkbox-indicator.tsx | 26 +- .../src/components/checkbox/checkbox-root.tsx | 69 +++ .../src/components/checkbox/checkbox.css | 3 + .../components/checkbox/checkbox.driver.ts | 17 +- .../src/components/checkbox/checkbox.test.ts | 556 ++++++++++-------- .../src/components/checkbox/checkbox.tsx | 225 ------- .../src/components/checkbox/context-id.ts | 11 - .../src/components/checkbox/index.ts | 3 +- .../src/components/checkbox/note.md | 1 + .../checklist/checklist-context-wrapper.tsx | 64 -- .../checklist/checklist-context.tsx | 12 + .../checklist/checklist-indicator.tsx | 31 +- .../components/checklist/checklist-item.tsx | 69 +++ .../components/checklist/checklist-root.tsx | 65 ++ .../checklist/checklist-selectall.tsx | 47 ++ .../src/components/checklist/checklist.tsx | 105 ---- .../src/components/checklist/context-id.ts | 10 - .../src/components/checklist/index.ts | 9 +- .../src/components/combobox/combobox.test.ts | 48 +- .../dropdown/dropdown-checkbox-item.tsx | 2 +- .../dropdown/dropdown-radio-item.tsx | 2 +- 38 files changed, 921 insertions(+), 928 deletions(-) create mode 100644 apps/website/src/routes/docs/headless/checkbox/examples/checklist.tsx create mode 100644 apps/website/src/routes/docs/headless/checkbox/examples/reactive.tsx rename apps/website/src/routes/docs/headless/checkbox/examples/{test-hero.tsx => test-initial-checked.tsx} (65%) create mode 100644 packages/kit-headless/src/components/checkbox/checkbox-context.tsx create mode 100644 packages/kit-headless/src/components/checkbox/checkbox-root.tsx create mode 100644 packages/kit-headless/src/components/checkbox/checkbox.css delete mode 100644 packages/kit-headless/src/components/checkbox/checkbox.tsx delete mode 100644 packages/kit-headless/src/components/checkbox/context-id.ts create mode 100644 packages/kit-headless/src/components/checkbox/note.md delete mode 100644 packages/kit-headless/src/components/checklist/checklist-context-wrapper.tsx create mode 100644 packages/kit-headless/src/components/checklist/checklist-context.tsx create mode 100644 packages/kit-headless/src/components/checklist/checklist-item.tsx create mode 100644 packages/kit-headless/src/components/checklist/checklist-root.tsx create mode 100644 packages/kit-headless/src/components/checklist/checklist-selectall.tsx delete mode 100644 packages/kit-headless/src/components/checklist/checklist.tsx delete mode 100644 packages/kit-headless/src/components/checklist/context-id.ts diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/checklist.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/checklist.tsx new file mode 100644 index 000000000..b0f592b8c --- /dev/null +++ b/apps/website/src/routes/docs/headless/checkbox/examples/checklist.tsx @@ -0,0 +1,28 @@ +import { component$ } from '@builder.io/qwik'; + +import { Checklist } from '@qwik-ui/headless'; + +export default component$(() => { + return ( + + +
+ โœ… +
{' '} + Select All +
+ + {Array.from({ length: 2 }, (_, index) => { + const uniqueKey = `cl-${index}-${Date.now()}`; + return ( + +
+ โœ… +
+ {`item ${index}`} +
+ ); + })} +
+ ); +}); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/controlled-values.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/controlled-values.tsx index 9b2fcb8f3..c841af64d 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/controlled-values.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/controlled-values.tsx @@ -1,4 +1,4 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +import { component$, useSignal } from '@builder.io/qwik'; import { Checkbox } from '@qwik-ui/headless'; export default component$(() => { const initialVal1 = false; diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/hero.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/hero.tsx index 932d1a50d..f35a438d0 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/hero.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/hero.tsx @@ -1,14 +1,18 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +import { component$, useSignal } from '@builder.io/qwik'; import { Checkbox } from '@qwik-ui/headless'; export default component$(() => { + const isCheckedSig = useSignal(false); + return ( - <> - - - โœ… - - I have read the README - - + +
+ โœ… +
+

I have read the README

+
); }); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/pizza.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/pizza.tsx index 37c671abe..633f5069b 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/pizza.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/pizza.tsx @@ -1,39 +1,42 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; -import { Checkbox, Checklist } from '@qwik-ui/headless'; -const toppingNames = ['hot peppers', 'ham', 'pineaple', 'mushroom']; -const toppingImages = ['๐ŸŒถ๏ธ', '๐Ÿ—', '๐Ÿ', '๐Ÿ„']; -export default component$(() => { - return ( - <> -

Pizza toppings

- - - -
- ๐Ÿ• -
+// import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +// import { Checkbox, Checklist } from '@qwik-ui/headless'; +// const toppingNames = ['hot peppers', 'ham', 'pineaple', 'mushroom']; +// const toppingImages = ['๐ŸŒถ๏ธ', '๐Ÿ—', '๐Ÿ', '๐Ÿ„']; +// export default component$(() => { +// return ( +// <> +//

Pizza toppings

+// +// +// +//
+// ๐Ÿ• +//
-
- โž– -
-
- Pick all toppings -
+//
+// โž– +//
+//
+// Pick all toppings +//
- {toppingNames.map((name, i) => { - return ( - - - {toppingImages[i]} - - {name} - - ); - })} -
- - ); -}); +// {toppingNames.map((name, i) => { +// return ( +// +// +// {toppingImages[i]} +// +// {name} +// +// ); +// })} +// +// +// ); +// }); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/reactive.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/reactive.tsx new file mode 100644 index 000000000..ad20aa681 --- /dev/null +++ b/apps/website/src/routes/docs/headless/checkbox/examples/reactive.tsx @@ -0,0 +1,27 @@ +import { component$, useSignal } from '@builder.io/qwik'; +import { Checkbox } from '@qwik-ui/headless'; +export default component$(() => { + const isCheckedSig = useSignal(false); + + return ( + <> + +
+ โœ… +
+

I have read the README

+
+ + + ); +}); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-false.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-false.tsx index 9946226e4..480307958 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-false.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-false.tsx @@ -1,4 +1,4 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +import { component$, useSignal } from '@builder.io/qwik'; import { Checkbox, Checklist } from '@qwik-ui/headless'; // TODO: add logic to handle user passed sigs with trues // this test basically ensures that the sig passed to the checklist controlls trumps all its children @@ -7,10 +7,10 @@ export default component$(() => { return ( <>

Pick a cat

- + diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-falses.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-falses.tsx index 5c458f05d..4e2180971 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-falses.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-falses.tsx @@ -1,36 +1,28 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; -import { Checkbox, Checklist } from '@qwik-ui/headless'; +import { component$ } from '@builder.io/qwik'; + +import { Checklist } from '@qwik-ui/headless'; + export default component$(() => { - const firstUserSig = useSignal(false); - const secondUserSig = useSignal(false); return ( - <> -

Pick a cat

- - - - โœ… - -

Controlls all

-
- - โœ… -

No other stuff is needed here

-
+ + +
+ โœ… +
{' '} + Select All +
- -
- โœ… -

No other stuff is needed here

-
-
-
- + {Array.from({ length: 2 }, (_, index) => { + const uniqueKey = `cl-${index}-${Date.now()}`; + return ( + +
+ โœ… +
+ {`item ${index}`} +
+ ); + })} +
); }); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-mixed.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-mixed.tsx index 97998cd2d..72e924095 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-mixed.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-mixed.tsx @@ -1,41 +1,46 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; -import { Checkbox, Checklist } from '@qwik-ui/headless'; -export default component$(() => { - const firstUserSig = useSignal(false); - const secondUserSig = useSignal(true); - return ( - <> -

Pick a cat

- - - -
- โœ… -
-
- โž– -
-
-

Controlls all

-
- - โœ… -

No other stuff is needed here

-
+// import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +// import { Checkbox, Checklist } from '@qwik-ui/headless'; +// export default component$(() => { +// const firstUserSig = useSignal(false); +// const secondUserSig = useSignal(true); +// return ( +// <> +//

Pick a cat

+// +// +// +//
+// โœ… +//
+//
+// โž– +//
+//
+//

Controlls all

+//
+// +// โœ… +//

No other stuff is needed here

+//
- -
- โœ… -

No other stuff is needed here

-
-
-
- - ); -}); +// +//
+// +// โœ… +// +//

No other stuff is needed here

+//
+//
+//
+// +// ); +// }); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-true.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-true.tsx index 1b07e991d..b1e16b705 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-true.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-true.tsx @@ -1,46 +1,19 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; -import { Checkbox, Checklist } from '@qwik-ui/headless'; +import { component$ } from '@builder.io/qwik'; +import { Checklist } from '@qwik-ui/headless'; // this test basically ensures that the sig passed to the checklist controlls trumps all its children export default component$(() => { - const checklistSig = useSignal(true); + // const checklistSig = useSignal(true); return ( - <> -

Pick a cat

- - - -
- โœ… -
- -
- โž– -
-
-

Controlls all

-
- - โœ… -

No other stuff is needed here

-
- - -
- โœ… -

Im a true.tsx

-
-
-
-

You signal is:

-

{`${checklistSig.value}`}

- + + + โœ… + + + โœ… first item + + + โœ… second item + + ); }); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-trues.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-trues.tsx index 43d0ebc4e..c723dcfbc 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-trues.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list-trues.tsx @@ -1,4 +1,4 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +import { component$, useSignal } from '@builder.io/qwik'; import { Checkbox, Checklist } from '@qwik-ui/headless'; export default component$(() => { const firstUserSig = useSignal(true); @@ -6,10 +6,10 @@ export default component$(() => { return ( <>

Pick a cat

- + โœ… diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list.tsx index 4717b279f..2e8ce5759 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-controlled-list.tsx @@ -1,4 +1,4 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +import { component$, useSignal } from '@builder.io/qwik'; import { Checkbox, Checklist } from '@qwik-ui/headless'; export default component$(() => { const firstUserSig = useSignal(true); @@ -6,10 +6,10 @@ export default component$(() => { return ( <>

Pick a cat

- + โœ… diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-default.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-default.tsx index 113802e5f..c13b66a80 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-default.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-default.tsx @@ -1,17 +1,18 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; -import { Checkbox, Checklist } from '@qwik-ui/headless'; +import { component$, useSignal } from '@builder.io/qwik'; +import { Checkbox } from '@qwik-ui/headless'; + export default component$(() => { + const bindChecked = useSignal(false); return ( - <> -

I'm the default checkbox!!!

- -
- -

โœ…

-
-

No other stuff is needed here

-
-
- + + +

โœ…

+
+ I have read the README +
); }); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-hero.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-initial-checked.tsx similarity index 65% rename from apps/website/src/routes/docs/headless/checkbox/examples/test-hero.tsx rename to apps/website/src/routes/docs/headless/checkbox/examples/test-initial-checked.tsx index 21f227d5c..211e05f08 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-hero.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-initial-checked.tsx @@ -1,19 +1,17 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +import { component$, useSignal } from '@builder.io/qwik'; import { Checkbox } from '@qwik-ui/headless'; export default component$(() => { - const userSig = useSignal(true); + const bindChecked = useSignal(true); return ( <> - +

โœ…

-

No other stuff is needed here

-
{`${userSig.value}`}
); }); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-list.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-list.tsx index ddd840a9b..3369e9101 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-list.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-list.tsx @@ -1,39 +1,33 @@ import { component$ } from '@builder.io/qwik'; -import { Checkbox, Checklist } from '@qwik-ui/headless'; -export default component$(() => { - return ( - <> -

Pick a cat

- - - -
- โœ… -
- -
- โž– -
-
-

Get All

-
+import { Checklist } from '@qwik-ui/headless'; - - โœ… -

Cat One

-
+export default component$(() => { + return ( + + +
+ โœ… +
{' '} + Select All +
- -
- โœ… -

Cat Two

-
-
-
- + +
+ โœ… +
+ first item +
+ +
+ โœ… +
+ second item +
+
); }); diff --git a/apps/website/src/routes/docs/headless/checkbox/examples/test-props-ids-list.tsx b/apps/website/src/routes/docs/headless/checkbox/examples/test-props-ids-list.tsx index 47e008451..2fd0ccc08 100644 --- a/apps/website/src/routes/docs/headless/checkbox/examples/test-props-ids-list.tsx +++ b/apps/website/src/routes/docs/headless/checkbox/examples/test-props-ids-list.tsx @@ -1,4 +1,4 @@ -import { component$, useSignal, useStyles$ } from '@builder.io/qwik'; +import { component$, useSignal } from '@builder.io/qwik'; import { Checkbox, Checklist } from '@qwik-ui/headless'; export default component$(() => { const firstUserSig = useSignal(true); @@ -6,10 +6,10 @@ export default component$(() => { return ( <>

Pick a cat

- + diff --git a/apps/website/src/routes/docs/headless/checkbox/index.mdx b/apps/website/src/routes/docs/headless/checkbox/index.mdx index ebe744394..278d807f3 100644 --- a/apps/website/src/routes/docs/headless/checkbox/index.mdx +++ b/apps/website/src/routes/docs/headless/checkbox/index.mdx @@ -44,6 +44,8 @@ In the example below, the left checkbox starts as **false**, while the right che + + ## Customization & Caveats You can apply CSS classes to any part of the component. Any valid HTML can be used as an icon, but children of the Checkbox Indicator are removed from the accessibility tree to prevent them from being added to the Checkbox Label. diff --git a/packages/kit-headless/src/components/checkbox/checkbox-context.tsx b/packages/kit-headless/src/components/checkbox/checkbox-context.tsx new file mode 100644 index 000000000..e1d3a0f93 --- /dev/null +++ b/packages/kit-headless/src/components/checkbox/checkbox-context.tsx @@ -0,0 +1,3 @@ +import { createContextId, type Signal } from '@builder.io/qwik'; + +export const CheckboxContext = createContextId>('CheckBox.context'); diff --git a/packages/kit-headless/src/components/checkbox/checkbox-indicator.tsx b/packages/kit-headless/src/components/checkbox/checkbox-indicator.tsx index 8adc76e70..b6183ded5 100644 --- a/packages/kit-headless/src/components/checkbox/checkbox-indicator.tsx +++ b/packages/kit-headless/src/components/checkbox/checkbox-indicator.tsx @@ -1,15 +1,29 @@ -import { component$, useContext, PropsOf, Slot } from '@builder.io/qwik'; -import { CheckboxContext } from './context-id'; +import { + component$, + useContext, + type PropsOf, + Slot, + useTask$, + useStyles$, +} from '@builder.io/qwik'; +import { CheckboxContext } from './checkbox-context'; +import './checkbox.css'; +import styles from './checkbox.css?inline'; export type CheckboxIndicatorProps = PropsOf<'div'>; export const CheckboxIndicator = component$((props) => { + useStyles$(styles); + const checkSig = useContext(CheckboxContext); + + useTask$(({ track }) => { + track(() => checkSig.value); + }); + return ( -
- + ); }); diff --git a/packages/kit-headless/src/components/checkbox/checkbox-root.tsx b/packages/kit-headless/src/components/checkbox/checkbox-root.tsx new file mode 100644 index 000000000..ddbcb9fb0 --- /dev/null +++ b/packages/kit-headless/src/components/checkbox/checkbox-root.tsx @@ -0,0 +1,69 @@ +import { + component$, + type PropsOf, + Slot, + type Signal, + $, + useContextProvider, + sync$, +} from '@builder.io/qwik'; +import { useBoundSignal } from '../../utils/bound-signal'; +import { CheckboxContext } from './checkbox-context'; +import type { QwikIntrinsicElements } from '@builder.io/qwik'; + +type AllowedElements = 'li' | 'div' | 'span'; + +export type CheckboxProps = { + 'bind:checked'?: Signal; + initialValue?: boolean; + index?: number; +} & PropsOf<'div'>; + +export const CheckboxRoot = component$( + ( + props: QwikIntrinsicElements[C] & { as?: C } & CheckboxProps, + ) => { + const { 'bind:checked': givenCheckedSig, initialValue, as, ...rest } = props; + const Comp = as ?? 'div'; + + const checkedSignal = useBoundSignal(givenCheckedSig, initialValue); + + useContextProvider(CheckboxContext, checkedSignal); + const handleKeyDownSync$ = sync$((e: KeyboardEvent) => { + if (e.key === ' ') { + e.preventDefault(); + } + }); + + const handleClick$ = $(() => { + checkedSignal.value = !checkedSignal.value; + }); + + const handleKeyDown$ = $((e: KeyboardEvent) => { + if (e.key === ' ') { + checkedSignal.value = !checkedSignal.value; + } + }); + + return ( + <> + {/* @ts-expect-error annoying polymorphism */} + + + + + ); + }, +); diff --git a/packages/kit-headless/src/components/checkbox/checkbox.css b/packages/kit-headless/src/components/checkbox/checkbox.css new file mode 100644 index 000000000..038a0a6f3 --- /dev/null +++ b/packages/kit-headless/src/components/checkbox/checkbox.css @@ -0,0 +1,3 @@ +[data-qds-indicator][data-hidden] { + display: none; +} diff --git a/packages/kit-headless/src/components/checkbox/checkbox.driver.ts b/packages/kit-headless/src/components/checkbox/checkbox.driver.ts index c1a3c2097..92280477b 100644 --- a/packages/kit-headless/src/components/checkbox/checkbox.driver.ts +++ b/packages/kit-headless/src/components/checkbox/checkbox.driver.ts @@ -1,4 +1,4 @@ -import { type Locator, type Page } from '@playwright/test'; +import type { Locator, Page } from '@playwright/test'; export type DriverLocator = Locator | Page; export function createTestDriver(rootLocator: T) { @@ -10,7 +10,7 @@ export function createTestDriver(rootLocator: T) { return getRoot().locator('#indicator'); }; const getCheckList = () => { - return getRoot().getByRole('group'); + return getRoot().getByRole('checkbox'); }; const getChecklistUL = () => { // note: filter method is always relative to the original locator not document root despite using root @@ -24,8 +24,17 @@ export function createTestDriver(rootLocator: T) { const getCheckbox = () => { return getRoot().getByRole('checkbox'); }; + + const getSelectAll = () => { + return getRoot().locator('[data-qds-selectall]'); + }; + + const getSelectAllIndicator = () => { + return getSelectAll().locator('[data-qds-indicator]'); + }; + const getTriCheckbox = () => { - return getRoot().locator('css=[aria-controls]'); + return getRoot().locator('#selectAll'); }; return { ...rootLocator, @@ -37,5 +46,7 @@ export function createTestDriver(rootLocator: T) { getChecklistUL, getChecklistLIs, getTriCheckbox, + getSelectAll, + getSelectAllIndicator, }; } diff --git a/packages/kit-headless/src/components/checkbox/checkbox.test.ts b/packages/kit-headless/src/components/checkbox/checkbox.test.ts index 75b808902..566c41012 100644 --- a/packages/kit-headless/src/components/checkbox/checkbox.test.ts +++ b/packages/kit-headless/src/components/checkbox/checkbox.test.ts @@ -1,8 +1,9 @@ +// TODO: refactor this into checkbox.test.ts and checklist.test.ts + import { expect, test, type Page } from '@playwright/test'; import { createTestDriver } from './checkbox.driver'; -import { getTriBool } from '../checklist/checklist-context-wrapper'; async function setup(page: Page, exampleName: string) { - await page.goto(`/headless/checkbox/${exampleName}`); + await page.goto(`http://localhost:6174/checkbox/${exampleName}`); const driver = createTestDriver(page); @@ -28,25 +29,138 @@ async function setup(page: Page, exampleName: string) { }; } -test.describe('checklist', () => { - test(`GIVEN a mixed checklist - WHEN the checklist renders - IT should render the mixed img - AND not the true img`, async ({ page }) => { - const exampleName = 'test-controlled-list-mixed'; - await setup(page, exampleName); - await expect(page.locator('#mixed-img')).toBeVisible(); - await expect(page.locator('#true-img')).toBeHidden(); +/** + * TYPESCRIPT SUPPORT + LESS IMPORTS + * test(`GIVEN a carousel + WHEN clicking on the next button + THEN it should move to the next slide`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + await d.getNextButton().click(); + await expect(d.getSlideAt(1)).toHaveAttribute('data-active'); }); + */ - test(`GIVEN an all-checked checklist - WHEN the checklist renders - IT should render the true img - AND not the mixed img`, async ({ page }) => { - const exampleName = 'test-controlled-list-true'; - await setup(page, exampleName); - await expect(page.locator('#true-img')).toBeVisible(); - await expect(page.locator('#mixed-img')).toBeHidden(); +// components -> usually modular, sometimes grouped depending on what it is +// modular -> tests ONE SPECIFIC thing + +// grouped -> tests groups of things + +/** + * + * test.describe('Critical functionality', () => { + * test(`GIVEN a checkbox + When the checkbox is clicked + Then it should be checked`, async ({ page }) => { + await expect() -> aria-checked + await expect() -> data-checked + }); + + // if there's too many of one thing (for example, maybe you need to check 10 different keys! make it a describe block) + test(`GIVEN a checkbox + When the checkbox is pressed with space + Then it should be checked`, async ({ page }) => { + + }); + + Ex: Select + + test.describe('Keyboard Behavior', () => { + + }); + * }) + * + */ + +test.describe('checkbox', () => { + // test default state and click from unchecked checkbox + test(`GIVEN a checkbox that is initially unchecked + and its icon is hidden + WHEN the checkbox is clicked + IT should toggle aria-checked to true + and make its icon visible`, async ({ page }) => { + const exampleName = 'test-default'; + const { getCheckbox, getIcon } = await setup(page, exampleName); + await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); + await expect(getIcon()).toBeHidden(); + await getCheckbox().click(); + await expect(getCheckbox()).toHaveAttribute('aria-checked', 'true'); + await expect(getIcon()).toBeVisible(); + }); + + // test default state and keyboard space from unchecked checkbox + test(`GIVEN a checkbox that is initially unchecked + and its icon is hidden + WHEN the checkbox is focused and the spacebar is pressed + IT should toggle aria-checked to true + and make its icon visible`, async ({ page }) => { + const exampleName = 'test-default'; + const { getCheckbox, getIcon } = await setup(page, exampleName); + await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); + await expect(getIcon()).toBeHidden(); + await getCheckbox().focus(); + await getCheckbox().press(' '); + await expect(getCheckbox()).toHaveAttribute('aria-checked', 'true'); + await expect(getIcon()).toBeVisible(); + }); + + // test default state from checked checkbox + test(`GIVEN a checkbox that is checked + WHEN the checkbox renders + IT should have aria-checked as true + and its icon visible`, async ({ page }) => { + const exampleName = 'test-initial-checked'; + const { getCheckbox, getIcon } = await setup(page, exampleName); + await expect(getCheckbox()).toBeVisible(); + await expect(getCheckbox()).toHaveAttribute('aria-checked', 'true'); + await expect(getIcon()).toBeVisible(); + }); + + // test mouse click + test(`GIVEN a checkbox that is checked + WHEN the checkbox is clicked + IT should have aria-checked as false + and its icon hidden`, async ({ page }) => { + const exampleName = 'test-initial-checked'; + const { getCheckbox, getIcon } = await setup(page, exampleName); + await expect(getCheckbox()).toBeVisible(); + await getCheckbox().click(); + await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); + await expect(getIcon()).toBeHidden(); + }); + + // test keyboard space from checked checkbox + test(`GIVEN a checkbox that is checked + WHEN the checkbox is focused and the spacebar is pressed + IT should have aria-checked as false + and its icon hidden`, async ({ page }) => { + const exampleName = 'test-initial-checked'; + const { getCheckbox, getIcon } = await setup(page, exampleName); + await expect(getCheckbox()).toBeVisible(); + await getCheckbox().focus(); + await getCheckbox().press(' '); + await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); + await expect(getIcon()).toBeHidden(); + }); +}); + +// Checklisst tests +test.describe('checklist', () => { + // test(`GIVEN a mixed checklist + // WHEN the checklist renders + // IT should render the mixed img + // AND not the true img`, async ({ page }) => { + // const exampleName = 'test-controlled-list-mixed'; + // await setup(page, exampleName); + // await expect(page.locator('#mixed-img')).toBeVisible(); + // await expect(page.locator('#true-img')).toBeHidden(); + // }); + + test(`GIVEN a checklist with all items checked + WHEN the checklist renders + The indicator in the toggle all checkbox should be visible`, async ({ page }) => { + const { driver: d } = await setup(page, 'test-controlled-list-true'); + await expect(d.getSelectAllIndicator()).toBeVisible(); }); test(`GIVEN an all-unchecked checklist @@ -58,33 +172,33 @@ test.describe('checklist', () => { await expect(page.locator('#mixed-img')).toBeHidden(); }); - test(`GIVEN a checklist with checkboxes - WHEN the elements render - THEN the checklist should be a
    with
  • s of checkboxes, all wrapped around a div with a role and aria-labeledby attributes`, async ({ - page, - }) => { - const { getCheckList, getChecklistUL, getChecklistLIs } = await setup( - page, - 'test-list', - ); - await expect(getCheckList()).toBeVisible(); - await expect(getCheckList()).toHaveAttribute('aria-labelledby', 'test123'); - await expect(getChecklistUL()).toBeVisible(); - await expect(getChecklistLIs()).toBeVisible(); - }); + // TODO: fix this test + // test(`GIVEN a checklist with checkboxes + // WHEN the elements render + // THEN the checklist should be a
      with
    • s of checkboxes, all wrapped around a div with a role and aria-labeledby attributes`, async ({ + // page, + // }) => { + // const { getRoot, getCheckList, getChecklistUL, getChecklistLIs } = + // await setup(page, 'test-list'); + // await expect(getCheckList()).toBeVisible(); + // await expect(getCheckList()).toHaveAttribute('aria-labelledby', 'test123'); + // await expect(getChecklistUL()).toBeVisible(); + // await expect(getChecklistLIs()).toBeVisible(); + // }); - test(`GIVEN a tri boolean function - WHEN it recieves an array of booleans - IT should return the correct tri bool`, async () => { - const indeterminateArr = [true, true, false]; - const trueArr = [true, true, true]; - const falseArr = [false, false, false]; - const emptyArr: boolean[] = []; - expect(getTriBool(indeterminateArr)).toBe('indeterminate'); - expect(getTriBool(trueArr)).toBe(true); - expect(getTriBool(falseArr)).toBe(false); - expect(getTriBool(emptyArr)).toBe('indeterminate'); - }); + // not using triboolean + // test(`GIVEN a tri boolean function + // WHEN it recieves an array of booleans + // IT should return the correct tri bool`, async () => { + // const indeterminateArr = [true, true, false]; + // const trueArr = [true, true, true]; + // const falseArr = [false, false, false]; + // const emptyArr: boolean[] = []; + // expect(getTriBool(indeterminateArr)).toBe('indeterminate'); + // expect(getTriBool(trueArr)).toBe(true); + // expect(getTriBool(falseArr)).toBe(false); + // expect(getTriBool(emptyArr)).toBe('indeterminate'); + // }); test(`GIVEN checklist with all unchecked checkboxes WHEN it renders @@ -96,29 +210,33 @@ test.describe('checklist', () => { await expect(getTriCheckbox()).toBeVisible(); await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'false'); }); - test(`GIVEN checklist with all unchecked checkboxes - WHEN the first checkbox is checked - the chekbox with aria-controls should have aria-checked mixed`, async ({ - page, - }) => { - const exampleName = 'test-list'; - const { getTriCheckbox, getCheckbox } = await setup(page, exampleName); - await expect(getTriCheckbox()).toBeVisible(); - await getCheckbox().nth(1).press(' '); - await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); - }); + // Not using mixed yet + // test(`GIVEN checklist with all unchecked checkboxes + // WHEN the first checkbox is checked + // the chekbox with aria-controls should have aria-checked mixed`, async ({ + // page, + // }) => { + // const exampleName = 'test-list'; + // const { getTriCheckbox, getCheckbox } = await setup(page, exampleName); + // await expect(getTriCheckbox()).toBeVisible(); + // await getCheckbox().nth(1).press(' '); + // await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); + // }); + + // 4 test(`GIVEN checklist with all unchecked checkboxes - WHEN all checkboxes are checked with space - the tri state checkbox should have aria-checked true`, async ({ page }) => { + WHEN all checkboxes are checked + the toggle all indicator should be checked`, async ({ page }) => { const exampleName = 'test-list'; - const { getTriCheckbox, getCheckbox } = await setup(page, exampleName); - await expect(getTriCheckbox()).toBeVisible(); - await getCheckbox().nth(1).press(' '); - await getCheckbox().nth(2).press(' '); - await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'true'); + const { driver: d } = await setup(page, exampleName); + await expect(d.getSelectAllIndicator()).toBeHidden(); + await d.getCheckbox().nth(1).click(); + await d.getCheckbox().nth(2).click(); + await expect(d.getSelectAllIndicator()).toBeVisible(); }); + //5 test(`GIVEN checklist with all unchecked checkboxes WHEN the checklist's checkbox is checked with space THEN all chekboxes should have aria-checked true`, async ({ page }) => { @@ -131,6 +249,7 @@ test.describe('checklist', () => { await expect(getCheckbox().nth(2)).toHaveAttribute('aria-checked', 'true'); }); + //6 // TODO: reme two part of test by adding new test file test(`GIVEN checklist with all unchecked checkboxes WHEN the checklist's checkbox is checked twice using space @@ -149,45 +268,53 @@ test.describe('checklist', () => { await expect(getCheckbox().nth(1)).toHaveAttribute('aria-checked', 'false'); await expect(getCheckbox().nth(2)).toHaveAttribute('aria-checked', 'false'); }); - test(`GIVEN checklist with checkboxes - WHEN the values of aria-controls are search - IT should always return a valid, non-duplicate, checkboxes`, async ({ page }) => { - const { getTriCheckbox } = await setup(page, 'test-list'); - await expect(getTriCheckbox()).toHaveAttribute('aria-controls'); - const magic = await getTriCheckbox().getAttribute('aria-controls'); - expect(magic).not.toBe(null); - if (magic === null) { - throw new Error( - 'no mixed checkbox found. Was the driver or test template changed?', - ); - } - const idArr = magic.split(' '); - expect(isUniqArr(idArr)).toBe(true); - for (let index = 0; index < idArr.length; index++) { - const elementId = idArr[index]; - const PosCheckbox = page.locator(`#${elementId}`); - await expect(PosCheckbox).toBeVisible(); - const role = await PosCheckbox.getAttribute('role'); - expect(role).toBe('checkbox'); - } - }); - test(`GIVEN a controlled checklist with one default checkbox and a controlled checkbox of true - WHEN it renders - IT should have aria-checked mixed`, async ({ page }) => { - const exampleName = 'test-controlled-list'; - const { getTriCheckbox } = await setup(page, exampleName); - await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); - }); + // Search? + // test(`GIVEN checklist with checkboxes + // WHEN the values of aria-controls are search + // IT should always return a valid, non-duplicate, checkboxes`, async ({ + // page, + // }) => { + // const { getTriCheckbox } = await setup(page, 'test-list'); + // await expect(getTriCheckbox()).toHaveAttribute('aria-controls'); + // const magic = await getTriCheckbox().getAttribute('aria-controls'); + // expect(magic).not.toBe(null); + // if (magic === null) { + // throw new Error( + // 'no mixed checkbox found. Was the driver or test template changed?' + // ); + // } + // const idArr = magic.split(' '); + // expect(isUniqArr(idArr)).toBe(true); + // for (let index = 0; index < idArr.length; index++) { + // const elementId = idArr[index]; + // const PosCheckbox = page.locator(`#${elementId}`); + // await expect(PosCheckbox).toBeVisible(); + // const role = await PosCheckbox.getAttribute('role'); + // expect(role).toBe('checkbox'); + // } + // }); - test(`GIVEN a controlled checklist with two true checkboxes + // Not using mixed yet + // test(`GIVEN a controlled checklist with one default checkbox and a controlled checkbox of true + // WHEN it renders + // IT should have aria-checked mixed`, async ({ page }) => { + // const exampleName = 'test-controlled-list'; + // const { getTriCheckbox } = await setup(page, exampleName); + // await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); + // }); + + //7 + test(`GIVEN a controlled checklist with two checked checkboxes WHEN it renders IT should have aria-checked true`, async ({ page }) => { const exampleName = 'test-controlled-list-trues'; const { getTriCheckbox } = await setup(page, exampleName); await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'true'); }); - test(`GIVEN a controlled checklist with two false checkboxes + + //8 + test(`GIVEN a controlled checklist with two unchecked checkboxes WHEN it renders IT should have aria-checked true`, async ({ page }) => { const exampleName = 'test-controlled-list-falses'; @@ -195,22 +322,26 @@ test.describe('checklist', () => { await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'false'); }); - test(`GIVEN a controlled checklist with mixed checkboxes - WHEN it renders - IT should have aria-checked mixed`, async ({ page }) => { - const exampleName = 'test-controlled-list-mixed'; - const { getTriCheckbox } = await setup(page, exampleName); - await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); - }); + //Not using mixed yet + // test(`GIVEN a controlled checklist with mixed checkboxes + // WHEN it renders + // IT should have aria-checked mixed`, async ({ page }) => { + // const exampleName = 'test-controlled-list-mixed'; + // const { getTriCheckbox } = await setup(page, exampleName); + // await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); + // }); - test(`GIVEN a controlled checklist with a checklist signal of true and default checkboxes as children + //9 + test(`GIVEN a checklist with intial value of true and default checkboxes as children WHEN the checklist renders IT shoud have aria-checked true`, async ({ page }) => { const exampleName = 'test-controlled-list-true'; const { getTriCheckbox } = await setup(page, exampleName); await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'true'); }); - test(`GIVEN a controlled checklist with a checklist signal of true and default checkboxes as children + + //10 + test(`GIVEN a checklist with intial value of true and default checkboxes as children WHEN the checklist renders ALL its child checkboxes should have aria-checked true`, async ({ page }) => { const exampleName = 'test-controlled-list-true'; @@ -222,59 +353,72 @@ test.describe('checklist', () => { } }); - // TODO: change api to not use indeterminate and used mixed instead - test(`GIVEN a controlled checklist with a checklist signal of true and default checkboxes as children - WHEN a child checkbox is unchecked - THEN the checklist signal should have aria-checked mixed`, async ({ page }) => { + // Not using mixed yet + // ONE CHILD UNCHECKED + //11 + test(`GIVEN a checklist that has all items checked + WHEN a child checkbox is unchecked + THEN the checklist signal should have a mixed state`, async ({ page }) => { const exampleName = 'test-controlled-list-true'; - const { getTriCheckbox } = await setup(page, exampleName); - const firstCheckbox = page.locator('#child-1'); - await firstCheckbox.press(' '); + const { getTriCheckbox, getCheckbox } = await setup(page, exampleName); + const firstCheckbox = getCheckbox().first(); + await firstCheckbox.click(); await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); }); - test(`GIVEN a controlled checklist with a checklist signal of true and default checkboxes as children - WHEN all child checkbox are unchecked - THEN the checklist signal should have aria-checked false`, async ({ page }) => { - const exampleName = 'test-controlled-list-true'; - const { getTriCheckbox } = await setup(page, exampleName); - await page.locator('#child-1').press(' '); - await page.locator('#child-2').press(' '); - await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'false'); - }); - test(`GIVEN a controlled checklist with every checkbox having a defined ID - WHEN it renders - ALL IDs should be present/rendered`, async ({ page }) => { - await setup(page, 'test-props-ids-list'); - const hardCodedIds = ['checklist', 'child-1', 'child-2']; - for (let index = 0; index < hardCodedIds.length; index++) { - const id = hardCodedIds[index]; - await expect(page.locator(`#${id}`)).toBeVisible(); - } - }); + // Not implemented yet + // ALL CHILDREN UNCHECKED + // test(`GIVEN a controlled checklist with a checklist signal of true and default checkboxes as children + // WHEN all child checkbox are unchecked + // THEN the checklist signal should have aria-checked false`, async ({ + // page, + // }) => { + // const exampleName = 'test-controlled-list-true'; + // const { getTriCheckbox } = await setup(page, exampleName); + // await page.locator('#child-1').press(' '); + // await page.locator('#child-2').press(' '); + // await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'false'); + // }); - test(`GIVEN a controlled checklist with every checkbox having a defined ID - WHEN it renders - THEN all IDs should be present in the aria-controls`, async ({ page }) => { - const { getTriCheckbox } = await setup(page, 'test-props-ids-list'); - const hardChildren = ['child-1', 'child-2']; - const magic = await getTriCheckbox().getAttribute('aria-controls'); - const twin = magic?.split(' '); - expect(hardChildren).toStrictEqual(twin); - }); + //Not using IDs yet and may not be needed + // test(`GIVEN a controlled checklist with every checkbox having a defined ID + // WHEN it renders + // ALL IDs should be present/rendered`, async ({ page }) => { + // await setup(page, 'test-props-ids-list'); + // const hardCodedIds = ['checklist', 'child-1', 'child-2']; + // for (let index = 0; index < hardCodedIds.length; index++) { + // const id = hardCodedIds[index]; + // await expect(page.locator(`#${id}`)).toBeVisible(); + // } + // }); - test(`GIVEN checklist with all unchecked checkboxes - WHEN the first child checkbox is clicked - the chekbox with aria-controls should have aria-checked mixed`, async ({ - page, - }) => { - const exampleName = 'test-list'; - const { getTriCheckbox, getCheckbox } = await setup(page, exampleName); - await expect(getTriCheckbox()).toBeVisible(); - await getCheckbox().nth(1).click(); - await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); - }); + //Not using IDs yet and may not be needed + // test(`GIVEN a controlled checklist with every checkbox having a defined ID + // WHEN it renders + // THEN all IDs should be present in the aria-controls`, async ({ page }) => { + // const { getTriCheckbox } = await setup(page, 'test-props-ids-list'); + // const hardChildren = ['child-1', 'child-2']; + // const magic = await getTriCheckbox().getAttribute('aria-controls'); + // const twin = magic?.split(' '); + // expect(hardChildren).toStrictEqual(twin); + // }); + + // Not using mixed yet + // ONE CHILD CHECKED + // test(`GIVEN checklist with all unchecked checkboxes + // WHEN the first child checkbox is clicked + // the chekbox with aria-controls should have aria-checked mixed`, async ({ + // page, + // }) => { + // const exampleName = 'test-list'; + // const { getTriCheckbox, getCheckbox } = await setup(page, exampleName); + // await expect(getTriCheckbox()).toBeVisible(); + // await getCheckbox().nth(1).click(); + // await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); + // }); + //Duplicates? Starts with Unchecked + //12 test(`GIVEN checklist with all unchecked checkboxes WHEN all checkboxes are checked using click THEN the checkbox with aria-controls should have aria-checked true`, async ({ @@ -288,6 +432,7 @@ test.describe('checklist', () => { await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'true'); }); + //13 test(`GIVEN checklist with all unchecked checkboxes WHEN the checklist's checkbox is checked by clicking THEN all checkboxes should have aria-checked true`, async ({ page }) => { @@ -300,6 +445,7 @@ test.describe('checklist', () => { await expect(getCheckbox().nth(2)).toHaveAttribute('aria-checked', 'true'); }); + //14 // TODO: reme two part of test by adding new test file test(`GIVEN checklist with all unchecked checkboxes WHEN the checklist's checkbox is checked twice using click @@ -337,104 +483,16 @@ test.describe('checklist', () => { // } // }); - test(`GIVEN a controlled checklist with a checklist signal of true and default checkboxes as children - WHEN a child checkbox is unchecked - THEN the checklist signal should have aria-checked mixed`, async ({ page }) => { - const exampleName = 'test-controlled-list-true'; - const { getTriCheckbox } = await setup(page, exampleName); - const firstCheckbox = page.locator('#child-1'); - await firstCheckbox.click(); - await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'mixed'); - }); - - test(`GIVEN a controlled checklist with a checklist signal of true and default checkboxes as children - WHEN all child checkbox are unchecked - THEN the checklist signal should have aria-checked false`, async ({ page }) => { - const exampleName = 'test-controlled-list-true'; - const { getTriCheckbox } = await setup(page, exampleName); - await page.locator('#child-1').click(); - await page.locator('#child-2').click(); - await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'false'); - }); + // Not using mixed yet + // test(`GIVEN a controlled checklist with a checklist signal of true and default checkboxes as children + // WHEN all child checkbox are unchecked + // THEN the checklist signal should have aria-checked false`, async ({ + // page, + // }) => { + // const exampleName = 'test-controlled-list-true'; + // const { getTriCheckbox } = await setup(page, exampleName); + // await page.locator('#child-1').click(); + // await page.locator('#child-2').click(); + // await expect(getTriCheckbox()).toHaveAttribute('aria-checked', 'false'); + // }); }); -test.describe('checkbox', () => { - test(`GIVEN a checkbox with a user sig value of true - WHEN the checkbox renders - IT should have aria-checked as true`, async ({ page }) => { - const exampleName = 'test-hero'; - const { getCheckbox } = await setup(page, exampleName); - await expect(getCheckbox()).toBeVisible(); - await expect(getCheckbox()).toHaveAttribute('aria-checked', 'true'); - }), - test(`GIVEN a checkbox with a user sig value of true -WHEN the checkbox is focused and the spacebar is pressed -IT should have aria-checked as false`, async ({ page }) => { - const exampleName = 'test-hero'; - const { getCheckbox } = await setup(page, exampleName); - await expect(getCheckbox()).toBeVisible(); - await getCheckbox().focus(); - await getCheckbox().press(' '); - await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); - }); - - test(`GIVEN a checkbox with a user sig value of true - WHEN the checkbox is focused and the spacebar is pressed - IT should have its icon hidden`, async ({ page }) => { - const exampleName = 'test-hero'; - const { getCheckbox, getIcon } = await setup(page, exampleName); - await expect(getIcon()).toBeVisible(); - await getCheckbox().focus(); - await getCheckbox().press(' '); - await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); - await expect(getIcon()).toBeHidden(); - }); - test(`GIVEN a default checkbox with a default sig value of false - WHEN the checkbox is focused and the spacebar is pressed - IT should have its icon visible`, async ({ page }) => { - const exampleName = 'test-default'; - const { getCheckbox, getIcon } = await setup(page, exampleName); - await expect(getIcon()).toBeHidden(); - await getCheckbox().focus(); - await getCheckbox().press(' '); - await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); - await expect(getIcon()).toBeVisible(); - }); - - test(`GIVEN a checkbox with a user sig value of true - WHEN the checkbox is clicked - IT should have aria-checked as false`, async ({ page }) => { - const exampleName = 'test-hero'; - const { getCheckbox } = await setup(page, exampleName); - await expect(getCheckbox()).toBeVisible(); - await getCheckbox().click(); - await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); - }); - - test(`GIVEN a checkbox with a user sig value of true - WHEN the checkbox is clicked - IT should have its icon hidden`, async ({ page }) => { - const exampleName = 'test-hero'; - const { getCheckbox, getIcon } = await setup(page, exampleName); - await expect(getIcon()).toBeVisible(); - await getCheckbox().click(); - await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); - await expect(getIcon()).toBeHidden(); - }); - test(`GIVEN a default checkbox with a default sig value of false - WHEN the checkbox is clicked - IT should have its icon visible`, async ({ page }) => { - const exampleName = 'test-default'; - const { getCheckbox, getIcon } = await setup(page, exampleName); - await expect(getIcon()).toBeHidden(); - await getCheckbox().click(); - await expect(getCheckbox()).toHaveAttribute('aria-checked', 'false'); - await expect(getIcon()).toBeVisible(); - }); -}); -//TODO: create util file -//TODO: add click -//TODO: refactor to use ids instead of nths since its test-only now -function isUniqArr(arr: string[]) { - const singleInstances = new Set(arr); - return arr.length === singleInstances.size; -} diff --git a/packages/kit-headless/src/components/checkbox/checkbox.tsx b/packages/kit-headless/src/components/checkbox/checkbox.tsx deleted file mode 100644 index 63b9966a1..000000000 --- a/packages/kit-headless/src/components/checkbox/checkbox.tsx +++ /dev/null @@ -1,225 +0,0 @@ -import { - PropsOf, - Signal, - Slot, - component$, - sync$, - useContextProvider, - useContext, - $, - useSignal, - useTask$, -} from '@builder.io/qwik'; -import { CheckListContext, CheckboxContext } from './context-id'; -import { TriBool, getTriBool } from '../checklist/checklist-context-wrapper'; -export type MixedStateCheckboxProps = { - 'bind:checked'?: Signal; - checklist?: boolean; - _useCheckListContext?: boolean; - _overWriteCheckbox?: boolean; -} & PropsOf<'div'>; -export type TwoStateCheckboxProps = { - 'bind:checked'?: Signal; - _useCheckListContext?: boolean; - _overWriteCheckbox?: boolean; -} & PropsOf<'div'>; - -type TwoStateCheckboxBehaviorProps = { - 'bind:checked': Signal; -} & PropsOf<'div'>; -export type ChecklistTwoStateCheckboxProps = { - 'bind:checked'?: Signal; - _useCheckListContext?: boolean; - _overWriteCheckbox?: boolean; -} & PropsOf<'div'>; -export const CheckboxRoot = component$((props) => { - // this is done to avoid consumers dealing with two types checkboxes, could go in different files - if (props.checklist) { - return ( - - - - ); - } - if (props._useCheckListContext) { - return ( - - - - ); - } - return ( - - - - ); -}); - -function getAriaChecked(triBool: TriBool): 'mixed' | 'true' | 'false' { - if (triBool === 'indeterminate') { - return 'mixed'; - } - return `${triBool === true}`; -} - -export const TwoStateCheckbox = component$((props) => { - // all the sig stuff should be refactored into a fancy hook - const defaultSig = useSignal(false); - const appliedSig = props['bind:checked'] ?? defaultSig; - const checklistID = useSignal(props.id); - useContextProvider(CheckboxContext, appliedSig); - return ( - - - - ); -}); - -export const ChecklistTwoStateCheckbox = component$( - (props) => { - // this code is duplicate bcs you cant use conditionals on hooks (checklistContext could be undefined) - // this has room for improvement: remove most of the code duplivation - // making this a wrapper over the simpler component or using hooks - const checklistContext = useContext(CheckListContext); - const defaultSig = useSignal(false); - const appliedSig = props['bind:checked'] ?? defaultSig; - const checklistID = useSignal(props.id); - // makes sure that the checklist's value is the same as its child - const syncToChecklist = useSignal(props._overWriteCheckbox); - useContextProvider(CheckboxContext, appliedSig); - useTask$(({ track }) => { - if (syncToChecklist.value !== undefined) { - appliedSig.value = syncToChecklist.value; - syncToChecklist.value = undefined; - } - track(() => { - appliedSig.value; - }); - - // now i can say that there's one good application for object identity - if (!checklistContext.checkboxes.value.some((e) => e === appliedSig)) { - const currIndex = checklistContext.checkboxes.value.length; - - // TODO: refactor id to not run on wrapper but after conditional - if (checklistID.value === undefined) { - checklistID.value = checklistContext.idArr[currIndex]; - } else { - checklistContext.idArr[currIndex] = checklistID.value; - } - checklistContext.checkboxes.value = [ - ...checklistContext.checkboxes.value, - appliedSig, - ]; - } - - const boolArr = checklistContext?.checkboxes.value.map((e) => e.value); - const newVal = getTriBool(boolArr); - checklistContext.checklistSig.value = newVal; - }); - return ( - - - - ); - }, -); - -export const MixedStateCheckbox = component$((props) => { - console.log('mixed'); - - // all the sig stuff should be refactored into a fancy hook - const checklistContext = useContext(CheckListContext); - const childCheckboxes = checklistContext.checkboxes; - const appliedSig = props['bind:checked'] ?? checklistContext.checklistSig; - const ariaControlsStrg = - checklistContext.idArr.length === 0 - ? '' - : checklistContext.idArr.reduce((p, c) => p + ' ' + c); - useContextProvider(CheckboxContext, appliedSig); - - // im not enterily sure why, but the if statement only runs once - if (props['bind:checked'] !== undefined) { - checklistContext.checklistSig = props['bind:checked']; - } - - const changeChecklistSig = $(() => { - if (appliedSig.value !== true) { - appliedSig.value = true; - childCheckboxes.value.forEach((sig) => (sig.value = true)); - return; - } - appliedSig.value = false; - childCheckboxes.value.forEach((sig) => (sig.value = false)); - }); - const handleKeyDownSync$ = sync$((e: KeyboardEvent) => { - if (e.key === ' ') { - e.preventDefault(); - } - }); - const handleKeyDown$ = $((e: KeyboardEvent) => { - if (e.key === ' ') { - changeChecklistSig(); - } - }); - const handleClick$ = $(() => { - changeChecklistSig(); - }); - - return ( -
      - -
      - ); -}); - -const TwoStateCheckboxBehavior = component$((props) => { - const handleKeyDownSync$ = sync$((e: KeyboardEvent) => { - if (e.key === ' ') { - e.preventDefault(); - } - }); - // this logic is duplicared thrice, make into hook pls - const handleClick = $(() => { - props['bind:checked'].value = !props['bind:checked'].value; - }); - const handleKeyDown$ = $((e: KeyboardEvent) => { - if (e.key === ' ') { - props['bind:checked'].value = !props['bind:checked'].value; - } - }); - // TODO: refactor to usetask code into fancy hook thingy - return ( -
      - -
      - ); -}); diff --git a/packages/kit-headless/src/components/checkbox/context-id.ts b/packages/kit-headless/src/components/checkbox/context-id.ts deleted file mode 100644 index fc0cfcabf..000000000 --- a/packages/kit-headless/src/components/checkbox/context-id.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { Signal, createContextId } from '@builder.io/qwik'; -import { TriBool } from '../checklist'; - -export type ArrSigs = Signal[]; -type CheckContextObj = { - checklistSig: Signal; - checkboxes: Signal; - idArr: string[]; -}; -export const CheckboxContext = createContextId>('CheckBox.context'); -export const CheckListContext = createContextId('CheckList.context'); diff --git a/packages/kit-headless/src/components/checkbox/index.ts b/packages/kit-headless/src/components/checkbox/index.ts index 12d259867..b16beff20 100644 --- a/packages/kit-headless/src/components/checkbox/index.ts +++ b/packages/kit-headless/src/components/checkbox/index.ts @@ -1,3 +1,2 @@ -export { CheckboxRoot as Root } from './checkbox'; +export { CheckboxRoot as Root } from './checkbox-root'; export { CheckboxIndicator as Indicator } from './checkbox-indicator'; -export * from './context-id'; diff --git a/packages/kit-headless/src/components/checkbox/note.md b/packages/kit-headless/src/components/checkbox/note.md new file mode 100644 index 000000000..1f457d4b4 --- /dev/null +++ b/packages/kit-headless/src/components/checkbox/note.md @@ -0,0 +1 @@ +# There isn't a route for the checklist. There should be one. That can be changed in menu.md as well as a new folder and examples. diff --git a/packages/kit-headless/src/components/checklist/checklist-context-wrapper.tsx b/packages/kit-headless/src/components/checklist/checklist-context-wrapper.tsx deleted file mode 100644 index 4e6c25611..000000000 --- a/packages/kit-headless/src/components/checklist/checklist-context-wrapper.tsx +++ /dev/null @@ -1,64 +0,0 @@ -import { - PropsOf, - Slot, - useId, - component$, - useContextProvider, - useSignal, - useTask$, -} from '@builder.io/qwik'; -import { CheckListContext } from './context-id'; - -export type CheckListContextWrapperProps = { - ariaLabeledBy: string; - arrSize: number; - idArr: Array; - initialTriBool: TriBool; -} & PropsOf<'div'>; - -export type TriBool = boolean | 'indeterminate'; -export const ChecklistContextWrapper = component$( - (props) => { - const helpme = useSignal([]); - const idArr: string[] = []; - // this sig vals should be a prop - const mehelp = useSignal(props.initialTriBool); - const id = useId(); - for (let index = 0; index < props.arrSize; index++) { - if (props.idArr[index] != false) { - idArr.push(props.idArr[index] as string); - continue; - } - - const uniqId = id + index.toString(); - idArr.push(uniqId); - } - const obj = { checkboxes: helpme, checklistSig: mehelp, idArr }; - useContextProvider(CheckListContext, obj); - useTask$(({ track }) => { - track(() => { - return obj.checkboxes; - }); - }); - return ( -
      - -
      - ); - }, -); - -export function getTriBool(boolArr: boolean[]): TriBool { - if (boolArr.length === 0) { - return 'indeterminate'; - } - if (boolArr.every((e) => e === true)) { - return true; - } - - if (boolArr.every((e) => e === false)) { - return false; - } - - return 'indeterminate'; -} diff --git a/packages/kit-headless/src/components/checklist/checklist-context.tsx b/packages/kit-headless/src/components/checklist/checklist-context.tsx new file mode 100644 index 000000000..0f85930e4 --- /dev/null +++ b/packages/kit-headless/src/components/checklist/checklist-context.tsx @@ -0,0 +1,12 @@ +import { createContextId, type Signal } from '@builder.io/qwik'; + +export interface ChecklistState { + items: Signal; + // toggleItem: (index: number) => void; + allSelected: Signal; + toggleAllSelected: () => void; + indeterminate: Signal; + initialStates: boolean[]; +} + +export const ChecklistContext = createContextId('ChecklistContext'); diff --git a/packages/kit-headless/src/components/checklist/checklist-indicator.tsx b/packages/kit-headless/src/components/checklist/checklist-indicator.tsx index 2ae376f3b..afe406d8e 100644 --- a/packages/kit-headless/src/components/checklist/checklist-indicator.tsx +++ b/packages/kit-headless/src/components/checklist/checklist-indicator.tsx @@ -1,29 +1,12 @@ -import { component$, useContext, PropsOf, Slot, useTask$ } from '@builder.io/qwik'; -import { CheckListContext } from './context-id'; +import { component$, type PropsOf, Slot } from '@builder.io/qwik'; +import { CheckboxIndicator } from '../checkbox/checkbox-indicator'; -export type ChecklistIndicatorProps = PropsOf<'div'>; +type ChecklistItemIndicatorProps = PropsOf<'div'>; -export const ChecklistIndicator = component$((props) => { - const { checklistSig } = useContext(CheckListContext); - - useTask$(({ track }) => { - track(() => checklistSig.value); - }); - - // weird comparions, but it gets the right behavior +export const ChecklistItemIndicator = component$((props: ChecklistItemIndicatorProps) => { return ( -
      -

      Here lol: {JSON.stringify(checklistSig.value)}

      - {checklistSig.value === true && ( -
      - -
      - )} - {checklistSig.value === 'indeterminate' && ( -
      - -
      - )} -
      + + + ); }); diff --git a/packages/kit-headless/src/components/checklist/checklist-item.tsx b/packages/kit-headless/src/components/checklist/checklist-item.tsx new file mode 100644 index 000000000..2a1052b96 --- /dev/null +++ b/packages/kit-headless/src/components/checklist/checklist-item.tsx @@ -0,0 +1,69 @@ +import { + component$, + type PropsOf, + Slot, + useContext, + useSignal, + useTask$, +} from '@builder.io/qwik'; +import { CheckboxRoot } from '../checkbox/checkbox-root'; +import { ChecklistContext } from './checklist-context'; + +interface ChecklistItemProps extends PropsOf<'div'> { + _index?: number; +} + +export const ChecklistItem = component$((props: ChecklistItemProps) => { + const { _index } = props; + + if (_index === undefined) { + throw new Error('Checklist Item must have an index.'); + } + + const context = useContext(ChecklistContext); + const isCheckedSig = useSignal(context.items.value[_index]); + const initialLoadSig = useSignal(true); + + useTask$(({ track }) => { + track(() => context.allSelected.value); + + if (initialLoadSig.value) { + return; + } + + if (context.allSelected.value) { + isCheckedSig.value = true; + } else { + isCheckedSig.value = false; + } + }); + + useTask$(function syncCheckboxState({ track }) { + track(() => isCheckedSig.value); + + // itemsSig + context.items.value[_index] = isCheckedSig.value; + + // root of both checkboxes updating. context.allselected is updated causing the other useTask$ to run again + // if (isCheckedSig.value === false) { + // context.allSelected.value = false; + // } + + const isAllSelected = context.items.value.every((item) => item === true); + // const isAnyChecked = context.items.value.some(Boolean); + + if (isAllSelected) { + context.allSelected.value = true; + } + }); + + useTask$(() => { + initialLoadSig.value = false; + }); + + return ( + + + + ); +}); diff --git a/packages/kit-headless/src/components/checklist/checklist-root.tsx b/packages/kit-headless/src/components/checklist/checklist-root.tsx new file mode 100644 index 000000000..0af93ae7e --- /dev/null +++ b/packages/kit-headless/src/components/checklist/checklist-root.tsx @@ -0,0 +1,65 @@ +import { + type JSXNode, + type JSXChildren, + type PropsOf, + component$, + useSignal, + Slot, + useContextProvider, + $, +} from '@builder.io/qwik'; +import { findComponent, processChildren } from '../../utils/inline-component'; +import { ChecklistContext, type ChecklistState } from './checklist-context'; +import { ChecklistItem } from './checklist-item'; + +export const ChecklistRoot = + //removing component to make inline causes Internal Server + (props: { initialStates: boolean[]; children: JSXChildren | JSXNode }) => { + const children = props.children; + let currItemIndex = 0; + const itemsMap = new Map(); + + findComponent(ChecklistItem, (itemProps) => { + itemProps._index = currItemIndex; + itemsMap.set(currItemIndex, itemProps.disabled); + currItemIndex++; + }); + + processChildren(props.children); + + return ( +
        + {children} +
      + ); + }; + +type ChecklistRootProps = PropsOf<'div'> & { + initialStates: boolean[]; +}; + +export const ChecklistBase = component$( + ({ initialStates, ...props }: ChecklistRootProps) => { + const items = useSignal(initialStates ?? []); + const allSelected = useSignal(false); + const toggleAllSelected = $(() => { + allSelected.value = !allSelected.value; + }); + const indeterminate = useSignal(false); + + const context: ChecklistState = { + items, + allSelected, + toggleAllSelected, + indeterminate, + initialStates, + }; + + useContextProvider(ChecklistContext, context); + return ( +
      + +
      + ); + }, +); diff --git a/packages/kit-headless/src/components/checklist/checklist-selectall.tsx b/packages/kit-headless/src/components/checklist/checklist-selectall.tsx new file mode 100644 index 000000000..94ca02d97 --- /dev/null +++ b/packages/kit-headless/src/components/checklist/checklist-selectall.tsx @@ -0,0 +1,47 @@ +import { + component$, + useContext, + $, + type PropsOf, + Slot, + useSignal, + useTask$, +} from '@builder.io/qwik'; +import { ChecklistContext } from './checklist-context'; +import { CheckboxRoot } from '../checkbox/checkbox-root'; + +export const ChecklistSelectAll = component$((props: PropsOf<'div'>) => { + const context = useContext(ChecklistContext); + const isInitiallySelected = context.initialStates.every(Boolean); + const allSelected = useSignal(isInitiallySelected); + const items = useSignal(context.items.value); + const toggleAll = $(() => { + const newState = !allSelected.value; + allSelected.value = newState; + items.value = items.value.map(() => newState); + context.toggleAllSelected(); + }); + + useTask$(({ track }) => { + track(() => context.items.value); + }); + + /** + * When in mixed state, it should show the mixed state indicator + * + * When in all selected state, it should show the checked indicator + */ + return ( + + + + ); +}); diff --git a/packages/kit-headless/src/components/checklist/checklist.tsx b/packages/kit-headless/src/components/checklist/checklist.tsx deleted file mode 100644 index a5d8d51ce..000000000 --- a/packages/kit-headless/src/components/checklist/checklist.tsx +++ /dev/null @@ -1,105 +0,0 @@ -import { type JSXNode, Component, PropsOf } from '@builder.io/qwik'; -import { CheckboxRoot, type MixedStateCheckboxProps } from '../checkbox/checkbox'; -import { ChecklistContextWrapper, getTriBool } from './checklist-context-wrapper'; - -type CheckListProps = PropsOf<'ul'> & { ariaLabeledBy: string }; -// type CheckBoxes= -/* - This is an inline component. An example use case of an inline component to get the proper indexes with CSR. See issue #4757 - for more information. -*/ -export const Checklist: Component = (props: CheckListProps) => { - const checkArr: JSXNode[] = []; - const hellSigs = []; - let checklistCheckbox = undefined; - const boolArr: boolean[] = []; - const idArr: Array = []; - const checklistChilds: JSXNode[] = []; - const { children: myChildren } = props; - - const childrenToProcess = ( - Array.isArray(myChildren) ? [...myChildren] : [myChildren] - ) as Array; - - while (childrenToProcess.length) { - const child = childrenToProcess.shift(); - - if (!child) { - continue; - } - - if (Array.isArray(child)) { - childrenToProcess.unshift(...child); - continue; - } - - switch (child.type) { - case CheckboxRoot: { - const typedProps = child.props as MixedStateCheckboxProps; - // FYI: Obj.assign mutates - Object.assign(typedProps, { _useCheckListContext: true }); - - checkArr.push(child); - // TODO: fix this if hell by making fn - if (!typedProps.checklist) { - checklistChilds.push(child); - - if (typedProps.id != undefined) { - idArr.push(typedProps.id as string); - } else { - idArr.push(false); - } - if (typedProps['bind:checked'] && typedProps['bind:checked'].value) { - boolArr.push(typedProps['bind:checked'].value); - hellSigs.push(typedProps['bind:checked']); - } else { - boolArr.push(false); - } - } else { - checklistCheckbox = child; - } - - break; - } - - default: { - if (child) { - const anyChildren = Array.isArray(child.children) - ? [...child.children] - : [child.children]; - childrenToProcess.unshift(...(anyChildren as JSXNode[])); - } - - break; - } - } - } - if (checklistCheckbox === undefined) { - throw Error( - "QWIKUI: checklist doesn't have a checkbox. Did you give the atribute to *checklist* to any of the checkboxes inside the checklist?", - ); - } - if (checklistCheckbox.props['bind:checked']) { - checklistChilds.forEach((checkbox) => { - Object.assign(checkbox.props, { _overWriteCheckbox: true }); - }); - } - - return ( - <> - -
        - {checkArr.map((checkbox, i) => ( -
      • {checkbox}
      • - ))} -
      -
      - - ); -}; -// TODO: deprecate ariaLabelledBy diff --git a/packages/kit-headless/src/components/checklist/context-id.ts b/packages/kit-headless/src/components/checklist/context-id.ts deleted file mode 100644 index 13bdbd3ba..000000000 --- a/packages/kit-headless/src/components/checklist/context-id.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { Signal, createContextId } from '@builder.io/qwik'; -import { TriBool } from './checklist-context-wrapper'; - -export type ArrSigs = Signal[]; -type CheckContextObj = { - checklistSig: Signal; - checkboxes: Signal; - idArr: string[]; -}; -export const CheckListContext = createContextId('CheckList.context'); diff --git a/packages/kit-headless/src/components/checklist/index.ts b/packages/kit-headless/src/components/checklist/index.ts index 9db387ec8..110a6b63a 100644 --- a/packages/kit-headless/src/components/checklist/index.ts +++ b/packages/kit-headless/src/components/checklist/index.ts @@ -1,4 +1,5 @@ -export { Checklist as Root } from './checklist'; -export { ChecklistIndicator as Indicator } from './checklist-indicator'; -export * from './checklist-context-wrapper'; -export * from './context-id'; +export { ChecklistRoot as Root } from './checklist-root'; +export { ChecklistSelectAll as SelectAll } from './checklist-selectall'; +export { ChecklistItem as Item } from './checklist-item'; +export { ChecklistItemIndicator as ItemIndicator } from './checklist-indicator'; +export { ChecklistItemIndicator as Indicator } from './checklist-indicator'; diff --git a/packages/kit-headless/src/components/combobox/combobox.test.ts b/packages/kit-headless/src/components/combobox/combobox.test.ts index f76d8a8fc..80cee0898 100644 --- a/packages/kit-headless/src/components/combobox/combobox.test.ts +++ b/packages/kit-headless/src/components/combobox/combobox.test.ts @@ -61,7 +61,7 @@ test.describe('Mouse Behavior', () => { await expect(d.getItemAt(1)).toHaveAttribute('aria-selected', 'true'); }); - test(`GIVEN a combobox with an open listbox + test(`GIVEN a combobox with an open listbox WHEN the 3rd option is clicked THEN the 3rd option should be the selected value`, async ({ page }) => { const { driver: d } = await setup(page, 'hero'); @@ -587,6 +587,52 @@ test.describe('Props', () => { await expect(d.getListbox()).toBeVisible(); }); + + test(`GIVEN a reactive combobox + WHEN the user empties the input with selecting all text and backspace + THEN there should be no selected value`, async ({ page }) => { + const { driver: d } = await setup(page, 'reactive'); + + const apricot = (await d.getItemAt(1).textContent()) ?? ''; + + await expect(d.getInput()).toHaveValue(apricot); + + await d.getInput().clear(); + + await expect(d.getInput()).toHaveValue(''); + + await expect( + page.getByRole('paragraph', { name: 'Your favorite fruit is:' }), + ).toBeVisible(); + }); + + test(`GIVEN a reactive combobox + WHEN the user empties the input and selects a new value + THEN that should update the given signal`, async ({ page }) => { + const { driver: d } = await setup(page, 'reactive'); + + const apricot = (await d.getItemAt(1).textContent()) ?? ''; + + await expect(d.getInput()).toHaveValue(apricot); + + await d.getInput().clear(); + + await expect(d.getInput()).toHaveValue(''); + + await expect( + page.getByRole('paragraph', { name: 'Your favorite fruit is:' }), + ).toBeVisible(); + + await d.openListbox('click'); + + await d.getItemAt(1).click(); + + await expect(d.getInput()).toHaveValue('Apricot'); + + await expect( + page.getByRole('paragraph', { name: 'Your favorite fruit is: Apricot' }), + ).toBeVisible(); + }); }); test.describe('option value', () => { diff --git a/packages/kit-headless/src/components/dropdown/dropdown-checkbox-item.tsx b/packages/kit-headless/src/components/dropdown/dropdown-checkbox-item.tsx index c86f4b1e5..a39da6f95 100644 --- a/packages/kit-headless/src/components/dropdown/dropdown-checkbox-item.tsx +++ b/packages/kit-headless/src/components/dropdown/dropdown-checkbox-item.tsx @@ -9,7 +9,7 @@ import { useTask$, } from '@builder.io/qwik'; -import { CheckboxRoot } from '../checkbox/checkbox'; +import { CheckboxRoot } from '../checkbox/checkbox-root'; import { DropdownItemProps } from './dropdown-item'; import { useDropdownItem } from './use-dropdown-item'; diff --git a/packages/kit-headless/src/components/dropdown/dropdown-radio-item.tsx b/packages/kit-headless/src/components/dropdown/dropdown-radio-item.tsx index 7bcecb690..d7491bfa4 100644 --- a/packages/kit-headless/src/components/dropdown/dropdown-radio-item.tsx +++ b/packages/kit-headless/src/components/dropdown/dropdown-radio-item.tsx @@ -8,7 +8,7 @@ import { useTask$, } from '@builder.io/qwik'; -import { CheckboxRoot } from '../checkbox/checkbox'; +import { CheckboxRoot } from '../checkbox/checkbox-root'; import { DropdownCheckboxItemProps } from './dropdown-checkbox-item'; import { dropdownRadioGroupContextId } from './dropdown-context'; import { useDropdownItem } from './use-dropdown-item'; From 6655bad1d686f5630f4eca01a928018997883431 Mon Sep 17 00:00:00 2001 From: Jack Shelton <104264123+thejackshelton@users.noreply.github.com> Date: Sun, 22 Sep 2024 17:18:29 -0500 Subject: [PATCH 2/3] fix: bound signal updates on the combobox (#968) * initial tests * get values with arrays instead * reactively update * make selection manager cleaner * remove unnecessary addition * fix: onChange$ only executes on client & simpler disabled state * feat: simpler invalid check * remove early return * feat: simplify inline component * simpler initial value naming * refactor: better context names * fix: combobox closes on enter key * fix: combobox properly resets when empty * get correct initial value reactively * fix: highlight jumping * multiple shows correct display * fix: undefined does not become part of the input * fix: handle proper empty input * feat: use local index instead * fix: last filtered item now gets highlighted on down key * fix: preserve item disabled state * fix: combobox empty now properly shows for both filter and non-filtered comboboxes * fix: removing items on backspace * respect input's given signal first * refactor: remove all the logs * fix: combobox scrolling * fix: form submissions * fix: pw tests * refactor: remove test since behavior is no longer warranted * fix: mouse should not affect initial keyboard highlight * feat: changeset --- .changeset/funny-pens-vanish.md | 31 +++ .../components/combobox/combobox-context.tsx | 9 +- .../components/combobox/combobox-empty.tsx | 2 +- .../combobox/combobox-hidden-option.tsx | 8 +- .../components/combobox/combobox-inline.tsx | 165 +++++++--------- .../components/combobox/combobox-input.tsx | 83 ++++---- .../src/components/combobox/combobox-item.tsx | 84 ++++---- .../components/combobox/combobox-popover.tsx | 19 +- .../src/components/combobox/combobox-root.tsx | 156 ++++++--------- .../src/components/combobox/combobox.test.ts | 75 +++---- .../src/components/combobox/use-combobox.tsx | 185 +++++++++++------- 11 files changed, 412 insertions(+), 405 deletions(-) create mode 100644 .changeset/funny-pens-vanish.md diff --git a/.changeset/funny-pens-vanish.md b/.changeset/funny-pens-vanish.md new file mode 100644 index 000000000..bd99ffd15 --- /dev/null +++ b/.changeset/funny-pens-vanish.md @@ -0,0 +1,31 @@ +--- +'@qwik-ui/headless': patch +--- + +# Combobox Improvements + +## ๐Ÿ”„ Reactive Improvements + +- Better handling of array-based values +- Improved handling of initial and reactive values + +## ๐Ÿ› Key Bug Fixes + +- Fixed highlight jumping issues +- Enhanced empty input handling +- Better filtered item highlighting + +## ๐Ÿ–ฑ๏ธ Interaction Enhancements + +- Smoother scrolling experience +- Improved keyboard and mouse coordination + +## ๐Ÿš€ Performance Optimizations + +- More efficient item filtering + +## ๐Ÿงช Reliability + +- Added tests for reactivity handling, item unselection, and mouse-to-pointer interaction switching + +- Improved handling of edge cases in user interactions diff --git a/packages/kit-headless/src/components/combobox/combobox-context.tsx b/packages/kit-headless/src/components/combobox/combobox-context.tsx index aff81076c..a22bce646 100644 --- a/packages/kit-headless/src/components/combobox/combobox-context.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-context.tsx @@ -18,14 +18,17 @@ export type ComboboxContext = { labelRef: Signal; controlRef: Signal; selectedValueSetSig: Signal>; + selectedValuesSig: Signal; + filteredIndexSetSig: Signal>; highlightedIndexSig: Signal; - currDisplayValueSig: Signal; + displayValuesSig: Signal; isMouseOverPopupSig: Signal; + isKeyboardFocusSig: Signal; disabledIndexSetSig: Signal>; removeOnBackspace: boolean; - hasVisibleItemsSig: Signal; + isNoItemsSig: Signal; initialLoadSig: Signal; - _value: string | undefined; + initialValue: string | undefined; loop: boolean; multiple: boolean | undefined; diff --git a/packages/kit-headless/src/components/combobox/combobox-empty.tsx b/packages/kit-headless/src/components/combobox/combobox-empty.tsx index d2eac1d7b..6c9833d12 100644 --- a/packages/kit-headless/src/components/combobox/combobox-empty.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-empty.tsx @@ -7,7 +7,7 @@ export const HComboboxEmpty = component$((props: PropsOf<'div'>) => { return (
      diff --git a/packages/kit-headless/src/components/combobox/combobox-hidden-option.tsx b/packages/kit-headless/src/components/combobox/combobox-hidden-option.tsx index 1ee0bc7a9..017cc2e5f 100644 --- a/packages/kit-headless/src/components/combobox/combobox-hidden-option.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-hidden-option.tsx @@ -22,7 +22,7 @@ export const ComboboxHiddenSelectOption = component$( const context = useContext(comboboxContextId); useTask$(async function modularFormsValidation({ track }) { - track(() => context.selectedValueSetSig.value); + track(() => context.selectedValuesSig.value); if (isServer || !nativeSelectRef.value || !optionRef.value) return; @@ -37,7 +37,11 @@ export const ComboboxHiddenSelectOption = component$( 'Qwik UI: value not found when trying to select or unselect an item.', ); } - optionRef.value.selected = context.selectedValueSetSig.value.has(value); + + const selectedValues = context.selectedValuesSig.value; + optionRef.value.selected = Array.isArray(selectedValues) + ? selectedValues.includes(value) + : selectedValues === value; }); return ( diff --git a/packages/kit-headless/src/components/combobox/combobox-inline.tsx b/packages/kit-headless/src/components/combobox/combobox-inline.tsx index ac8e604d4..2ca5a5c7f 100644 --- a/packages/kit-headless/src/components/combobox/combobox-inline.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-inline.tsx @@ -1,9 +1,10 @@ -import { type JSXNode, Component } from '@builder.io/qwik'; +import { Component, JSXChildren } from '@builder.io/qwik'; import { HComboboxRootImpl, HComboboxRootImplProps } from './combobox-root'; import { HComboboxItem as InternalComboboxItem } from './combobox-item'; import { HComboboxItemLabel as InternalComboboxItemLabel } from './combobox-item-label'; import { HComboboxEmpty as InternalComboboxEmpty } from './combobox-empty'; import { HComboboxErrorMessage } from './combobox-error-message'; +import { findComponent, processChildren } from '../../utils/inline-component'; export type TItemsMap = Map< number, @@ -13,8 +14,8 @@ export type TItemsMap = Map< export type InternalComboboxProps = { /** When a value is passed, we check if it's an actual item value, and get its index at pre-render time. **/ - _valuePropIndex?: number | null; - _value?: string; + initialIndex?: number | null; + initialValue?: string; /** Checks if the consumer added the label in their JSX */ _label?: boolean; @@ -35,7 +36,7 @@ export const HComboboxRoot: Component { const { - children: myChildren, + children, comboboxItemComponent: UserItem, comboboxItemLabelComponent: UserItemLabel, ...rest @@ -47,121 +48,87 @@ export const HComboboxRoot: Component; + findComponent(HComboboxItem, (itemProps) => { + itemProps._index = currItemIndex; + + isItemDisabled = itemProps.disabled === true; + + if (itemProps.value) { + givenItemValue = itemProps.value as string; + } + + // the default case isn't handled here, so we need to process the children to get to the label component + if (itemProps.children) { + return processChildren(itemProps.children as JSXChildren); + } + }); + + findComponent(HComboboxItemLabel, (labelProps) => { + const displayValue = labelProps.children as string; - while (childrenToProcess.length) { - const child = childrenToProcess.shift(); + // distinct value, or the display value is the same as the value + const value = (givenItemValue !== null ? givenItemValue : displayValue) as string; - if (!child) { - continue; + itemsMap.set(currItemIndex, { value, displayValue, disabled: isItemDisabled }); + + if (props.value && props.multiple) { + throw new Error( + `Qwik UI: When in multiple selection mode, the value prop is disabled. Use the bind:value prop's initial signal value instead.`, + ); } - if (Array.isArray(child)) { - childrenToProcess.unshift(...child); - continue; + // if the current option value is equal to the initial value + if (value === props.value) { + // minus one because it is incremented already in SelectOption + initialIndex = currItemIndex; + initialValue = value; } - switch (child.type) { - case HComboboxItem: { - // get the index of the current option - child.props._index = currItemIndex; - - isItemDisabled = child.props.disabled === true; - - if (child.props.value) { - givenItemValue = child.props.value; - } - - // the default case isn't handled here, so we need to process the children to get to the label component - if (child.props.children) { - const childChildren = Array.isArray(child.props.children) - ? [...child.props.children] - : [child.props.children]; - childrenToProcess.unshift(...childChildren); - } - - break; - } - - case HComboboxItemLabel: { - const displayValue = child.props.children as string; - - // distinct value, or the display value is the same as the value - const value = (givenItemValue !== null ? givenItemValue : displayValue) as string; - - itemsMap.set(currItemIndex, { value, displayValue, disabled: isItemDisabled }); - - if (props.value && props.multiple) { - throw new Error( - `Qwik UI: When in multiple selection mode, the value prop is disabled. Use the bind:value prop's initial signal value instead.`, - ); - } - - // if the current option value is equal to the initial value - if (value === props.value) { - // minus one because it is incremented already in SelectOption - valuePropIndex = currItemIndex; - _value = value; - } - - const isString = typeof child.props.children === 'string'; - - if (!isString) { - throw new Error( - `Qwik UI: select item label passed was not a string. It was a ${typeof child - .props.children}.`, - ); - } - - // increment after processing children - currItemIndex++; - - break; - } - - case HComboboxEmpty: { - hasEmptyComp = true; - break; - } - - case HComboboxErrorMessage: { - hasErrorComp = true; - break; - } - - default: { - if (child) { - const anyChildren = Array.isArray(child.children) - ? [...child.children] - : [child.children]; - childrenToProcess.unshift(...(anyChildren as JSXNode[])); - } - - break; - } + const isString = typeof labelProps.children === 'string'; + + if (!isString) { + throw new Error( + `Qwik UI: select item label passed was not a string. It was a ${typeof labelProps.children}.`, + ); } - } + + // increment after processing children + currItemIndex++; + }); + + findComponent(HComboboxEmpty, () => { + hasEmptyComp = true; + }); + + findComponent(HComboboxErrorMessage, () => { + hasErrorComp = true; + }); + + processChildren(children); return ( - {props.children} + {children} ); }; diff --git a/packages/kit-headless/src/components/combobox/combobox-input.tsx b/packages/kit-headless/src/components/combobox/combobox-input.tsx index e4bb482f7..c0c5bd7e5 100644 --- a/packages/kit-headless/src/components/combobox/combobox-input.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-input.tsx @@ -15,7 +15,7 @@ import { useCombinedRef } from '../../hooks/combined-refs'; type HComboboxInputProps = PropsOf<'input'>; export const HComboboxInput = component$( - ({ 'bind:value': inputValueSig, ...props }: HComboboxInputProps) => { + ({ 'bind:value': givenInputValueSig, ...props }: HComboboxInputProps) => { const context = useContext(comboboxContextId); const contextRefOpts = { context, givenContextRef: context.inputRef }; const inputRef = useCombinedRef(props.ref, contextRefOpts); @@ -25,7 +25,7 @@ export const HComboboxInput = component$( const labelId = `${context.localId}-label`; const descriptionId = `${context.localId}-description`; const errorMessageId = `${context.localId}-error-message`; - const initialValueSig = useSignal(); + const initialValueSig = useSignal(); const wasEmptyBeforeBackspaceSig = useSignal(false); const isInputResetSig = useSignal(false); @@ -54,6 +54,8 @@ export const HComboboxInput = component$( const handleKeyDown$ = $(async (e: KeyboardEvent) => { if (!context.itemsMapSig.value) return; + context.isKeyboardFocusSig.value = true; + if (e.key === 'Backspace') { // check if input was empty before backspace wasEmptyBeforeBackspaceSig.value = context.inputValueSig.value.length === 0; @@ -107,7 +109,7 @@ export const HComboboxInput = component$( if (!context.isListboxOpenSig.value) break; await selectionManager$(context.highlightedIndexSig.value, 'toggle'); - if (context.selectedValueSetSig.value.size <= 0) break; + if (context.selectedValuesSig.value.length <= 0) break; if (!context.multiple) { context.isListboxOpenSig.value = false; @@ -142,9 +144,15 @@ export const HComboboxInput = component$( context.highlightedIndexSig.value = null; isInputResetSig.value = false; + if (target.value === '' && !context.multiple) { + context.selectedValuesSig.value = ''; + } else { + context.isListboxOpenSig.value = true; + } + // bind:value on the input - if (inputValueSig) { - inputValueSig.value = el.value; + if (givenInputValueSig) { + givenInputValueSig.value = el.value; context.inputValueSig.value = el.value; } }); @@ -153,64 +161,51 @@ export const HComboboxInput = component$( if (e.key === 'Backspace') { // removeOnBackspace if (!context.multiple) return; - if (context.selectedValueSetSig.value.size === 0) return; + if (context.selectedValuesSig.value.length === 0) return; if (!context.removeOnBackspace) return; if ( (wasEmptyBeforeBackspaceSig.value || isInputResetSig.value) && context.inputValueSig.value.length === 0 ) { - const selectedValuesArray = [...context.selectedValueSetSig.value]; - selectedValuesArray.pop(); // Remove the last element - context.selectedValueSetSig.value = new Set(selectedValuesArray); + const selectedValuesArray = [...context.selectedValuesSig.value]; + selectedValuesArray.pop(); + + context.selectedValuesSig.value = selectedValuesArray; } } }); /** Users may pass an initial value to bind:value on the input, use the value, or bind:value props on the root. */ - useTask$(function initialState() { - const selectedValue = - context.selectedValueSetSig.value.size > 0 - ? context.selectedValueSetSig.value.values().next().value - : ''; - - let initialValue = ''; - let matchingItemValue = null; - let matchingItemIndex = -1; - - context.itemsMapSig.value.forEach((item, index) => { - if (item.value === selectedValue) { - initialValue = item.displayValue; - } - if (inputValueSig?.value && item.displayValue === inputValueSig.value) { - matchingItemValue = item.value; - matchingItemIndex = index; + useTask$(function getInitialValues() { + const { selectedValuesSig, multiple, itemsMapSig, highlightedIndexSig } = context; + const selectedValues = selectedValuesSig.value; + const initialValue: string[] = []; + + for (const [index, item] of itemsMapSig.value.entries()) { + const isSelected = multiple + ? Array.isArray(selectedValues) && selectedValues.includes(item.value) + : item.value === selectedValues; + + if (isSelected) { + initialValue.push(item.displayValue); + highlightedIndexSig.value = index; + // end the loop when we've found our selected value in single mode + if (!multiple) break; } - }); - - initialValueSig.value = initialValue; - - if (matchingItemValue !== null) { - context.selectedValueSetSig.value.add(matchingItemValue); - context.highlightedIndexSig.value = matchingItemIndex; } - }); - const computedInputValueSig = useComputed$(() => { - if (initialValueSig.value) { - return initialValueSig.value; - } else { - if (inputValueSig?.value) { - return inputValueSig.value; - } - return ''; - } + initialValueSig.value = multiple ? initialValue : initialValue[0] || ''; }); + const inputValueSig = useComputed$( + () => givenInputValueSig?.value ?? initialValueSig.value ?? '', + ); + return ( & { }; export const HComboboxItem = component$((props: HComboboxItemProps) => { + const context = useContext(comboboxContextId); + const debounceTimeoutSig = useSignal(); + if (props._index === undefined) { throw new Error('Qwik UI: Combobox component item cannot find its proper index.'); } - const context = useContext(comboboxContextId); + const localIndexSig = useComputed$(() => props._index ?? -1); const itemRef = useCombinedRef(props.ref); - const itemLabelId = `${context.localId}-${props._index ?? -1}-item-label`; const { selectionManager$, filterManager$ } = useCombobox(); + + const itemLabelId = `${context.localId}-${localIndexSig.value}-item-label`; + const isDisabledSig = useComputed$(() => - context.disabledIndexSetSig.value.has(props._index ?? -1), + context.disabledIndexSetSig.value.has(localIndexSig.value), ); const isSelectedSig = useComputed$(() => { - const index = props._index ?? -1; + const index = localIndexSig.value; const selectedValue = context.itemsMapSig.value.get(index)?.value; - return !isDisabledSig.value && context.selectedValueSetSig.value.has(selectedValue!); + if (!selectedValue || isDisabledSig.value) return false; + + if (Array.isArray(context.selectedValuesSig.value)) { + return context.selectedValuesSig.value.includes(selectedValue); + } else { + return context.selectedValuesSig.value === selectedValue; + } }); const isHighlightedSig = useComputed$(() => { if (isDisabledSig.value) return; - if (context.highlightedIndexSig.value === props._index ?? -1) { + if (context.highlightedIndexSig.value === localIndexSig.value) { return true; } else { return false; @@ -64,7 +76,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { const containerRect = context.panelRef.value?.getBoundingClientRect(); const itemRect = itemRef.value?.getBoundingClientRect(); - if (!containerRect || !itemRect || context.isMouseOverPopupSig.value) return; + if (!containerRect || !itemRect || !context.isKeyboardFocusSig.value) return; // Calculates the offset to center the item within the container const offset = @@ -78,9 +90,9 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { }); const handleClick$ = $(async () => { - if (isDisabledSig.value || (props._index ?? -1) === null) return; + if (isDisabledSig.value || localIndexSig.value === null) return; - await selectionManager$(props._index ?? -1, 'toggle'); + await selectionManager$(localIndexSig.value, 'toggle'); if (!isSelectedSig.value && !context.multiple) { context.isListboxOpenSig.value = false; @@ -88,10 +100,10 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { }); const handlePointerOver$ = $(() => { - if (isDisabledSig.value) return; + if (isDisabledSig.value || context.isKeyboardFocusSig.value) return; if (props._index !== undefined && context.isMouseOverPopupSig.value) { - context.highlightedIndexSig.value = props._index ?? -1; + context.highlightedIndexSig.value = localIndexSig.value; } }); @@ -107,50 +119,56 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { useContextProvider(comboboxItemContextId, itemContext); - useTask$(async function navigationTask({ track, cleanup }) { + useTask$(function handleScrolling({ track, cleanup }) { track(() => context.highlightedIndexSig.value); if (isServer || !context.panelRef.value) return; - if (props._index !== context.highlightedIndexSig.value) return; const hasScrollbar = context.panelRef.value.scrollHeight > context.panelRef.value.clientHeight; - if (!hasScrollbar) { - return; + if (!hasScrollbar) return; + + if (debounceTimeoutSig.value !== undefined) { + clearTimeout(debounceTimeoutSig.value); } - const observer = new IntersectionObserver(checkVisibility$, { - root: context.panelRef.value, - threshold: 1.0, - }); + debounceTimeoutSig.value = setTimeout(() => { + if (props._index !== context.highlightedIndexSig.value) return; - cleanup(() => observer?.disconnect()); + const observer = new IntersectionObserver(checkVisibility$, { + root: context.panelRef.value, + threshold: 1.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); + } + }); }); - useTask$(async ({ track }) => { + useTask$(async function defaultFilter({ track }) { track(() => context.inputValueSig.value); if (isServer || !itemRef.value) return; - if (context.inputValueSig.value === '' && !context.multiple) { - context.selectedValueSetSig.value = new Set(); - } else { - context.isListboxOpenSig.value = true; - } + const displayValue = context.itemsMapSig.value.get(localIndexSig.value)?.displayValue; let isVisible; - const displayValue = context.itemsMapSig.value.get(props._index ?? -1)?.displayValue; if (!displayValue) return; if (context.filter) { const lowerCaseDisplayValue = displayValue?.toLowerCase(); const lowerCaseInputValue = context.inputValueSig.value.toLowerCase(); isVisible = lowerCaseDisplayValue?.includes(lowerCaseInputValue); - filterManager$(!!isVisible, itemRef, props._index ?? -1); + filterManager$(!!isVisible, itemRef, localIndexSig.value); } }); @@ -167,7 +185,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { role="option" ref={itemRef} tabIndex={-1} - id={`${context.localId}-${props._index ?? -1}`} + id={`${context.localId}-${localIndexSig.value}`} aria-selected={isSelectedSig.value} aria-disabled={isDisabledSig.value === true ? 'true' : 'false'} data-disabled={isDisabledSig.value ? '' : undefined} @@ -177,7 +195,7 @@ export const HComboboxItem = component$((props: HComboboxItemProps) => { onFocus$={[handleFocus$, props.onFocus$]} aria-labelledby={itemLabelId} data-item - onClick$={handleClick$} + onClick$={[handleClick$, props.onClick$]} {...props} > diff --git a/packages/kit-headless/src/components/combobox/combobox-popover.tsx b/packages/kit-headless/src/components/combobox/combobox-popover.tsx index b935f42f2..aafd115ca 100644 --- a/packages/kit-headless/src/components/combobox/combobox-popover.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-popover.tsx @@ -14,7 +14,6 @@ import { comboboxContextId } from './combobox-context'; import { HPopoverRoot } from '../popover/popover-root'; import { isServer } from '@builder.io/qwik/build'; import { useCombinedRef } from '../../hooks/combined-refs'; -import { useDebouncer } from '../../hooks/use-debouncer'; export const HComboboxPopover = component$>((props) => { const context = useContext(comboboxContextId); @@ -85,12 +84,10 @@ export const HComboboxPopover = component$>((props) initialLoadSig.value = false; }); - const resetScrollMove = useDebouncer( - $(() => { - context.isMouseOverPopupSig.value = false; - }), - 650, - ); + const handleMouseEnter$ = $(() => { + context.isKeyboardFocusSig.value = false; + context.isMouseOverPopupSig.value = true; + }); return ( >((props) role="listbox" aria-expanded={context.isListboxOpenSig.value ? 'true' : 'false'} aria-multiselectable={context.multiple ? 'true' : undefined} - onMouseMove$={async () => { - context.isMouseOverPopupSig.value = true; - - await resetScrollMove(); - }} - onMouseOut$={() => (context.isMouseOverPopupSig.value = false)} - onKeyDown$={() => (context.isMouseOverPopupSig.value = true)} + onMouseEnter$={[handleMouseEnter$, props.onMouseEnter$]} {...rest} > diff --git a/packages/kit-headless/src/components/combobox/combobox-root.tsx b/packages/kit-headless/src/components/combobox/combobox-root.tsx index 6e8a03477..b901f662f 100644 --- a/packages/kit-headless/src/components/combobox/combobox-root.tsx +++ b/packages/kit-headless/src/components/combobox/combobox-root.tsx @@ -12,8 +12,8 @@ import { } from '@builder.io/qwik'; import { ComboboxContext, comboboxContextId } from './combobox-context'; import { InternalComboboxProps } from './combobox-inline'; -import { useCombobox } from './use-combobox'; import { useCombinedRef } from '../../hooks/combined-refs'; +import { useBoundSignal } from '../../utils/bound-signal'; export type TMultiple = M extends true ? string[] : string; @@ -118,13 +118,12 @@ export type HComboboxRootImplProps = Omit< export const HComboboxRootImpl = component$< HComboboxRootImplProps & InternalComboboxProps >((props: HComboboxRootImplProps & InternalComboboxProps) => { - const isListboxOpenSig = useSignal(false); const { onInput$, onChange$, onOpenChange$, - _valuePropIndex: givenValuePropIndex, - _value, + initialIndex, + initialValue, loop: givenLoop, scrollOptions: givenScrollOptions, multiple = false, @@ -132,15 +131,23 @@ export const HComboboxRootImpl = component$< filter = true, _itemsMap, hasEmptyComp, - hasErrorComp, removeOnBackspace = false, name, required, - disabled, + 'bind:value': givenValueSig, + 'bind:open': givenOpenSig, + 'bind:displayValue': givenDisplayValueSig, ...rest } = props; // source of truth + const isListboxOpenSig = useBoundSignal(givenOpenSig, false); + const displayValuesSig = useBoundSignal(givenDisplayValueSig, []); + const selectedValuesSig = useBoundSignal( + givenValueSig, + multiple ? (initialValue ? [initialValue] : []) : initialValue || '', + ); + const itemsMapSig = useComputed$(() => { return props._itemsMap ?? new Map(); }); @@ -154,20 +161,24 @@ export const HComboboxRootImpl = component$< const loop = givenLoop ?? false; const localId = useId(); /** We use selected values to preserve the item state when the consumer filters the items. */ - const selectedValueSetSig = useSignal>(new Set(_value ? [_value] : [])); + const selectedValueSetSig = useSignal>( + new Set(initialValue ? [initialValue] : []), + ); + const disabledIndexSetSig = useSignal>(new Set()); + const filteredIndexSetSig = useSignal>(new Set()); const isMouseOverPopupSig = useSignal(false); - const isDisabledSig = useSignal(disabled ?? false); - const highlightedIndexSig = useSignal(givenValuePropIndex ?? null); + const highlightedIndexSig = useSignal(initialIndex ?? null); const initialLoadSig = useSignal(true); - const currDisplayValueSig = useSignal(); const scrollOptions = givenScrollOptions ?? { behavior: 'smooth', block: 'center', inline: 'nearest', }; const inputValueSig = useSignal(inputRef.value?.value ?? ''); - const isInvalidSig = useSignal(hasErrorComp ?? false); + const isDisabledSig = useComputed$(() => props.disabled ?? false); + const isInvalidSig = useComputed$(() => props.hasErrorComp ?? false); + const isKeyboardFocusSig = useSignal(false); // check any initial disabled items before the computed read below useTask$(() => { @@ -181,19 +192,19 @@ export const HComboboxRootImpl = component$< disabledIndexSetSig.value = disabledIndices; }); - useTask$(({ track }) => { - isInvalidSig.value = track(() => props.hasErrorComp ?? false); - }); - - const hasVisibleItemsSig = useComputed$(() => { - return itemsMapSig.value.size !== disabledIndexSetSig.value.size; + const isNoItemsSig = useComputed$(() => { + return ( + itemsMapSig.value.size === filteredIndexSetSig.value.size || + itemsMapSig.value.size === 0 + ); }); useTask$(function closeIfEmptyComp({ track }) { - track(() => disabledIndexSetSig.value); + track(() => itemsMapSig.value); + track(() => filteredIndexSetSig.value); // automatically closes the listbox if there are no visible items AND the combobox does not have an empty component - if (!hasEmptyComp && !hasVisibleItemsSig.value) { + if (!hasEmptyComp && isNoItemsSig.value) { isListboxOpenSig.value = false; } }); @@ -210,19 +221,22 @@ export const HComboboxRootImpl = component$< controlRef, localId, highlightedIndexSig, + selectedValuesSig, selectedValueSetSig, disabledIndexSetSig, - currDisplayValueSig, + filteredIndexSetSig, + displayValuesSig, isMouseOverPopupSig, + isKeyboardFocusSig, initialLoadSig, removeOnBackspace, filter, loop, multiple, - _value, + initialValue, scrollOptions, placeholder, - hasVisibleItemsSig, + isNoItemsSig, name, required, isDisabledSig, @@ -231,104 +245,50 @@ export const HComboboxRootImpl = component$< useContextProvider(comboboxContextId, context); - const { selectionManager$ } = useCombobox(); - - useTask$(async function reactiveUserValue({ track }) { - const bindValueSig = props['bind:value']; - if (!bindValueSig) return; - track(() => bindValueSig.value); - - for (const [index, item] of itemsMapSig.value) { - if (bindValueSig.value?.includes(item.value)) { - await selectionManager$(index, 'add'); - - if (initialLoadSig.value) { - // for both SSR and CSR, we need to set the initial index - context.highlightedIndexSig.value = index; - } - } else { - await selectionManager$(index, 'remove'); - } - } - }); - - useTask$(function reactiveUserOpen({ track }) { - const bindOpenSig = props['bind:open']; - if (!bindOpenSig) return; - track(() => bindOpenSig.value); - - isListboxOpenSig.value = bindOpenSig.value ?? isListboxOpenSig.value; - }); - - useTask$(function onOpenChangeTask({ track }) { - const bindOpenSig = props['bind:open']; + useTask$(async function handleOpenChange({ track }) { track(() => isListboxOpenSig.value); if (!initialLoadSig.value) { - onOpenChange$?.(isListboxOpenSig.value); - } - - // sync the user's given signal for the open state - if (bindOpenSig && bindOpenSig.value !== isListboxOpenSig.value) { - bindOpenSig.value = isListboxOpenSig.value; + await onOpenChange$?.(isListboxOpenSig.value); } }); - useTask$(function onInputTask({ track }) { + useTask$(async function handleInput({ track }) { track(() => context.inputValueSig.value); if (!initialLoadSig.value) { - onInput$?.(context.inputValueSig.value); + await onInput$?.(context.inputValueSig.value); } }); - useTask$(async function updateConsumerProps({ track }) { - const bindValueSig = props['bind:value']; - const bindDisplayTextSig = props['bind:displayValue']; - track(() => selectedValueSetSig.value); + useTask$(async function handleChange({ track }) { + track(() => selectedValuesSig.value); - const values = Array.from(selectedValueSetSig.value); - const displayValues = []; - - for (const value of values) { - for (const item of context.itemsMapSig.value.values()) { - if (item.value === value) { - displayValues.push(item.displayValue); - break; - } - } + if (!initialLoadSig.value && selectedValuesSig.value.length > 0) { + await onChange$?.(selectedValuesSig.value); } - if (onChange$ && selectedValueSetSig.value.size > 0) { - await onChange$(context.multiple ? values : values[0]); - } + const selectedValues = Array.isArray(selectedValuesSig.value) + ? selectedValuesSig.value + : [selectedValuesSig.value]; - // sync the user's given signal when an option is selected - if (bindValueSig && bindValueSig.value) { - const currUserSigValues = JSON.stringify(bindValueSig.value); - const newUserSigValues = JSON.stringify(values); + const displayValues: string[] = []; - if (currUserSigValues !== newUserSigValues) { - if (context.multiple) { - bindValueSig.value = values; - } else { - bindValueSig.value = values[0]; + for (const [index, item] of itemsMapSig.value.entries()) { + if (selectedValues.includes(item.value)) { + displayValues.push(item.displayValue); + + /* avoid "highlight jumping" on multiple selections */ + if (context.isListboxOpenSig.value === false) { + context.highlightedIndexSig.value = index; } } } - context.currDisplayValueSig.value = displayValues; - - // sync the user's given signal for the display value - if (bindDisplayTextSig && context.currDisplayValueSig.value) { - bindDisplayTextSig.value = context.multiple - ? context.currDisplayValueSig.value - : context.currDisplayValueSig.value[0]; - } - }); + displayValuesSig.value = displayValues; - useTask$(({ track }) => { - isDisabledSig.value = track(() => disabled ?? false); + if (multiple || !context.inputRef.value || !displayValues[0]) return; + context.inputRef.value.value = displayValues[0]; }); useTask$(() => { diff --git a/packages/kit-headless/src/components/combobox/combobox.test.ts b/packages/kit-headless/src/components/combobox/combobox.test.ts index 80cee0898..012bc4a1a 100644 --- a/packages/kit-headless/src/components/combobox/combobox.test.ts +++ b/packages/kit-headless/src/components/combobox/combobox.test.ts @@ -238,6 +238,32 @@ test.describe('Keyboard Behavior', () => { await d.getInput().press('ArrowUp'); await expect(d.getItemAt(1)).toHaveAttribute('data-highlighted'); }); + + test(`GIVEN a combobox with a selected value via mouse + WHEN using the arrow keys to select another option + THEN the latest selected option should be highlighted when the menu is opened again`, async ({ + page, + }) => { + const { driver: d } = await setup(page, 'hero'); + + // initial mouse selection + await d.openListbox('click'); + await d.getItemAt(2).click(); + + await d.openListbox('ArrowDown'); + await expect(d.getItemAt(2)).toHaveAttribute('data-highlighted'); + + // selection via arrow keys + await d.getInput().press('ArrowDown'); + await expect(d.getItemAt(3)).toHaveAttribute('data-highlighted'); + + await d.getInput().press('Enter'); + await expect(d.getItemAt(3)).toHaveAttribute('data-selected'); + + // latest selected option should be highlighted when the menu is opened again + await d.openListbox('ArrowDown'); + await expect(d.getItemAt(3)).toHaveAttribute('data-highlighted'); + }); }); test.describe('selecting options', () => { @@ -589,49 +615,39 @@ test.describe('Props', () => { }); test(`GIVEN a reactive combobox - WHEN the user empties the input with selecting all text and backspace - THEN there should be no selected value`, async ({ page }) => { + WHEN the user empties the input with selecting all text and backspace + THEN there should be no selected value`, async ({ page }) => { const { driver: d } = await setup(page, 'reactive'); - const apricot = (await d.getItemAt(1).textContent()) ?? ''; - - await expect(d.getInput()).toHaveValue(apricot); + await expect(d.getInput()).toHaveValue('Apricot'); + await expect(page.getByRole('paragraph')).toContainText('Apricot'); await d.getInput().clear(); await expect(d.getInput()).toHaveValue(''); - await expect( - page.getByRole('paragraph', { name: 'Your favorite fruit is:' }), - ).toBeVisible(); + await expect(page.getByRole('paragraph')).not.toContainText('Apricot'); }); test(`GIVEN a reactive combobox - WHEN the user empties the input and selects a new value - THEN that should update the given signal`, async ({ page }) => { + WHEN the user empties the input and selects a new value + THEN that should update the given signal`, async ({ page }) => { const { driver: d } = await setup(page, 'reactive'); - const apricot = (await d.getItemAt(1).textContent()) ?? ''; - - await expect(d.getInput()).toHaveValue(apricot); + await expect(d.getInput()).toHaveValue('Apricot'); + await expect(page.getByRole('paragraph')).toContainText('Apricot'); await d.getInput().clear(); await expect(d.getInput()).toHaveValue(''); - await expect( - page.getByRole('paragraph', { name: 'Your favorite fruit is:' }), - ).toBeVisible(); + await expect(page.getByRole('paragraph')).not.toContainText('Apricot'); await d.openListbox('click'); + await d.getItemAt(0).click(); - await d.getItemAt(1).click(); - - await expect(d.getInput()).toHaveValue('Apricot'); - - await expect( - page.getByRole('paragraph', { name: 'Your favorite fruit is: Apricot' }), - ).toBeVisible(); + await expect(d.getInput()).toHaveValue('Apple'); + await expect(page.getByRole('paragraph')).toContainText('Apple'); }); }); @@ -892,19 +908,6 @@ test.describe('Filtering options', () => { expect(await d.getVisibleItemsLength()).toBe(8); }); - test(`GIVEN a combobox - WHEN an option is selected - AND the typed string does not match the filter function - THEN the option should be unselected`, async ({ page }) => { - const { driver: d } = await setup(page, 'hero'); - await d.openListbox('ArrowDown'); - await d.getInput().press('Enter'); - await expect(d.getItemAt(0)).toHaveAttribute('aria-selected', 'true'); - - await d.getInput().press('z'); - await expect(d.getItemAt(0)).toHaveAttribute('aria-selected', 'false'); - }); - test(`GIVEN a combobox WHEN a custom filter is set THEN it should respect the filter function`, async ({ page }) => { diff --git a/packages/kit-headless/src/components/combobox/use-combobox.tsx b/packages/kit-headless/src/components/combobox/use-combobox.tsx index 3a970b7eb..a1462a1e3 100644 --- a/packages/kit-headless/src/components/combobox/use-combobox.tsx +++ b/packages/kit-headless/src/components/combobox/use-combobox.tsx @@ -1,13 +1,46 @@ import { useContext, $, Signal } from '@builder.io/qwik'; import { comboboxContextId } from './combobox-context'; +class ValueManager { + constructor( + private isMultiple: boolean, + private initialValue: T | T[], + ) {} + + add(value: T): T | T[] { + if (this.isMultiple) { + const currentArray = Array.isArray(this.initialValue) ? [...this.initialValue] : []; + return [...currentArray, value]; + } + return value; + } + + remove(value: T): T | T[] { + if (this.isMultiple && Array.isArray(this.initialValue)) { + return this.initialValue.filter((v) => v !== value); + } + return '' as T; + } + + toggle(value: T): T | T[] { + if (this.isMultiple) { + const currentArray = Array.isArray(this.initialValue) ? [...this.initialValue] : []; + return currentArray.includes(value) + ? currentArray.filter((v) => v !== value) + : [...currentArray, value]; + } + return this.initialValue === value ? ('' as T) : value; + } +} + export function useCombobox() { const context = useContext(comboboxContextId); + const selectionManager$ = $( async (index: number | null, action: 'add' | 'toggle' | 'remove') => { if (index === null) return; - const selectedDisplayValue = context.itemsMapSig.value.get(index)?.displayValue; + const selectedDisplayValue = context.itemsMapSig.value.get(index)?.displayValue; const value = context.itemsMapSig.value.get(index)?.value; if (!value) { @@ -16,49 +49,26 @@ export function useCombobox() { ); } - if (action === 'add') { - if (context.multiple) { - context.selectedValueSetSig.value = new Set([ - ...context.selectedValueSetSig.value, - value, - ]); - } else { - context.selectedValueSetSig.value = new Set([value]); - } - } - if (action === 'toggle') { - if (context.selectedValueSetSig.value.has(value)) { - context.selectedValueSetSig.value = new Set( - [...context.selectedValueSetSig.value].filter( - (selectedValue) => selectedValue !== value, - ), - ); - } else { - if (context.multiple) { - context.selectedValueSetSig.value = new Set([ - ...context.selectedValueSetSig.value, - value, - ]); - } else { - context.selectedValueSetSig.value = new Set([value]); - } - } - } - if (action === 'remove') { - context.selectedValueSetSig.value = new Set( - [...context.selectedValueSetSig.value].filter( - (selectedValue) => selectedValue !== value, - ), - ); - } + const valueManager = new ValueManager( + context.multiple ?? false, + context.selectedValuesSig.value, + ); - if (action === 'add' || action === 'toggle') { - if (!context.inputRef.value) return; - if (!selectedDisplayValue) return; + switch (action) { + case 'add': + context.selectedValuesSig.value = valueManager.add(value); + break; + case 'remove': + context.selectedValuesSig.value = valueManager.remove(value); + return; // Early return for 'remove' action + case 'toggle': + context.selectedValuesSig.value = valueManager.toggle(value); + break; + } - if (!context.multiple && context.selectedValueSetSig.value.has(value)) { - context.inputRef.value.value = selectedDisplayValue; - } + // Update input value for single-select combobox + if (!context.multiple && context.inputRef.value && selectedDisplayValue) { + context.inputRef.value.value = selectedDisplayValue; } }, ); @@ -66,56 +76,81 @@ export function useCombobox() { const filterManager$ = $((isVisible: boolean, itemRef: Signal, index: number) => { if (!itemRef.value) return; + const isDisabled = context.itemsMapSig.value.get(index)?.disabled; + itemRef.value.style.display = isVisible ? '' : 'none'; - context.disabledIndexSetSig.value = new Set( + + context.filteredIndexSetSig.value = new Set( isVisible - ? [...context.disabledIndexSetSig.value].filter( - (selectedIndex) => selectedIndex !== index, + ? [...context.filteredIndexSetSig.value].filter( + (filteredIndex) => filteredIndex !== index, ) - : [...context.disabledIndexSetSig.value, index], + : [...context.filteredIndexSetSig.value, index], ); + + // Update disabledIndexSetSig + if (isDisabled) { + context.disabledIndexSetSig.value = new Set([ + ...context.disabledIndexSetSig.value, + index, + ]); + } else { + context.disabledIndexSetSig.value = new Set( + [...context.disabledIndexSetSig.value].filter( + (disabledIndex) => disabledIndex !== index, + ), + ); + } }); const getNextEnabledItemIndex$ = $((index: number) => { const len = context.itemsMapSig.value.size; - if (len === 1) { - return context.disabledIndexSetSig.value.has(0) ? -1 : 0; - } + if (len === 0) return -1; - let offset = 1; - if (!context.loop && index + 1 >= len) { - return index; - } - while (offset < len) { - const nextIndex = (index + offset) % len; - if (!context.disabledIndexSetSig.value.has(nextIndex)) { - return nextIndex; - } - offset++; - if (!context.loop && index + offset >= len) { - break; + const findNextEnabled = (start: number) => { + for (let i = 0; i < len; i++) { + const nextIndex = (start + i) % len; + if ( + !context.disabledIndexSetSig.value.has(nextIndex) && + !context.filteredIndexSetSig.value.has(nextIndex) + ) { + return nextIndex; + } } + return -1; + }; + + if (index === -1 || len === 1) { + return findNextEnabled(0); } - return index; + + const nextIndex = findNextEnabled(index + 1); + return context.loop || nextIndex > index ? nextIndex : index; }); const getPrevEnabledItemIndex$ = $((index: number) => { - let offset = 1; const len = context.itemsMapSig.value.size; - if (!context.loop && index - 1 < 0) { - return index; - } - while (offset <= len) { - const prevIndex = (index - offset + len) % len; - if (!context.disabledIndexSetSig.value.has(prevIndex)) { - return prevIndex; - } - offset++; - if (!context.loop && index - offset < 0) { - break; + if (len === 0) return -1; + + const findPrevEnabled = (start: number) => { + for (let i = 0; i < len; i++) { + const prevIndex = (start - i + len) % len; + if ( + !context.disabledIndexSetSig.value.has(prevIndex) && + !context.filteredIndexSetSig.value.has(prevIndex) + ) { + return prevIndex; + } } + return -1; + }; + + if (index === -1 || len === 1) { + return findPrevEnabled(len - 1); } - return index; + + const prevIndex = findPrevEnabled(index - 1); + return context.loop || prevIndex < index ? prevIndex : index; }); const getActiveDescendant$ = $((index: number) => { From af6aff98dd2a77c4e20c1e98b8619d322c6717f6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 22 Sep 2024 17:21:48 -0500 Subject: [PATCH 3/3] Version Packages (#971) Co-authored-by: github-actions[bot] --- .changeset/funny-pens-vanish.md | 31 ------------------------------ packages/kit-headless/CHANGELOG.md | 31 ++++++++++++++++++++++++++++++ packages/kit-headless/package.json | 2 +- 3 files changed, 32 insertions(+), 32 deletions(-) delete mode 100644 .changeset/funny-pens-vanish.md diff --git a/.changeset/funny-pens-vanish.md b/.changeset/funny-pens-vanish.md deleted file mode 100644 index bd99ffd15..000000000 --- a/.changeset/funny-pens-vanish.md +++ /dev/null @@ -1,31 +0,0 @@ ---- -'@qwik-ui/headless': patch ---- - -# Combobox Improvements - -## ๐Ÿ”„ Reactive Improvements - -- Better handling of array-based values -- Improved handling of initial and reactive values - -## ๐Ÿ› Key Bug Fixes - -- Fixed highlight jumping issues -- Enhanced empty input handling -- Better filtered item highlighting - -## ๐Ÿ–ฑ๏ธ Interaction Enhancements - -- Smoother scrolling experience -- Improved keyboard and mouse coordination - -## ๐Ÿš€ Performance Optimizations - -- More efficient item filtering - -## ๐Ÿงช Reliability - -- Added tests for reactivity handling, item unselection, and mouse-to-pointer interaction switching - -- Improved handling of edge cases in user interactions diff --git a/packages/kit-headless/CHANGELOG.md b/packages/kit-headless/CHANGELOG.md index 91be78861..3a02b0b5d 100644 --- a/packages/kit-headless/CHANGELOG.md +++ b/packages/kit-headless/CHANGELOG.md @@ -1,5 +1,36 @@ # Changelog +## 0.6.1 + +### Patch Changes + +- # Combobox Improvements (by [@thejackshelton](https://github.com/thejackshelton) in [#968](https://github.com/qwikifiers/qwik-ui/pull/968)) + + ## ๐Ÿ”„ Reactive Improvements + + - Better handling of array-based values + - Improved handling of initial and reactive values + + ## ๐Ÿ› Key Bug Fixes + + - Fixed highlight jumping issues + - Enhanced empty input handling + - Better filtered item highlighting + + ## ๐Ÿ–ฑ๏ธ Interaction Enhancements + + - Smoother scrolling experience + - Improved keyboard and mouse coordination + + ## ๐Ÿš€ Performance Optimizations + + - More efficient item filtering + + ## ๐Ÿงช Reliability + + - Added tests for reactivity handling, item unselection, and mouse-to-pointer interaction switching + - Improved handling of edge cases in user interactions + ## 0.6.0 ### Minor Changes diff --git a/packages/kit-headless/package.json b/packages/kit-headless/package.json index b34a44fe4..4890d7bb3 100644 --- a/packages/kit-headless/package.json +++ b/packages/kit-headless/package.json @@ -1,6 +1,6 @@ { "name": "@qwik-ui/headless", - "version": "0.6.0", + "version": "0.6.1", "description": "Qwik UI headless components library", "publishConfig": { "access": "public"