Skip to content

Commit f75d497

Browse files
EmyrkEmyrk
authored
chore: touch ups to wsproxy UX (#8350)
* chore: update wording on wsproxy help * chore: show help if no fields specified in wsproxy edit * chore: Add run command example to wsproxy create * chore: remove localhost warning * chore: navbar match page title * chore: Add helper text to latency picker * chore: add confirm delete to workspace proxy delete cli * chore: add errors + warnings to workspace proxy table
1 parent 396e5e9 commit f75d497

File tree

6 files changed

+188
-46
lines changed

6 files changed

+188
-46
lines changed

‎enterprise/cli/proxyserver.go

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func (*RootCmd) proxyServer() *clibase.Cmd {
9595
),
9696
Handler: func(inv *clibase.Invocation) error {
9797
if !(primaryAccessURL.Scheme == "http" || primaryAccessURL.Scheme == "https") {
98-
return xerrors.Errorf("primary access URL must be http or https: url=%s", primaryAccessURL.String())
98+
return xerrors.Errorf("'--primary-access-url' value must be http or https: url=%s", primaryAccessURL.String())
9999
}
100100

101101
var closers closers
@@ -175,20 +175,6 @@ func (*RootCmd) proxyServer() *clibase.Cmd {
175175
defer httpClient.CloseIdleConnections()
176176
closers.Add(httpClient.CloseIdleConnections)
177177

178-
// Warn the user if the access URL appears to be a loopback address.
179-
isLocal, err := cli.IsLocalURL(ctx, cfg.AccessURL.Value())
180-
if isLocal || err != nil {
181-
reason := "could not be resolved"
182-
if isLocal {
183-
reason = "isn't externally reachable"
184-
}
185-
cliui.Warnf(
186-
inv.Stderr,
187-
"The access URL %s %s, this may cause unexpected problems when creating workspaces. Generate a unique *.try.coder.app URL by not specifying an access URL.\n",
188-
cliui.DefaultStyles.Field.Render(cfg.AccessURL.String()), reason,
189-
)
190-
}
191-
192178
// A newline is added before for visibility in terminal output.
193179
cliui.Infof(inv.Stdout, "\nView the Web UI: %s", cfg.AccessURL.String())
194180

‎enterprise/cli/workspaceproxy.go

Lines changed: 54 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ func (r *RootCmd) workspaceProxy() *clibase.Cmd {
1818
Use: "workspace-proxy",
1919
Short: "Workspace proxies provide low-latency experiences for geo-distributed teams.",
2020
Long: "Workspace proxies provide low-latency experiences for geo-distributed teams. " +
21-
"It will act as a connection gateway to your workspace providing a lower latency solution " +
22-
"to connecting to your workspace if Coder and your workspace are deployed in different regions.",
21+
"It will act as a connection gateway to your workspace. " +
22+
"Best used if Coder and your workspace are deployed in different regions.",
2323
Aliases: []string{"wsproxy"},
2424
Hidden: true,
2525
Handler: func(inv *clibase.Invocation) error {
@@ -51,6 +51,7 @@ func (r *RootCmd) regenerateProxyToken() *clibase.Cmd {
5151
),
5252
Handler: func(inv *clibase.Invocation) error {
5353
ctx := inv.Context()
54+
formatter.primaryAccessURL = client.URL.String()
5455
// This is cheeky, but you can also use a uuid string in
5556
// 'DeleteWorkspaceProxyByName' and it will work.
5657
proxy, err := client.WorkspaceProxyByName(ctx, inv.Args[0])
@@ -120,6 +121,7 @@ func (r *RootCmd) patchProxy() *clibase.Cmd {
120121
Handler: func(inv *clibase.Invocation) error {
121122
ctx := inv.Context()
122123
if proxyIcon == "" && displayName == "" && proxyName == "" {
124+
_ = inv.Command.HelpHandler(inv)
123125
return xerrors.Errorf("specify at least one field to update")
124126
}
125127

@@ -187,13 +189,32 @@ func (r *RootCmd) deleteProxy() *clibase.Cmd {
187189
cmd := &clibase.Cmd{
188190
Use: "delete <name|id>",
189191
Short: "Delete a workspace proxy",
192+
Options: clibase.OptionSet{
193+
cliui.SkipPromptOption(),
194+
},
190195
Middleware: clibase.Chain(
191196
clibase.RequireNArgs(1),
192197
r.InitClient(client),
193198
),
194199
Handler: func(inv *clibase.Invocation) error {
195200
ctx := inv.Context()
196-
err := client.DeleteWorkspaceProxyByName(ctx, inv.Args[0])
201+
202+
wsproxy, err := client.WorkspaceProxyByName(ctx, inv.Args[0])
203+
if err != nil {
204+
return xerrors.Errorf("fetch workspace proxy %q: %w", inv.Args[0], err)
205+
}
206+
207+
// Confirm deletion of the template.
208+
_, err = cliui.Prompt(inv, cliui.PromptOptions{
209+
Text: fmt.Sprintf("Delete this workspace proxy: %s?", cliui.DefaultStyles.Code.Render(wsproxy.DisplayName)),
210+
IsConfirm: true,
211+
Default: cliui.ConfirmNo,
212+
})
213+
if err != nil {
214+
return err
215+
}
216+
217+
err = client.DeleteWorkspaceProxyByName(ctx, inv.Args[0])
197218
if err != nil {
198219
return xerrors.Errorf("delete workspace proxy %q: %w", inv.Args[0], err)
199220
}
@@ -225,6 +246,7 @@ func (r *RootCmd) createProxy() *clibase.Cmd {
225246
),
226247
Handler: func(inv *clibase.Invocation) error {
227248
ctx := inv.Context()
249+
formatter.primaryAccessURL = client.URL.String()
228250
var err error
229251
if proxyName == "" && !noPrompts {
230252
proxyName, err = cliui.Prompt(inv, cliui.PromptOptions{
@@ -367,8 +389,9 @@ func (r *RootCmd) listProxies() *clibase.Cmd {
367389

368390
// updateProxyResponseFormatter is used for both create and regenerate proxy commands.
369391
type updateProxyResponseFormatter struct {
370-
onlyToken bool
371-
formatter *cliui.OutputFormatter
392+
onlyToken bool
393+
formatter *cliui.OutputFormatter
394+
primaryAccessURL string
372395
}
373396

374397
func (f *updateProxyResponseFormatter) Format(ctx context.Context, data codersdk.UpdateWorkspaceProxyResponse) (string, error) {
@@ -392,31 +415,37 @@ func (f *updateProxyResponseFormatter) AttachOptions(opts *clibase.OptionSet) {
392415
func newUpdateProxyResponseFormatter() *updateProxyResponseFormatter {
393416
up := &updateProxyResponseFormatter{
394417
onlyToken: false,
395-
formatter: cliui.NewOutputFormatter(
396-
// Text formatter should be human readable.
397-
cliui.ChangeFormatterData(cliui.TextFormat(), func(data any) (any, error) {
418+
}
419+
up.formatter = cliui.NewOutputFormatter(
420+
// Text formatter should be human readable.
421+
cliui.ChangeFormatterData(cliui.TextFormat(), func(data any) (any, error) {
422+
response, ok := data.(codersdk.UpdateWorkspaceProxyResponse)
423+
if !ok {
424+
return nil, xerrors.Errorf("unexpected type %T", data)
425+
}
426+
427+
return fmt.Sprintf("Workspace Proxy %[1]q updated successfully.\n"+
428+
cliui.DefaultStyles.Placeholder.Render("—————————————————————————————————————————————————")+"\n"+
429+
"Save this authentication token, it will not be shown again.\n"+
430+
"Token: %[2]s\n"+
431+
"\n"+
432+
"Start the proxy by running:\n"+
433+
cliui.DefaultStyles.Code.Render("CODER_PROXY_SESSION_TOKEN=%[2]s coder wsproxy server --primary-access-url %[3]s --http-address=0.0.0.0:3001")+
434+
// This is required to turn off the code style. Otherwise it appears in the code block until the end of the line.
435+
cliui.DefaultStyles.Placeholder.Render(""),
436+
response.Proxy.Name, response.ProxyToken, up.primaryAccessURL), nil
437+
}),
438+
cliui.JSONFormat(),
439+
// Table formatter expects a slice, make a slice of one.
440+
cliui.ChangeFormatterData(cliui.TableFormat([]codersdk.UpdateWorkspaceProxyResponse{}, []string{"proxy name", "proxy url", "proxy token"}),
441+
func(data any) (any, error) {
398442
response, ok := data.(codersdk.UpdateWorkspaceProxyResponse)
399443
if !ok {
400444
return nil, xerrors.Errorf("unexpected type %T", data)
401445
}
402-
403-
return fmt.Sprintf("Workspace Proxy %q updated successfully.\n"+
404-
cliui.DefaultStyles.Placeholder.Render("—————————————————————————————————————————————————")+"\n"+
405-
"Save this authentication token, it will not be shown again.\n"+
406-
"Token: %s\n", response.Proxy.Name, response.ProxyToken), nil
446+
return []codersdk.UpdateWorkspaceProxyResponse{response}, nil
407447
}),
408-
cliui.JSONFormat(),
409-
// Table formatter expects a slice, make a slice of one.
410-
cliui.ChangeFormatterData(cliui.TableFormat([]codersdk.UpdateWorkspaceProxyResponse{}, []string{"proxy name", "proxy url", "proxy token"}),
411-
func(data any) (any, error) {
412-
response, ok := data.(codersdk.UpdateWorkspaceProxyResponse)
413-
if !ok {
414-
return nil, xerrors.Errorf("unexpected type %T", data)
415-
}
416-
return []codersdk.UpdateWorkspaceProxyResponse{response}, nil
417-
}),
418-
),
419-
}
448+
)
420449

421450
return up
422451
}

‎enterprise/cli/workspaceproxy_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func Test_ProxyCRUD(t *testing.T) {
125125

126126
inv, conf := newCLI(
127127
t,
128-
"wsproxy", "delete", expectedName,
128+
"wsproxy", "delete", "-y", expectedName,
129129
)
130130

131131
pty := ptytest.New(t)

‎site/src/components/DeploySettingsLayout/Sidebar.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export const Sidebar: React.FC = () => {
8484
href="workspace-proxies"
8585
icon={<SidebarNavItemIcon icon={HubOutlinedIcon} />}
8686
>
87-
Workspace Proxy
87+
Workspace Proxies
8888
</SidebarNavItem>
8989
)}
9090
<SidebarNavItem

‎site/src/components/Navbar/NavbarView.tsx

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ import Skeleton from "@mui/material/Skeleton"
2424
import { BUTTON_SM_HEIGHT } from "theme/theme"
2525
import { ProxyStatusLatency } from "components/ProxyStatusLatency/ProxyStatusLatency"
2626
import { usePermissions } from "hooks/usePermissions"
27+
import {
28+
HelpTooltip,
29+
HelpTooltipText,
30+
HelpTooltipTitle,
31+
} from "components/Tooltips/HelpTooltip"
2732

2833
export const USERS_LINK = `/users?filter=${encodeURIComponent("status:active")}`
2934

@@ -186,6 +191,7 @@ export const NavbarView: FC<NavbarViewProps> = ({
186191
const ProxyMenu: FC<{ proxyContextValue: ProxyContextValue }> = ({
187192
proxyContextValue,
188193
}) => {
194+
const styles = useStyles()
189195
const buttonRef = useRef<HTMLButtonElement>(null)
190196
const [isOpen, setIsOpen] = useState(false)
191197
const [refetchDate, setRefetchDate] = useState<Date>()
@@ -269,6 +275,34 @@ const ProxyMenu: FC<{ proxyContextValue: ProxyContextValue }> = ({
269275
onClose={closeMenu}
270276
sx={{ "& .MuiMenu-paper": { py: 1 } }}
271277
>
278+
<MenuItem
279+
sx={[
280+
{ fontSize: 14 },
281+
{ "&:hover": { backgroundColor: "transparent" } },
282+
{ wordWrap: "break-word" },
283+
{ inlineSize: "200px" },
284+
{ whiteSpace: "normal" },
285+
{ textAlign: "center" },
286+
]}
287+
onClick={(e) => {
288+
// Stop the menu from closing
289+
e.stopPropagation()
290+
}}
291+
>
292+
<div>
293+
Reduce workspace latency by selecting the region nearest you.
294+
{/* This was always on a newline below the text. This puts it on the same line.
295+
It still doesn't look great, but it is marginally better. */}
296+
<HelpTooltip buttonClassName={styles.displayInitial}>
297+
<HelpTooltipTitle>Workspace Proxy Selection</HelpTooltipTitle>
298+
<HelpTooltipText>
299+
Only applies to web connections. Local ssh connections will
300+
automatically select the nearest region based on latency.
301+
</HelpTooltipText>
302+
</HelpTooltip>
303+
</div>
304+
</MenuItem>
305+
<Divider sx={{ borderColor: (theme) => theme.palette.divider }} />
272306
{proxyContextValue.proxies?.map((proxy) => (
273307
<MenuItem
274308
onClick={() => {
@@ -335,6 +369,9 @@ const ProxyMenu: FC<{ proxyContextValue: ProxyContextValue }> = ({
335369
}
336370

337371
const useStyles = makeStyles((theme) => ({
372+
displayInitial: {
373+
display: "initial",
374+
},
338375
root: {
339376
height: navHeight,
340377
background: theme.palette.background.paper,

‎site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorkspaceProxyRow.tsx

Lines changed: 94 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { AvatarData } from "components/AvatarData/AvatarData"
33
import { Avatar } from "components/Avatar/Avatar"
44
import TableCell from "@mui/material/TableCell"
55
import TableRow from "@mui/material/TableRow"
6-
import { FC } from "react"
6+
import { FC, useState } from "react"
77
import {
88
HealthyBadge,
99
NotHealthyBadge,
@@ -12,22 +12,51 @@ import {
1212
} from "components/DeploySettingsLayout/Badges"
1313
import { ProxyLatencyReport } from "contexts/useProxyLatency"
1414
import { getLatencyColor } from "utils/latency"
15+
import Collapse from "@mui/material/Collapse"
16+
import { makeStyles } from "@mui/styles"
17+
import { combineClasses } from "utils/combineClasses"
18+
import ListItem from "@mui/material/ListItem"
19+
import List from "@mui/material/List"
20+
import ListSubheader from "@mui/material/ListSubheader"
21+
import { Maybe } from "components/Conditionals/Maybe"
22+
import { CodeExample } from "components/CodeExample/CodeExample"
1523

1624
export const ProxyRow: FC<{
1725
latency?: ProxyLatencyReport
1826
proxy: Region
1927
}> = ({ proxy, latency }) => {
28+
const styles = useStyles()
2029
// If we have a more specific proxy status, use that.
2130
// All users can see healthy/unhealthy, some can see more.
2231
let statusBadge = <ProxyStatus proxy={proxy} />
32+
let shouldShowMessages = false
2333
if ("status" in proxy) {
24-
statusBadge = <DetailedProxyStatus proxy={proxy as WorkspaceProxy} />
34+
const wsproxy = proxy as WorkspaceProxy
35+
statusBadge = <DetailedProxyStatus proxy={wsproxy} />
36+
shouldShowMessages = Boolean(
37+
(wsproxy.status?.report?.warnings &&
38+
wsproxy.status?.report?.warnings.length > 0) ||
39+
(wsproxy.status?.report?.errors &&
40+
wsproxy.status?.report?.errors.length > 0),
41+
)
2542
}
2643

44+
const [isMsgsOpen, setIsMsgsOpen] = useState(false)
45+
const toggle = () => {
46+
if (shouldShowMessages) {
47+
setIsMsgsOpen((v) => !v)
48+
}
49+
}
2750
return (
2851
<>
29-
<TableRow key={proxy.name} data-testid={`${proxy.name}`}>
30-
<TableCell>
52+
<TableRow
53+
className={combineClasses({
54+
[styles.clickable]: shouldShowMessages,
55+
})}
56+
key={proxy.name}
57+
data-testid={`${proxy.name}`}
58+
>
59+
<TableCell onClick={toggle}>
3160
<AvatarData
3261
title={
3362
proxy.display_name && proxy.display_name.length > 0
@@ -44,6 +73,7 @@ export const ProxyRow: FC<{
4473
/>
4574
)
4675
}
76+
subtitle={shouldShowMessages ? "Click to view details" : undefined}
4777
/>
4878
</TableCell>
4979

@@ -62,10 +92,60 @@ export const ProxyRow: FC<{
6292
{latency ? `${latency.latencyMS.toFixed(0)} ms` : "Not available"}
6393
</TableCell>
6494
</TableRow>
95+
<Maybe condition={shouldShowMessages}>
96+
<TableRow>
97+
<TableCell colSpan={4} sx={{ padding: "0px !important" }}>
98+
<Collapse in={isMsgsOpen}>
99+
<ProxyMessagesRow proxy={proxy as WorkspaceProxy} />
100+
</Collapse>
101+
</TableCell>
102+
</TableRow>
103+
</Maybe>
104+
</>
105+
)
106+
}
107+
108+
const ProxyMessagesRow: FC<{
109+
proxy: WorkspaceProxy
110+
}> = ({ proxy }) => {
111+
return (
112+
<>
113+
<ProxyMessagesList
114+
title="Errors"
115+
messages={proxy.status?.report?.errors}
116+
/>
117+
<ProxyMessagesList
118+
title="Warnings"
119+
messages={proxy.status?.report?.warnings}
120+
/>
65121
</>
66122
)
67123
}
68124

125+
const ProxyMessagesList: FC<{
126+
title: string
127+
messages?: string[]
128+
}> = ({ title, messages }) => {
129+
if (!messages) {
130+
return <></>
131+
}
132+
return (
133+
<List
134+
subheader={
135+
<ListSubheader component="div" id="nested-list-subheader">
136+
{title}
137+
</ListSubheader>
138+
}
139+
>
140+
{messages.map((error, index) => (
141+
<ListItem key={"message" + index}>
142+
<CodeExample code={error} />
143+
</ListItem>
144+
))}
145+
</List>
146+
)
147+
}
148+
69149
// DetailedProxyStatus allows a more precise status to be displayed.
70150
const DetailedProxyStatus: FC<{
71151
proxy: WorkspaceProxy
@@ -100,3 +180,13 @@ const ProxyStatus: FC<{
100180

101181
return icon
102182
}
183+
184+
const useStyles = makeStyles((theme) => ({
185+
clickable: {
186+
cursor: "pointer",
187+
188+
"&:hover": {
189+
backgroundColor: theme.palette.action.hover,
190+
},
191+
},
192+
}))

0 commit comments

Comments
 (0)