Browse Source

ref: remove default margin from Alert in sentry (#85128)

## Context

Because `Alert` used to have a default bottom margin, we had a lot of
instances of styled `Alerts` across the codebase that looked like this:

```
const StyledAlert = styled(Alert)`
   margin: 0;
`;
```

The goal was to remove these instances entirely. In this PR, the default
margin is removed from `Alert` and placed inside a `Alert.Container`
export instead.

This PR also updates all `Alert` instances to either
- wrap with `Alert.Container` if it was using the default margins of the
`Alert` component, or
- remove the `margin: 0` style

In summary:

| BEFORE | AFTER |
| -------- | ------- |
| <img width="225" alt="SCR-20250212-pdos"
src="https://github.com/user-attachments/assets/592f8ca1-e0d5-4dc4-b17d-86637e064f63"
/> | <img width="302" alt="SCR-20250212-pdnt"
src="https://github.com/user-attachments/assets/a238fc09-12a5-486c-8292-8019cec5dd9d"
/> |
| <img width="319" alt="SCR-20250212-pdyh"
src="https://github.com/user-attachments/assets/92afabfe-1543-4588-83a5-fe763c20ef80"
/> | <img width="221" alt="SCR-20250212-peah"
src="https://github.com/user-attachments/assets/0538494d-c432-46c4-bab1-245475e8f15a"
/> |
| <img width="370" alt="SCR-20250212-peqt"
src="https://github.com/user-attachments/assets/60f22081-5eeb-4594-8b6d-acfd974a6875"
/> | <img width="328" alt="SCR-20250212-petn"
src="https://github.com/user-attachments/assets/75e458d5-b056-4211-ac69-bc5d2b54596a"
/>|
| <img width="328" alt="SCR-20250212-petn"
src="https://github.com/user-attachments/assets/75e458d5-b056-4211-ac69-bc5d2b54596a"
/>| <img width="342" alt="SCR-20250212-pfbk"
src="https://github.com/user-attachments/assets/3cbb1f31-f7da-4a5d-a93c-e130ddfcd147"
/> |
|<img width="342" alt="SCR-20250212-phuy"
src="https://github.com/user-attachments/assets/78d7fb1f-297b-4de5-9835-754c952328b2"
/>|<img width="342" alt="SCR-20250212-phuy"
src="https://github.com/user-attachments/assets/78d7fb1f-297b-4de5-9835-754c952328b2"
/> (no change)|


      


## tldr, using `Alert` going forward

`Alert` will no longer have a default bottom margin. If you'd still like
to utilize that old built-in margin, you can wrap your component with
`<Alert.Container>`:

```
<Alert.Container>
   <Alert/>
</Alert.Container>
```

note that components that utilize `Alert` as a child, such as
`PanelAlert` and `ProjectPermissionAlert`, already have the
`<Alert.Container>` wrapped around it inside the component definition.

relates to https://github.com/getsentry/sentry/issues/85109
see also https://github.com/getsentry/getsentry/pull/16482 (getsentry
counterpart)
Michelle Zhang 3 weeks ago
parent
commit
72c69690f2

+ 5 - 3
static/app/components/acl/comingSoon.tsx

@@ -3,9 +3,11 @@ import {t} from 'sentry/locale';
 
 function ComingSoon() {
   return (
-    <Alert type="info" showIcon>
-      {t('This feature is coming soon!')}
-    </Alert>
+    <Alert.Container>
+      <Alert type="info" showIcon>
+        {t('This feature is coming soon!')}
+      </Alert>
+    </Alert.Container>
   );
 }
 

+ 5 - 3
static/app/components/acl/featureDisabled.tsx

@@ -117,9 +117,11 @@ function FeatureDisabled({
 
   const AlertComponent = typeof alert === 'boolean' ? Alert : alert;
   return (
-    <AlertComponent type="warning" showIcon expand={renderHelp()}>
-      {message}
-    </AlertComponent>
+    <Alert.Container>
+      <AlertComponent type="warning" showIcon expand={renderHelp()}>
+        {message}
+      </AlertComponent>
+    </Alert.Container>
   );
 }
 

+ 81 - 49
static/app/components/alert.stories.tsx

@@ -43,21 +43,23 @@ export default storyBook('Alert', story => {
           The <JSXProperty name="type" value /> prop specifies the alert category, and
           changes the color and default icon of the alert.
         </p>
-        <Alert type="error" showIcon>
-          This is an error alert. Something went wrong.
-        </Alert>
-        <Alert type="info" showIcon>
-          Info alert. Put some exciting info here.
-        </Alert>
-        <Alert type="muted" showIcon>
-          Muted alerts look like this.
-        </Alert>
-        <Alert type="success" showIcon>
-          Success alert. Yay!
-        </Alert>
-        <Alert type="warning" showIcon>
-          Warning alert. Something is about to go wrong, probably.
-        </Alert>
+        <Alert.Container>
+          <Alert type="error" showIcon>
+            This is an error alert. Something went wrong.
+          </Alert>
+          <Alert type="info" showIcon>
+            Info alert. Put some exciting info here.
+          </Alert>
+          <Alert type="muted" showIcon>
+            Muted alerts look like this.
+          </Alert>
+          <Alert type="success" showIcon>
+            Success alert. Yay!
+          </Alert>
+          <Alert type="warning" showIcon>
+            Warning alert. Something is about to go wrong, probably.
+          </Alert>
+        </Alert.Container>
       </Fragment>
     );
   });
@@ -70,12 +72,14 @@ export default storyBook('Alert', story => {
           customize the default icon. Just directly pass in the icon, like{' '}
           <JSXNode name="IconDelete" /> for example, to the prop.
         </p>
-        <Alert type="warning" showIcon icon={<IconDelete />}>
-          Are you sure you want to delete?
-        </Alert>
-        <Alert type="error" showIcon icon={<IconSad />}>
-          Oh no!
-        </Alert>
+        <Alert.Container>
+          <Alert type="warning" showIcon icon={<IconDelete />}>
+            Are you sure you want to delete?
+          </Alert>
+          <Alert type="error" showIcon icon={<IconSad />}>
+            Oh no!
+          </Alert>
+        </Alert.Container>
       </Fragment>
     );
   });
@@ -88,12 +92,14 @@ export default storyBook('Alert', story => {
           <code>false</code> by default.
         </p>
         <SizingWindow display="block">
-          <Alert type="success" showIcon opaque>
-            This one is opaque.
-          </Alert>
-          <Alert type="success" showIcon>
-            This is not opaque.
-          </Alert>
+          <Alert.Container>
+            <Alert type="success" showIcon opaque>
+              This one is opaque.
+            </Alert>
+            <Alert type="success" showIcon>
+              This is not opaque.
+            </Alert>
+          </Alert.Container>
         </SizingWindow>
       </Fragment>
     );
@@ -107,12 +113,14 @@ export default storyBook('Alert', story => {
           <JSXProperty name="defaultExpanded" value /> props can be used to show
           additional content when the alert is expanded.
         </p>
-        <Alert type="info" showIcon expand="Some extra info here.">
-          Expand me
-        </Alert>
-        <Alert type="info" showIcon defaultExpanded expand="Some extra info here.">
-          This one is expanded by default.
-        </Alert>
+        <Alert.Container>
+          <Alert type="info" showIcon expand="Some extra info here.">
+            Expand me
+          </Alert>
+          <Alert type="info" showIcon defaultExpanded expand="Some extra info here.">
+            This one is expanded by default.
+          </Alert>
+        </Alert.Container>
       </Fragment>
     );
   });
@@ -131,23 +139,26 @@ export default storyBook('Alert', story => {
           enable this functionality with local storage. Or you can use{' '}
           <code>useState</code> to bring it back on re-render.
         </p>
+
         {isDismissed ? null : (
-          <Alert
-            type="info"
-            showIcon
-            icon={<IconSentry />}
-            trailingItems={
-              <Button
-                aria-label="Dismiss banner"
-                icon={<IconClose />}
-                onClick={dismiss}
-                size="zero"
-                borderless
-              />
-            }
-          >
-            This alert can be dismissed!
-          </Alert>
+          <Alert.Container>
+            <Alert
+              type="info"
+              showIcon
+              icon={<IconSentry />}
+              trailingItems={
+                <Button
+                  aria-label="Dismiss banner"
+                  icon={<IconClose />}
+                  onClick={dismiss}
+                  size="zero"
+                  borderless
+                />
+              }
+            >
+              This alert can be dismissed!
+            </Alert>
+          </Alert.Container>
         )}
         {stateDismissed ? (
           <Button onClick={() => setStateDismissed(false)}>Bring the alert back</Button>
@@ -172,4 +183,25 @@ export default storyBook('Alert', story => {
       </Fragment>
     );
   });
+
+  story('Alert.Container', () => {
+    return (
+      <Fragment>
+        <p>
+          The <JSXNode name="Alert" /> component is marginless by default. The{' '}
+          <JSXNode name="Alert.Container" /> component is an exported wrapper in the same
+          file that adds the original bottom margin back in, and also automatically spaces
+          all items within the container evenly.
+        </p>
+        <Alert.Container>
+          <Alert type="info" showIcon>
+            These two alerts...
+          </Alert>
+          <Alert type="info" showIcon>
+            ...are both in one container.
+          </Alert>
+        </Alert.Container>
+      </Fragment>
+    );
+  });
 });

+ 11 - 1
static/app/components/alert.tsx

@@ -167,7 +167,6 @@ const AlertContainer = styled('div')<
     ${p => defined(p.trailingItems) && 'max-content'}
     ${p => defined(p.expand) && 'max-content'};
   gap: ${space(1)};
-  margin: 0 0 ${space(2)};
   color: ${p => p.alertColors.color};
   font-size: ${p => p.theme.fontSizeMedium};
   border-radius: ${p => p.theme.borderRadius};
@@ -294,3 +293,14 @@ function AlertIcon({type}: {type: AlertProps['type']}): React.ReactNode {
 
   return null;
 }
+
+// Spaces items within the container evenly.
+const Container = styled('div')`
+  > div {
+    margin-bottom: ${space(2)};
+  }
+`;
+
+Alert.Container = Container;
+
+export default Alert;

+ 14 - 12
static/app/components/alerts/onDemandMetricAlert.tsx

@@ -52,18 +52,20 @@ export function OnDemandMetricAlert({
   }
 
   return (
-    <InfoAlert type="info" showIcon>
-      {message}
-      {dismissable && (
-        <DismissButton
-          priority="link"
-          size="sm"
-          icon={<IconClose />}
-          aria-label={t('Close Alert')}
-          onClick={dismiss}
-        />
-      )}
-    </InfoAlert>
+    <Alert.Container>
+      <InfoAlert type="info" showIcon>
+        {message}
+        {dismissable && (
+          <DismissButton
+            priority="link"
+            size="sm"
+            icon={<IconClose />}
+            aria-label={t('Close Alert')}
+            onClick={dismiss}
+          />
+        )}
+      </InfoAlert>
+    </Alert.Container>
   );
 }
 

+ 11 - 7
static/app/components/alerts/unsupportedAlert.tsx

@@ -9,12 +9,16 @@ interface Props {
 
 export default function UnsupportedAlert({featureName, projectSlug}: Props) {
   return (
-    <Alert data-test-id="unsupported-alert" type="info" icon={<IconInfo />}>
-      {projectSlug ? (
-        <strong>{t(`%s isn't available for %s.`, featureName, projectSlug)}</strong>
-      ) : (
-        <strong>{t(`%s isn't available for the selected projects.`, featureName)}</strong>
-      )}
-    </Alert>
+    <Alert.Container>
+      <Alert data-test-id="unsupported-alert" type="info" icon={<IconInfo />}>
+        {projectSlug ? (
+          <strong>{t(`%s isn't available for %s.`, featureName, projectSlug)}</strong>
+        ) : (
+          <strong>
+            {t(`%s isn't available for the selected projects.`, featureName)}
+          </strong>
+        )}
+      </Alert>
+    </Alert.Container>
   );
 }

+ 3 - 1
static/app/components/confirmDelete.tsx

@@ -22,7 +22,9 @@ function ConfirmDelete({message, confirmInput, ...props}: Props) {
       disableConfirmButton
       renderMessage={({disableConfirmButton}) => (
         <Fragment>
-          <Alert type="error">{message}</Alert>
+          <Alert.Container>
+            <Alert type="error">{message}</Alert>
+          </Alert.Container>
           <FieldGroup
             flexibleControlStateSize
             inline={false}

+ 5 - 3
static/app/components/customIgnoreDurationModal.tsx

@@ -96,9 +96,11 @@ export default function CustomIgnoreDurationModal(props: Props) {
         </form>
       </Body>
       {dateWarning && (
-        <Alert type="error" showIcon>
-          {t('Please enter a valid date in the future')}
-        </Alert>
+        <Alert.Container>
+          <Alert type="error" showIcon>
+            {t('Please enter a valid date in the future')}
+          </Alert>
+        </Alert.Container>
       )}
       <Footer>
         <ButtonBar gap={1}>

+ 0 - 1
static/app/components/discover/performanceCardTable.tsx

@@ -830,7 +830,6 @@ const StyledAlert = styled(Alert)`
   border-top: 1px solid ${p => p.theme.border};
   border-right: 1px solid ${p => p.theme.border};
   border-left: 1px solid ${p => p.theme.border};
-  margin-bottom: 0;
 `;
 
 const StyledNotAvailable = styled(NotAvailable)`

+ 5 - 3
static/app/components/errorBoundary.tsx

@@ -110,9 +110,11 @@ class ErrorBoundary extends Component<Props, State> {
 
     if (mini) {
       return (
-        <Alert type="error" showIcon className={className}>
-          {message || t('There was a problem rendering this component')}
-        </Alert>
+        <Alert.Container>
+          <Alert type="error" showIcon className={className}>
+            {message || t('There was a problem rendering this component')}
+          </Alert>
+        </Alert.Container>
       );
     }
 

Some files were not shown because too many files changed in this diff