-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Update Alert banner message-body to match design system #9463
Conversation
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.
I really like the way you approached this (creating a new scss file for the component). Just one small comment, but looks great!
@@ -0,0 +1,6 @@ | |||
.alert-banner-message-body { | |||
border: 0; | |||
margin-top: $spacing-xxs; |
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.
it looks like the border
and margin-top
here have the same values as the original message-body
class. in that case, what do you think about extending message-body
so these styles aren't duplicated? this way if we make any other styling changes to all messages (perhaps a new font or font weight, etc), they'll apply here too. i.e.:
.alert-banner-message-body {
@extend .message-body
color: $black;
}
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.
Ah, that's a great idea. I didn't know you could do that. Thanks!
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.
@noelledaley ... um. 🤔 this doesn't seem to work because message-body
color is overriding the black here. I would have to use !important
after black to make it work, and that doesn't seem like an optimal solution.
* make alert banner text black * remove comment in scss file
The new design system has the alert banner text black. This PR updates that.
To have the least impact on other elements/components using the
.message-body
class I created a new .scss file for alert-banners and renamed the class it's using. This keeps the change contained to thealert-banner
component.Here are the design system pictures:

Here is the before the fix:

Here is the fix with the PR:
