From 3c57df1d8454e419e2096263e204e7ab78b98075 Mon Sep 17 00:00:00 2001
From: "andrii.dudar" <andriid@comet.com>
Date: Mon, 2 Dec 2024 15:49:50 +0100
Subject: [PATCH] [OPIK-512] Reduce table paddings (#785)

---
 .../CompareExperimentsDetails.tsx             | 10 ++++--
 .../CompareExperimentsFeedbackScoreCell.tsx   |  2 +-
 .../CompareExperimentsNameCell.tsx            |  2 +-
 .../CompareExperimentsOutputCell.tsx          |  2 +-
 .../VerticallySplitCellWrapper.tsx            |  2 ++
 .../pages/ExperimentsShared/table.tsx         | 30 +++++++++---------
 .../src/components/shared/DataTable/utils.tsx | 31 ++++++++++++-------
 .../shared/DataTableCells/CellWrapper.tsx     |  3 ++
 .../shared/DataTableHeaders/HeaderWrapper.tsx | 12 ++++---
 .../opik-frontend/src/components/ui/table.tsx |  4 +--
 10 files changed, 62 insertions(+), 36 deletions(-)

diff --git a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/CompareExperimentsDetails.tsx b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/CompareExperimentsDetails.tsx
index 415763373f..7d4f96a62f 100644
--- a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/CompareExperimentsDetails.tsx
+++ b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/CompareExperimentsDetails.tsx
@@ -152,7 +152,10 @@ const CompareExperimentsDetails: React.FunctionComponent<
               {experiments.map((e) => (
                 <TableRow key={e.id}>
                   <TableCell>
-                    <div className="flex h-14 min-w-20 items-center truncate p-2">
+                    <div
+                      className="flex h-14 min-w-20 items-center truncate p-2"
+                      data-cell-wrapper="true"
+                    >
                       {e.name}
                     </div>
                   </TableCell>
@@ -161,7 +164,10 @@ const CompareExperimentsDetails: React.FunctionComponent<
 
                     return (
                       <TableCell key={id}>
-                        <div className="flex h-14 min-w-20 items-center truncate p-2">
+                        <div
+                          className="flex h-14 min-w-20 items-center truncate p-2"
+                          data-cell-wrapper="true"
+                        >
                           {isUndefined(value) ? (
                             "–"
                           ) : (
diff --git a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsFeedbackScoreCell.tsx b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsFeedbackScoreCell.tsx
index ceead9b4c4..9b26b26699 100644
--- a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsFeedbackScoreCell.tsx
+++ b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsFeedbackScoreCell.tsx
@@ -43,7 +43,7 @@ const CompareExperimentsFeedbackScoreCell: React.FunctionComponent<
       metadata={context.column.columnDef.meta}
       tableMetadata={context.table.options.meta}
       rowId={context.row.id}
-    ></VerticallySplitCellWrapper>
+    />
   );
 };
 
diff --git a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsNameCell.tsx b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsNameCell.tsx
index ad62ec136b..3e08397ecb 100644
--- a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsNameCell.tsx
+++ b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsNameCell.tsx
@@ -43,7 +43,7 @@ const CompareExperimentsOutputCell: React.FunctionComponent<
       metadata={context.column.columnDef.meta}
       tableMetadata={context.table.options.meta}
       rowId={context.row.id}
-    ></VerticallySplitCellWrapper>
+    />
   );
 };
 
diff --git a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsOutputCell.tsx b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsOutputCell.tsx
index 6690b40e11..c915ca8db5 100644
--- a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsOutputCell.tsx
+++ b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsOutputCell.tsx
@@ -66,7 +66,7 @@ const CompareExperimentsOutputCell: React.FunctionComponent<
       metadata={context.column.columnDef.meta}
       tableMetadata={context.table.options.meta}
       rowId={context.row.id}
-    ></VerticallySplitCellWrapper>
+    />
   );
 };
 
diff --git a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/VerticallySplitCellWrapper.tsx b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/VerticallySplitCellWrapper.tsx
index b640d1d894..94b18306df 100644
--- a/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/VerticallySplitCellWrapper.tsx
+++ b/apps/opik-frontend/src/components/pages/CompareExperimentsPage/ExperimentItemsTab/VerticallySplitCellWrapper.tsx
@@ -78,6 +78,7 @@ const VerticallySplitCellWrapper: React.FC<VerticallySplitCellWrapperProps> = ({
         data-virtual-row-id={virtualRowId}
         onMouseEnter={() => highlightSubRow(virtualRowId, true)}
         onMouseLeave={() => highlightSubRow(virtualRowId, false)}
+        data-cell-wrapper="true"
       >
         {content}
       </div>
@@ -89,6 +90,7 @@ const VerticallySplitCellWrapper: React.FC<VerticallySplitCellWrapperProps> = ({
       metadata={metadata}
       tableMetadata={tableMetadata}
       className="flex-col items-stretch justify-center overflow-hidden p-0"
+      dataCellWrapper={false}
     >
       {items.map(renderItem)}
     </CellWrapper>
diff --git a/apps/opik-frontend/src/components/pages/ExperimentsShared/table.tsx b/apps/opik-frontend/src/components/pages/ExperimentsShared/table.tsx
index ae52b38f19..47b1a091eb 100644
--- a/apps/opik-frontend/src/components/pages/ExperimentsShared/table.tsx
+++ b/apps/opik-frontend/src/components/pages/ExperimentsShared/table.tsx
@@ -11,8 +11,8 @@ import { COLUMN_NAME_ID, ColumnData, OnChangeFn } from "@/types/shared";
 import { Button } from "@/components/ui/button";
 import { ChevronDown, ChevronUp, Text } from "lucide-react";
 import { mapColumnDataFields } from "@/lib/table";
-import { cn } from "@/lib/utils";
 import CellWrapper from "@/components/shared/DataTableCells/CellWrapper";
+import HeaderWrapper from "@/components/shared/DataTableHeaders/HeaderWrapper";
 import {
   checkIsMoreRowId,
   DEFAULT_EXPERIMENTS_PER_GROUP,
@@ -45,22 +45,24 @@ export const generateExperimentNameColumDef = <TData,>({
 }) => {
   return {
     accessorKey: COLUMN_NAME_ID,
-    header: ({ table }) => (
-      <div
-        className={cn("flex size-full items-center gap-2 pr-2")}
-        onClick={(e) => e.stopPropagation()}
+    header: (context) => (
+      <HeaderWrapper
+        metadata={context.column.columnDef.meta}
+        tableMetadata={context.table.options.meta}
       >
         <Checkbox
           checked={
-            table.getIsAllPageRowsSelected() ||
-            (table.getIsSomePageRowsSelected() && "indeterminate")
+            context.table.getIsAllPageRowsSelected() ||
+            (context.table.getIsSomePageRowsSelected() && "indeterminate")
+          }
+          onCheckedChange={(value) =>
+            context.table.toggleAllPageRowsSelected(!!value)
           }
-          onCheckedChange={(value) => table.toggleAllPageRowsSelected(!!value)}
           aria-label="Select all"
         />
-        <Text className="ml-3 size-4 shrink-0" />
+        <Text className="ml-3 size-3.5 shrink-0 text-slate-300" />
         <span className="truncate">Name</span>
-      </div>
+      </HeaderWrapper>
     ),
     cell: (context) => {
       const data = context.row.original as GroupedExperiment;
@@ -109,7 +111,7 @@ export const generateGroupedCellDef = <TData, TValue>(
       const { row, cell } = context;
       return (
         <div className="flex size-full h-11 items-center">
-          <div className="flex shrink-0 items-center">
+          <div className="flex shrink-0 items-center pl-5">
             <Checkbox
               onClick={(event) => event.stopPropagation()}
               checked={
@@ -123,16 +125,16 @@ export const generateGroupedCellDef = <TData, TValue>(
             <Button
               variant="minimal"
               size="sm"
-              className="-mr-3 ml-3"
+              className="-mr-3 ml-1.5"
               onClick={(event) => {
                 row.toggleExpanded();
                 event.stopPropagation();
               }}
             >
               {row.getIsExpanded() ? (
-                <ChevronUp className="mr-1.5 size-4" />
+                <ChevronUp className="mr-1 size-4" />
               ) : (
-                <ChevronDown className="mr-1.5 size-4" />
+                <ChevronDown className="mr-1 size-4" />
               )}
               Dataset:
             </Button>
diff --git a/apps/opik-frontend/src/components/shared/DataTable/utils.tsx b/apps/opik-frontend/src/components/shared/DataTable/utils.tsx
index 6fc635ec79..61c0a012ee 100644
--- a/apps/opik-frontend/src/components/shared/DataTable/utils.tsx
+++ b/apps/opik-frontend/src/components/shared/DataTable/utils.tsx
@@ -17,6 +17,7 @@ import {
   ROW_HEIGHT,
 } from "@/types/shared";
 import CellWrapper from "@/components/shared/DataTableCells/CellWrapper";
+import HeaderWrapper from "@/components/shared/DataTableHeaders/HeaderWrapper";
 
 export const calculateHeightStyle = (rowHeight: ROW_HEIGHT) => {
   return ROW_HEIGHT_MAP[rowHeight];
@@ -59,22 +60,30 @@ export const getCommonPinningClasses = <TData,>(
 export const generateSelectColumDef = <TData,>() => {
   return {
     accessorKey: COLUMN_SELECT_ID,
-    header: ({ table }) => (
-      <Checkbox
-        onClick={(event) => event.stopPropagation()}
-        checked={
-          table.getIsAllPageRowsSelected() ||
-          (table.getIsSomePageRowsSelected() && "indeterminate")
-        }
-        onCheckedChange={(value) => table.toggleAllPageRowsSelected(!!value)}
-        aria-label="Select all"
-      />
+    header: (context) => (
+      <HeaderWrapper
+        metadata={context.column.columnDef.meta}
+        tableMetadata={context.table.options.meta}
+        supportStatistic={false}
+      >
+        <Checkbox
+          onClick={(event) => event.stopPropagation()}
+          checked={
+            context.table.getIsAllPageRowsSelected() ||
+            (context.table.getIsSomePageRowsSelected() && "indeterminate")
+          }
+          onCheckedChange={(value) =>
+            context.table.toggleAllPageRowsSelected(!!value)
+          }
+          aria-label="Select all"
+        />
+      </HeaderWrapper>
     ),
     cell: (context) => (
       <CellWrapper
         metadata={context.column.columnDef.meta}
         tableMetadata={context.table.options.meta}
-        className="px-0 py-3.5"
+        className="py-3.5"
       >
         <Checkbox
           onClick={(event) => event.stopPropagation()}
diff --git a/apps/opik-frontend/src/components/shared/DataTableCells/CellWrapper.tsx b/apps/opik-frontend/src/components/shared/DataTableCells/CellWrapper.tsx
index 48c1df9f3c..69407bc536 100644
--- a/apps/opik-frontend/src/components/shared/DataTableCells/CellWrapper.tsx
+++ b/apps/opik-frontend/src/components/shared/DataTableCells/CellWrapper.tsx
@@ -13,6 +13,7 @@ type CellWrapperProps = {
   metadata?: ColumnMeta<unknown, unknown>;
   tableMetadata?: TableMeta<unknown>;
   className?: string;
+  dataCellWrapper?: boolean;
 };
 
 const ALIGN_END_TYPES = [COLUMN_TYPE.number, COLUMN_TYPE.cost];
@@ -22,6 +23,7 @@ const CellWrapper: React.FunctionComponent<CellWrapperProps> = ({
   metadata,
   tableMetadata,
   className,
+  dataCellWrapper = true,
 }) => {
   const { type } = metadata || {};
   const { rowHeight, rowHeightStyle } = tableMetadata || {};
@@ -46,6 +48,7 @@ const CellWrapper: React.FunctionComponent<CellWrapperProps> = ({
         className,
       )}
       style={rowHeightStyle}
+      data-cell-wrapper={dataCellWrapper}
     >
       {children}
     </div>
diff --git a/apps/opik-frontend/src/components/shared/DataTableHeaders/HeaderWrapper.tsx b/apps/opik-frontend/src/components/shared/DataTableHeaders/HeaderWrapper.tsx
index 5425335a18..613ab6777e 100644
--- a/apps/opik-frontend/src/components/shared/DataTableHeaders/HeaderWrapper.tsx
+++ b/apps/opik-frontend/src/components/shared/DataTableHeaders/HeaderWrapper.tsx
@@ -11,6 +11,7 @@ type CellWrapperProps = {
   tableMetadata?: TableMeta<unknown>;
   className?: string;
   onClick?: (e: React.MouseEvent<HTMLDivElement>) => void;
+  supportStatistic?: boolean;
 };
 
 const HeaderWrapper: React.FunctionComponent<CellWrapperProps> = ({
@@ -19,6 +20,7 @@ const HeaderWrapper: React.FunctionComponent<CellWrapperProps> = ({
   tableMetadata,
   className,
   onClick,
+  supportStatistic = true,
 }) => {
   const { type, statisticKey } = metadata || {};
   const { columnsStatistic } = tableMetadata || {};
@@ -28,17 +30,18 @@ const HeaderWrapper: React.FunctionComponent<CellWrapperProps> = ({
 
   const heightClass = columnsStatistic ? "h-14" : "h-11";
 
-  if (columnsStatistic) {
+  if (supportStatistic && columnsStatistic) {
     const data = find(columnsStatistic, (s) => s.name === statisticKey);
 
     return (
       <div
-        className={cn("flex flex-col py-2", heightClass, className)}
+        className={cn("flex flex-col py-2 px-3", heightClass, className)}
         onClick={onClick}
+        data-header-wrapper="true"
       >
         <div
           className={cn(
-            "flex size-full items-center gap-1 px-3",
+            "flex size-full items-center gap-1",
             horizontalAlignClass,
           )}
         >
@@ -46,7 +49,7 @@ const HeaderWrapper: React.FunctionComponent<CellWrapperProps> = ({
         </div>
         <div
           className={cn(
-            "flex size-full items-center gap-1 px-3",
+            "flex size-full items-center gap-1",
             horizontalAlignClass,
           )}
         >
@@ -65,6 +68,7 @@ const HeaderWrapper: React.FunctionComponent<CellWrapperProps> = ({
         className,
       )}
       onClick={onClick}
+      data-header-wrapper="true"
     >
       {children}
     </div>
diff --git a/apps/opik-frontend/src/components/ui/table.tsx b/apps/opik-frontend/src/components/ui/table.tsx
index f39e711c82..c05a345140 100644
--- a/apps/opik-frontend/src/components/ui/table.tsx
+++ b/apps/opik-frontend/src/components/ui/table.tsx
@@ -73,7 +73,7 @@ const TableHead = React.forwardRef<
   <th
     ref={ref}
     className={cn(
-      "relative p-0 text-left align-middle font-medium text-light-slate [&:has([role=checkbox])]:pr-0 first:pl-5 last:pr-5",
+      "relative p-0 text-left align-middle font-medium text-light-slate [&:has([role=checkbox])]:pr-0 [&:first-child_[data-header-wrapper=true]]:pl-5 [&:last-child_[data-header-wrapper=true]]:pr-5",
       className,
     )}
     {...props}
@@ -88,7 +88,7 @@ const TableCell = React.forwardRef<
   <td
     ref={ref}
     className={cn(
-      "relative p-0 align-middle [&:has([role=checkbox])]:pr-0 first:pl-5 last:pr-5 h-px group-hover/row:bg-destructive-foreground",
+      "relative p-0 align-middle [&:has([role=checkbox])]:pr-0 [&:first-child_[data-cell-wrapper=true]]:pl-5 [&:last-child_[data-cell-wrapper=true]]:pr-5 h-px group-hover/row:bg-destructive-foreground",
       className,
     )}
     {...props}