From 59214b181fcac4de37be4dd6b4f7dbdced554c42 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 26 Sep 2024 05:30:10 +0000 Subject: [PATCH 01/12] fix: make sure workspace avatars display correctly when URLs fail to load --- site/src/components/AvatarData/AvatarData.tsx | 23 +++++++++++++++---- .../pages/WorkspacePage/WorkspaceTopbar.tsx | 15 ++++++++++-- site/src/theme/externalImages.ts | 6 ++--- site/src/utils/appearance.ts | 17 ++++++++++++++ 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index e1598feb29d4b..ad7837e2b97c9 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -8,19 +8,34 @@ export interface AvatarDataProps { subtitle?: ReactNode; src?: string; avatar?: React.ReactNode; + + /** + * Useful for when you need to pass in a ReactNode for the title of the + * component. + * + * MUI will try to take any string titles and turn them into the first + * character, but if you pass in a ReactNode, MUI can't do that, because it + * has no way to reliably grab the text content during renders. + * + * Tried writing some layout effect/JSX parsing logic to do the extraction, + * but it added complexity and render overhead, and wasn't reliable enough + */ + fallbackLetter?: string; } export const AvatarData: FC = ({ title, subtitle, src, + fallbackLetter, avatar, }) => { const theme = useTheme(); - - if (!avatar) { - avatar = {title}; - } + avatar ??= ( + + {typeof title === "string" ? title : (fallbackLetter?.slice(0, 1) ?? "-")} + + ); return ( = ({ subtitle="Organization" avatar={ orgIconUrl && ( - + ) } /> @@ -405,7 +411,12 @@ const WorkspaceBreadcrumb: FC = ({ } avatar={ - + } /> diff --git a/site/src/theme/externalImages.ts b/site/src/theme/externalImages.ts index 834e8fb7d80e7..612d7ab2c75d4 100644 --- a/site/src/theme/externalImages.ts +++ b/site/src/theme/externalImages.ts @@ -75,8 +75,8 @@ const parseInvertFilterParameters = ( let extraStyles: CSSObject | undefined; - const brightness = params.get("brightness"); - if (multiplier.test(brightness!)) { + const brightness = params.get("brightness") ?? ""; + if (multiplier.test(brightness)) { let filter = baseStyles.filter ?? ""; filter += ` brightness(${brightness})`; extraStyles = { ...extraStyles, filter }; @@ -131,7 +131,7 @@ export function getExternalImageStylesFromUrl( ) { return parseImageParameters( modes, - defaultParametersForBuiltinIcons.get(url.pathname)!, + defaultParametersForBuiltinIcons.get(url.pathname) as string, ); } diff --git a/site/src/utils/appearance.ts b/site/src/utils/appearance.ts index d025ec15df197..9855ee1bcaf22 100644 --- a/site/src/utils/appearance.ts +++ b/site/src/utils/appearance.ts @@ -1,3 +1,5 @@ +import type { ReactFragment, ReactNode, ReactPortal } from "react"; + export const getApplicationName = (): string => { const c = document.head .querySelector("meta[name=application-name]") @@ -14,3 +16,18 @@ export const getLogoURL = (): string => { ?.getAttribute("content"); return c && !c.startsWith("{{ .") ? c : ""; }; + +/** + * Exposes an easy way to determine if a given URL is for an emoji hosted on + * the Coder deployment. + * + * Helps when you need to style emojis differently (i.e., not adding rounding to + * the container so that the emoji doesn't get cut off). + */ +export function isEmojiUrl(url: string | undefined): boolean { + if (!url) { + return false; + } + + return url.startsWith("/emojis/"); +} From ab432e33f5db067cfde348635ea4535e9ae72791 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 26 Sep 2024 05:38:53 +0000 Subject: [PATCH 02/12] blah --- site/src/components/AvatarData/AvatarData.tsx | 6 +++--- site/src/pages/WorkspacePage/WorkspaceTopbar.tsx | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index ad7837e2b97c9..b9d46fb5f0f8d 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -20,20 +20,20 @@ export interface AvatarDataProps { * Tried writing some layout effect/JSX parsing logic to do the extraction, * but it added complexity and render overhead, and wasn't reliable enough */ - fallbackLetter?: string; + imgFallbackText?: string; } export const AvatarData: FC = ({ title, subtitle, src, - fallbackLetter, + imgFallbackText, avatar, }) => { const theme = useTheme(); avatar ??= ( - {typeof title === "string" ? title : (fallbackLetter?.slice(0, 1) ?? "-")} + {typeof title === "string" ? title : (imgFallbackText ?? "-")} ); diff --git a/site/src/pages/WorkspacePage/WorkspaceTopbar.tsx b/site/src/pages/WorkspacePage/WorkspaceTopbar.tsx index 91d174dcbc4e0..e3be26462cc5b 100644 --- a/site/src/pages/WorkspacePage/WorkspaceTopbar.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceTopbar.tsx @@ -25,12 +25,12 @@ import { WorkspaceStatusBadge } from "modules/workspaces/WorkspaceStatusBadge/Wo import type { FC } from "react"; import { useQuery } from "react-query"; import { Link as RouterLink } from "react-router-dom"; +import { isEmojiUrl } from "utils/appearance"; import { displayDormantDeletion } from "utils/dormant"; import { WorkspaceActions } from "./WorkspaceActions/WorkspaceActions"; import { WorkspaceNotifications } from "./WorkspaceNotifications/WorkspaceNotifications"; import { WorkspaceScheduleControls } from "./WorkspaceScheduleControls"; import type { WorkspacePermissions } from "./permissions"; -import { isEmojiUrl } from "utils/appearance"; export type WorkspaceError = | "getBuildsError" @@ -353,6 +353,7 @@ const OrganizationBreadcrumb: FC = ({ /> ) } + imgFallbackText={orgName} /> @@ -418,6 +419,7 @@ const WorkspaceBreadcrumb: FC = ({ fitImage /> } + imgFallbackText={templateVersionDisplayName} /> From 597348b55dea60360bf61ede4cceb165a2732f00 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 26 Sep 2024 05:39:16 +0000 Subject: [PATCH 03/12] blah blah --- site/src/components/AvatarData/AvatarData.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index b9d46fb5f0f8d..f0c8657846094 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -33,7 +33,7 @@ export const AvatarData: FC = ({ const theme = useTheme(); avatar ??= ( - {typeof title === "string" ? title : (imgFallbackText ?? "-")} + {typeof title === "string" ? title : imgFallbackText ?? "-"} ); From a4096d5ba0b800febea72c9e6c17838a5cc1571b Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 26 Sep 2024 05:41:21 +0000 Subject: [PATCH 04/12] dead --- site/src/utils/appearance.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/site/src/utils/appearance.ts b/site/src/utils/appearance.ts index 9855ee1bcaf22..a7c2fb142b4f8 100644 --- a/site/src/utils/appearance.ts +++ b/site/src/utils/appearance.ts @@ -1,5 +1,3 @@ -import type { ReactFragment, ReactNode, ReactPortal } from "react"; - export const getApplicationName = (): string => { const c = document.head .querySelector("meta[name=application-name]") From 50cfa3fd8b121ed4b290f8f04c8aa940a01623af Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 27 Sep 2024 16:56:22 +0000 Subject: [PATCH 05/12] fix: update logic for handling fallback title --- site/src/components/AvatarData/AvatarData.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index f0c8657846094..39f16bf131fff 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -33,7 +33,7 @@ export const AvatarData: FC = ({ const theme = useTheme(); avatar ??= ( - {typeof title === "string" ? title : imgFallbackText ?? "-"} + {title === undefined ? (imgFallbackText ?? "-") : title} ); From 4cc2b56558c57432f6a0934c42ce31d408fbdf42 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 27 Sep 2024 16:58:17 +0000 Subject: [PATCH 06/12] fix: restore fallback icons for falsey strings --- site/src/components/AvatarData/AvatarData.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index 39f16bf131fff..457aa9a3f02a4 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -31,11 +31,13 @@ export const AvatarData: FC = ({ avatar, }) => { const theme = useTheme(); - avatar ??= ( - - {title === undefined ? (imgFallbackText ?? "-") : title} - - ); + if (!avatar) { + avatar = ( + + {title === undefined ? (imgFallbackText ?? "-") : title} + + ); + } return ( Date: Fri, 27 Sep 2024 16:59:08 +0000 Subject: [PATCH 07/12] refactor: clean up avatar fallback title logic --- site/src/components/AvatarData/AvatarData.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index 457aa9a3f02a4..aeee6af4d8afc 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -34,7 +34,7 @@ export const AvatarData: FC = ({ if (!avatar) { avatar = ( - {title === undefined ? (imgFallbackText ?? "-") : title} + {title ?? imgFallbackText ?? "-"} ); } From 5672d926d069b7ccd1f25248d2213d24348f122e Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 27 Sep 2024 19:05:01 +0000 Subject: [PATCH 08/12] fix: adjust color --- site/src/theme/dark/branding.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/theme/dark/branding.ts b/site/src/theme/dark/branding.ts index 614bf630b51bf..d3b47254df4dc 100644 --- a/site/src/theme/dark/branding.ts +++ b/site/src/theme/dark/branding.ts @@ -18,7 +18,7 @@ export const branding: Branding = { featureStage: { background: colors.sky[950], divider: colors.sky[900], - border: colors.sky[400], + border: colors.sky[600], text: colors.sky[400], hover: { From 5b9a6b36d57bb72654ed339d8db1056781627eb0 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 27 Sep 2024 19:30:22 +0000 Subject: [PATCH 09/12] revert: undo color change --- .../ManagementSettingsPage/SidebarView.tsx | 78 +++++++++---------- site/src/theme/dark/branding.ts | 2 +- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.tsx index eeae70bec80dd..954ca338d813b 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.tsx @@ -389,49 +389,49 @@ const styles = { const classNames = { link: (css, theme) => css` - color: inherit; - display: block; - font-size: 14px; - text-decoration: none; - padding: 10px 12px 10px 16px; - border-radius: 4px; - transition: background-color 0.15s ease-in-out; - position: relative; - - &:hover { - background-color: ${theme.palette.action.hover}; - } - - border-left: 3px solid transparent; - `, + color: inherit; + display: block; + font-size: 14px; + text-decoration: none; + padding: 10px 12px 10px 16px; + border-radius: 4px; + transition: background-color 0.15s ease-in-out; + position: relative; + + &:hover { + background-color: ${theme.palette.action.hover}; + } + + border-left: 3px solid transparent; + `, activeLink: (css, theme) => css` - border-left-color: ${theme.palette.primary.main}; - border-top-left-radius: 0; - border-bottom-left-radius: 0; - `, + border-left-color: ${theme.palette.primary.main}; + border-top-left-radius: 0; + border-bottom-left-radius: 0; + `, subLink: (css, theme) => css` - color: ${theme.palette.text.secondary}; - text-decoration: none; - - display: block; - font-size: 13px; - margin-left: 44px; - padding: 4px 12px; - border-radius: 4px; - transition: background-color 0.15s ease-in-out; - margin-bottom: 1px; - position: relative; - - &:hover { - color: ${theme.palette.text.primary}; - background-color: ${theme.palette.action.hover}; - } - `, + color: ${theme.palette.text.secondary}; + text-decoration: none; + + display: block; + font-size: 13px; + margin-left: 44px; + padding: 4px 12px; + border-radius: 4px; + transition: background-color 0.15s ease-in-out; + margin-bottom: 1px; + position: relative; + + &:hover { + color: ${theme.palette.text.primary}; + background-color: ${theme.palette.action.hover}; + } + `, activeSubLink: (css, theme) => css` - color: ${theme.palette.text.primary}; - font-weight: 600; - `, + color: ${theme.palette.text.primary}; + font-weight: 600; + `, } satisfies Record; diff --git a/site/src/theme/dark/branding.ts b/site/src/theme/dark/branding.ts index d3b47254df4dc..614bf630b51bf 100644 --- a/site/src/theme/dark/branding.ts +++ b/site/src/theme/dark/branding.ts @@ -18,7 +18,7 @@ export const branding: Branding = { featureStage: { background: colors.sky[950], divider: colors.sky[900], - border: colors.sky[600], + border: colors.sky[400], text: colors.sky[400], hover: { From 392e896bce2ad07418e495ee70ac74a4e7ee0efe Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 27 Sep 2024 20:00:55 +0000 Subject: [PATCH 10/12] fix: update typo in runtime logic --- site/src/components/AvatarData/AvatarData.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index aeee6af4d8afc..4365c9b491fd2 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -34,7 +34,7 @@ export const AvatarData: FC = ({ if (!avatar) { avatar = ( - {title ?? imgFallbackText ?? "-"} + {typeof title === "string" ? title || "-" : (imgFallbackText ?? "-")} ); } From dee7611ef5c7eab9986389a2849ebc617cfa815d Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 27 Sep 2024 20:07:28 +0000 Subject: [PATCH 11/12] fix: format --- site/src/components/AvatarData/AvatarData.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index 4365c9b491fd2..0376411ffe137 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -34,7 +34,7 @@ export const AvatarData: FC = ({ if (!avatar) { avatar = ( - {typeof title === "string" ? title || "-" : (imgFallbackText ?? "-")} + {typeof title === "string" ? title || "-" : imgFallbackText ?? "-"} ); } From 720ca7683c51ea0a174d3b8b5ad93234a41db0d9 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 2 Oct 2024 14:36:38 +0000 Subject: [PATCH 12/12] fix: apply suggestions --- site/src/components/AvatarData/AvatarData.tsx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/site/src/components/AvatarData/AvatarData.tsx b/site/src/components/AvatarData/AvatarData.tsx index 0376411ffe137..eb9fa81d4981d 100644 --- a/site/src/components/AvatarData/AvatarData.tsx +++ b/site/src/components/AvatarData/AvatarData.tsx @@ -10,15 +10,11 @@ export interface AvatarDataProps { avatar?: React.ReactNode; /** - * Useful for when you need to pass in a ReactNode for the title of the - * component. + * Lets you specify the character(s) displayed in an avatar when an image is + * unavailable (like when the network request fails). * - * MUI will try to take any string titles and turn them into the first - * character, but if you pass in a ReactNode, MUI can't do that, because it - * has no way to reliably grab the text content during renders. - * - * Tried writing some layout effect/JSX parsing logic to do the extraction, - * but it added complexity and render overhead, and wasn't reliable enough + * If not specified, the component will try to parse the first character + * from the title prop if it is a string. */ imgFallbackText?: string; } @@ -34,7 +30,7 @@ export const AvatarData: FC = ({ if (!avatar) { avatar = ( - {typeof title === "string" ? title || "-" : imgFallbackText ?? "-"} + {(typeof title === "string" ? title : imgFallbackText) || "-"} ); }