Skip to content
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

Only apply margin to details summary when open #5352

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

owenatgov
Copy link
Contributor

@owenatgov owenatgov commented Sep 24, 2024

What

Only applies the 5px bottom margin of the details summary when the details element is open. The details element when closed how has a bottom margin of 30px whereas before it had a calcualted bottom margin of 35px, a combination of the bttom margin of govuk-details and govuk-details__summary.

Why

This is an offshoot of #5089

We're confident about removing this space as it allows us to bypass the concerns we had about managing the non-collpasing margin in Safari iOS 12 and below. Additionally, we weren't able to discern the need for the extra spacing other than speculation. This has been verified by a designer.

Visual changes

Before

spacing of the details component withh non-collapsed summary spacing

After

spacing of the details component with no summary spacing on closed

This comment was marked as outdated.

Copy link

github-actions bot commented Sep 24, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 118.58 KiB
dist/govuk-frontend-development.min.js 43.63 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 90.19 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 84.69 KiB
packages/govuk-frontend/dist/govuk/all.mjs 1.05 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 118.57 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 43.62 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 4.97 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 81.8 KiB 41.48 KiB
accordion.mjs 23.5 KiB 12.39 KiB
button.mjs 5.98 KiB 2.69 KiB
character-count.mjs 22.4 KiB 9.92 KiB
checkboxes.mjs 5.83 KiB 2.83 KiB
error-summary.mjs 7.89 KiB 3.46 KiB
exit-this-page.mjs 17.1 KiB 9.26 KiB
header.mjs 4.46 KiB 2.6 KiB
notification-banner.mjs 6.26 KiB 2.62 KiB
password-input.mjs 15.15 KiB 7.25 KiB
radios.mjs 4.83 KiB 2.38 KiB
service-navigation.mjs 4.46 KiB 2.69 KiB
skip-link.mjs 4.39 KiB 2.18 KiB
tabs.mjs 10.05 KiB 6.06 KiB

View stats and visualisations on the review app


Action run for 3d39b0b

Copy link

github-actions bot commented Sep 24, 2024

Stylesheets changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
index 7fbe9acd4..0d72bd2ab 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
@@ -2973,7 +2973,10 @@ screen and (forced-colors:active) {
 }
 
 .govuk-details__summary {
-    display: inline-block;
+    display: inline-block
+}
+
+.govuk-details[open] .govuk-details__summary {
     margin-bottom: 5px
 }
 

Action run for 3d39b0b

Copy link

github-actions bot commented Sep 24, 2024

Other changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/components/details/_index.scss b/packages/govuk-frontend/dist/govuk/components/details/_index.scss
index 296fcfdb0..0a500c62f 100644
--- a/packages/govuk-frontend/dist/govuk/components/details/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/details/_index.scss
@@ -10,7 +10,9 @@
   .govuk-details__summary {
     // Make the focus outline shrink-wrap the text content of the summary
     display: inline-block;
+  }
 
+  .govuk-details[open] .govuk-details__summary {
     margin-bottom: govuk-spacing(1);
   }
 

Action run for 3d39b0b

Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice little tidy up! 🙌🏻 Can I let you add a little CHANGELOG entry within 'Fixes' before you merge it, please? 😊

@owenatgov owenatgov force-pushed the remove-details-summary-closed-spacing branch from 36355e6 to 3d39b0b Compare September 24, 2024 14:43
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-5352 September 24, 2024 14:43 Inactive
@owenatgov owenatgov merged commit 2d3fbaa into main Sep 24, 2024
50 checks passed
@owenatgov owenatgov deleted the remove-details-summary-closed-spacing branch September 24, 2024 15:10
@owenatgov owenatgov mentioned this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants