Skip to content

refactor(site): Refactor alerts #7587

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 13 commits into from
May 18, 2023
Merged
4 changes: 4 additions & 0 deletions site/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ rules:
message:
"You should use the Avatar component provided on
components/Avatar/Avatar"
- name: "@mui/material/Alert"
message:
"You should use the Alert component provided on
components/Alert/Alert"
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

no-unused-vars: "off"
"object-curly-spacing": "off"
react-hooks/exhaustive-deps: warn
Expand Down
61 changes: 61 additions & 0 deletions site/src/components/Alert/Alert.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Alert } from "./Alert"
import Button from "@mui/material/Button"
import Link from "@mui/material/Link"
import type { Meta, StoryObj } from "@storybook/react"

const meta: Meta<typeof Alert> = {
title: "components/Alert",
component: Alert,
}

export default meta
type Story = StoryObj<typeof Alert>

const ExampleAction = (
<Button onClick={() => null} size="small" variant="text">
Button
</Button>
)

export const Warning: Story = {
args: {
children: "This is a warning",
severity: "warning",
},
}

export const WarningWithDismiss: Story = {
args: {
children: "This is a warning",
dismissible: true,
severity: "warning",
},
}

export const WarningWithAction: Story = {
args: {
children: "This is a warning",
actions: [ExampleAction],
severity: "warning",
},
}

export const WarningWithActionAndDismiss: Story = {
args: {
children: "This is a warning",
actions: [ExampleAction],
dismissible: true,
severity: "warning",
},
}

export const WithChildren: Story = {
args: {
severity: "warning",
children: (
<div>
This is a message with a <Link href="#">link</Link>
</div>
),
},
}
63 changes: 63 additions & 0 deletions site/src/components/Alert/Alert.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { useState, FC, ReactNode, PropsWithChildren } from "react"
import Collapse from "@mui/material/Collapse"
// eslint-disable-next-line no-restricted-imports -- It is the base component
import MuiAlert, { AlertProps as MuiAlertProps } from "@mui/material/Alert"
import Button from "@mui/material/Button"

export interface AlertProps extends PropsWithChildren {
severity: MuiAlertProps["severity"]
Copy link
Member

Choose a reason for hiding this comment

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

If ErrorAlert is going to be a separate component, should we omit the severity error from this prop type? At first glance, it's hard to tell if I should be using ErrorAlert for errors, or Alert with severity="error" or both in different ways.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alert is the generic one with extra things and it should still be usable for errors if you need some customization. The advantage of using the ErrorAlert component is, you can pass the error and get the message and detail displayed without having to do it yourself.

actions?: ReactNode[]
dismissible?: boolean
onRetry?: () => void
onDismiss?: () => void
}

export const Alert: FC<AlertProps> = ({
children,
actions = [],
onRetry,
dismissible,
severity,
onDismiss,
}) => {
const [open, setOpen] = useState(true)

return (
<Collapse in={open}>
<MuiAlert
severity={severity}
action={
<>
{/* CTAs passed in by the consumer */}
{actions.length > 0 &&
actions.map((action) => <div key={String(action)}>{action}</div>)}

{/* retry CTA */}
{onRetry && (
<Button variant="text" size="small" onClick={onRetry}>
Retry
</Button>
)}

{/* close CTA */}
{dismissible && (
<Button
variant="text"
size="small"
onClick={() => {
setOpen(false)
onDismiss && onDismiss()
}}
data-testid="dismiss-banner-btn"
Copy link
Member

Choose a reason for hiding this comment

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

Should we stick an aria-label on here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since it has a text, I think we don't need it 🤔 but I'm not sure if I understood your point

Copy link
Member

Choose a reason for hiding this comment

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

No, you're right! I had to google it but I think since we have the text we can forget the label.

>
Dismiss
</Button>
)}
</>
}
>
{children}
</MuiAlert>
</Collapse>
)
}
77 changes: 77 additions & 0 deletions site/src/components/Alert/ErrorAlert.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import Button from "@mui/material/Button"
import { mockApiError } from "testHelpers/entities"
import type { Meta, StoryObj } from "@storybook/react"
import { action } from "@storybook/addon-actions"
import { ErrorAlert } from "./ErrorAlert"

const mockError = mockApiError({
message: "Email or password was invalid",
detail: "Password is invalid",
})

const meta: Meta<typeof ErrorAlert> = {
title: "components/ErrorAlert",
component: ErrorAlert,
args: {
error: mockError,
dismissible: false,
onRetry: undefined,
},
}

export default meta
type Story = StoryObj<typeof ErrorAlert>

const ExampleAction = (
<Button onClick={() => null} size="small" variant="text">
Button
</Button>
)

export const WithOnlyMessage: Story = {
args: {
error: mockApiError({
message: "Email or password was invalid",
}),
},
}

export const WithDismiss: Story = {
args: {
dismissible: true,
},
}

export const WithAction: Story = {
args: {
actions: [ExampleAction],
},
}

export const WithActionAndDismiss: Story = {
args: {
actions: [ExampleAction],
dismissible: true,
},
}

export const WithRetry: Story = {
args: {
onRetry: action("retry"),
dismissible: true,
},
}

export const WithActionRetryAndDismiss: Story = {
args: {
actions: [ExampleAction],
onRetry: action("retry"),
dismissible: true,
},
}

export const WithNonApiError: Story = {
args: {
error: new Error("Non API error here"),
},
}
32 changes: 32 additions & 0 deletions site/src/components/Alert/ErrorAlert.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { AlertProps, Alert } from "./Alert"
import AlertTitle from "@mui/material/AlertTitle"
import Box from "@mui/material/Box"
import { getErrorMessage, getErrorDetail } from "api/errors"
import { FC } from "react"

export const ErrorAlert: FC<
Omit<AlertProps, "severity" | "children"> & { error: unknown }
> = ({ error, ...alertProps }) => {
const message = getErrorMessage(error, "Something went wrong.")
const detail = getErrorDetail(error)

return (
<Alert severity="error" {...alertProps}>
{detail ? (
<>
<AlertTitle>{message}</AlertTitle>
<Box
component="span"
color={(theme) => theme.palette.text.secondary}
fontSize={13}
data-chromatic="ignore"
>
{detail}
</Box>
</>
) : (
message
)}
</Alert>
)
}
122 changes: 0 additions & 122 deletions site/src/components/AlertBanner/AlertBanner.stories.tsx

This file was deleted.

Loading