refactor: migrate _alert.scss to tailwind #2226
Conversation
| <form ref={formRef}> | ||
| {props.payouts_paused_by !== null ? ( | ||
| <Alert role="status" variant="warning" className="mx-8 mb-12"> | ||
| <Alert className="m-4 md:m-8" role="status" variant="warning"> |
There was a problem hiding this comment.
Fixing margins over here
| Before | After |
|---|---|
 |
PayoutsPage-After.mov |
| <div> | ||
| <Alert asChild role="status" variant="info"> | ||
| <small> | ||
| <span> | ||
| This sale was driven by a{" "} | ||
| <a href={link.utm_url} target="_blank" rel="noreferrer"> | ||
| UTM link | ||
| </a> | ||
| . | ||
| </span> | ||
| </small> | ||
| <Alert className="flex-1 text-sm" role="status" variant="info"> | ||
| <span> | ||
| This sale was driven by a{" "} | ||
| <a href={link.utm_url} target="_blank" rel="noreferrer"> | ||
| UTM link | ||
| </a> | ||
| . | ||
| </span> | ||
| </Alert> |
There was a problem hiding this comment.
The first one is before and second one is after. Slight increase in icon size over here
| export const AlertIcon = React.forwardRef<HTMLSpanElement, AlertIconProps>(({ children, className, ...props }, ref) => ( | ||
| <span ref={ref} className={classNames(className)} {...props}> | ||
| {children} | ||
| </span> | ||
| )); | ||
| AlertIcon.displayName = "AlertIcon"; |
There was a problem hiding this comment.
After #2181 (comment), I've removed the default icon override case and context/provider usage.
However, we still needed a way to insert custom icon for variant="accent" and for that the following two options came to my mind
// Custom icon as a prop
<Alert variant="accent" icon={<img src={hands} className="size-12 self-center" alt="" />}>
<div>Content</div>
</Alert>
// or
// Shadcn/radix style composition pattern
<Alert variant="accent">
<AlertIcon className="self-center">
<img src={hands} className="size-12" />
</AlertIcon>
<div>Content</div>
</Alert>I'm just slightly leaned towards radix style composition pattern because other base components are built using it and someone would expect Alert to work similarly.
No strong preference though, have used both patterns across various projects and both tend to do well in the long term
|
@binary-koan I've addresed some of the review comments please have a look Also, left a few comments on #2181 which might need some action |
3 participants
Part of
_alert.scssto tailwind #2181In this PR, we migrate the usage of
alert.scssfile indesign.scssto tailwind based component.Since there were a lot of the components, I've only attached before and after shots of the tricky ones and base variants. Please let me know if you need before/after shots of any to help with the review
Changes
Created
Alert+AlertIconcomponent with tailwind based styling for six variants namelyAlertsupports automatic icon rendering using variants + custom icon usage viaAlertIconUpdated
ToastAlert(server-components/Alert.tsx) to use the newAlertcomponent while preserving toast behavior[Bug-Fix] Raised
ToastAlertz-index to 100, allowing it to appear aboveSheets/ModalsScreenshots of changes from #2181
Payouts Page (variant="warning")
PayoutsPage-After.mov
UTM Link (variant="info") after removing
asChildfromAlertcomponentThere is a slight increase in size of icon over here because tailwind class usage. Line height has increased because of it from ~1.3 to ~1.4
Shots from previous PR
In all of the comparision videos/image, the tailwind one is below the exisiting scss based
AlertAlert Variants Responsive Showcase (5 existing types - Default/Danger/Info/Success/Warning)
BaseVariants-Bef.Aft-light.mov
BaseVariants-Bef.Aft-dark.mov
ToastAlertResponsive Showcase (Light + Dark)
ToastAlert-Bef.Aft.mov
Bug fix
Increase z-index to make
ToastAlertappear above sheet/modalsbefore-z-index-30.mov
after-z-index-100.mov
AINotification-custom-Bef.Aft.mov
BlackFridayDiscountInfo-bef.aft.mov
DiscountsPage-BlackFriday-bef.aft.mov
DiscoverEligibility-bef.aft.mov
ProductNew-Bef.Aft.mov
SettingsTeamCloseButton.undo.position.-bef.after.mov
AI Usage
No AI usage after #2181
Live Stream Disclosure