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

Fix non-semantic use of text colour for borders. #551

Merged
merged 4 commits into from
Feb 26, 2018

Conversation

36degrees
Copy link
Contributor

@36degrees 36degrees commented Feb 23, 2018

At the minute we are using the $govuk-text-colour variable for the colour of form control borders, which doesn’t make much sense semantically.

This introduces a new variable, $govuk-input-border-colour which is set to the same $govuk-black that $govuk-text-colour currently refers to, and updates form components to use it for the border colour.

It also modifies the radio and checkbox components to consistently use currentColor for the pseudo-elements, and be explicit about doing so.

  • Changes documented in changelog

At the minute we are using the `$govuk-text-colour` variable for the colour of form control borders, which doesn’t make much sense semantically.

This introduces a new variable, `$govuk-input-border-colour` which is set to the same `$govuk-black` that `$govuk-text-colour` currently refers to, and updates form components to use it for the border colour.
This matches the existing behaviour in Elements and the behaviour of the equivalent pseudo element in the Radios component.
We want to use currentColor which is the default value but as users are likely used to seeing the colour defined as part of the shorthand border property being explicit makes it less likely someone will try and ‘fix’ this by adding another colour.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-551 February 23, 2018 17:19 Inactive
@kr8n3r
Copy link

kr8n3r commented Feb 23, 2018

looking good, just the changelog

@36degrees 36degrees merged commit 30e4239 into master Feb 26, 2018
@36degrees 36degrees deleted the more-semantic-border-colours branch February 26, 2018 10:31
This was referenced Mar 1, 2018
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