Clean up table head styles (#3674)

webview/src/experiments/components/AddStage.tsx

@@ -22,7 +22,7 @@ export const AddStage: React.FC<AddStageProps> = ({ hasValidDvcYaml }) => (
       disabled={!hasValidDvcYaml}
     />
     {!hasValidDvcYaml && (
-      <p className={styles.error}>
+      <p className={styles.errorText}>
         Your dvc.yaml file should contain valid yaml before adding any pipeline
         stages.
       </p>

webview/src/experiments/components/App.test.tsx

@@ -74,20 +74,6 @@ afterEach(() => {
 })
 
 describe('App', () => {
-  describe('Sorting Classes', () => {
-    it('should apply the sortingHeaderCellDesc class to a header cell of a column sorted descending', () => {
-      renderTableWithPlaceholder()
-
-      const headerCell = screen.getByTestId(
-        'header-params:params.yaml:nested1.doubled'
-      )
-
-      expect(
-        headerCell.classList.contains(styles.sortingHeaderCellDesc)
-      ).toBeTruthy()
-    })
-  })
-
   it('should send a message to the extension on the first render', () => {
     renderTable(undefined, true)
     expect(mockPostMessage).toHaveBeenCalledWith({

webview/src/experiments/components/Experiments.tsx

@@ -53,7 +53,7 @@ const getDefaultColumnWithIndicatorsPlaceHolder = () =>
         }
       } = cell as unknown as CellContext<Experiment, CellValue>
       return (
-        <div className={styles.experimentCellContents}>
+        <div className={styles.experimentCellText}>
           <span>{label}</span>
           {displayName && (
             <CellSecondaryName

webview/src/experiments/components/table/Cell.tsx

@@ -55,16 +55,16 @@ export const FirstCell: React.FC<
   const { toggleExperiment } = rowActionsProps
 
   return (
-    <td className={styles.experimentCell}>
+    <td className={cx(styles.experimentsTd, styles.experimentCell)}>
       <div className={styles.innerCell} style={{ width: getSize() }}>
         <CellRowActions status={status} {...rowActionsProps} />
         <RowExpansionButton row={row} />
         {getIsPlaceholder() ? null : (
           <ErrorTooltip error={error}>
             <div
-              className={cx(styles.experimentCellContentsContainer, {
-                [styles.workspaceChange]: changesIfWorkspace,
-                [styles.error]: error
+              className={cx(styles.experimentCellTextWrapper, {
+                [styles.workspaceChangeText]: changesIfWorkspace,
+                [styles.errorText]: error
               })}
               {...clickAndEnterProps(
                 toggleExperiment,
@@ -92,9 +92,9 @@ export const CellWrapper: React.FC<
 > = ({ cell, cellId, changes }) => {
   return (
     <td
-      className={cx({
-        [styles.workspaceChange]: changes?.includes(cell.column.id),
-        [styles.depChange]: cellHasChanges(cell.getValue() as CellValue)
+      className={cx(styles.experimentsTd, {
+        [styles.workspaceChangeText]: changes?.includes(cell.column.id),
+        [styles.depChangeText]: cellHasChanges(cell.getValue() as CellValue)
       })}
       data-testid={cellId}
     >

webview/src/experiments/components/table/CellSecondaryName.tsx

@@ -13,7 +13,8 @@ export const CellSecondaryName: React.FC<{
 }> = ({ displayName, commit, sha }) => {
   const children = (
     <span className={styles.experimentCellSecondaryName}>
-      {commit && <Icon width={14} height={14} icon={GitCommit} />} {displayName}
+      {commit && <Icon width={14} height={14} icon={GitCommit} />}{' '}
+      <span>{displayName}</span>
     </span>
   )
   if (!commit) {

webview/src/experiments/components/table/Row.tsx

@@ -44,7 +44,7 @@ const getRowClassNames = (
 ) => {
   return cx(
     className,
-    styles.tr,
+    styles.experimentsTr,
     styles.bodyRow,
     getExperimentTypeClass(original),
     cond(

webview/src/experiments/components/table/Table.test.tsx

@@ -68,9 +68,6 @@ describe('Table', () => {
     const mockColumnName = 'C'
     const mockColumnPath = 'params:C'
 
-    const findSortableColumn = async () =>
-      await screen.findByTestId(`header-${mockColumnPath}`)
-
     const clickOnSortOption = async (optionLabel: SortOrder) => {
       const column = await screen.findByText(mockColumnName)
       fireEvent.contextMenu(column, {
@@ -113,9 +110,6 @@ describe('Table', () => {
           ]
         })
 
-        const column = await findSortableColumn()
-        expect(column).toHaveClass('sortingHeaderCellAsc')
-
         await clickOnSortOption(SortOrder.DESCENDING)
 
         expect(mockedPostMessage).toHaveBeenCalledWith({
@@ -138,9 +132,6 @@ describe('Table', () => {
           ]
         })
 
-        const column = await findSortableColumn()
-        expect(column).toHaveClass('sortingHeaderCellDesc')
-
         await clickOnSortOption(SortOrder.NONE)
 
         expect(mockedPostMessage).toHaveBeenCalledWith({
@@ -174,17 +165,17 @@ describe('Table', () => {
 
       const workspaceCell = await screen.findByText(EXPERIMENT_WORKSPACE_ID)
 
-      expect(workspaceCell?.className.includes(styles.workspaceChange)).toBe(
-        false
-      )
+      expect(
+        workspaceCell?.className.includes(styles.workspaceChangeText)
+      ).toBe(false)
     })
 
     it("should have the workspaceChange class on the workspace's first cell (text) if there are workspace changes", () => {
       renderExperimentsTable({ changes: ['something_changed'] })
 
       const workspaceCell = screen.getByTestId('id___workspace')
 
-      expect(workspaceCell.className.includes(styles.workspaceChange)).toBe(
+      expect(workspaceCell.className.includes(styles.workspaceChangeText)).toBe(
         true
       )
     })
@@ -194,23 +185,23 @@ describe('Table', () => {
 
       const row = await screen.findByTestId('Created___workspace')
 
-      expect(row?.className.includes(styles.workspaceChange)).toBe(false)
+      expect(row?.className.includes(styles.workspaceChangeText)).toBe(false)
     })
 
     it('should not have the workspaceChange class on a cell if there are changes to other columns but not this one', async () => {
       renderExperimentsTable({ changes: ['a_change'] })
 
       const row = await screen.findByTestId('Created___workspace')
 
-      expect(row?.className.includes(styles.workspaceChange)).toBe(false)
+      expect(row?.className.includes(styles.workspaceChangeText)).toBe(false)
     })
 
     it('should have the workspaceChange class on a cell if there are changes matching the column id', async () => {
       renderExperimentsTable({ changes: ['Created'] })
 
       const row = await screen.findByTestId('Created___workspace')
 
-      expect(row?.className.includes(styles.workspaceChange)).toBe(true)
+      expect(row?.className.includes(styles.workspaceChangeText)).toBe(true)
     })
   })
 
@@ -391,13 +382,13 @@ describe('Table', () => {
       dragEnter(startingNode, targetNode.id, DragEnterDirection.AUTO)
 
       expect(
-        header?.classList.contains(styles.headerCellDropTarget)
+        header?.classList.contains(styles.dropTargetHeaderCell)
       ).toBeTruthy()
 
       dragLeave(targetNode)
 
       expect(
-        header?.classList.contains(styles.headerCellDropTarget)
+        header?.classList.contains(styles.dropTargetHeaderCell)
       ).toBeFalsy()
     })
   })

webview/src/experiments/components/table/Table.tsx

@@ -77,7 +77,10 @@ export const Table: React.FC<TableProps> = ({
     >
       {/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */}
       <table
-        className={cx(expColumnNeedsShadow && styles.withExpColumnShadow)}
+        className={cx(
+          styles.experimentsTable,
+          expColumnNeedsShadow && styles.withExpColumnShadow
+        )}
         ref={tableRef}
         onKeyUp={e => {
           if (e.key === 'Escape') {

webview/src/experiments/components/table/TableBody.tsx

@@ -78,19 +78,19 @@ export const TableBody: React.FC<
     <>
       {row.index === 2 && row.depth === 0 && (
         <tbody>
-          <tr className={cx(styles.tr, styles.previousCommitsRow)}>
-            <td className={styles.th}>
+          <tr className={cx(styles.experimentsTr, styles.previousCommitsRow)}>
+            <td className={styles.experimentsTd}>
               {isBranchesView ? 'Other Branches' : 'Previous Commits'}
             </td>
             <td
-              className={styles.th}
+              className={styles.experimentsTd}
               colSpan={row.getAllCells().length - 1}
             ></td>
           </tr>
         </tbody>
       )}
       <tbody
-        className={cx(styles.rowGroup, styles.tbody, styles.normalRowGroup, {
+        className={cx(styles.rowGroup, {
           [styles.experimentGroup]: row.depth > 0,
           [styles.expandedGroup]: row.getIsExpanded() && row.subRows.length > 0
         })}

webview/src/experiments/components/table/content/ErrorCell.tsx

@@ -10,7 +10,7 @@ interface ErrorCellProps {
 
 export const ErrorCell: React.FC<ErrorCellProps> = ({ error }) => (
   <ErrorTooltip error={error}>
-    <div className={cx(styles.innerCell, styles.error)}>
+    <div className={cx(styles.innerCell, styles.errorText)}>
       <CellContents>!</CellContents>
     </div>
   </ErrorTooltip>

webview/src/experiments/components/table/content/Header.tsx

@@ -9,7 +9,7 @@ interface HeaderProps {
 export const Header: React.FC<HeaderProps> = ({ name }) => {
   return (
     <OverflowHoverTooltip content={name}>
-      <div className={styles.headerCellWrapper}>
+      <div className={styles.headerCellText}>
         <span>{name}</span>
       </div>
     </OverflowHoverTooltip>

webview/src/experiments/components/table/header/MergeHeaderGroups.tsx

@@ -1,4 +1,5 @@
 import React from 'react'
+import cx from 'classnames'
 import { Experiment } from 'dvc/src/experiments/webview/contract'
 import { HeaderGroup, Header } from '@tanstack/react-table'
 import { TableHeader } from './TableHeader'
@@ -27,7 +28,7 @@ export const MergedHeaderGroups: React.FC<{
   onlyOneLine
 }) => {
   return (
-    <tr className={styles.headRow}>
+    <tr className={cx(styles.experimentsTr, styles.headRow)}>
       {headerGroup.headers.map((header: Header<Experiment, unknown>) => (
         <TableHeader
           setExpColumnNeedsShadow={setExpColumnNeedsShadow}

webview/src/experiments/components/table/header/TableHead.tsx

@@ -13,6 +13,8 @@ import {
   isExperimentColumn
 } from '../../../util/columns'
 import { DragFunction } from '../../../../shared/components/dragDrop/Draggable'
+import styles from '../styles.module.scss'
+
 interface TableHeadProps {
   instance: Table<Experiment>
   root: HTMLElement | null
@@ -117,7 +119,7 @@ export const TableHead = ({
   }
 
   return (
-    <thead ref={wrapper}>
+    <thead className={styles.experimentsThead} ref={wrapper}>
       {headerGroups.map(headerGroup => (
         <MergedHeaderGroups
           key={headerGroup.id}

webview/src/experiments/components/table/header/TableHeaderCell.tsx

@@ -5,11 +5,7 @@ import { Header } from '@tanstack/react-table'
 import cx from 'classnames'
 import { useInView } from 'react-intersection-observer'
 import { TableHeaderCellContents } from './TableHeaderCellContents'
-import {
-  ContextMenuContent,
-  getMenuOptions,
-  SortOrder
-} from './ContextMenuContent'
+import { ContextMenuContent, getMenuOptions } from './ContextMenuContent'
 import styles from '../styles.module.scss'
 import { isExperimentColumn, isFirstLevelHeader } from '../../../util/columns'
 import { ExperimentsState } from '../../../store'
@@ -39,28 +35,22 @@ const calcResizerHeight = (header: Header<Experiment, unknown>) => {
 const getHeaderPropsArgs = (
   header: Header<Experiment, unknown>,
   headerDropTargetId: string,
-  sortEnabled: boolean,
-  sortOrder: SortOrder,
   onlyOneLine?: boolean
 ) => {
   const columnWithGroup = header.column.columnDef as ColumnWithGroup
   return {
     className: cx(
+      styles.experimentsTh,
       header.isPlaceholder ? styles.placeholderHeaderCell : styles.headerCell,
       {
         [styles.paramHeaderCell]: columnWithGroup.group === ColumnType.PARAMS,
         [styles.metricHeaderCell]: columnWithGroup.group === ColumnType.METRICS,
         [styles.depHeaderCell]: columnWithGroup.group === ColumnType.DEPS,
-        [styles.createdHeaderCell]: header.id === 'Created',
-        [styles.firstLevelHeader]: isFirstLevelHeader(header.column.id),
-        [styles.leafHeader]: header.subHeaders === undefined,
-        [styles.menuEnabled]: sortEnabled,
-        [styles.sortingHeaderCellAsc]: sortOrder === SortOrder.ASCENDING,
-        [styles.sortingHeaderCellDesc]:
-          sortOrder === SortOrder.DESCENDING && !header.isPlaceholder,
-        [styles.oneRowHeaderCell]: onlyOneLine
-      },
-      headerDropTargetId === header.id && styles.headerCellDropTarget
+        [styles.firstLevelHeaderCell]: isFirstLevelHeader(header.column.id),
+        [styles.leafHeaderCell]: header.subHeaders === undefined,
+        [styles.oneRowHeaderCell]: onlyOneLine,
+        [styles.dropTargetHeaderCell]: headerDropTargetId === header.id
+      }
     ),
     style: {
       position: undefined
@@ -161,13 +151,7 @@ export const TableHeaderCell: React.FC<{
       trigger="contextmenu"
     >
       <th
-        {...getHeaderPropsArgs(
-          header,
-          headerDropTargetId,
-          isSortable,
-          sortOrder,
-          onlyOneLine
-        )}
+        {...getHeaderPropsArgs(header, headerDropTargetId, onlyOneLine)}
         data-testid={`header-${id}${previousPlaceholder}`}
         key={id}
         tabIndex={0}

webview/src/experiments/components/table/header/TableHeaderCellContents.tsx

@@ -1,6 +1,6 @@
 import React, { useEffect } from 'react'
 import cx from 'classnames'
-import { ColumnType, Experiment } from 'dvc/src/experiments/webview/contract'
+import { Experiment } from 'dvc/src/experiments/webview/contract'
 import { flexRender, Header } from '@tanstack/react-table'
 import { SortOrder } from './ContextMenuContent'
 import { ColumnResizer, ResizerHeight } from './ColumnResizer'
@@ -104,7 +104,7 @@ export const TableHeaderCellContents: React.FC<{
   setMenuSuppressed,
   resizerHeight
 }) => {
-  const isTimestamp = header.headerGroup.id === ColumnType.TIMESTAMP
+  const isTimestamp = header.id === 'Created'
   const columnIsResizing = header.column.getIsResizing()
 
   useEffect(() => {