-
Notifications
You must be signed in to change notification settings - Fork 331
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
Reintroduce additional bottom margin to Error Summary content #5060
Conversation
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for 9c6828e |
Stylesheets changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
index de0a3a8db..f0aa66d38 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
@@ -3184,6 +3184,10 @@ screen and (forced-colors:active) {
}
}
+.govuk-error-summary__body>:last-child {
+ margin-bottom: 5px
+}
+
.govuk-error-summary__list,
.govuk-error-summary__list li:last-child {
margin-bottom: 0
Action run for 9c6828e |
Other changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/components/error-summary/_index.scss b/packages/govuk-frontend/dist/govuk/components/error-summary/_index.scss
index 0cb1cc2cc..69bdbaf78 100644
--- a/packages/govuk-frontend/dist/govuk/components/error-summary/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/error-summary/_index.scss
@@ -30,6 +30,10 @@
> * + * {
@include govuk-responsive-margin(4, "top");
}
+
+ > :last-child {
+ @include govuk-responsive-margin(1, "bottom");
+ }
}
// Cross-component class - adjusts styling of list component
Action run for 9c6828e |
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.
Code change looks good to me! @mia-allers-gds (or @Ciandelle, @CharlotteDowns) would you be able to give a quick 'designer approved' stamp on this one as well (the value added back is the same as the one that was there before, which was the bottom margin of the list) 😊
@romaricpascal I think it looks good! |
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.
@romaricpascal I think it looks good!
Let's merge it! ⛵
Reverses a visual change introduced in #4971 where the negative space at the bottom of the Error Summary component was reduced to equalise the container padding on all sides. Although this was originally approved of by a designer on the team, further discussion after the 5.4.0 release concluded that having additional negative space below the Error Summary's content was desirable, as it gave extra space for text descenders to occupy (if present).
3cf09bc
to
9c6828e
Compare
Reverses a visual change introduced in #4971 where the negative space at the bottom of the Error Summary component was reduced to create equal padding on all sides of the container.
Although this was originally approved of by a designer on the team, further discussion after the 5.4.0 release concluded that having additional negative space below the Error Summary's content was desirable, as it allowed extra space for text descenders to occupy, if any are present.
This PR reintroduces that space, but does so in a way that's compatible with the new configuration variants made possible by #4971. (Originally, this additional spacing only existed if the
errorList
parameter was being used, whereas this is no longer a requirement.)Closes #4997.