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
73.85k stars 4.55k forks source link

[Bug]: Custom onRowClick function disrupts row selection functionality #3155

Open dhpcc opened 7 months ago

dhpcc commented 7 months ago

Describe the bug

Hey all,

I am using the data table component (similar to the Tasks template). I made each row in the data table clickable by modifying the data-table.tsx file. This is how I did that:

<TableRow
    key={row.id}
    data-state={row.getIsSelected() && 'selected'}
    onClick={() => onRowClick?.(row)}
    className={cn({ 'cursor-pointer': !!onRowClick })}
>
    {row.getVisibleCells().map((cell) => (
        <TableCell key={cell.id}>
            {flexRender(
                cell.column.columnDef.cell,
                cell.getContext()
             )}
        </TableCell>
    ))}
</TableRow>

My onRowClick function simply redirects to user to a different page. This works perfectly. The issue is with the built-in row selection feature (i.e. checkboxes on each row that can be selected). When I click on a checkbox, the onRowClick function is being executed prior to the checkbox selection.

Here is what my columns.tsx file looks like:

export const columns: ColumnDef<Alert>[] = [
  {
    id: 'select',
    header: ({ table }) => (
      <Checkbox
        checked={
          table.getIsAllPageRowsSelected() ||
          (table.getIsSomePageRowsSelected() && 'indeterminate')
        }
        onCheckedChange={(value) => table.toggleAllPageRowsSelected(!!value)}
        aria-label="Select all"
      />
    ),
    cell: ({ row }) => (
      <Checkbox
        checked={row.getIsSelected()}
        onCheckedChange={(value) => row.toggleSelected(!!value)}
        aria-label="Select row"
      />
    ),
    enableSorting: false,
    enableHiding: false,
  },
  ...

I still want the checkbox functionality because users should be able to perform a bulk action by selecting multiple entries within the data table. I have two main questions:

  1. How can I ensure that a checkbox can be selected without triggering my onRowClick function?
  2. How can I implement bulk actions whereby I select multiple rows and perform an action on all of those rows?

If anyone has any insight, I'd be super grateful. Thank you for reading this!

Affected component/components

Data Table

How to reproduce

  1. Add row click functionality by modifying the <TableRow> props within the data-table.tsx file
  2. Add the row selection field by following ShadCN's Data Table tutorial
  3. Try to click on a checkbox of a row

Codesandbox/StackBlitz link

No response

Logs

No response

System Info

Mac OS, Google Chrome

Before submitting

ylyra commented 7 months ago

Try adding to your Checkboxs

onClick={(e) => {
  e.stopPropagation();
}}

Like:

export const columns: ColumnDef<Alert>[] = [
  {
    id: 'select',
    header: ({ table }) => (
      <Checkbox
        checked={
          table.getIsAllPageRowsSelected() ||
          (table.getIsSomePageRowsSelected() && 'indeterminate')
        }
        onCheckedChange={(value) => table.toggleAllPageRowsSelected(!!value)}
        aria-label="Select all"
        onClick={(e) => {
          e.stopPropagation();
        }}
      />
    ),
    cell: ({ row }) => (
      <Checkbox
        checked={row.getIsSelected()}
        onCheckedChange={(value) => row.toggleSelected(!!value)}
        aria-label="Select row"
        onClick={(e) => {
          e.stopPropagation();
        }}
      />
    ),
    enableSorting: false,
    enableHiding: false,
  },
  ...
dhpcc commented 7 months ago

That worked. Thank you @ylyra! Sorry for the follow-up but would you be able to point me in the right direction for figuring out how to achieve bulk action (aka selecting multiple rows and performing an action on all selected resources). Thank you again.

ylyra commented 7 months ago

@dhpcc this is how I do, there is probably a better way. Be careful if you have pagination or your data is always changing, you need to reset the selectedRows since it's index based.