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.

Sign up for GitHub

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

Jump to bottom

The $govuk-border-width-mobile variable doesn't make any sense #1240

Closed
36degrees opened this issue Mar 6, 2019 · 2 comments
Closed

The $govuk-border-width-mobile variable doesn't make any sense #1240

36degrees opened this issue Mar 6, 2019 · 2 comments
Labels
breaking change 🕔 hours A well understood issue which we expect to take less than a day to resolve.
Milestone
v3.0.0

Comments

@36degrees
Copy link
Contributor

We have a variable called $govuk-border-width-mobile.

It's documented as 'Border width on mobile'

However, it's only used in a couple of places:

  • The width of the conditional reveal on checkboxes and radio buttons at all breakpoints
  • The width of the error summary at all breakpoints

We should consider renaming or removing this variable.
@NickColley NickColley added breaking change awaiting triage Needs triaging by team labels Mar 6, 2019
@kellylee-gds kellylee-gds added 🕔 hours A well understood issue which we expect to take less than a day to resolve. Priority: Low and removed awaiting triage Needs triaging by team labels Mar 6, 2019
@36degrees 36degrees added this to the v3.0.0 milestone Mar 11, 2019
@hannalaakso
Copy link
Member

@dashouse Would you mind taking a look? 🙏

We use this border width (4px) for

  1. error summary on mobile view
  2. radios conditional
  3. checkboxes conditional

Do we think having 4px border there is useful or would we want to consider simplifying this by bumping those scenarios to use the standard 5px border width?

(If having 4px border variable is useful, we could rename it to $govuk-border-width-narrow - we already have $govuk-border-width-wide)

@dashouse
Copy link

The 4px border is used in the conditional reveals because it has to be an even number in order to be centred under the 40px checkbox or radio. 5px would put it 1px too far left or right and it was noticeable.

The error summary change is probably not needed.

So I'd say we will need a 4px border, it should just have a different name.

hannalaakso added a commit that referenced this issue Apr 24, 2019
@hannalaakso
Rename border-width-mobile to reflect how it's used
b5e64f2 
Despite its name, the variable isn't used on mobile.

Standardise the border width on error summary on mobile to use
the standard border width.

From @dashouse): "The 4px border is used in the conditional reveals
because it has to be an even number in order to be centred under the 40px
checkbox or radio. 5px would put it 1px too far left or right and it was
noticeable. The error summary change is probably not needed."

Fixes #1240
@hannalaakso hannalaakso mentioned this issue Apr 24, 2019
Merged
hannalaakso added a commit that referenced this issue Apr 24, 2019
@hannalaakso
Rename border-width-mobile to reflect how it's used
150f263 
Despite its name, the variable isn't used on mobile.

Standardise the border width on error summary on mobile to use
the standard border width.

From @dashouse): "The 4px border is used in the conditional reveals
because it has to be an even number in order to be centred under the 40px
checkbox or radio. 5px would put it 1px too far left or right and it was
noticeable. The error summary change is probably not needed."

Fixes #1240
hannalaakso added a commit that referenced this issue Apr 30, 2019
@hannalaakso
Rename border-width-mobile to reflect how it's used
231c062 
Despite its name, the variable isn't used on mobile.

Standardise the border width on error summary on mobile to use
the standard border width.

From @dashouse): "The 4px border is used in the conditional reveals
because it has to be an even number in order to be centred under the 40px
checkbox or radio. 5px would put it 1px too far left or right and it was
noticeable. The error summary change is probably not needed."

Fixes #1240
hannalaakso added a commit that referenced this issue Apr 30, 2019
@hannalaakso
Rename border-width-mobile to reflect how it's used
8695973 
Despite its name, the variable isn't used on mobile.

Standardise the border width on error summary on mobile to use
the standard border width.

From @dashouse): "The 4px border is used in the conditional reveals
because it has to be an even number in order to be centred under the 40px
checkbox or radio. 5px would put it 1px too far left or right and it was
noticeable. The error summary change is probably not needed."

Fixes #1240
aliuk2012 pushed a commit that referenced this issue Jun 14, 2019
@hannalaakso @aliuk2012
Rename border-width-mobile to reflect how it's used
1abd2b2 
Despite its name, the variable isn't used on mobile.

Standardise the border width on error summary on mobile to use
the standard border width.

From @dashouse): "The 4px border is used in the conditional reveals
because it has to be an even number in order to be centred under the 40px
checkbox or radio. 5px would put it 1px too far left or right and it was
noticeable. The error summary change is probably not needed."

Fixes #1240
aliuk2012 pushed a commit that referenced this issue Jun 14, 2019
@hannalaakso @aliuk2012
Rename border-width-mobile to reflect how it's used
5c73def 
Despite its name, the variable isn't used on mobile.

Standardise the border width on error summary on mobile to use
the standard border width.

From @dashouse): "The 4px border is used in the conditional reveals
because it has to be an even number in order to be centred under the 40px
checkbox or radio. 5px would put it 1px too far left or right and it was
noticeable. The error summary change is probably not needed."

Fixes #1240
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking change 🕔 hours A well understood issue which we expect to take less than a day to resolve.
Projects
GOV.UK Design System cycle board
Archived in project
Milestone
v3.0.0
Development

No branches or pull requests
5 participants
@36degrees @NickColley @hannalaakso @dashouse @kellylee-gds