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

Improve accessibility and accessibility documentation of components #449

Merged
merged 4 commits into from Aug 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/helpers/contents_list_helper.rb
Expand Up @@ -11,7 +11,7 @@ def wrap_numbers_with_spans(content_item_link)

if number
words = content_item_text.sub(number.to_s, '').strip #remove the number from the text
content_item_link.sub(content_item_text, "<span class=\"app-c-contents-list__number\">#{number}</span><span class=\"app-c-contents-list__numbered-text\">#{words}</span>").squish.html_safe
content_item_link.sub(content_item_text, "<span class=\"app-c-contents-list__number\">#{number} </span><span class=\"app-c-contents-list__numbered-text\">#{words}</span>").squish.html_safe
else
content_item_link
end
Expand Down
33 changes: 14 additions & 19 deletions app/views/components/_banner.html.erb
Expand Up @@ -3,7 +3,6 @@
aside ||= false
large_text = !title && !aside
%>

<% content_block = capture do %>
<% if title %>
<h2 class="app-c-banner__title"><%= title %></h2>
Expand All @@ -12,23 +11,19 @@
<%= text %>
</p>
<% end %>

<% if aside %>
<div class="app-c-banner app-c-banner--grid">
<div class="grid-row">
<div class="column-third">
<div class="app-c-banner__aside">
<p><%= aside %></p>
<section class="app-c-banner<% if aside %> app-c-banner--grid<% end %>" aria-label="Notice">
<% if aside %>
<div class="grid-row">
<div class="column-third">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably out of scope of this PR, and we may have discussed this already, but is the component isolated if it relies on external CSS? (in this case the grid)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope, yes. Might want to track with an issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case the grid classes would be better suited to being .govuk-o-grid__row .govuk-o-grid__column etc. Since objects are shared across components.

<div class="app-c-banner__aside">
<p><%= aside %></p>
</div>
</div>
<div class="column-two-thirds app-c-banner__main">
<%= content_block %>
</div>
</div>

<div class="column-two-thirds app-c-banner__main">
<%= content_block %>
</div>
</div>
</div>
<% else %>
<div class="app-c-banner">
<%= content_block %>
</div>
<% end %>
<% else %>
<%= content_block %>
<% end %>
</section>
4 changes: 2 additions & 2 deletions app/views/components/_notice.html.erb
Expand Up @@ -3,13 +3,13 @@
description_text ||= ""
description_govspeak ||= ""
%>
<div class="app-c-notice">
<section class="app-c-notice" aria-label="Notice">
<h2 class="app-c-notice__title"><%= title %></h2>
<% if description_text.present? %>
<p class="app-c-notice__description"><%= description_text %></p>
<% end %>
<% if description_govspeak.present? %>
<%= render 'govuk_component/govspeak', content: description_govspeak %>
<% end %>
</div>
</section>
<% end %>
8 changes: 6 additions & 2 deletions app/views/components/docs/banner.yml
@@ -1,11 +1,15 @@
name: Banner
description: A page banner, designed to highlight important information
body: Passing an aside to the banner will mean the banner renders in a grid layout.
body: |
Passing an aside to the banner will mean the banner renders in a grid layout.

Real world example: [Consultation with history banner](/government/consultations/child-trust-fund-consultation-on-allowing-the-transfer-of-savings-from-a-child-trust-fund-to-a-junior-isa)
accessibility_criteria: |
The banner must:

- be visually distinct from other content on the page
- have a contrast ratio higher than 4.5:1 against the background colour to meet WCAG AA
- have an accessible name that describes the banner as a notice
- have a text contrast ratio higher than 4.5:1 against the background colour to meet [WCAG AA](https://www.w3.org/TR/WCAG20/#visual-audio-contrast-contrast)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual wording of the WCAG requirement is "of at least", do we need to go beyond that? Also doesn't this qualify as "large text" and therefore only need to have a contrast ratio of 3:1?

Copy link
Contributor Author

@fofr fofr Aug 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We avoid confusion over the definition of large text by sticking with 4.5:1 for everything.
I will update to say "at least".

fixtures:
default:
text: 'This was published under the 2010 to 2015 Conservative and Liberal Democrat coalition government'
Expand Down
11 changes: 10 additions & 1 deletion app/views/components/docs/contents-list.yml
Expand Up @@ -9,17 +9,26 @@ body: |
`format_numbers` option will pull out numbers in the link text to render them as though they were the list style type. Applies to
numbers at the start of text, with or without a decimal. See the [format complex numbers fixture](/component-guide/contents-list/formats_complex_numbers) for details.
accessibility_criteria: |
The component must be [a landmark with a navigation role](https://accessibility.blog.gov.uk/2016/05/27/using-navigation-landmarks/).

The contents list must:

- have a navigation role
- inform the user how many items are in the list
- convey the content structure

The contents item links must:

- accept focus
- be focusable with a keyboard
- be usable with a keyboard
- indicate when it has focus
- change in appearance when touched (in the touch-down state)
- change in appearance when hovered
- be usable with touch
- be usable with [voice commands](https://www.w3.org/WAI/perspectives/voice.html)
- have visible text

Links with formatted numbers must separate the number and text with a space for correct screen reader pronunciation. This changes pronunciation from "1 dot Item" to "1 Item".
fixtures:
default:
contents:
Expand Down
7 changes: 7 additions & 0 deletions app/views/components/docs/download-link.yml
@@ -1,12 +1,19 @@
name: Download link
description: A link with a file download icon
body: |
For usability the provided link text should indicate the file type and size of the download.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update one of the examples on this page to include this information?

accessibility_criteria: |
The download link must:

- accept focus
- be focusable with a keyboard
- be usable with a keyboard
- indicate when it has focus
- change in appearance when touched (in the touch-down state)
- change in appearance when hovered
- be usable with touch
- be usable with [voice commands](https://www.w3.org/WAI/perspectives/voice.html)
- have visible text

The download icon must:

Expand Down
10 changes: 8 additions & 2 deletions app/views/components/docs/heading.yml
@@ -1,12 +1,18 @@
name: Heading
description: A text heading
body: |
Provides a heading tag with an optional id attribute, used predominantly to the left of content on consultations and publications.
A heading tag with an optional id attribute, used predominantly to the left of content on consultations and publications.

Real world examples:

- [Publication](/government/publications/recognising-the-terrorist-threat)
- [Consultation](/government/consultations/proposal-for-the-future-of-rotherham-goldthorpe-jobcentre)
accessibility_criteria: |
The heading must:

- be part of a correct heading structure for a page

- be semantically represented as a heading
- convey the heading level
fixtures:
default:
text: 'Download the full outcome'
Expand Down
4 changes: 1 addition & 3 deletions app/views/components/docs/lead-paragraph.yml
@@ -1,9 +1,7 @@
name: Lead paragraph
description: The opening paragraph of content. Typically a content item’s description field.
accessibility_criteria: |
The lead paragraph must:

- be visually distinct from other paragraphs
The lead paragraph must be visually distinct from other paragraphs.
fixtures:
default:
text: 'UK Visas and Immigration is making changes to the Immigration Rules affecting various categories.'
4 changes: 1 addition & 3 deletions app/views/components/docs/notice.yml
Expand Up @@ -5,9 +5,7 @@ body: |

The component accepts either a simple string description_text parameter that it wraps in a paragraph, or a description_govspeak parameter that is rendered through govspeak for more complex HTML layout.
accessibility_criteria: |
The notice must:

- have a border of AA contrast ratio 4.5:1
The notice border colour must have a contrast ratio of more than 4.5:1 with its background to be visually distinct.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contrast ratio for text is "at least" rather than "more than" but is this level of contrast necessary for non-text elements? What's the WCAG guidance here?

Copy link
Contributor Author

@fofr fofr Aug 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no guidance for border colours in WCAG. But we know this contrast is necessary for text so can infer it's usefulness in distinguishing things from other things.

fixtures:
default:
title: 'Statistics release cancelled'
Expand Down
9 changes: 6 additions & 3 deletions app/views/components/docs/print-link.yml
Expand Up @@ -7,10 +7,13 @@ accessibility_criteria: |
- be focusable with a keyboard
- be usable with a keyboard
- indicate when it has focus
- change in appearance when touched (in the touch-down state)
- change in appearance when hovered
- be usable with touch
- be usable with [voice commands](https://www.w3.org/WAI/perspectives/voice.html)
- have visible text

The print icon must:

- be presentational and ignored by screen readers
The print icon must be presentational and ignored by screen readers.
fixtures:
default:
href: '/print-this'
Expand Down
16 changes: 8 additions & 8 deletions app/views/components/docs/share-links.yml
@@ -1,15 +1,17 @@
name: Share links
description: Links to allow the user to share the current page on Facebook or Twitter
description: Links to share the current page on Facebook or Twitter
body: |
Pass complete share URLs to the component. The component will not process a URL into a share link itself.

Use only with content that benefits from being shared, for example a consultation.

The component will track interactions with the share links to Google Analytics using [Social Interactions](https://developers.google.com/analytics/devguides/collection/analyticsjs/social-interactions)

Examples:
Real world examples:

- [News Article with Share Links](/government/news/fast-tracking-uk-innovation-apply-for-business-funding)
- [Consultation Page with Share Links](/government/consultations/soft-drinks-industry-levy)
- [Right to Left Page with Share Links](/government/news/uk-sets-out-long-term-support-for-stable-secure-and-prosperous-afghanistan-to-2020.ur)
- [News article](/government/news/fast-tracking-uk-innovation-apply-for-business-funding)
- [Consultation](/government/consultations/soft-drinks-industry-levy)
- [Right to left](/government/news/uk-sets-out-long-term-support-for-stable-secure-and-prosperous-afghanistan-to-2020.ur)

accessibility_criteria: |
The share link must:
Expand All @@ -19,9 +21,7 @@ accessibility_criteria: |
- be usable with a keyboard
- indicate when it has focus

The share link icons must:

- be presentational and ignored by screen readers
The share link icons must be presentational and ignored by screen readers.
fixtures:
default:
facebook_href: '/facebook-link'
Expand Down
11 changes: 7 additions & 4 deletions app/views/components/docs/subscription-links.yml
@@ -1,16 +1,19 @@
name: Subscription links
description: Links to 'Get email alerts' and 'Subscribe to feed'
description: Links to Get email alerts and Subscribe to feed
accessibility_criteria: |
The subscription links must:

- accept focus
- be focusable with a keyboard
- be usable with a keyboard
- indicate when they have focus
- change in appearance when touched (in the touch-down state)
- change in appearance when hovered
- be usable with touch
- be usable with [voice commands](https://www.w3.org/WAI/perspectives/voice.html)
- have visible text

Icons in subscription links must:

- be presentational and ignored by screen readers
Icons in subscription links must be presentational and ignored by screen readers.
fixtures:
default:
email_signup_link: '/foreign-travel-advice/singapore/email-signup'
Expand Down
16 changes: 10 additions & 6 deletions app/views/components/docs/translation-nav.yml
@@ -1,14 +1,19 @@
name: Translation Nav
description: A navigation item for available language translations.
body: The active property should be used to indicate the currently selected language.
name: Translation navigation
description: A list of links to available translations
body: The active property indicates the current language.
accessibility_criteria: |
The translation nav must:
The translation navigation links must:

- identify the language of the link
- accept focus
- be focusable with a keyboard
- indicate when it has focus
- be usable with a keyboard
- inform the user of the language option, if the user has that language installed
- change in appearance when touched (in the touch-down state)
- change in appearance when hovered
- be usable with touch
- be usable with [voice commands](https://www.w3.org/WAI/perspectives/voice.html)
- have visible text
fixtures:
# TODO: We need to add a fixture to show component rendering right-to-left.
# This is dependant on being able to provide context to fixtures.
Expand Down Expand Up @@ -42,4 +47,3 @@ fixtures:
- locale: 'zh'
base_path: '/zh'
text: '中文'

5 changes: 5 additions & 0 deletions test/components/banner_test.rb
Expand Up @@ -18,6 +18,11 @@ def component_name
assert_select ".app-c-banner__desc", text: 'This was published under the 2010 to 2015 Conservative government'
end

test "renders a banner with an aria label" do
render_component(title: 'Summary', text: 'Text')
assert_select "section[aria-label]"
end

test "renders a banner with title and text correctly" do
render_component(title: 'Summary', text: 'This was published under the 2010 to 2015 Conservative government')

Expand Down
5 changes: 5 additions & 0 deletions test/components/notice_test.rb
Expand Up @@ -24,6 +24,11 @@ class NoticeGovspeakTest < ActionDispatch::IntegrationTest
end
end

test "renders a notice with an aria label" do
visit '/component-guide/notice/default'
assert page.has_selector?(".component-guide-preview section[aria-label]")
end

test "renders a notice with a title and description text" do
visit '/component-guide/notice/with_description_text'

Expand Down
12 changes: 11 additions & 1 deletion test/helpers/contents_list_helper_test.rb
@@ -1,12 +1,22 @@
require 'test_helper'

class ContentsListHelperTest < ActionView::TestCase
include ActionView::Helpers::SanitizeHelper

test "it wraps a number and text in separate span elements" do
assert_split_number_and_text('1. Thing', '1.', 'Thing')
assert_split_number_and_text('10. Thing', '10.', 'Thing')
assert_split_number_and_text('100. Thing', '100.', 'Thing')
end

test "it keeps a space between number and text for screen reader pronunciation" do
# 1.Thing can be pronounced "1 dot Thing"
# 1. Thing is always pronounced "1 Thing"
text = "1. Thing"
wrapped_html = wrap_numbers_with_spans("<a href=\"#link\">#{text}</a>")
assert_equal text, strip_tags(wrapped_html)
end

test "it wraps a number and text in span elements if it's a number without a period" do
assert_split_number_and_text('1 Thing', '1', 'Thing')
end
Expand Down Expand Up @@ -62,7 +72,7 @@ def assert_split_number_and_text(number_and_text, number, text)
numbered_text_class = "app-c-contents-list__numbered-text"

input = "<a href=\"#link\">#{number_and_text}</a>"
expected = "<a href=\"#link\"><span class=\"#{number_class}\">#{number}</span><span class=\"#{numbered_text_class}\">#{text}</span></a>"
expected = "<a href=\"#link\"><span class=\"#{number_class}\">#{number} </span><span class=\"#{numbered_text_class}\">#{text}</span></a>"
assert_equal expected, wrap_numbers_with_spans(input)
end
end