[OPIK-512] Reduce table paddings (#785)

comet-ml · Dec 2, 2024 · 3c57df1 · 3c57df1
1 parent fc1b7a3
commit 3c57df1
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 36 deletions.
diff --git 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/...s/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsFeedbackScoreCell.tsx b/...s/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/...components/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsNameCell.tsx b/...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/...mponents/pages/CompareExperimentsPage/ExperimentItemsTab/CompareExperimentsOutputCell.tsx b/...mponents/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/...components/pages/CompareExperimentsPage/ExperimentItemsTab/VerticallySplitCellWrapper.tsx b/...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
@@ -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
@@ -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
@@ -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
@@ -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,25 +30,26 @@ 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,
           )}
         >
           {children}
         </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
@@ -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}