Changed Transaction Banner font size to match GOV Elements banner due… #781
Conversation
assets/scss/components/_header.scss
Outdated
| @@ -88,10 +88,17 @@ Styleguide Header.Transaction | |||
| text-align: left; | |||
| padding: 0; | |||
| } | |||
|
|
|||
| p { | |||
| font-size: 36px; | |||
There was a problem hiding this comment.
Can you use the core-36 mixin instead of hard coding the font size here.
There was a problem hiding this comment.
Ahhh - I couldn't find "core" and "bold" was obviously the wrong choice. Thanks - I will update this now
There was a problem hiding this comment.
Yeah you can find the core mixins here - https://github.com/hmrc/assets-frontend/blob/master/assets/govuk_frontend_toolkit/stylesheets/_typography.scss#L60
You can pass in a custom line height if you need to but I'd say leave at default unless you really need to.
|
Made that change to use the mixin, thanks @timsb |
|
@CEPoole did you consider adding this in as an additional style to match GOV Elements rather than replacing the current one? |
|
@gavinwye If you think that is better? I thought this would be an important change due to the fact it failed accessibility testing, is it right to let the default implementation fail WCAG 2.0 AA? |
|
@CEPoole I didn't realise this was for an accessibility fail. I don't think it should fail accessibility. I'm thinking about the upgrade path for teams though. But I guess this would create a new version of AF so teams would need to bump their version to get this change. |
|
@gordonmcmullan, @gavinwye and I moved this conversation out of the PR. I believe as it currently stands @CEPoole is making some changes to the PR so that it copies the GDS pattern more closely and he'll either edit this PR or open a new one. |
|
@rpowis I think it's this one |
|
Thanks @adamliptrot-oc. That's an odd page for the example to be on. Anyway... @CEPoole If you can add the GDS styles with their class name then we can mark ours as deprecated and teams can start adopting the new one. Is that what your conversation was about @timsb? Anyone prefer an alternative approach? |
|
Just spoke with @russellthorn about this and he's going to add some styles for the GOV.UK markup. The AF markup will be deprecated by way of updating the component library to the design pattern library and the soon to be added GOV.UK styles can be removed in the upcoming dependencies update. @russellthorn let us know on here if this PR should be merged or if you're able to add your stuff to it. |
… to a colour contrast accessibility issue
|
@rpowis the PR has been updated with GOV.UK styles and annotated in the comments. The new _components file has been imported into the elements main.scss. Let me know if the change is OK. If it is and you're happy, fire away and merge! 👍 |
CHANGELOG.md
Outdated
| @@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). | |||
|
|
|||
| ### Changed | |||
| - Added the correct class to the Tab component to enable targetting an open tab via the url with a fragment [#778](https://github.com/hmrc/assets-frontend/issues/778) | |||
| - Increased font size in transaction banner headers to match the GDS example banner to fix an accessibility issue [#781](https://github.com/hmrc/assets-frontend/pull/781) | |||
There was a problem hiding this comment.
Careful when rebasing this file. This entry needs moving up into the Unreleased section.
|
Looks good 👍 Thanks @CEPoole @russellthorn Worth noting that the margins on the heading in the GOV.UK Elements version are bigger than those in the Assets Frontend version. Is that expected or should they be the same before this gets merged? Top: assets-frontend markup
GOVUK Elements example
|
|
The margins should be larger, that's fine and correct @rpowis 👍 Happy for this to be merged. |
|
Great. Thanks for confirming @russellthorn 👍 |
generated from commit 7b221ae
… to a colour contrast accessibility issue
DAC reports showed that the current transaction banner failed WCAG AAA colour contrast standards.
Problem
The font size of the
<p> text </p>was too small for the colour contrast to pass accessibility standards.Example Screenshot
LEFT: Assets frontend RIGHT: GOV Elements
Solution
Increased the font sizes to match those used in the GOV Elements kit