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 regression: Don't style strong text as bold unless using rich govspeak #922

Merged
merged 1 commit into from Mar 2, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Mar 1, 2017

We don’t style bold text in markdown except for specific places where rich-govspeak is enabled (eg the highway code manual), see #657

If an app uses core-layout then it uses a reset that styles strong as bold. See: #483. Any app using core-layout and the govspeak component would render the strong text in bold. This is a regression that has gradually affected a greater proportion of content as we have migrated.

Whitehall did not set strong styles, so bold was not used in content.

Our style guide says:

bold
Don’t use bold. It can distract the user and it makes the text longer and more confusing for users of screen readers. Use headings or bullets instead if you want to emphasise particular words or sections.

This PR means we are:

  • being explicit about strong being rendered as normal text
  • continuing to whitelist rich-govspeak use
  • letting these rules work with whatever flavour of reset an app has

Other reading on our approach to bold:
https://gdsengagement.blog.gov.uk/2016/11/28/how-to-make-blog-posts-accessible/
https://www.gov.uk/guidance/style-guide/a-to-z-of-gov-uk-style#bold
https://www.gov.uk/design-principles/accessiblepdfs#use-headings

We correctly omit bold from our markdown guidance here:
https://www.gov.uk/guidance/how-to-publish-on-gov-uk/markdown

screen shot 2017-03-02 at 12 00 07

cc @markhurrell

@mcgoooo
mcgoooo approved these changes Mar 2, 2017
Copy link
Contributor

@mcgoooo mcgoooo left a comment

no bold

app/assets/stylesheets/govuk-component/_govspeak.scss Outdated
@@ -20,6 +20,10 @@
text-align: start;
}

strong {

This comment has been minimized.

@36degrees

36degrees Mar 2, 2017
Member

Can this go into govspeak/typography? Specifically in with em, sup, etc?

This comment has been minimized.

@36degrees

36degrees Mar 2, 2017
Member

It might then make sense to move the .rich-govspeak strong there too…

This comment has been minimized.

@fofr

fofr Mar 2, 2017
Author Contributor

I left it here as it's next to the rich-govspeak declaration that overrides it.

This comment has been minimized.

@36degrees

36degrees Mar 2, 2017
Member

Talked about this IRL. I suggested that we should move both strong declerations into govspeak/typography and add a comment there that rich-govspeak is a… Govspeak ‘modifier’(?) than re-enables bold.

This comment has been minimized.

@fofr

fofr Mar 2, 2017
Author Contributor

@36degrees Changes made, comment added. Also added example to docs.

@fofr fofr force-pushed the disable-bold branch Mar 2, 2017
We don’t allow bold text in markdown except for specific places where
rich-govspeak is enabled (eg the highway code manual)

If an app uses `core-layout` then they use a reset that sets strong to
bold. See: #483

Any app using core-layout and the govspeak component would render the
`strong` text in bold. This is a regression that has gradually affected
a great proportion of content. (Whitehall does not set `strong` styles)

* Be explicit about strong being rendered as normal text
* Rich-govspeak will continue to enable
* This will work with whatever flavour of CSS reset an app has
@fofr fofr force-pushed the disable-bold branch to 757acec Mar 2, 2017
Copy link
Member

@36degrees 36degrees left a comment

👏 Thanks!

@fofr fofr merged commit f87e766 into master Mar 2, 2017
2 checks passed
2 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
security/snyk No new vulnerabilities
Details
@fofr fofr deleted the disable-bold branch Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.