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

Change spacing relationship on default and small legends and hints #940

Merged
merged 2 commits into from
Aug 13, 2018

Conversation

dashouse
Copy link

@dashouse dashouse commented Aug 3, 2018

Before the release of GOV.UK Frontend we changed the spacing of labels and hints when using the default or govuk-label--s class. (#806)

We chose not update the legend and hint, in isolation they looked good.

However when a service uses a small label + hint and a small legend + hint on the same page the inconsistency is noticeable.

Amending the spacing on the <legend> is slightly different to the <label>, there was also a need to only reduce spacing on the <legend> when a hint was applied, hence the use of an adjacent selector and negative margin.

Before
screen shot 2018-08-03 at 10 44 36

After
screen shot 2018-08-03 at 10 40 04

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-940 August 3, 2018 09:54 Inactive
}

.govuk-fieldset__legend--s {
@include govuk-font($size: 19, $weight: bold);
}

.govuk-fieldset__legend
Copy link
Contributor

Choose a reason for hiding this comment

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

Guessing this change isn't intentional?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks!

@kr8n3r
Copy link

kr8n3r commented Aug 7, 2018

this looks good but it needs a changelog entry as well. is this a bug fix or a new feature?

@36degrees 36degrees added this to the 1.2.0 milestone Aug 13, 2018
// or govuk-fieldset__legend--s for better vertical alignment.

// This adjustment will not work when the legend is outside the <h1>, however
// it is unlikely that the default or govuk-fieldset__legend--s class would // be used in this case.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment's got a mis-placed // – it'd be good to wrap these comments correctly at 80 characters.

@36degrees 36degrees modified the milestones: 1.2.0, [NEXT] Aug 13, 2018
@kr8n3r
Copy link

kr8n3r commented Aug 13, 2018

resolved conflict and comment line length


// This adjustment will not work in browsers that do not support :not()
// Users with these browsers will see the default size margin (5px larger)
// This adjustment will not work in browsers that do not support :not() Users
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a full stop here to keep these sentences from running into each other like this?

Copy link

Choose a reason for hiding this comment

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

adjusted

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

👍

@kr8n3r
Copy link

kr8n3r commented Aug 13, 2018

merged on behalf of @dashouse

@kr8n3r kr8n3r merged commit b805864 into master Aug 13, 2018
@kr8n3r kr8n3r deleted the legend-hint-spacing branch August 13, 2018 08:52
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.

5 participants