-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Shadcn migration - alert #13617
Shadcn migration - alert #13617
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@nloureiro tagging you since this needs design review. |
@pettinarip this one is for the alerts on top? like this one? |
No, we are not currently using them on prod. You can see them only on Chromatic/Storybook for now. You can see them on the UI tests check or in this link for the Storybook instance for this build. |
Great! will approve the UI tests for now then and mark this PR ready for review soon.
Ah hahaha, I don't know, was me doing silly things. Wanted to highlight the code name for each variant. Will remove the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice 💪 Looks great! Definitely an improvement for this component
--error-light: var(--error-light); | ||
--error: var(--red-500); | ||
--error-light: var(--red-100); | ||
--error-outline: var(--error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious the reason for the -outline
token variants?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm good call, we are not using it, right? I guess the idea was to use this color as the border
color. I missed that.
{...props} | ||
> | ||
<MdClose className="h-6 w-6" /> | ||
<span className="sr-only">Close</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice (ref for anyone else: https://tailwindcss.com/docs/screen-readers)
Description
Adds the new shadcn Alert component.