fix: Wrap TableCell with Link for native browser routing (#2532)

Tables were previously using an onClick handler which replicated some
Link behavior, but not natively through the browser.

Fixes #2525.

site/src/components/BuildsTable/BuildsTable.tsx

@@ -12,6 +12,7 @@ import { useNavigate, useParams } from "react-router-dom"
 import * as TypesGen from "../../api/typesGenerated"
 import { displayWorkspaceBuildDuration, getDisplayWorkspaceBuildStatus } from "../../util/workspace"
 import { EmptyState } from "../EmptyState/EmptyState"
+import { TableCellLink } from "../TableCellLink/TableCellLink"
 import { TableLoader } from "../TableLoader/TableLoader"
 
 export const Language = {
@@ -51,10 +52,7 @@ export const BuildsTable: FC<BuildsTableProps> = ({ builds, className }) => {
         {builds &&
           builds.map((build) => {
             const status = getDisplayWorkspaceBuildStatus(theme, build)
-
-            const navigateToBuildPage = () => {
-              navigate(`/@${username}/${workspaceName}/builds/${build.build_number}`)
-            }
+            const buildPageLink = `/@${username}/${workspaceName}/builds/${build.build_number}`
 
             return (
               <TableRow
@@ -62,31 +60,30 @@ export const BuildsTable: FC<BuildsTableProps> = ({ builds, className }) => {
                 key={build.id}
                 data-testid={`build-${build.id}`}
                 tabIndex={0}
-                onClick={navigateToBuildPage}
                 onKeyDown={(event) => {
                   if (event.key === "Enter") {
-                    navigateToBuildPage()
+                    navigate(buildPageLink)
                   }
                 }}
                 className={styles.clickableTableRow}
               >
-                <TableCell>{build.transition}</TableCell>
-                <TableCell>
+                <TableCellLink to={buildPageLink}>{build.transition}</TableCellLink>
+                <TableCellLink to={buildPageLink}>
                   <span style={{ color: theme.palette.text.secondary }}>{displayWorkspaceBuildDuration(build)}</span>
-                </TableCell>
-                <TableCell>
+                </TableCellLink>
+                <TableCellLink to={buildPageLink}>
                   <span style={{ color: theme.palette.text.secondary }}>
                     {new Date(build.created_at).toLocaleString()}
                   </span>
-                </TableCell>
-                <TableCell>
+                </TableCellLink>
+                <TableCellLink to={buildPageLink}>
                   <span style={{ color: status.color }}>{status.status}</span>
-                </TableCell>
-                <TableCell>
+                </TableCellLink>
+                <TableCellLink to={buildPageLink}>
                   <div className={styles.arrowCell}>
                     <KeyboardArrowRight className={styles.arrowRight} />
                   </div>
-                </TableCell>
+                </TableCellLink>
               </TableRow>
             )
           })}
@@ -107,8 +104,6 @@ export const BuildsTable: FC<BuildsTableProps> = ({ builds, className }) => {
 
 const useStyles = makeStyles((theme) => ({
   clickableTableRow: {
-    cursor: "pointer",
-
     "&:hover td": {
       backgroundColor: fade(theme.palette.primary.light, 0.1),
     },

site/src/components/TableCellLink/TableCellLink.tsx

@@ -0,0 +1,47 @@
+import Link from "@material-ui/core/Link"
+import { makeStyles } from "@material-ui/core/styles"
+import TableCell, { TableCellProps } from "@material-ui/core/TableCell"
+import { Link as RouterLink } from "react-router-dom"
+import { combineClasses } from "../../util/combineClasses"
+
+// TableCellLink wraps a TableCell filling the entirety with a Link.
+// This allows table rows to be clickable with browser-behavior like ctrl+click.
+export const TableCellLink: React.FC<
+  TableCellProps & {
+    to: string
+  }
+> = (props) => {
+  const styles = useStyles()
+
+  return (
+    <TableCell className={styles.cell} {...props}>
+      <Link
+        component={RouterLink}
+        to={props.to}
+        className={combineClasses([styles.link, "MuiTableCell-root", "MuiTableCell-body"])}
+      >
+        {props.children}
+      </Link>
+    </TableCell>
+  )
+}
+
+const useStyles = makeStyles((theme) => ({
+  cell: {
+    // This must override all padding for all rules on a TableCell.
+    // Otherwise, the link will not cover the entire region.
+    // It's unfortuante to use `!important`, but this seems to be
+    // a reasonable use-case.
+    padding: "0 !important",
+  },
+  link: {
+    display: "block",
+    width: "100%",
+    border: "none",
+    background: "none",
+    paddingTop: theme.spacing(2),
+    paddingBottom: theme.spacing(2),
+    // This is required to hide all underlines for child elements!
+    textDecoration: "none !important",
+  },
+}))

site/src/pages/TemplatesPage/TemplatesPageView.tsx

@@ -17,6 +17,7 @@ import { EmptyState } from "../../components/EmptyState/EmptyState"
 import { Margins } from "../../components/Margins/Margins"
 import { PageHeader, PageHeaderSubtitle, PageHeaderTitle } from "../../components/PageHeader/PageHeader"
 import { Stack } from "../../components/Stack/Stack"
+import { TableCellLink } from "../../components/TableCellLink/TableCellLink"
 import { TableLoader } from "../../components/TableLoader/TableLoader"
 import {
   HelpTooltip,
@@ -115,36 +116,37 @@ export const TemplatesPageView: FC<TemplatesPageViewProps> = (props) => {
             </TableRow>
           )}
           {props.templates?.map((template) => {
-            const navigateToTemplatePage = () => {
-              navigate(`/templates/${template.name}`)
-            }
+            const templatePageLink = `/templates/${template.name}`
             return (
               <TableRow
                 key={template.id}
                 hover
                 data-testid={`template-${template.id}`}
                 tabIndex={0}
-                onClick={navigateToTemplatePage}
                 onKeyDown={(event) => {
                   if (event.key === "Enter") {
-                    navigateToTemplatePage()
+                    navigate(templatePageLink)
                   }
                 }}
                 className={styles.clickableTableRow}
               >
-                <TableCell>
+                <TableCellLink to={templatePageLink}>
                   <AvatarData title={template.name} subtitle={template.description} />
-                </TableCell>
+                </TableCellLink>
 
-                <TableCell>{Language.developerCount(template.workspace_owner_count)}</TableCell>
+                <TableCellLink to={templatePageLink}>
+                  {Language.developerCount(template.workspace_owner_count)}
+                </TableCellLink>
 
-                <TableCell data-chromatic="ignore">{dayjs().to(dayjs(template.updated_at))}</TableCell>
-                <TableCell>{template.created_by_name}</TableCell>
-                <TableCell>
+                <TableCellLink data-chromatic="ignore" to={templatePageLink}>
+                  {dayjs().to(dayjs(template.updated_at))}
+                </TableCellLink>
+                <TableCellLink to={templatePageLink}>{template.created_by_name}</TableCellLink>
+                <TableCellLink to={templatePageLink}>
                   <div className={styles.arrowCell}>
                     <KeyboardArrowRight className={styles.arrowRight} />
                   </div>
-                </TableCell>
+                </TableCellLink>
               </TableRow>
             )
           })}
@@ -159,8 +161,6 @@ const useStyles = makeStyles((theme) => ({
     maxWidth: theme.spacing(62),
   },
   clickableTableRow: {
-    cursor: "pointer",
-
     "&:hover td": {
       backgroundColor: fade(theme.palette.primary.light, 0.1),
     },

site/src/pages/WorkspacesPage/WorkspacesPageView.tsx

@@ -28,6 +28,7 @@ import { EmptyState } from "../../components/EmptyState/EmptyState"
 import { Margins } from "../../components/Margins/Margins"
 import { PageHeader, PageHeaderSubtitle, PageHeaderTitle } from "../../components/PageHeader/PageHeader"
 import { Stack } from "../../components/Stack/Stack"
+import { TableCellLink } from "../../components/TableCellLink/TableCellLink"
 import { TableLoader } from "../../components/TableLoader/TableLoader"
 import {
   HelpTooltip,
@@ -104,27 +105,25 @@ const WorkspaceRow: React.FC<{ workspaceRef: WorkspaceItemMachineRef }> = ({ wor
   const [workspaceState, send] = useActor(workspaceRef)
   const { data: workspace } = workspaceState.context
   const status = getDisplayStatus(theme, workspace.latest_build)
-  const navigateToWorkspacePage = () => {
-    navigate(`/@${workspace.owner_name}/${workspace.name}`)
-  }
+  const workspacePageLink = `/@${workspace.owner_name}/${workspace.name}`
+
   return (
     <TableRow
       hover
       data-testid={`workspace-${workspace.id}`}
       tabIndex={0}
-      onClick={navigateToWorkspacePage}
       onKeyDown={(event) => {
         if (event.key === "Enter") {
-          navigateToWorkspacePage()
+          navigate(workspacePageLink)
         }
       }}
       className={styles.clickableTableRow}
     >
-      <TableCell>
+      <TableCellLink to={workspacePageLink}>
         <AvatarData title={workspace.name} subtitle={workspace.owner_name} />
-      </TableCell>
-      <TableCell>{workspace.template_name}</TableCell>
-      <TableCell>
+      </TableCellLink>
+      <TableCellLink to={workspacePageLink}>{workspace.template_name}</TableCellLink>
+      <TableCellLink to={workspacePageLink}>
         {workspace.outdated ? (
           <span className={styles.outdatedLabel}>
             {Language.outdatedLabel}
@@ -137,20 +136,20 @@ const WorkspaceRow: React.FC<{ workspaceRef: WorkspaceItemMachineRef }> = ({ wor
         ) : (
           <span style={{ color: theme.palette.text.secondary }}>{Language.upToDateLabel}</span>
         )}
-      </TableCell>
-      <TableCell>
+      </TableCellLink>
+      <TableCellLink to={workspacePageLink}>
         <span data-chromatic="ignore" style={{ color: theme.palette.text.secondary }}>
           {dayjs().to(dayjs(workspace.latest_build.created_at))}
         </span>
-      </TableCell>
-      <TableCell>
+      </TableCellLink>
+      <TableCellLink to={workspacePageLink}>
         <span style={{ color: status.color }}>{status.status}</span>
-      </TableCell>
-      <TableCell>
+      </TableCellLink>
+      <TableCellLink to={workspacePageLink}>
         <div className={styles.arrowCell}>
           <KeyboardArrowRight className={styles.arrowRight} />
         </div>
-      </TableCell>
+      </TableCellLink>
     </TableRow>
   )
 }
@@ -349,8 +348,6 @@ const useStyles = makeStyles((theme) => ({
     },
   },
   clickableTableRow: {
-    cursor: "pointer",
-
     "&:hover td": {
       backgroundColor: fade(theme.palette.primary.light, 0.1),
     },

site/src/theme/overrides.ts

@@ -61,10 +61,12 @@ export const getOverrides = (palette: PaletteOptions) => {
         background: palette.background?.paper,
         borderBottom: `1px solid ${palette.divider}`,
         padding: 8,
-        "&:first-child": {
+        // This targets the first+last td elements, and also the first+last elements
+        // of a TableCellLink.
+        "&:not(:only-child):first-child, &:not(:only-child):first-child > a": {
           paddingLeft: 32,
         },
-        "&:last-child": {
+        "&:not(:only-child):last-child, &:not(:only-child):last-child > a": {
           paddingRight: 32,
         },
       },