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

Add list--spaced modifier #1775

Merged
merged 4 commits into from
May 21, 2020
Merged

Add list--spaced modifier #1775

merged 4 commits into from
May 21, 2020

Conversation

36degrees
Copy link
Member

This modifier is designed to make lists easier to scan when the list items are long, and span multiple lines.

Closes #1272

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1775 March 31, 2020 12:25 Inactive
Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

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

Makes sense, think we'll need to add some new guidance to the Design System too.

@timpaul
Copy link
Contributor

timpaul commented Mar 31, 2020

Makes sense, think we'll need to add some new guidance to the Design System too.

Absolutely - I'm going to work on that later

@timpaul timpaul marked this pull request as ready for review March 31, 2020 12:57
@36degrees 36degrees added this to the Next milestone Mar 31, 2020
@36degrees 36degrees removed this from the Next milestone May 6, 2020
This modifier is designed to make lists easier to scan when the list items are long, and span multiple lines.

Closes #1272
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1775 May 20, 2020 13:24 Inactive
@36degrees
Copy link
Member Author

On mobile, we do not include any margin after bulleted or numbered list items – the margin is only added on tablet and above.

I'm wondering if we should be using a slightly smaller margin on mobile to be consistent and to better associate each bullet point with the lead-in paragraph:

  %govuk-list--spaced > li {
    margin-bottom: govuk-spacing(2);

    @include govuk-media-query($from: tablet) {
      margin-bottom: govuk-spacing(3);
    }

    &:last-child {
      margin-bottom: govuk-spacing(0);
    }
  }

To my (non-designer) eyes the 15px on mobile otherwise seems excessive.

Thoughts @timpaul or @christopherthomasdesign?

Current (15px) Proposed (10px)
current proposed

@timpaul
Copy link
Contributor

timpaul commented May 20, 2020

I think that typically the internal padding and margins of textual components should shrink by a similar proportion to the text shrinkage, so yep - I'd agree with your suggestion. @christopherthomasdesign - what do you think? There's no crazy vertical rhythm thing we're doing that I've forgotten about is there?

@christopherthomasdesign
Copy link
Member

Looks good to me @timpaul, good suggestion @36degrees 👍

Reduce the spacing on the spaced list items on mobile relative to the text size, and the fact that on mobile list items have 0 margin by default (vs 5px on tablet and mobile).
Because the ul has a bottom margin of 15px (mobile) or 20px (tablet, desktop) and the bottom margin of the last li will always have a smaller bottom margin, the two should collapse and so the margin on the last list item has no effect – thus we don't need any special treatment for it.
@timpaul
Copy link
Contributor

timpaul commented May 20, 2020

This proposal was approved by the Design System working group on 14 May 2020

Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

🎉

@36degrees 36degrees merged commit 1f19db3 into master May 21, 2020
@36degrees 36degrees deleted the add-list-spaced-modifier branch May 21, 2020 07:49
@frankieroberto
Copy link
Contributor

Nice work! 🎊 I've used this style a few times in the past.

@36degrees 36degrees mentioned this pull request Jun 1, 2020
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.

More spacing between bulleted list items containing longer text
7 participants