Fix checkbox tests (#767)

Co-authored-by: philon- <philon-@users.noreply.github.com>

packages/web/src/components/UI/Checkbox/Checkbox.test.tsx

@@ -1,56 +1,75 @@
 import { Checkbox } from "@components/UI/Checkbox/index.tsx";
 import { cleanup, fireEvent, render, screen } from "@testing-library/react";
-import type React from "react";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 
-vi.mock("@components/UI/Label.tsx", () => ({
-  Label: ({
-    children,
-    className,
-    htmlFor,
-    id,
-  }: {
-    children: React.ReactNode;
-    className: string;
-    htmlFor: string;
-    id: string;
-  }) => (
-    <label
-      data-testid="label-component"
-      className={className}
-      htmlFor={htmlFor}
-      id={id}
-    >
-      {children}
-    </label>
-  ),
-}));
-
 describe("Checkbox", () => {
   beforeEach(cleanup);
 
-  it("renders unchecked by default", () => {
+  it("renders unchecked by default (uncontrolled)", () => {
     render(<Checkbox />);
     const checkbox = screen.getByRole("checkbox");
+    const presentation = screen.getByRole("presentation");
     expect(checkbox).not.toBeChecked();
-    expect(screen.queryByText("Check")).not.toBeInTheDocument();
+    // unchecked -> no filled bg class
+    expect(presentation).not.toHaveClass("bg-slate-500");
   });
 
-  it("renders checked when checked prop is true", () => {
+  it("respects defaultChecked in uncontrolled mode", () => {
-    render(<Checkbox checked />);
+    render(<Checkbox defaultChecked />);
     expect(screen.getByRole("checkbox")).toBeChecked();
-    expect(screen.getByRole("presentation")).toBeInTheDocument();
   });
 
-  it("calls onChange when clicked", () => {
+  it("renders checked when controlled with checked=true", () => {
+    render(<Checkbox checked />);
+    const checkbox = screen.getByRole("checkbox");
+    const presentation = screen.getByRole("presentation");
+    expect(checkbox).toBeChecked();
+    expect(presentation).toHaveClass("bg-slate-500");
+  });
+
+  it("calls onChange when clicked (uncontrolled) and toggles DOM state", () => {
     const onChange = vi.fn();
     render(<Checkbox onChange={onChange} />);
 
-    fireEvent.click(screen.getByRole("presentation"));
+    const checkbox = screen.getByRole("checkbox");
-    expect(onChange).toHaveBeenCalledWith(true);
+    const presentation = screen.getByRole("presentation");
 
-    fireEvent.click(screen.getByRole("presentation"));
+    fireEvent.click(presentation);
-    expect(onChange).toHaveBeenCalledWith(false);
+    expect(onChange).toHaveBeenLastCalledWith(true);
+    expect(checkbox).toBeChecked();
+
+    fireEvent.click(presentation);
+    expect(onChange).toHaveBeenLastCalledWith(false);
+    expect(checkbox).not.toBeChecked();
+  });
+
+  it("controlled: calls onChange but does not toggle without prop update", () => {
+    const onChange = vi.fn();
+    render(<Checkbox checked={false} onChange={onChange} />);
+
+    const checkbox = screen.getByRole("checkbox");
+    const presentation = screen.getByRole("presentation");
+
+    fireEvent.click(presentation);
+    expect(onChange).toHaveBeenLastCalledWith(true);
+    // still unchecked because parent didn't update prop
+    expect(checkbox).not.toBeChecked();
+  });
+
+  it("controlled: reflects external prop changes after onChange", () => {
+    const onChange = vi.fn();
+    const { rerender } = render(<Checkbox checked={false} onChange={onChange} />);
+
+    const checkbox = screen.getByRole("checkbox");
+    const presentation = screen.getByRole("presentation");
+
+    fireEvent.click(presentation);
+    expect(onChange).toHaveBeenLastCalledWith(true);
+
+    // parent updates `checked` based on onChange
+    rerender(<Checkbox checked={true} onChange={onChange} />);
+    expect(checkbox).toBeChecked();
+    expect(presentation).toHaveClass("bg-slate-500");
   });
 
   it("uses provided id", () => {
@@ -58,23 +77,16 @@ describe("Checkbox", () => {
     expect(screen.getByRole("checkbox").id).toBe("custom-id");
   });
 
-  it("renders children in Label component", () => {
+  it("renders children inside the label", () => {
     render(<Checkbox>Test Label</Checkbox>);
-    expect(screen.getByTestId("label-component")).toHaveTextContent(
+    expect(screen.getByTestId("label-component")).toHaveTextContent("Test Label");
-      "Test Label",
-    );
   });
 
-  it("applies custom className", () => {
+  it("applies custom className to wrapper label", () => {
     const { container } = render(<Checkbox className="custom-class" />);
     expect(container.firstChild).toHaveClass("custom-class");
   });
 
-  it("applies labelClassName to Label", () => {
-    render(<Checkbox labelClassName="label-class">Test</Checkbox>);
-    expect(screen.getByTestId("label-component")).toHaveClass("label-class");
-  });
-
   it("disables checkbox when disabled prop is true", () => {
     render(<Checkbox disabled />);
     expect(screen.getByRole("checkbox")).toBeDisabled();
@@ -84,7 +96,6 @@ describe("Checkbox", () => {
   it("does not call onChange when disabled", () => {
     const onChange = vi.fn();
     render(<Checkbox onChange={onChange} disabled />);
-
     fireEvent.click(screen.getByRole("presentation"));
     expect(onChange).not.toHaveBeenCalled();
   });
@@ -99,24 +110,19 @@ describe("Checkbox", () => {
     expect(screen.getByRole("checkbox")).toHaveAttribute("name", "test-name");
   });
 
-  it("passes through additional props", () => {
+  it("passes through additional props to the input", () => {
     render(<Checkbox data-testid="extra-prop" />);
-    expect(screen.getByRole("checkbox")).toHaveAttribute(
+    expect(screen.getByRole("checkbox")).toHaveAttribute("data-testid", "extra-prop");
-      "data-testid",
-      "extra-prop",
-    );
   });
 
-  it("toggles checked state correctly", () => {
+  it("uncontrolled: toggles checked state when clicking the visual box", () => {
     render(<Checkbox />);
     const checkbox = screen.getByRole("checkbox");
     const presentation = screen.getByRole("presentation");
 
     expect(checkbox).not.toBeChecked();
-
     fireEvent.click(presentation);
     expect(checkbox).toBeChecked();
-
     fireEvent.click(presentation);
     expect(checkbox).not.toBeChecked();
   });

packages/web/src/components/UI/Checkbox/index.tsx

@@ -1,9 +1,10 @@
 import { cn } from "@core/utils/cn.ts";
 import { Check } from "lucide-react";
-import { useId } from "react";
+import { useId, useState } from "react";
 
 interface CheckboxProps {
   checked?: boolean;
+  defaultChecked?: boolean;
   onChange?: (checked: boolean) => void;
   className?: string;
   labelClassName?: string;
@@ -15,7 +16,8 @@ interface CheckboxProps {
 }
 
 export function Checkbox({
-  checked = false,
+  checked,
+  defaultChecked = false,
   onChange,
   className,
   id: propId,
@@ -28,10 +30,21 @@ export function Checkbox({
   const generatedId = useId();
   const id = propId || generatedId;
 
-  const handleToggle = (): void => {
+  const isControlled = checked !== undefined;
-    if (!disabled) {
+  const [internal, setInternal] = useState<boolean>(defaultChecked);
-      onChange?.(!checked);
+  const value = checked ?? internal;
+
+  const handleToggle = (e: React.ChangeEvent<HTMLInputElement>) => {
+    if (disabled) {
+      return;
     }
+
+    const next = e.target.checked;
+
+    if (!isControlled) {
+      setInternal(next);
+    }
+    onChange?.(next);
   };
 
   return (
@@ -41,11 +54,12 @@ export function Checkbox({
         disabled ? "cursor-not-allowed opacity-50" : "cursor-pointer",
         className,
       )}
+      data-testid="label-component"
     >
       <input
         type="checkbox"
         id={id}
-        checked={checked}
+        checked={value}
         onChange={handleToggle}
         disabled={disabled}
         required={required}
@@ -57,10 +71,12 @@ export function Checkbox({
         className={cn(
           "flex h-6 w-6 shrink-0 items-center justify-center rounded-md border-2 border-gray-500 transition-colors",
           "peer-focus-visible:ring-2 peer-focus-visible:ring-blue-500 peer-focus-visible:ring-offset-2",
-          { "border-slate-500 bg-slate-500": checked },
+          { "border-slate-500 bg-slate-500": value },
+          { "opacity-50": disabled },
         )}
+        role="presentation"
       >
-        {checked && (
+        {value && (
           <div className="animate-fade-in">
             <Check className="h-4 w-4 text-white" />
           </div>

packages/web/src/components/generic/Filter/FilterComponents.tsx

@@ -170,6 +170,7 @@ export const FilterMulti = <K extends EnumArrayKeys<FilterState>>({
               key={val}
               checked={selected.includes(val)}
               onChange={(checked) => toggleValue(val, checked)}
+              className="flex items-center gap-2"
             >
               <span className="dark:text-slate-200">{getLabel(val)}</span>
             </Checkbox>