From 788a7b20c2c51404c1c8e53a81db8af3e13a6b02 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 28 May 2024 08:39:24 -0500 Subject: [PATCH 1/7] chore: add story for failed refresh error --- .../ExternalAuthPageView.stories.tsx | 15 +++++++++++++++ site/src/testHelpers/entities.ts | 10 ++++++++++ 2 files changed, 25 insertions(+) diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.stories.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.stories.tsx index f5f5cb3e21963..f6421f3cffc54 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.stories.tsx @@ -2,6 +2,7 @@ import type { Meta, StoryObj } from "@storybook/react"; import { MockGithubAuthLink, MockGithubExternalProvider, + MockGithubValidateErrorAuthLink, } from "testHelpers/entities"; import { ExternalAuthPageView } from "./ExternalAuthPageView"; @@ -60,3 +61,17 @@ export const Unauthenticated: Story = { }, }, }; + +export const Failed: Story = { + args: { + ...meta.args, + auths: { + providers: [MockGithubExternalProvider], + links: [ + { + ...MockGithubValidateErrorAuthLink, + }, + ], + }, + }, +}; diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 1fbb18aa86a07..88198abf02af8 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -3455,6 +3455,16 @@ export const MockGithubAuthLink: TypesGen.ExternalAuthLink = { validate_error: "", }; +export const MockGithubValidateErrorAuthLink: TypesGen.ExternalAuthLink = { + provider_id: "github", + created_at: "", + updated_at: "", + has_refresh_token: true, + expires: "", + authenticated: false, + validate_error: "Failed to refresh token.", +}; + export const MockOAuth2ProviderApps: TypesGen.OAuth2ProviderApp[] = [ { id: "1", From 4b2558b02fc053299b8b46010d4c5cc29543eac5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 28 May 2024 08:48:11 -0500 Subject: [PATCH 2/7] add terrible ui element to show the validate error --- .../UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx index 4433fee43045b..d8e42ddd37147 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx @@ -141,6 +141,8 @@ const ExternalAuthRow: FC = ({ ) } /> + {/* TODO: Style this! */} + {link?.validate_error} Date: Tue, 28 May 2024 09:23:08 -0500 Subject: [PATCH 3/7] chore: add refresh icon to tokens that can refresh --- .../ExternalAuthPageView.stories.tsx | 19 +++++++- .../ExternalAuthPage/ExternalAuthPageView.tsx | 46 +++++++++++++------ site/src/testHelpers/entities.ts | 10 ---- 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.stories.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.stories.tsx index f6421f3cffc54..4f04feab54b9f 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.stories.tsx @@ -2,7 +2,6 @@ import type { Meta, StoryObj } from "@storybook/react"; import { MockGithubAuthLink, MockGithubExternalProvider, - MockGithubValidateErrorAuthLink, } from "testHelpers/entities"; import { ExternalAuthPageView } from "./ExternalAuthPageView"; @@ -69,7 +68,23 @@ export const Failed: Story = { providers: [MockGithubExternalProvider], links: [ { - ...MockGithubValidateErrorAuthLink, + ...MockGithubAuthLink, + validate_error: "Failed to refresh token", + }, + ], + }, + }, +}; + +export const NoRefresh: Story = { + args: { + ...meta.args, + auths: { + providers: [MockGithubExternalProvider], + links: [ + { + ...MockGithubAuthLink, + has_refresh_token: false, }, ], }, diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx index d8e42ddd37147..4382c33786ae0 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx @@ -1,4 +1,6 @@ +import CachedIcon from "@mui/icons-material/Cached"; import LoadingButton from "@mui/lab/LoadingButton"; +import Badge from "@mui/material/Badge"; import Divider from "@mui/material/Divider"; import Table from "@mui/material/Table"; import TableBody from "@mui/material/TableBody"; @@ -6,6 +8,7 @@ import TableCell from "@mui/material/TableCell"; import TableContainer from "@mui/material/TableContainer"; import TableHead from "@mui/material/TableHead"; import TableRow from "@mui/material/TableRow"; +import Tooltip from "@mui/material/Tooltip"; import visuallyHidden from "@mui/utils/visuallyHidden"; import { type FC, useState, useCallback, useEffect } from "react"; import { useQuery } from "react-query"; @@ -125,22 +128,39 @@ const ExternalAuthRow: FC = ({ ? externalAuth.authenticated : link?.authenticated ?? false; + let avatar = app.display_icon ? ( + + ) : ( + {name} + ); + + // If the link is authenticated and has a refresh token, show that it will automatically + // attempt to authenticate when the token expires. + if (link?.has_refresh_token && authenticated) { + avatar = ( + + + + } + > + {avatar} + + ); + } + return ( - - ) - } - /> + {/* TODO: Style this! */} {link?.validate_error} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 88198abf02af8..1fbb18aa86a07 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -3455,16 +3455,6 @@ export const MockGithubAuthLink: TypesGen.ExternalAuthLink = { validate_error: "", }; -export const MockGithubValidateErrorAuthLink: TypesGen.ExternalAuthLink = { - provider_id: "github", - created_at: "", - updated_at: "", - has_refresh_token: true, - expires: "", - authenticated: false, - validate_error: "Failed to refresh token.", -}; - export const MockOAuth2ProviderApps: TypesGen.OAuth2ProviderApp[] = [ { id: "1", From 855e8b8c2e96a57a84e659267f7ef72d79cc15ae Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 28 May 2024 09:28:31 -0500 Subject: [PATCH 4/7] Add styled error --- .../ExternalAuthPage/ExternalAuthPageView.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx index 4382c33786ae0..1ad2989de17a6 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx @@ -1,3 +1,4 @@ +import { useTheme } from "@emotion/react"; import CachedIcon from "@mui/icons-material/Cached"; import LoadingButton from "@mui/lab/LoadingButton"; import Badge from "@mui/material/Badge"; @@ -114,6 +115,7 @@ const ExternalAuthRow: FC = ({ onUnlinkExternalAuth, onValidateExternalAuth, }) => { + const theme = useTheme(); const name = app.display_name || app.id || app.type; const authURL = "/external-auth/" + app.id; @@ -161,8 +163,16 @@ const ExternalAuthRow: FC = ({ - {/* TODO: Style this! */} - {link?.validate_error} + {link?.validate_error && ( + <> + + Error:{" "} + + {link?.validate_error} + + )} Date: Tue, 28 May 2024 13:31:31 -0500 Subject: [PATCH 5/7] Fixup styling of the badge --- .../ExternalAuthPage/ExternalAuthPageView.tsx | 39 +++++++++++++++++-- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx index 1ad2989de17a6..b799d1b636d0c 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx @@ -1,8 +1,9 @@ import { useTheme } from "@emotion/react"; -import CachedIcon from "@mui/icons-material/Cached"; +import AutorenewIcon from "@mui/icons-material/Autorenew"; import LoadingButton from "@mui/lab/LoadingButton"; import Badge from "@mui/material/Badge"; import Divider from "@mui/material/Divider"; +import { styled } from "@mui/material/styles"; import Table from "@mui/material/Table"; import TableBody from "@mui/material/TableBody"; import TableCell from "@mui/material/TableCell"; @@ -32,6 +33,7 @@ import { } from "components/MoreMenu/MoreMenu"; import { TableEmpty } from "components/TableEmpty/TableEmpty"; import type { ExternalAuthPollingState } from "pages/CreateWorkspacePage/CreateWorkspacePage"; +import { minWidth, padding, width } from "@mui/system"; export type ExternalAuthPageViewProps = { isLoading: boolean; @@ -108,6 +110,29 @@ interface ExternalAuthRowProps { onValidateExternalAuth: () => void; } +const StyledBadge = styled(Badge)(({ theme }) => ({ + "& .MuiBadge-badge": { + // Make a circular background for the icon. Background provides contrast, with a thin + // border to separate it from the avatar image. + backgroundColor: `${theme.palette.background.paper}`, + borderStyle: "solid", + borderColor: `${theme.palette.secondary.main}`, + borderWidth: "thin", + + // The size of the badge content should be small, and should perfectly encapsulate the icon. + // By default, the style has padding and a fixed size, which is too large. + // Remove all default padding, and base the size off of the icon size. All based around text + // sizing. + // Ideally, we could use padding to accomplis this, but you have to override the default 'height' and + // 'width' properties. + padding: "0px", + minHeight: "0px", + minWidth: "0px", + height: "1.4em", + width: "1.4em", + }, +})); + const ExternalAuthRow: FC = ({ app, unlinked, @@ -140,22 +165,28 @@ const ExternalAuthRow: FC = ({ // attempt to authenticate when the token expires. if (link?.has_refresh_token && authenticated) { avatar = ( - - + } > {avatar} - + ); } From b39bfa4709bde7bf00645d95feae0e85583d4d16 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 28 May 2024 13:36:33 -0500 Subject: [PATCH 6/7] add padding instead of fixed sizes --- .../ExternalAuthPage/ExternalAuthPageView.tsx | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx index b799d1b636d0c..776a4b417ac71 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx @@ -33,7 +33,7 @@ import { } from "components/MoreMenu/MoreMenu"; import { TableEmpty } from "components/TableEmpty/TableEmpty"; import type { ExternalAuthPollingState } from "pages/CreateWorkspacePage/CreateWorkspacePage"; -import { minWidth, padding, width } from "@mui/system"; +import { margin, minWidth, padding, width } from "@mui/system"; export type ExternalAuthPageViewProps = { isLoading: boolean; @@ -119,17 +119,13 @@ const StyledBadge = styled(Badge)(({ theme }) => ({ borderColor: `${theme.palette.secondary.main}`, borderWidth: "thin", - // The size of the badge content should be small, and should perfectly encapsulate the icon. - // By default, the style has padding and a fixed size, which is too large. - // Remove all default padding, and base the size off of the icon size. All based around text - // sizing. - // Ideally, we could use padding to accomplis this, but you have to override the default 'height' and - // 'width' properties. - padding: "0px", + // Override the default minimum sizes, as they are larger than what we want. minHeight: "0px", minWidth: "0px", - height: "1.4em", - width: "1.4em", + // Override the default "height", which is usually set to some constant value. + height: "auto", + // Padding adds some room for the icon to live in. + padding: "0.1em", }, })); From 9639a9410510bd8f652d64930c762e2c2e1af9fd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 28 May 2024 13:42:05 -0500 Subject: [PATCH 7/7] remove import --- .../UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx index 776a4b417ac71..b73286a6158f0 100644 --- a/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx +++ b/site/src/pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPageView.tsx @@ -33,7 +33,6 @@ import { } from "components/MoreMenu/MoreMenu"; import { TableEmpty } from "components/TableEmpty/TableEmpty"; import type { ExternalAuthPollingState } from "pages/CreateWorkspacePage/CreateWorkspacePage"; -import { margin, minWidth, padding, width } from "@mui/system"; export type ExternalAuthPageViewProps = { isLoading: boolean;