shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
68.51k stars 4.06k forks source link

[Bug]: `Checkbox` inside a `DataTable` component moves once checked #3077

Open lohrm-stabl opened 5 months ago

lohrm-stabl commented 5 months ago

Describe the bug

The Checkbox inside a DataTable component moves once checked and unchecked.

Unchecked image

Checked image

Affected component/components

DataTable, Checkbodx

How to reproduce

  1. Go to https://ui.shadcn.com/docs/components/data-table
  2. Check one of the checkboxes of the columns in the data table example

You will see that the checkbox floats up once its checked.

Link to reproduce

https://ui.shadcn.com/docs/components/data-table

Logs

No response

System Info

https://ui.shadcn.com/docs/components/data-table

Before submitting

anantraghuvanshi commented 5 months ago

Hello @lohrm-stabl I tried recreating this issue, but I think the float is not appearing to me. May you look into this? Unchecked: image Checked: image Could it be a browser/OS specific issue as I am currently using brave and ubuntu to run this locally?

lohrm-stabl commented 5 months ago

Hi @anantraghuvanshi, I encountered this locally, but then verified that it's not an issue with my implementation by visiting https://ui.shadcn.com/docs/components/data-table, where this issues persists as well.

I can see this issue on latest Firefox on Windows 11 and latest Firefox on Arch. With Edge browser on Windows 11 the effect is really subtle, but still there.

anantraghuvanshi commented 5 months ago

That could be the case, @lohrm-stabl. I tested it in chrome in Windows 11 no issues over there as well. So no progress from my side in replicating this issue, sadly!

linus-jansson commented 4 months ago

This seems to only appear for me on firefox but not Edge. Seems to be a browser issue?

pepegc commented 4 months ago

Same bug, Firefox 124.0.2 (64-bit), Sonoma 14.4.1 (23E224)

Bippitee commented 3 months ago

I think maybe it's a combination of the TableCell as well as the Checkbox component:

const TableCell = React.forwardRef(({ className, ...props }, ref) => (
  <td
    ref={ref}
    className={cn("p-2 align-middle [&:has([role=checkbox])]:pr-0 [&>[role=checkbox]]:translate-y-[2px]", className)}
    {...props}
  />
));

specifically this bit: [&>[role=checkbox]]:translate-y-[2px]

I also had to add flex to CheckboxPrimitive.Root and move the h-4 w-4 (or size-4) from the CheckIcon to the Indicator <CheckboxPrimitive.Indicator className={cn("size-4 items-center justify-center text-current")}> Haven't looked at any knock on effect this might have as I've only used it in a Table

DanielBaulig commented 3 months ago

Removing the tailwind classes of "w-4 h-4" from the CheckIcon fixed it for me. The button that renders from CheckboxPrimitive.Root doesn't change in dimensions, but the containing table cell increases by 0.8px in computer height when the CheckIcon gets rendered. Removing the w/h classes and letting the SVG figure out its height by itself fixes that.

To not cause issues in other places I decided to change the CheckIcon w/h to max-w/max-h instead of entirely removing them. That also fixed the table issue for me and should prevent any major breakages in other locations, where there might not be any implicit constraints on the CheckIcon size. Although the CheckboxPrimitive.Root also has explicit w/h set, so not sure if this concern is founded. In either case, this change seems safe while fixing the table behavior.

ZakEDS commented 2 months ago

This for checkbox.tsx seems to do the trick, easy to copy for those arriving to this thread ;)

"use client"

import * as React from "react"
import * as CheckboxPrimitive from "@radix-ui/react-checkbox"
import { Check } from "lucide-react"

import { cn } from "@/lib/utils"

const Checkbox = React.forwardRef<
  React.ElementRef<typeof CheckboxPrimitive.Root>,
  React.ComponentPropsWithoutRef<typeof CheckboxPrimitive.Root>
>(({ className, ...props }, ref) => (
  <CheckboxPrimitive.Root
    ref={ref}
    className={cn(
      "flex peer size-4 justify-center items-center shrink-0 rounded-sm border border-primary ring-offset-background focus-visible:outline-none focus-visible:ring-2   focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground ",
      className
    )}
    {...props}
  >
    <CheckboxPrimitive.Indicator
      className={cn("flex items-center justify-center text-current")}
    >
      <Check className="size-3" />
    </CheckboxPrimitive.Indicator>
  </CheckboxPrimitive.Root>
))
Checkbox.displayName = CheckboxPrimitive.Root.displayName

export { Checkbox }
michaelschufi commented 2 months ago

For those who use the non-variables strategy. @ZakEDS's fix basically just adds the following classes to the checkbox flex items-center justify-center and size-3 for the icon. I recommend size-full for the icon.