fix: Better alignment for Alert icons

[frankieyan]

Short description

This fixes the alignment for icons in the <Alert> component, spotted by @pauloslund over at https://github.com/Doist/todoist-web/pull/8223#pullrequestreview-1706995679

PR Checklist

• Added tests for bugs / new features
• Updated docs (storybooks, readme)
• Executed npm run validate and made sure no errors / warnings were shown
• Described changes in CHANGELOG.md
• Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
• Reviewed and approved Chromatic visual regression tests in CI

Versioning

Patch

[pauloslund]

Unfortunately, this results in a similar but different issue. On single line alerts without a close button, the icon and the content itself end up not being vertically aligned within the alert. You can see in the image below that the alignment between the two Alerts don't quite match:

I think part of this is caused by the min-height being calculated for the container.

I have a solution for it though! This is the diff for the changes I'd make:

diff --git a/src/alert/alert.module.css b/src/alert/alert.module.css
index d5b95121..e319e425 100644
--- a/src/alert/alert.module.css
+++ b/src/alert/alert.module.css
@@ -3,8 +3,14 @@
     border-width: 1px;
     color: var(--reactist-content-primary);
     padding: var(--reactist-spacing-small);
-    /* this is to make sure it always has the same minimum height, whether it has a close button or not */
-    min-height: calc(2 * var(--reactist-spacing-small) + var(--reactist-button-small-height) + 2px);
+}
+
+.icon {
+    display: block;
+}
+
+.content {
+    /* this is to make sure it always has the same minimum height, whether it has a close button or not */
+    min-height: var(--reactist-button-small-height);
 }

 .tone-info {
diff --git a/src/alert/alert.tsx b/src/alert/alert.tsx
index 9bf372ba..00399ac6 100644
--- a/src/alert/alert.tsx
+++ b/src/alert/alert.tsx
@@ -35,14 +35,15 @@ function Alert({ id, children, tone, closeLabel, onClose }: AlertProps) {
         >
             <Columns space="small" alignY="center">
                 <Column width="content">
-                    <Box display="flex" alignItems="center">
-                        <AlertIcon tone={tone} className={styles.icon} />
-                    </Box>
+                    <AlertIcon tone={tone} className={styles.icon} />
                 </Column>
                 <Column>
                     <Box
                         paddingY="xsmall"
                         paddingRight={onClose != null && closeLabel != null ? undefined : 'small'}
+                        display="flex"
+                        alignItems="center"
+                        className={styles.content}
                     >
                         {children}
                     </Box>

There are two fixes here:

1. Setting the .icon to display: block; solves the icon centering issue. However, this results in the same problem of the icon and content not being centered within the alert itself.
2. Move the minimum height to the alert content instead of the alert container.

This is what it looks like after those changes:

Thoughts?

[frankieyan]

I will take care of this when I'm back after OOO!

[frankieyan]

@pauloslund I'm embarrassed that this took almost a year, but I've added your suggested changes now 🥲

[frankieyan]

We lost the dismiss button awhile ago due to the IconButton change, so we're bringing it back here as well: