Various improvements to Button component #552
Conversation
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-552
src/components/button/_button.scss
Outdated
| @@ -130,6 +99,10 @@ | |||
| cursor: default; | |||
| } | |||
|
|
|||
| :focus { | |||
There was a problem hiding this comment.
Does this need to be &:focus?
36degrees
force-pushed
the
button-improvements
branch
from
0dbeaaa
to
529a532
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-552
36degrees
force-pushed
the
button-improvements
branch
from
529a532
to
9e573fb
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-552
|
Rebased against master. Pushed an additional commit which ensures that link styles are overridden, which was removed in 7fd770c. Not sure how to document this in the changelog… the only 'effective' change now is the removal of the IE7 outline fix, everything else should just be removing redundant styling and tidying up the Sass. Thoughts? |
|
Checked the last commit against the various browsers again, looks good 👍 Re: changelog, we do have very small changes like "Button hover colour now has a semantic Sass name: $govuk-button-hover-colour" already documented so the ones in this PR don't feel any less worthy of being mentioned. However I thought our intended audience were service teams who want to know what has changed since last version and I don't know how useful such granular information is... But this is not to do with this PR... happy to approve this as soon as the changelog is updated. |
36degrees
force-pushed
the
button-improvements
branch
from
9e573fb
to
dab67ef
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-review-pr-552
This is a hangover from when the button mixin could be used to generate buttons of different colours. Now that we have one colour palette for buttons, we can remove this logic and define the button colour in the same place we define the background colour.
We needed this in Elements, when we did not give buttons a border. Now that they have a border, they do not need an outline as well.
This is exactly the same as the shadow defined on the `govuk-c-button` class, so there’s no need to redefine it here.
I see this as part of the styling of the ‘default’ state of the button, so it makes sense to group it with the other ‘core’ govuk-c-button rules, before we start introducing states.
This code was copied from Frontend Toolkit. Unfortunately, there’s nothing in the comment or its history to tell us what version of IE8 the issue it is trying to address occurs in. As far as I can tell, this is not an issue that affects IE8 (I can’t see any difference to the button in any state in IE8 when this line is removed, and references to this `filter: chroma` hack tend to talk about IE7) so I think we can remove it.
36degrees
force-pushed
the
button-improvements
branch
from
dab67ef
to
3166f58
Compare
I suggest walking through the individual commits to understand the changes.