Skip to content

Commit 9c26023

Browse files
kylecarbskylecarbs
committed
fix: Wrap TableCell with Link for native browser routing
Tables were previously using an onClick handler which replicated some Link behavior, but not natively through the browser. Fixes #2525.
1 parent 7a4fd12 commit 9c26023

File tree

5 files changed

+90
-51
lines changed

5 files changed

+90
-51
lines changed

‎site/src/components/BuildsTable/BuildsTable.tsx

+12-17
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { useNavigate, useParams } from "react-router-dom"
1212
import * as TypesGen from "../../api/typesGenerated"
1313
import { displayWorkspaceBuildDuration, getDisplayWorkspaceBuildStatus } from "../../util/workspace"
1414
import { EmptyState } from "../EmptyState/EmptyState"
15+
import { TableCellLink } from "../TableCellLink/TableCellLink"
1516
import { TableLoader } from "../TableLoader/TableLoader"
1617

1718
export const Language = {
@@ -51,42 +52,38 @@ export const BuildsTable: FC<BuildsTableProps> = ({ builds, className }) => {
5152
{builds &&
5253
builds.map((build) => {
5354
const status = getDisplayWorkspaceBuildStatus(theme, build)
54-
55-
const navigateToBuildPage = () => {
56-
navigate(`/@${username}/${workspaceName}/builds/${build.build_number}`)
57-
}
55+
const buildPageLink = `/@${username}/${workspaceName}/builds/${build.build_number}`
5856

5957
return (
6058
<TableRow
6159
hover
6260
key={build.id}
6361
data-testid={`build-${build.id}`}
6462
tabIndex={0}
65-
onClick={navigateToBuildPage}
6663
onKeyDown={(event) => {
6764
if (event.key === "Enter") {
68-
navigateToBuildPage()
65+
navigate(buildPageLink)
6966
}
7067
}}
7168
className={styles.clickableTableRow}
7269
>
73-
<TableCell>{build.transition}</TableCell>
74-
<TableCell>
70+
<TableCellLink to={buildPageLink}>{build.transition}</TableCellLink>
71+
<TableCellLink to={buildPageLink}>
7572
<span style={{ color: theme.palette.text.secondary }}>{displayWorkspaceBuildDuration(build)}</span>
76-
</TableCell>
77-
<TableCell>
73+
</TableCellLink>
74+
<TableCellLink to={buildPageLink}>
7875
<span style={{ color: theme.palette.text.secondary }}>
7976
{new Date(build.created_at).toLocaleString()}
8077
</span>
81-
</TableCell>
82-
<TableCell>
78+
</TableCellLink>
79+
<TableCellLink to={buildPageLink}>
8380
<span style={{ color: status.color }}>{status.status}</span>
84-
</TableCell>
85-
<TableCell>
81+
</TableCellLink>
82+
<TableCellLink to={buildPageLink}>
8683
<div className={styles.arrowCell}>
8784
<KeyboardArrowRight className={styles.arrowRight} />
8885
</div>
89-
</TableCell>
86+
</TableCellLink>
9087
</TableRow>
9188
)
9289
})}
@@ -107,8 +104,6 @@ export const BuildsTable: FC<BuildsTableProps> = ({ builds, className }) => {
107104

108105
const useStyles = makeStyles((theme) => ({
109106
clickableTableRow: {
110-
cursor: "pointer",
111-
112107
"&:hover td": {
113108
backgroundColor: fade(theme.palette.primary.light, 0.1),
114109
},

‎site/src/components/TableCellLink/TableCellLink.tsx

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import Link from "@material-ui/core/Link"
2+
import { makeStyles } from "@material-ui/core/styles"
3+
import TableCell, { TableCellProps } from "@material-ui/core/TableCell"
4+
import { Link as RouterLink } from "react-router-dom"
5+
import { combineClasses } from "../../util/combineClasses"
6+
7+
// TableCellLink wraps a TableCell filling the entirety with a Link.
8+
// This allows table rows to be clickable with browser-behavior like ctrl+click.
9+
export const TableCellLink: React.FC<
10+
TableCellProps & {
11+
to: string
12+
}
13+
> = (props) => {
14+
const styles = useStyles()
15+
16+
return (
17+
<TableCell className={styles.cell} {...props}>
18+
<Link
19+
component={RouterLink}
20+
to={props.to}
21+
className={combineClasses([styles.link, "MuiTableCell-root", "MuiTableCell-body"])}
22+
>
23+
{props.children}
24+
</Link>
25+
</TableCell>
26+
)
27+
}
28+
29+
const useStyles = makeStyles((theme) => ({
30+
cell: {
31+
"&.MuiTableCell-root:not(:only-child):first-child, &.MuiTableCell-root:not(:only-child):last-child": {
32+
padding: "0",
33+
},
34+
},
35+
link: {
36+
display: "block",
37+
width: "100%",
38+
border: "none",
39+
background: "none",
40+
paddingTop: theme.spacing(2),
41+
paddingBottom: theme.spacing(2),
42+
// This is required to hide all underlines for child elements!
43+
textDecoration: "none !important",
44+
},
45+
}))

‎site/src/pages/TemplatesPage/TemplatesPageView.tsx

+14-14
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { EmptyState } from "../../components/EmptyState/EmptyState"
1717
import { Margins } from "../../components/Margins/Margins"
1818
import { PageHeader, PageHeaderSubtitle, PageHeaderTitle } from "../../components/PageHeader/PageHeader"
1919
import { Stack } from "../../components/Stack/Stack"
20+
import { TableCellLink } from "../../components/TableCellLink/TableCellLink"
2021
import { TableLoader } from "../../components/TableLoader/TableLoader"
2122
import {
2223
HelpTooltip,
@@ -115,36 +116,37 @@ export const TemplatesPageView: FC<TemplatesPageViewProps> = (props) => {
115116
</TableRow>
116117
)}
117118
{props.templates?.map((template) => {
118-
const navigateToTemplatePage = () => {
119-
navigate(`/templates/${template.name}`)
120-
}
119+
const templatePageLink = `/templates/${template.name}`
121120
return (
122121
<TableRow
123122
key={template.id}
124123
hover
125124
data-testid={`template-${template.id}`}
126125
tabIndex={0}
127-
onClick={navigateToTemplatePage}
128126
onKeyDown={(event) => {
129127
if (event.key === "Enter") {
130-
navigateToTemplatePage()
128+
navigate(templatePageLink)
131129
}
132130
}}
133131
className={styles.clickableTableRow}
134132
>
135-
<TableCell>
133+
<TableCellLink to={templatePageLink}>
136134
<AvatarData title={template.name} subtitle={template.description} />
137-
</TableCell>
135+
</TableCellLink>
138136

139-
<TableCell>{Language.developerCount(template.workspace_owner_count)}</TableCell>
137+
<TableCellLink to={templatePageLink}>
138+
{Language.developerCount(template.workspace_owner_count)}
139+
</TableCellLink>
140140

141-
<TableCell data-chromatic="ignore">{dayjs().to(dayjs(template.updated_at))}</TableCell>
142-
<TableCell>{template.created_by_name}</TableCell>
143-
<TableCell>
141+
<TableCellLink data-chromatic="ignore" to={templatePageLink}>
142+
{dayjs().to(dayjs(template.updated_at))}
143+
</TableCellLink>
144+
<TableCellLink to={templatePageLink}>{template.created_by_name}</TableCellLink>
145+
<TableCellLink to={templatePageLink}>
144146
<div className={styles.arrowCell}>
145147
<KeyboardArrowRight className={styles.arrowRight} />
146148
</div>
147-
</TableCell>
149+
</TableCellLink>
148150
</TableRow>
149151
)
150152
})}
@@ -159,8 +161,6 @@ const useStyles = makeStyles((theme) => ({
159161
maxWidth: theme.spacing(62),
160162
},
161163
clickableTableRow: {
162-
cursor: "pointer",
163-
164164
"&:hover td": {
165165
backgroundColor: fade(theme.palette.primary.light, 0.1),
166166
},

‎site/src/pages/WorkspacesPage/WorkspacesPageView.tsx

+15-18
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { EmptyState } from "../../components/EmptyState/EmptyState"
2828
import { Margins } from "../../components/Margins/Margins"
2929
import { PageHeader, PageHeaderSubtitle, PageHeaderTitle } from "../../components/PageHeader/PageHeader"
3030
import { Stack } from "../../components/Stack/Stack"
31+
import { TableCellLink } from "../../components/TableCellLink/TableCellLink"
3132
import { TableLoader } from "../../components/TableLoader/TableLoader"
3233
import {
3334
HelpTooltip,
@@ -104,27 +105,25 @@ const WorkspaceRow: React.FC<{ workspaceRef: WorkspaceItemMachineRef }> = ({ wor
104105
const [workspaceState, send] = useActor(workspaceRef)
105106
const { data: workspace } = workspaceState.context
106107
const status = getDisplayStatus(theme, workspace.latest_build)
107-
const navigateToWorkspacePage = () => {
108-
navigate(`/@${workspace.owner_name}/${workspace.name}`)
109-
}
108+
const workspacePageLink = `/@${workspace.owner_name}/${workspace.name}`
109+
110110
return (
111111
<TableRow
112112
hover
113113
data-testid={`workspace-${workspace.id}`}
114114
tabIndex={0}
115-
onClick={navigateToWorkspacePage}
116115
onKeyDown={(event) => {
117116
if (event.key === "Enter") {
118-
navigateToWorkspacePage()
117+
navigate(workspacePageLink)
119118
}
120119
}}
121120
className={styles.clickableTableRow}
122121
>
123-
<TableCell>
122+
<TableCellLink to={workspacePageLink}>
124123
<AvatarData title={workspace.name} subtitle={workspace.owner_name} />
125-
</TableCell>
126-
<TableCell>{workspace.template_name}</TableCell>
127-
<TableCell>
124+
</TableCellLink>
125+
<TableCellLink to={workspacePageLink}>{workspace.template_name}</TableCellLink>
126+
<TableCellLink to={workspacePageLink}>
128127
{workspace.outdated ? (
129128
<span className={styles.outdatedLabel}>
130129
{Language.outdatedLabel}
@@ -137,20 +136,20 @@ const WorkspaceRow: React.FC<{ workspaceRef: WorkspaceItemMachineRef }> = ({ wor
137136
) : (
138137
<span style={{ color: theme.palette.text.secondary }}>{Language.upToDateLabel}</span>
139138
)}
140-
</TableCell>
141-
<TableCell>
139+
</TableCellLink>
140+
<TableCellLink to={workspacePageLink}>
142141
<span data-chromatic="ignore" style={{ color: theme.palette.text.secondary }}>
143142
{dayjs().to(dayjs(workspace.latest_build.created_at))}
144143
</span>
145-
</TableCell>
146-
<TableCell>
144+
</TableCellLink>
145+
<TableCellLink to={workspacePageLink}>
147146
<span style={{ color: status.color }}>{status.status}</span>
148-
</TableCell>
149-
<TableCell>
147+
</TableCellLink>
148+
<TableCellLink to={workspacePageLink}>
150149
<div className={styles.arrowCell}>
151150
<KeyboardArrowRight className={styles.arrowRight} />
152151
</div>
153-
</TableCell>
152+
</TableCellLink>
154153
</TableRow>
155154
)
156155
}
@@ -349,8 +348,6 @@ const useStyles = makeStyles((theme) => ({
349348
},
350349
},
351350
clickableTableRow: {
352-
cursor: "pointer",
353-
354351
"&:hover td": {
355352
backgroundColor: fade(theme.palette.primary.light, 0.1),
356353
},

‎site/src/theme/overrides.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,12 @@ export const getOverrides = (palette: PaletteOptions) => {
6161
background: palette.background?.paper,
6262
borderBottom: `1px solid ${palette.divider}`,
6363
padding: 8,
64-
"&:first-child": {
64+
// This targets the first+last td elements, and also the first+last elements
65+
// of a TableCellLink.
66+
"&:not(:only-child):first-child, &:not(:only-child):first-child > a": {
6567
paddingLeft: 32,
6668
},
67-
"&:last-child": {
69+
"&:not(:only-child):last-child, &:not(:only-child):last-child > a": {
6870
paddingRight: 32,
6971
},
7072
},

0 commit comments

Comments
 (0)