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

Top padding not applied to fieldset legends when they appear after a paragraph. #953

Open
edwardhorsford opened this issue May 24, 2024 · 4 comments

Comments

@edwardhorsford
Copy link
Contributor

Bug Report

This section of SASS adds top padding to headings where they appear after a paragraph. It doesn't apply to fieldset-legends though - so if you're using a radio question after a paragraph, it looks too close.

What is the issue?

Fieldset legends should be included in the padding override.

@frankieroberto
Copy link
Contributor

@edwardhorsford screenshots might help explain this?

@edwardhorsford
Copy link
Contributor Author

Here's a screenshot from my service. There's several h2s and a legend. The h2s get extra top padding when they follow a paragraph. However the legend (which should visually look like an h2) does not - this causes it to be too close to the proceeding paragraph.
Screenshot 2024-05-24 at 15 21 28

@edwardhorsford
Copy link
Contributor Author

It's probably tricky to extend the current SASS to cover fieldset legends and labels because there's so many different combinations (and they're not direct siblings). I asked ChatGPT and it suggested a refactor like this:

@mixin nhsuk-heading-padding($large-elements, $small-elements) {
  @each $element in $large-elements {
    .nhsuk-body-l + #{$element},
    .nhsuk-body-m + #{$element},
    .nhsuk-body-s + #{$element},
    .nhsuk-list + #{$element},
    .nhsuk-body-l + .nhsuk-form-group > .nhsuk-label-wrapper > #{$element},
    .nhsuk-body-m + .nhsuk-form-group > .nhsuk-label-wrapper > #{$element},
    .nhsuk-body-s + .nhsuk-form-group > .nhsuk-label-wrapper > #{$element},
    .nhsuk-list + .nhsuk-form-group > .nhsuk-label-wrapper > #{$element},
    .nhsuk-body-l + .nhsuk-form-group .nhsuk-fieldset > .nhsuk-fieldset__legend#{$element},
    .nhsuk-body-m + .nhsuk-form-group .nhsuk-fieldset > .nhsuk-fieldset__legend#{$element},
    .nhsuk-body-s + .nhsuk-form-group .nhsuk-fieldset > .nhsuk-fieldset__legend#{$element},
    .nhsuk-list + .nhsuk-form-group .nhsuk-fieldset > .nhsuk-fieldset__legend#{$element} {
      @include nhsuk-responsive-padding(4, "top");
    }
  }

  @each $element in $small-elements {
    .nhsuk-body-l + #{$element},
    .nhsuk-body-m + #{$element},
    .nhsuk-body-s + #{$element},
    .nhsuk-list + #{$element},
    .nhsuk-body-l + .nhsuk-form-group > .nhsuk-label-wrapper > #{$element},
    .nhsuk-body-m + .nhsuk-form-group > .nhsuk-label-wrapper > #{$element},
    .nhsuk-body-s + .nhsuk-form-group > .nhsuk-label-wrapper > #{$element},
    .nhsuk-list + .nhsuk-form-group > .nhsuk-label-wrapper > #{$element},
    .nhsuk-body-l + .nhsuk-form-group .nhsuk-fieldset > .nhsuk-fieldset__legend#{$element},
    .nhsuk-body-m + .nhsuk-form-group .nhsuk-fieldset > .nhsuk-fieldset__legend#{$element},
    .nhsuk-body-s + .nhsuk-form-group .nhsuk-fieldset > .nhsuk-fieldset__legend#{$element},
    .nhsuk-list + .nhsuk-form-group .nhsuk-fieldset > .nhsuk-fieldset__legend#{$element} {
      padding-top: nhsuk-spacing(1);

      @include mq($from: tablet) {
        padding-top: nhsuk-spacing(2);
      }
    }

    .nhsuk-lede-text + #{$element} {
      padding-top: 0;
    }
  }
}

@include nhsuk-heading-padding(
  ('.nhsuk-heading-l', '.nhsuk-label--l', '.nhsuk-fieldset__legend--l'),
  ('.nhsuk-heading-m', '.nhsuk-heading-s', '.nhsuk-label--m', '.nhsuk-fieldset__legend--m')
);

We probably wouldn't want exactly this, but the refactor might be clearer.

@edwardhorsford
Copy link
Contributor Author

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

No branches or pull requests

2 participants