Skip to content

refactor(site): Normalize avatar components #5860

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions site/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ rules:
message:
"Use path imports to avoid pulling in unused modules. See:
https://material-ui.com/guides/minimizing-bundle-size/"
- name: "@material-ui/core/Avatar"
message:
"You should use the Avatar component provided on
components/Avatar/Avatar"
no-unused-vars: "off"
"object-curly-spacing": "off"
react-hooks/exhaustive-deps: warn
Expand Down
61 changes: 61 additions & 0 deletions site/src/components/Avatar/Avatar.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Story } from "@storybook/react"
import { Avatar, AvatarIcon, AvatarProps } from "./Avatar"
import PauseIcon from "@material-ui/icons/PauseOutlined"

export default {
title: "components/Avatar",
component: Avatar,
}

const Template: Story<AvatarProps> = (args: AvatarProps) => <Avatar {...args} />

export const Letter = Template.bind({})
Letter.args = {
children: "Coder",
}

export const LetterXL = Template.bind({})
LetterXL.args = {
children: "Coder",
size: "xl",
}

export const LetterDarken = Template.bind({})
LetterDarken.args = {
children: "Coder",
colorScheme: "darken",
}

export const Image = Template.bind({})
Image.args = {
src: "https://avatars.githubusercontent.com/u/95932066?s=200&v=4",
}

export const ImageXL = Template.bind({})
ImageXL.args = {
src: "https://avatars.githubusercontent.com/u/95932066?s=200&v=4",
size: "xl",
}

export const MuiIcon = Template.bind({})
MuiIcon.args = {
children: <PauseIcon />,
}

export const MuiIconDarken = Template.bind({})
MuiIconDarken.args = {
children: <PauseIcon />,
colorScheme: "darken",
}

export const MuiIconXL = Template.bind({})
MuiIconXL.args = {
children: <PauseIcon />,
size: "xl",
}

export const AvatarIconDarken = Template.bind({})
AvatarIconDarken.args = {
children: <AvatarIcon src="/icon/database.svg" />,
colorScheme: "darken",
}
77 changes: 77 additions & 0 deletions site/src/components/Avatar/Avatar.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// This is the only place MuiAvatar can be used
// eslint-disable-next-line no-restricted-imports -- Read above
import MuiAvatar, {
AvatarProps as MuiAvatarProps,
} from "@material-ui/core/Avatar"
import { makeStyles } from "@material-ui/core/styles"
import { FC } from "react"
import { combineClasses } from "util/combineClasses"
import { firstLetter } from "./firstLetter"

export type AvatarProps = MuiAvatarProps & {
size?: "md" | "xl"
colorScheme?: "light" | "darken"
fitImage?: boolean
}

export const Avatar: FC<AvatarProps> = ({
size = "md",
colorScheme = "light",
fitImage,
className,
children,
...muiProps
}) => {
const styles = useStyles()

return (
<MuiAvatar
{...muiProps}
className={combineClasses([
className,
styles[size],
styles[colorScheme],
fitImage && styles.fitImage,
])}
>
{/* If the children is a string, we always want to render the first letter */}
{typeof children === "string" ? firstLetter(children) : children}
</MuiAvatar>
)
}

/**
* Use it to make an img element behaves like a MaterialUI Icon component
*/
export const AvatarIcon: FC<{ src: string }> = ({ src }) => {
const styles = useStyles()
return <img src={src} alt="" className={styles.avatarIcon} />
}

const useStyles = makeStyles((theme) => ({
// Size styles
// Just use the default value from theme
md: {},
xl: {
width: theme.spacing(6),
height: theme.spacing(6),
fontSize: theme.spacing(3),
},
// Colors
// Just use the default value from theme
light: {},
darken: {
background: theme.palette.divider,
color: theme.palette.text.primary,
},
// Avatar icon
avatarIcon: {
maxWidth: "50%",
},
// Fit image
fitImage: {
"& .MuiAvatar-img": {
objectFit: "contain",
},
},
}))
File renamed without changes.
13 changes: 3 additions & 10 deletions site/src/components/AvatarData/AvatarData.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,9 @@ Example.args = {
subtitle: "coder@coder.com",
}

export const WithHighlightTitle = Template.bind({})
WithHighlightTitle.args = {
export const WithImage = Template.bind({})
WithImage.args = {
title: "coder",
subtitle: "coder@coder.com",
highlightTitle: true,
}

export const WithLink = Template.bind({})
WithLink.args = {
title: "coder",
subtitle: "coder@coder.com",
link: "/users/coder",
src: "https://avatars.githubusercontent.com/u/95932066?s=200&v=4",
}
67 changes: 23 additions & 44 deletions site/src/components/AvatarData/AvatarData.tsx
Original file line number Diff line number Diff line change
@@ -1,71 +1,50 @@
import Avatar from "@material-ui/core/Avatar"
import Link from "@material-ui/core/Link"
import { makeStyles } from "@material-ui/core/styles"
import { Avatar } from "components/Avatar/Avatar"
import { FC, PropsWithChildren } from "react"
import { Link as RouterLink } from "react-router-dom"
import { firstLetter } from "../../util/firstLetter"
import {
TableCellData,
TableCellDataPrimary,
TableCellDataSecondary,
} from "../TableCellData/TableCellData"
import { Stack } from "components/Stack/Stack"
import { makeStyles } from "@material-ui/core/styles"

export interface AvatarDataProps {
title: string
subtitle?: string
highlightTitle?: boolean
link?: string
src?: string
avatar?: React.ReactNode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not really important, just a thought but would it make sense to make AvatarData take Avatar props and pass them along rather than accept avatar? My reasoning is that it seems unfortunate that there are two ways of doing things like:

<AvatarData src={src} />

and

<AvatarData avatar={<Avatar src={src} />} />

Or alternatively we could always make it so we always have to pass an avatar, either way works so there is one way of doing things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AvatarData is a bad name for what it does and not a good abstraction. Wondering if Heading or DataHeading would be better with the following interface:

<Heading
  title="Bruno"
  subtitle="bruno@hotmail.com"
  leftElement={<UserAvatar user="{user}" />}
/>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely agree!

}

export const AvatarData: FC<PropsWithChildren<AvatarDataProps>> = ({
title,
subtitle,
link,
highlightTitle,
src,
avatar,
}) => {
const styles = useStyles()

if (!avatar) {
avatar = <Avatar>{firstLetter(title)}</Avatar>
avatar = <Avatar src={src}>{title}</Avatar>
}

return (
<div className={styles.root}>
<div className={styles.avatarWrapper}>{avatar}</div>
<Stack spacing={1.5} direction="row" alignItems="center">
{avatar}

{link ? (
<Link to={link} underline="none" component={RouterLink}>
<TableCellData>
<TableCellDataPrimary highlight={highlightTitle}>
{title}
</TableCellDataPrimary>
{subtitle && (
<TableCellDataSecondary>{subtitle}</TableCellDataSecondary>
)}
</TableCellData>
</Link>
) : (
<TableCellData>
<TableCellDataPrimary highlight={highlightTitle}>
{title}
</TableCellDataPrimary>
{subtitle && (
<TableCellDataSecondary>{subtitle}</TableCellDataSecondary>
)}
</TableCellData>
)}
</div>
<Stack spacing={0}>
<span className={styles.title}>{title}</span>
{subtitle && <span className={styles.subtitle}>{subtitle}</span>}
</Stack>
</Stack>
)
}

const useStyles = makeStyles((theme) => ({
root: {
display: "flex",
alignItems: "center",
title: {
color: theme.palette.text.primary,
fontWeight: 600,
},
avatarWrapper: {
marginRight: theme.spacing(1.5),

subtitle: {
fontSize: 12,
color: theme.palette.text.secondary,
lineHeight: "140%",
marginTop: 2,
maxWidth: 540,
},
}))
28 changes: 5 additions & 23 deletions site/src/components/BuildsTable/BuildAvatar.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import Avatar from "@material-ui/core/Avatar"
import Badge from "@material-ui/core/Badge"
import { Theme, useTheme, withStyles } from "@material-ui/core/styles"
import { FC } from "react"
Expand All @@ -8,6 +7,7 @@ import DeleteOutlined from "@material-ui/icons/DeleteOutlined"
import { WorkspaceBuild, WorkspaceTransition } from "api/typesGenerated"
import { getDisplayWorkspaceBuildStatus } from "util/workspace"
import { PaletteIndex } from "theme/palettes"
import { Avatar, AvatarProps } from "components/Avatar/Avatar"

interface StylesBadgeProps {
type: PaletteIndex
Expand All @@ -25,27 +25,9 @@ const StyledBadge = withStyles((theme) => ({
},
}))(Badge)

interface StyledAvatarProps {
size?: number
}

const StyledAvatar = withStyles((theme) => ({
root: {
background: theme.palette.divider,
color: theme.palette.text.primary,
border: `2px solid ${theme.palette.divider}`,
width: ({ size }: StyledAvatarProps) => size,
height: ({ size }: StyledAvatarProps) => size,

"& svg": {
width: ({ size }: StyledAvatarProps) => (size ? size / 2 : 18),
height: ({ size }: StyledAvatarProps) => (size ? size / 2 : 18),
},
},
}))(Avatar)

export interface BuildAvatarProps extends StyledAvatarProps {
export interface BuildAvatarProps {
build: WorkspaceBuild
size?: AvatarProps["size"]
}

const iconByTransition: Record<WorkspaceTransition, JSX.Element> = {
Expand All @@ -71,9 +53,9 @@ export const BuildAvatar: FC<BuildAvatarProps> = ({ build, size }) => {
}}
badgeContent={<div></div>}
>
<StyledAvatar size={size}>
<Avatar size={size} colorScheme="darken">
{iconByTransition[build.transition]}
</StyledAvatar>
</Avatar>
</StyledBadge>
)
}
5 changes: 2 additions & 3 deletions site/src/components/GroupAvatar/GroupAvatar.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import Avatar from "@material-ui/core/Avatar"
import { Avatar } from "components/Avatar/Avatar"
import Badge from "@material-ui/core/Badge"
import { withStyles } from "@material-ui/core/styles"
import Group from "@material-ui/icons/Group"
import { FC } from "react"
import { firstLetter } from "util/firstLetter"

const StyledBadge = withStyles((theme) => ({
badge: {
Expand Down Expand Up @@ -38,7 +37,7 @@ export const GroupAvatar: FC<GroupAvatarProps> = ({ name, avatarURL }) => {
}}
badgeContent={<Group />}
>
<Avatar src={avatarURL}>{firstLetter(name)}</Avatar>
<Avatar src={avatarURL}>{name}</Avatar>
</StyledBadge>
)
}
22 changes: 6 additions & 16 deletions site/src/components/Resources/ResourceAvatar.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import Avatar from "@material-ui/core/Avatar"
import { makeStyles } from "@material-ui/core/styles"
import { Avatar, AvatarIcon } from "components/Avatar/Avatar"
import { FC } from "react"
import { WorkspaceResource } from "../../api/typesGenerated"

const FALLBACK_ICON = "/icon/widgets.svg"

// NOTE @jsjoeio, @BrunoQuaresma
// These resources (i.e. docker_image, kubernetes_deployment) map to Terraform
// resource types. These are the most used ones and are based on user usage.
// We may want to update from time-to-time.
Expand Down Expand Up @@ -37,18 +35,10 @@ export type ResourceAvatarProps = { resource: WorkspaceResource }
export const ResourceAvatar: FC<ResourceAvatarProps> = ({ resource }) => {
const hasIcon = resource.icon && resource.icon !== ""
const avatarSrc = hasIcon ? resource.icon : getIconPathResource(resource.type)
const styles = useStyles()

return <Avatar className={styles.resourceAvatar} src={avatarSrc} />
return (
<Avatar colorScheme="darken">
<AvatarIcon src={avatarSrc} />
</Avatar>
)
}

const useStyles = makeStyles((theme) => ({
resourceAvatar: {
backgroundColor: theme.palette.divider,

"& img": {
width: 18,
height: 18,
},
},
}))
Loading