Skip to content

Commit 471564d

Browse files
Kira-PilotKira-Pilot
authored
feat: improve update button visibility (coder#3115)
* feat: give update button primary focus when applicable resolves coder#3024 * added update tooltip * cleanup * prettier * PR feedback
1 parent 2dd98c7 commit 471564d

File tree

16 files changed

+131
-79
lines changed

16 files changed

+131
-79
lines changed

‎site/src/components/TemplateStats/TemplateStats.tsx

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
import { makeStyles } from "@material-ui/core/styles"
2-
import dayjs from "dayjs"
3-
import relativeTime from "dayjs/plugin/relativeTime"
42
import { FC } from "react"
3+
import { createDayString } from "util/createDayString"
54
import { Template, TemplateVersion } from "../../api/typesGenerated"
65
import { CardRadius, MONOSPACE_FONT_FAMILY } from "../../theme/constants"
76

8-
dayjs.extend(relativeTime)
9-
107
const Language = {
118
usedByLabel: "Used by",
129
activeVersionLabel: "Active version",
@@ -45,7 +42,7 @@ export const TemplateStats: FC<TemplateStatsProps> = ({ template, activeVersion
4542
<div className={styles.statItem}>
4643
<span className={styles.statsLabel}>{Language.lastUpdateLabel}</span>
4744
<span className={styles.statsValue} data-chromatic="ignore">
48-
{dayjs().to(dayjs(template.updated_at))}
45+
{createDayString(template.updated_at)}
4946
</span>
5047
</div>
5148
<div className={styles.statsDivider} />

‎site/src/components/Tooltips/HelpTooltip/HelpTooltip.tsx

+6-5
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,17 @@ export const HelpTooltipLink: React.FC<{ href: string }> = ({ children, href })
110110
)
111111
}
112112

113-
export const HelpTooltipAction: React.FC<{ icon: Icon; onClick: () => void }> = ({
114-
children,
115-
icon: Icon,
116-
onClick,
117-
}) => {
113+
export const HelpTooltipAction: React.FC<{
114+
icon: Icon
115+
onClick: () => void
116+
ariaLabel?: string
117+
}> = ({ children, icon: Icon, onClick, ariaLabel }) => {
118118
const styles = useStyles()
119119
const tooltip = useHelpTooltip()
120120

121121
return (
122122
<button
123+
aria-label={ariaLabel ?? ""}
123124
className={styles.action}
124125
onClick={(event) => {
125126
event.stopPropagation()

‎site/src/components/Tooltips/OutdatedHelpTooltip.tsx

+4-3
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,24 @@ import {
88
HelpTooltipTitle,
99
} from "./HelpTooltip"
1010

11-
const Language = {
11+
export const Language = {
1212
outdatedLabel: "Outdated",
1313
versionTooltipText: "This workspace version is outdated and a newer version is available.",
1414
updateVersionLabel: "Update version",
1515
}
1616

1717
interface TooltipProps {
1818
onUpdateVersion: () => void
19+
ariaLabel?: string
1920
}
2021

21-
export const OutdatedHelpTooltip: FC<TooltipProps> = ({ onUpdateVersion }) => {
22+
export const OutdatedHelpTooltip: FC<TooltipProps> = ({ onUpdateVersion, ariaLabel }) => {
2223
return (
2324
<HelpTooltip size="small">
2425
<HelpTooltipTitle>{Language.outdatedLabel}</HelpTooltipTitle>
2526
<HelpTooltipText>{Language.versionTooltipText}</HelpTooltipText>
2627
<HelpTooltipLinksGroup>
27-
<HelpTooltipAction icon={RefreshIcon} onClick={onUpdateVersion}>
28+
<HelpTooltipAction icon={RefreshIcon} onClick={onUpdateVersion} ariaLabel={ariaLabel}>
2829
{Language.updateVersionLabel}
2930
</HelpTooltipAction>
3031
</HelpTooltipLinksGroup>

‎site/src/components/Workspace/Workspace.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export const Workspace: FC<WorkspaceProps> = ({
9494
handleClick={() => navigate(`/templates`)}
9595
/>
9696

97-
<WorkspaceStats workspace={workspace} />
97+
<WorkspaceStats workspace={workspace} handleUpdate={handleUpdate} />
9898

9999
{!!resources && !!resources.length && (
100100
<Resources

‎site/src/components/WorkspaceActions/ActionCtas.tsx

+10-32
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import DeleteOutlineIcon from "@material-ui/icons/DeleteOutline"
66
import HighlightOffIcon from "@material-ui/icons/HighlightOff"
77
import PlayCircleOutlineIcon from "@material-ui/icons/PlayCircleOutline"
88
import { FC } from "react"
9-
import { Workspace } from "../../api/typesGenerated"
10-
import { WorkspaceStatus } from "../../util/workspace"
119
import { WorkspaceActionButton } from "../WorkspaceActionButton/WorkspaceActionButton"
1210

1311
export const Language = {
@@ -22,6 +20,16 @@ interface WorkspaceAction {
2220
handleAction: () => void
2321
}
2422

23+
export const UpdateButton: FC<WorkspaceAction> = ({ handleAction }) => {
24+
const styles = useStyles()
25+
26+
return (
27+
<Button className={styles.actionButton} startIcon={<CloudQueueIcon />} onClick={handleAction}>
28+
{Language.update}
29+
</Button>
30+
)
31+
}
32+
2533
export const StartButton: FC<WorkspaceAction> = ({ handleAction }) => {
2634
const styles = useStyles()
2735

@@ -61,36 +69,6 @@ export const DeleteButton: FC<WorkspaceAction> = ({ handleAction }) => {
6169
)
6270
}
6371

64-
type UpdateAction = WorkspaceAction & {
65-
workspace: Workspace
66-
workspaceStatus: WorkspaceStatus
67-
}
68-
69-
export const UpdateButton: FC<UpdateAction> = ({ handleAction, workspace, workspaceStatus }) => {
70-
const styles = useStyles()
71-
72-
/**
73-
* Jobs submitted while another job is in progress will be discarded,
74-
* so check whether workspace job status has reached completion (whether successful or not).
75-
*/
76-
const canAcceptJobs = (workspaceStatus: WorkspaceStatus) =>
77-
["started", "stopped", "deleted", "error", "canceled"].includes(workspaceStatus)
78-
79-
return (
80-
<>
81-
{workspace.outdated && canAcceptJobs(workspaceStatus) && (
82-
<Button
83-
className={styles.actionButton}
84-
startIcon={<CloudQueueIcon />}
85-
onClick={handleAction}
86-
>
87-
{Language.update}
88-
</Button>
89-
)}
90-
</>
91-
)
92-
}
93-
9472
export const CancelButton: FC<WorkspaceAction> = ({ handleAction }) => {
9573
const styles = useStyles()
9674

‎site/src/components/WorkspaceActions/WorkspaceActions.test.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,11 @@ describe("WorkspaceActions", () => {
7979
})
8080
})
8181
describe("when the workspace is outdated", () => {
82-
it("primary is start; secondary are delete, update", async () => {
82+
it("primary is update; secondary are start, delete", async () => {
8383
await renderAndClick({ workspace: Mocks.MockOutdatedWorkspace })
84-
expect(screen.getByTestId("primary-cta")).toHaveTextContent(Language.start)
84+
expect(screen.getByTestId("primary-cta")).toHaveTextContent(Language.update)
85+
expect(screen.getByTestId("secondary-ctas")).toHaveTextContent(Language.start)
8586
expect(screen.getByTestId("secondary-ctas")).toHaveTextContent(Language.delete)
86-
expect(screen.getByTestId("secondary-ctas")).toHaveTextContent(Language.update)
8787
})
8888
})
8989
})

‎site/src/components/WorkspaceActions/WorkspaceActions.tsx

+27-10
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
import Button from "@material-ui/core/Button"
22
import Popover from "@material-ui/core/Popover"
33
import { makeStyles } from "@material-ui/core/styles"
4-
import { FC, ReactNode, useEffect, useRef, useState } from "react"
4+
import { FC, ReactNode, useEffect, useMemo, useRef, useState } from "react"
55
import { Workspace } from "../../api/typesGenerated"
6-
import { getWorkspaceStatus } from "../../util/workspace"
6+
import { getWorkspaceStatus, WorkspaceStatus } from "../../util/workspace"
77
import { CloseDropdown, OpenDropdown } from "../DropdownArrows/DropdownArrows"
88
import { CancelButton, DeleteButton, StartButton, StopButton, UpdateButton } from "./ActionCtas"
99
import { ButtonTypesEnum, WorkspaceStateActions, WorkspaceStateEnum } from "./constants"
1010

11+
/**
12+
* Jobs submitted while another job is in progress will be discarded,
13+
* so check whether workspace job status has reached completion (whether successful or not).
14+
*/
15+
const canAcceptJobs = (workspaceStatus: WorkspaceStatus) =>
16+
["started", "stopped", "deleted", "error", "canceled"].includes(workspaceStatus)
17+
1118
export interface WorkspaceActionsProps {
1219
workspace: Workspace
1320
handleStart: () => void
@@ -34,7 +41,23 @@ export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
3441
workspace.latest_build,
3542
)
3643
const workspaceState = WorkspaceStateEnum[workspaceStatus]
37-
const actions = WorkspaceStateActions[workspaceState]
44+
45+
const canBeUpdated = workspace.outdated && canAcceptJobs(workspaceStatus)
46+
47+
// actions are the primary and secondary CTAs that appear in the workspace actions dropdown
48+
const actions = useMemo(() => {
49+
if (!canBeUpdated) {
50+
return WorkspaceStateActions[workspaceState]
51+
}
52+
53+
// if an update is available, we make the update button the primary CTA
54+
// and move the former primary CTA to the secondary actions list
55+
const updatedActions = { ...WorkspaceStateActions[workspaceState] }
56+
updatedActions.secondary.unshift(updatedActions.primary)
57+
updatedActions.primary = ButtonTypesEnum.update
58+
59+
return updatedActions
60+
}, [canBeUpdated, workspaceState])
3861

3962
/**
4063
* Ensures we close the popover before calling any action handler
@@ -58,16 +81,10 @@ export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
5881

5982
// A mapping of button type to the corresponding React component
6083
const buttonMapping: ButtonMapping = {
84+
[ButtonTypesEnum.update]: <UpdateButton handleAction={handleUpdate} />,
6185
[ButtonTypesEnum.start]: <StartButton handleAction={handleStart} />,
6286
[ButtonTypesEnum.stop]: <StopButton handleAction={handleStop} />,
6387
[ButtonTypesEnum.delete]: <DeleteButton handleAction={handleDelete} />,
64-
[ButtonTypesEnum.update]: (
65-
<UpdateButton
66-
handleAction={handleUpdate}
67-
workspace={workspace}
68-
workspaceStatus={workspaceStatus}
69-
/>
70-
),
7188
[ButtonTypesEnum.cancel]: <CancelButton handleAction={handleCancel} />,
7289
[ButtonTypesEnum.canceling]: disabledButton,
7390
[ButtonTypesEnum.disabled]: disabledButton,

‎site/src/components/WorkspaceActions/constants.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -45,24 +45,24 @@ export const WorkspaceStateActions: StateActionsType = {
4545
},
4646
[WorkspaceStateEnum.started]: {
4747
primary: ButtonTypesEnum.stop,
48-
secondary: [ButtonTypesEnum.delete, ButtonTypesEnum.update],
48+
secondary: [ButtonTypesEnum.delete],
4949
},
5050
[WorkspaceStateEnum.stopping]: {
5151
primary: ButtonTypesEnum.cancel,
5252
secondary: [],
5353
},
5454
[WorkspaceStateEnum.stopped]: {
5555
primary: ButtonTypesEnum.start,
56-
secondary: [ButtonTypesEnum.delete, ButtonTypesEnum.update],
56+
secondary: [ButtonTypesEnum.delete],
5757
},
5858
[WorkspaceStateEnum.canceled]: {
5959
primary: ButtonTypesEnum.start,
60-
secondary: [ButtonTypesEnum.stop, ButtonTypesEnum.delete, ButtonTypesEnum.update],
60+
secondary: [ButtonTypesEnum.stop, ButtonTypesEnum.delete],
6161
},
6262
// in the case of an error
6363
[WorkspaceStateEnum.error]: {
6464
primary: ButtonTypesEnum.start, // give the user the ability to start a workspace again
65-
secondary: [ButtonTypesEnum.delete, ButtonTypesEnum.update], // allows the user to delete or update
65+
secondary: [ButtonTypesEnum.delete], // allows the user to delete
6666
},
6767
/**
6868
* disabled states

‎site/src/components/WorkspaceStats/WorkspaceStats.test.tsx

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { fireEvent, screen } from "@testing-library/react"
2+
import { Language } from "components/Tooltips/OutdatedHelpTooltip"
3+
import { WorkspaceStats } from "components/WorkspaceStats/WorkspaceStats"
4+
import { MockOutdatedWorkspace } from "testHelpers/entities"
5+
import { renderWithAuth } from "testHelpers/renderHelpers"
6+
import * as CreateDayString from "util/createDayString"
7+
8+
describe("WorkspaceStats", () => {
9+
it("shows an outdated tooltip", async () => {
10+
// Mocking the dayjs module within the createDayString file
11+
const mock = jest.spyOn(CreateDayString, "createDayString")
12+
mock.mockImplementation(() => "a minute ago")
13+
14+
const handleUpdateMock = jest.fn()
15+
renderWithAuth(
16+
<WorkspaceStats handleUpdate={handleUpdateMock} workspace={MockOutdatedWorkspace} />,
17+
{
18+
route: `/@${MockOutdatedWorkspace.owner_name}/${MockOutdatedWorkspace.name}`,
19+
path: "/@:username/:workspace",
20+
},
21+
)
22+
const tooltipButton = await screen.findByRole("button")
23+
fireEvent.click(tooltipButton)
24+
expect(await screen.findByText(Language.versionTooltipText)).toBeInTheDocument()
25+
const updateButton = screen.getByRole("button", {
26+
name: "update version",
27+
})
28+
fireEvent.click(updateButton)
29+
expect(handleUpdateMock).toBeCalledTimes(1)
30+
})
31+
})

‎site/src/components/WorkspaceStats/WorkspaceStats.tsx

+17-6
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import Link from "@material-ui/core/Link"
22
import { makeStyles, useTheme } from "@material-ui/core/styles"
3-
import dayjs from "dayjs"
3+
import { OutdatedHelpTooltip } from "components/Tooltips"
44
import { FC } from "react"
55
import { Link as RouterLink } from "react-router-dom"
6+
import { combineClasses } from "util/combineClasses"
7+
import { createDayString } from "util/createDayString"
8+
import { getDisplayStatus, getDisplayWorkspaceBuildInitiatedBy } from "util/workspace"
69
import { Workspace } from "../../api/typesGenerated"
710
import { MONOSPACE_FONT_FAMILY } from "../../theme/constants"
8-
import { combineClasses } from "../../util/combineClasses"
9-
import { getDisplayStatus, getDisplayWorkspaceBuildInitiatedBy } from "../../util/workspace"
1011

1112
const Language = {
1213
workspaceDetails: "Workspace Details",
@@ -21,9 +22,10 @@ const Language = {
2122

2223
export interface WorkspaceStatsProps {
2324
workspace: Workspace
25+
handleUpdate: () => void
2426
}
2527

26-
export const WorkspaceStats: FC<WorkspaceStatsProps> = ({ workspace }) => {
28+
export const WorkspaceStats: FC<WorkspaceStatsProps> = ({ workspace, handleUpdate }) => {
2729
const styles = useStyles()
2830
const theme = useTheme()
2931
const status = getDisplayStatus(theme, workspace.latest_build)
@@ -46,7 +48,10 @@ export const WorkspaceStats: FC<WorkspaceStatsProps> = ({ workspace }) => {
4648
<span className={styles.statsLabel}>{Language.versionLabel}</span>
4749
<span className={styles.statsValue}>
4850
{workspace.outdated ? (
49-
<span style={{ color: theme.palette.error.main }}>{Language.outdated}</span>
51+
<span className={styles.outdatedLabel}>
52+
{Language.outdated}
53+
<OutdatedHelpTooltip onUpdateVersion={handleUpdate} ariaLabel="update version" />
54+
</span>
5055
) : (
5156
<span style={{ color: theme.palette.text.secondary }}>{Language.upToDate}</span>
5257
)}
@@ -56,7 +61,7 @@ export const WorkspaceStats: FC<WorkspaceStatsProps> = ({ workspace }) => {
5661
<div className={styles.statItem}>
5762
<span className={styles.statsLabel}>{Language.lastBuiltLabel}</span>
5863
<span className={styles.statsValue} data-chromatic="ignore">
59-
{dayjs().to(dayjs(workspace.latest_build.created_at))}
64+
{createDayString(workspace.latest_build.created_at)}
6065
</span>
6166
</div>
6267
<div className={styles.statsDivider} />
@@ -133,4 +138,10 @@ const useStyles = makeStyles((theme) => ({
133138
color: theme.palette.text.primary,
134139
fontWeight: 600,
135140
},
141+
outdatedLabel: {
142+
color: theme.palette.error.main,
143+
display: "flex",
144+
alignItems: "center",
145+
gap: theme.spacing(0.5),
146+
},
136147
}))

‎site/src/components/WorkspacesTable/WorkspacesRow.tsx

+2-5
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ import TableRow from "@material-ui/core/TableRow"
33
import KeyboardArrowRight from "@material-ui/icons/KeyboardArrowRight"
44
import useTheme from "@material-ui/styles/useTheme"
55
import { useActor } from "@xstate/react"
6-
import dayjs from "dayjs"
7-
import relativeTime from "dayjs/plugin/relativeTime"
86
import { FC } from "react"
97
import { useNavigate } from "react-router-dom"
8+
import { createDayString } from "util/createDayString"
109
import { getDisplayStatus, getDisplayWorkspaceBuildInitiatedBy } from "../../util/workspace"
1110
import { WorkspaceItemMachineRef } from "../../xServices/workspaces/workspacesXService"
1211
import { AvatarData } from "../AvatarData/AvatarData"
@@ -18,8 +17,6 @@ import {
1817
import { TableCellLink } from "../TableCellLink/TableCellLink"
1918
import { OutdatedHelpTooltip } from "../Tooltips"
2019

21-
dayjs.extend(relativeTime)
22-
2320
const Language = {
2421
upToDateLabel: "Up to date",
2522
outdatedLabel: "Outdated",
@@ -58,7 +55,7 @@ export const WorkspacesRow: FC<{ workspaceRef: WorkspaceItemMachineRef }> = ({ w
5855
<TableCellLink to={workspacePageLink}>
5956
<AvatarData
6057
title={initiatedBy}
61-
subtitle={dayjs().to(dayjs(workspace.latest_build.created_at))}
58+
subtitle={createDayString(workspace.latest_build.created_at)}
6259
/>
6360
</TableCellLink>
6461
<TableCellLink to={workspacePageLink}>

‎site/src/pages/TemplatePage/TemplatePage.test.tsx

+5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { screen } from "@testing-library/react"
2+
import * as CreateDayString from "util/createDayString"
23
import {
34
MockTemplate,
45
MockTemplateVersion,
@@ -9,6 +10,10 @@ import { TemplatePage } from "./TemplatePage"
910

1011
describe("TemplatePage", () => {
1112
it("shows the template name, readme and resources", async () => {
13+
// Mocking the dayjs module within the createDayString file
14+
const mock = jest.spyOn(CreateDayString, "createDayString")
15+
mock.mockImplementation(() => "a minute ago")
16+
1217
renderWithAuth(<TemplatePage />, {
1318
route: `/templates/${MockTemplate.id}`,
1419
path: "/templates/:template",

0 commit comments

Comments
 (0)