fix: Checkbox.Group onValueChange not being called (#1146)

huntabyte · Feb 11, 2025 · b871643 · b871643
1 parent 90435cf
commit b871643
Show file tree

Hide file tree

Showing 18 changed files with 769 additions and 637 deletions.
diff --git a/.changeset/nice-cougars-trade.md b/.changeset/nice-cougars-trade.md
@@ -0,0 +1,5 @@
+---
+"bits-ui": patch
+---
+
+fix: `Checkbox.Group` not firing `onValueChange`
diff --git a/packages/bits-ui/src/lib/bits/accordion/types.ts b/packages/bits-ui/src/lib/bits/accordion/types.ts
@@ -89,6 +89,12 @@ export type AccordionRootPropsWithoutHTML =
 export type AccordionRootProps = AccordionRootPropsWithoutHTML &
 	Without<BitsPrimitiveDivAttributes, AccordionRootPropsWithoutHTML>;
 
+export type AccordionRootSingleProps = AccordionRootSinglePropsWithoutHTML &
+	Without<BitsPrimitiveDivAttributes, AccordionRootSinglePropsWithoutHTML>;
+
+export type AccordionMultipleProps = AccordionRootMultiplePropsWithoutHTML &
+	Without<BitsPrimitiveDivAttributes, AccordionRootMultiplePropsWithoutHTML>;
+
 export type AccordionTriggerPropsWithoutHTML = WithChild;
 
 export type AccordionTriggerProps = AccordionTriggerPropsWithoutHTML &

diff --git a/packages/bits-ui/src/lib/bits/checkbox/checkbox.svelte.ts b/packages/bits-ui/src/lib/bits/checkbox/checkbox.svelte.ts
@@ -2,7 +2,12 @@ import { srOnlyStyles, styleToString, useRefById } from "svelte-toolbelt";
 import type { HTMLButtonAttributes } from "svelte/elements";
 import { Context, watch } from "runed";
 import type { ReadableBoxedValues, WritableBoxedValues } from "$lib/internal/box.svelte.js";
-import type { BitsKeyboardEvent, BitsMouseEvent, WithRefProps } from "$lib/internal/types.js";
+import type {
+	BitsKeyboardEvent,
+	BitsMouseEvent,
+	OnChangeFn,
+	WithRefProps,
+} from "$lib/internal/types.js";
 import { getAriaChecked, getAriaRequired, getDataDisabled } from "$lib/internal/attrs.js";
 import { kbd } from "$lib/internal/kbd.js";
 
@@ -15,6 +20,7 @@ type CheckboxGroupStateProps = WithRefProps<
 		name: string | undefined;
 		disabled: boolean;
 		required: boolean;
+		onValueChange: OnChangeFn<string[]>;
 	}> &
 		WritableBoxedValues<{
 			value: string[];
@@ -32,6 +38,7 @@ class CheckboxGroupState {
 		if (!checkboxValue) return;
 		if (!this.opts.value.current.includes(checkboxValue)) {
 			this.opts.value.current.push(checkboxValue);
+			this.opts.onValueChange.current(this.opts.value.current);
 		}
 	}
 
@@ -40,6 +47,7 @@ class CheckboxGroupState {
 		const index = this.opts.value.current.indexOf(checkboxValue);
 		if (index === -1) return;
 		this.opts.value.current.splice(index, 1);
+		this.opts.onValueChange.current(this.opts.value.current);
 	}
 
 	props = $derived.by(

diff --git a/packages/bits-ui/src/lib/bits/checkbox/components/checkbox-group.svelte b/packages/bits-ui/src/lib/bits/checkbox/components/checkbox-group.svelte
@@ -29,8 +29,12 @@
 		name: box.with(() => name),
 		value: box.with(
 			() => value,
-			(v) => onValueChange(v)
+			(v) => {
+				value = v;
+				onValueChange(v);
+			}
 		),
+		onValueChange: box.with(() => onValueChange),
 	});
 
 	const mergedProps = $derived(mergeProps(restProps, groupState.props));

diff --git a/packages/tests/src/tests/accordion/accordion-multi-test-controlled.svelte b/packages/tests/src/tests/accordion/accordion-multi-test-controlled.svelte
@@ -10,10 +10,10 @@
 	};
 
 	type Props = {
-		disabled: boolean;
-		items: Item[];
-		value: string[];
-		onValueChange: (v: string[]) => void;
+		disabled?: boolean;
+		items?: Item[];
+		value?: string[];
+		onValueChange?: (v: string[]) => void;
 	} & Omit<BitsPrimitiveDivAttributes, "value">;
 
 	let { disabled = false, items = [], value: valueProp = [], ...restProps }: Props = $props();

diff --git a/packages/tests/src/tests/accordion/accordion-multi-test.svelte b/packages/tests/src/tests/accordion/accordion-multi-test.svelte
@@ -10,10 +10,10 @@
 	};
 
 	type Props = {
-		disabled: boolean;
-		items: Item[];
-		value: string[];
-		onValueChange: (v: string[]) => void;
+		disabled?: boolean;
+		items?: Item[];
+		value?: string[];
+		onValueChange?: (v: string[]) => void;
 	} & Omit<BitsPrimitiveDivAttributes, "value">;
 
 	let { disabled = false, items = [], value = [], ...restProps }: Props = $props();

diff --git a/packages/tests/src/tests/accordion/accordion-single-force-mount-test.svelte b/packages/tests/src/tests/accordion/accordion-single-force-mount-test.svelte
@@ -1,5 +1,5 @@
 <script lang="ts">
-	import { Accordion } from "bits-ui";
+	import { Accordion, type AccordionRootSingleProps } from "bits-ui";
 
 	type Item = {
 		value: string;
@@ -14,13 +14,10 @@
 		items = [],
 		value = "",
 		withOpenCheck = false,
-	}: Omit<
-		Accordion.RootProps & {
-			items: Item[];
-			withOpenCheck?: boolean;
-		},
-		"type"
-	> = $props();
+	}: Omit<AccordionRootSingleProps, "type"> & {
+		items?: Item[];
+		withOpenCheck?: boolean;
+	} = $props();
 </script>
 
 <Accordion.Root type="single" value={value as string} {disabled} data-testid="root">

diff --git a/packages/tests/src/tests/accordion/accordion-single-test-controlled.svelte b/packages/tests/src/tests/accordion/accordion-single-test-controlled.svelte
@@ -1,5 +1,5 @@
 <script lang="ts">
-	import { Accordion } from "bits-ui";
+	import { Accordion, type AccordionRootSingleProps } from "bits-ui";
 
 	type Item = {
 		value: string;
@@ -13,12 +13,9 @@
 		disabled = false,
 		items = [],
 		value: valueProp = "",
-	}: Omit<
-		Accordion.RootProps & {
-			items: Item[];
-		},
-		"type"
-	> = $props();
+	}: Omit<AccordionRootSingleProps, "type"> & {
+		items: Item[];
+	} = $props();
 
 	let value = $state(valueProp);
 </script>

diff --git a/packages/tests/src/tests/accordion/accordion-single-test.svelte b/packages/tests/src/tests/accordion/accordion-single-test.svelte
@@ -1,5 +1,5 @@
 <script lang="ts">
-	import { Accordion } from "bits-ui";
+	import { Accordion, type AccordionRootSingleProps } from "bits-ui";
 
 	type Item = {
 		value: string;
@@ -13,15 +13,13 @@
 		disabled = false,
 		items = [],
 		value = "",
-	}: Omit<
-		Accordion.RootProps & {
-			items: Item[];
-		},
-		"type"
-	> = $props();
+		...restProps
+	}: Omit<AccordionRootSingleProps, "type"> & {
+		items: Item[];
+	} = $props();
 </script>
 
-<Accordion.Root type="single" value={value as string} {disabled} data-testid="root">
+<Accordion.Root {value} {disabled} data-testid="root" type="single" {...restProps}>
 	{#each items as { value, title, disabled, content, level }}
 		<Accordion.Item {value} {disabled} data-testid="{value}-item">
 			<Accordion.Header {level} data-testid="{value}-header">