-
Notifications
You must be signed in to change notification settings - Fork 973
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
USWDS - Site Alert - Repair top margin, broken by #4922 #5550
USWDS - Site Alert - Repair top margin, broken by #4922 #5550
Conversation
PR uswds#4922 added a top margin to the site alert, which looks wrong when a site alert immediately follows a gov banner.
Great patch. We're going to work to get this into 3.7.0. |
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.
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.
Lgtm! Thanks for taking care of this @lpsinger 👏
- Site alert no longer has a margin between banner and alert
- Standard alerts are unaffected by this change
- No visual regressions or build errors
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.
Thank you!
Summary
Removed the top margin from the site alert component.
PR #4922 added a top margin to the site alert, which looks wrong when a site alert immediately follows a gov banner.
Breaking change
This is not a breaking change.
Related PR
Changelog PR
Preview link
This affects https://test.gcn.nasa.gov.
USWDS 3.1.0
USWDS 3.6.1, before this patch
After this patch
Problem statement
The site alert should not have any top margin. It didn't before USWDS 3.6.1. The margin looks like a glitch when a site alert immediately follows a gov banner.
Solution
Move the top margin back from the mixin to _usa-alert.scss so that applies to alerts but not site alerts.
Testing and review
I tested this locally to produce the screenshots above.
Dependency updates
None.