refactor(site): Refactor alerts #7587
Conversation
| - name: "@mui/material/Alert" | ||
| message: | ||
| "You should use the Alert component provided on | ||
| components/Alert/Alert" |
|
Feels like the icon on the "info" severity alert doesn't have enough contrast: |
|
I think I was under the mistaken impression that we were going to be using the snackbar from this PR as the new alert widget going forward. Is that just for version updates? When do you think we should use an alert vs a toast vs a snackbar? Asking so I can get it right in my upcoming PR :) |
| import Button from "@mui/material/Button" | ||
|
|
||
| export interface AlertProps extends PropsWithChildren { | ||
| severity: MuiAlertProps["severity"] |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| severity="error" | ||
| text="No authentication methods configured!" | ||
| /> | ||
| <Alert severity="error">No authentication methods configured!</Alert> |
There was a problem hiding this comment.
Related to my comment above, what is the difference between these two ways of displaying errors?
There was a problem hiding this comment.
I answered in the previous comment.
| setOpen(false) | ||
| onDismiss && onDismiss() | ||
| }} | ||
| data-testid="dismiss-banner-btn" |
There was a problem hiding this comment.
Should we stick an aria-label on here?
There was a problem hiding this comment.
Since it has a text, I think we don't need it 🤔 but I'm not sure if I understood your point
There was a problem hiding this comment.
No, you're right! I had to google it but I think since we have the text we can forget the label.
This reverts commit 709b1ed.
This is a REALLY good question and I'm not sure if I have a good answer for that, but I think snack bars are good for displaying errors or warnings that need to be displayed after a certain action as feedback like an update or delete error after a user clicked on a button. Alerts are good for static warnings where the users didn't trigger any action like deprecation notices and errors during initial loading but any of these are "written in stone" and we may find situations where display alerts for actions feedback or display snack bars for static warnings can be better, the PR that you mentioned is one of these cases. We didn't have a good place to place the "global alert" so it was just better and easy to make it a snack bar. |
This seems like a good distinction to me! |
Motivation:
Our alerts were inconsistent when displaying messages with details or actions, looking a bit ugly and unprofessional.
Changes:
AlertBannertoAlert.Alertis our custom component with a few standard features we usually need when displaying errors and warnings.ErrorAlert. It extends theAlertcomponent and uses thegetErrorMessageandgetErrorDetailto mount the text that will be displayedPreview:
Before:
After:
Before:
After:
