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

Add axe accessibility testing #33

Merged
merged 6 commits into from
Aug 25, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
### Running tests

The default rake task runs all tests:
```
bundle exec rake
```

Javascript is tested using Jasmine and the [Jasmine gem](https://github.com/pivotal/jasmine-gem). Tests can be run either in the browser or on the command line via the dummy app’s tasks:
```sh
# browser
bundle exec rake app:jasmine

# command line
bundle exec rake app:jasmine:ci
```
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,6 @@ This will create a template, scss file and yml documentation file for a new comp
## Licence

[MIT Licence](LICENCE.md)

## Development
For documentation on how to update and develop this gem see [DEVELOPMENT.md](./DEVELOPMENT.md)
8 changes: 2 additions & 6 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,11 @@ require 'rspec/core/rake_task'
RSpec::Core::RakeTask.new(:spec)

APP_RAKEFILE = File.expand_path("../test/dummy/Rakefile", __FILE__)
load 'rails/tasks/engine.rake'


load 'rails/tasks/engine.rake'
load 'rails/tasks/statistics.rake'



require 'bundler/gem_tasks'

require 'rake/testtask'

Rake::TestTask.new(:test) do |t|
Expand All @@ -32,4 +28,4 @@ namespace :assets do
end
end

task default: :spec
task default: [:spec, 'app:jasmine:ci']
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
(function (window, document, axe) {
window.GOVUK = window.GOVUK || {}

function AccessibilityTest (selector, callback) {
if (typeof callback !== 'function') {
return
}

if (!document.querySelector(selector)) {
return callback('No selector "' + selector + '" found')
}

var axeOptions = {
restoreScroll: true,
include: [selector]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace here: run through standardjs linter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some space 👍


axe.run(axeOptions, function (err, results) {
if (err) {
callback('aXe Error: ' + err)
}

var violations = (typeof results === 'undefined') ? [] : results.violations

if (violations.length === 0) {
return callback('No accessibility issues found')
}

var errorText = (
'\n' + 'Accessibility issues at ' +
results.url + '\n\n' +
violations.map(function (violation) {
var help = 'Problem: ' + violation.help
var helpUrl = 'Try fixing it with this help: ' + _formatHelpUrl(violation.helpUrl)
var htmlAndTarget = violation.nodes.map(_renderNode).join('\n\n')

return [
help,
htmlAndTarget,
helpUrl
].join('\n\n\n')
}).join('\n\n- - -\n\n')
)
callback(undefined, errorText)
})
}

var _formatHelpUrl = function (helpUrl) {
if (axe.version.indexOf('alpha') === -1) {
console.warn('Deprecation warning: helpUrl formatting is no longer needed so can be deleted')
return helpUrl
}
return helpUrl.replace('3.0.0-alpha', '2.3')
}

var _renderNode = function (node) {
return (
' Check the HTML:\n' +
' `' + node.html + '`\n' +
' found with the selector:\n' +
' "' + node.target.join(', ') + '"'
)
}

var _throwUncaughtError = function (error) {
// aXe swallows throw errors so we pass it through a setTimeout
// so that it's not in aXe's context
setTimeout(function () {
throw new Error(error)
}, 0)
}

window.GOVUK.AccessibilityTest = AccessibilityTest

// Cut the mustard at IE9+ since aXe only works with those browsers.
// http://responsivenews.co.uk/post/18948466399/cutting-the-mustard
if (document.addEventListener) {
document.addEventListener('DOMContentLoaded', function () {
AccessibilityTest('[data-module="test-a11y"]', function (err, results) {
if (err) {
return
}
_throwUncaughtError(results)
})
})
}
})(window, document, window.axe)
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@
// Read Sprockets README (https://github.com/rails/sprockets#sprockets-directives) for details
// about supported directives.
//
//= require_tree ./vendor
//= require_tree .

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,3 @@ html {
}
}
}

Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<div class="component-guide-preview">
<div data-module="test-a11y" class="component-guide-preview">
<%= render @component_doc.partial_path, fixture.html_safe_data %>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
<% if @component_fixtures.length > 1 %>
<h2 class="preview-title"><a href="<%= component_fixture_path(@component_doc.id, fixture.id) %>"><%= fixture.name %></a></h2>
<% end %>
<%= render @component_doc.partial_path, fixture.html_safe_data %>
<div data-module="test-a11y">
<%= render @component_doc.partial_path, fixture.html_safe_data %>
</div>
</div>
<% end %>
6 changes: 4 additions & 2 deletions govuk_publishing_components.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Gem::Specification.new do |s|
s.homepage = "https://github.com/alphagov/govuk_publishing_components"
s.license = "MIT"

s.files = Dir["{app,config,db,lib}/**/*", "LICENCE.md", "Rakefile", "README.md"]
s.files = Dir["{app,config,db,lib}/**/*", "DEVELOPMENT.md", "LICENCE.md", "Rakefile", "README.md"]

s.add_dependency "rails", ">= 5.0.0.1"
s.add_dependency "slimmer"
Expand All @@ -25,8 +25,10 @@ Gem::Specification.new do |s|
s.add_development_dependency "govuk-lint", "~> 2.1.0"
s.add_development_dependency "rspec", "~> 3.6"
s.add_development_dependency "capybara", "~> 2.14.4"
s.add_development_dependency "poltergeist", "~> 1.16.0"
s.add_development_dependency "jasmine", "~> 2.4.0"

# Needed to load slimmer test helpers
# https://github.com/alphagov/slimmer/issues/201
s.add_development_dependency "webmock", "~> 1.18.0"
s.add_development_dependency "webmock", "~> 3.0.1"
end
23 changes: 18 additions & 5 deletions spec/component_guide/component_guide_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
visit '/component-guide/test-component'

expect(body).to include('How to call this component')
expect(body).to include('render \'components/test-component\'')
expect(body).to include('render <span class="token string">\'components/test-component\'</span>')
end

it 'includes the component partial' do
Expand All @@ -64,12 +64,13 @@
it 'passes params in fixtures to component example' do
visit '/component-guide/test-component-with-params'
expect(body).to include('A test component that takes a required parameter')
expect(body).to include('render \'components/test-component-with-params\'')
expect(body).to include('render <span class="token string">\'components/test-component-with-params\'</span>')

expect(body).to include('test_component_parameter: &quot;Some value&quot;')

expect(body).to include('test_component_parameter<span class="token punctuation">:</span> <span class="token string">"Some value"</span>')
expect(page).to have_selector('.component-guide-preview .test-component-with-params', text: 'Some value')

expect(body).to include('test_component_parameter: &quot;A different value&quot;')
expect(body).to include('test_component_parameter<span class="token punctuation">:</span> <span class="token string">"A different value"</span>')
expect(page).to have_selector('.component-guide-preview .test-component-with-params', text: 'A different value')
end

Expand All @@ -83,12 +84,17 @@
end
end

it 'guide throws errors if component has an accessibility issue' do
expect { visit '/component-guide/test-component-with-a11y-issue' }
.to raise_error(Capybara::Poltergeist::JavascriptError)
end

it 'creates a page for each fixture' do
visit '/component-guide/test-component-with-params/another_fixture'
expect(body).to include('How to call this example')
expect(body).to include('How it looks')

expect(body).to include('test_component_parameter: &quot;A different value&quot;')
expect(body).to include('test_component_parameter<span class="token punctuation">:</span> <span class="token string">"A different value"</span>')
expect(page).to have_selector('.component-guide-preview .test-component-with-params', text: 'A different value')
end

Expand All @@ -110,6 +116,11 @@
end
end

it 'has accessibility testing hooks' do
visit '/component-guide/test-component'
expect(page).to have_selector('.component-guide-preview[data-module="test-a11y"]')
end

it 'displays the accessibility acceptance criteria of a component as html using static’s govspeak component' do
visit '/component-guide/test-component'
within ".component-accessibility-criteria" do
Expand All @@ -132,6 +143,7 @@
expect(page).to have_selector('.component-guide-preview-page .test-component-with-params', text: 'Some value')
expect(page).to have_selector('.component-guide-preview-page .preview-title', text: 'Another fixture')
expect(page).to have_selector('.component-guide-preview-page .test-component-with-params', text: 'A different value')
expect(page).to have_selector('.component-guide-preview-page [data-module="test-a11y"]')
end

it 'loads a preview page for a specific fixture' do
Expand All @@ -142,5 +154,6 @@

expect(page).to have_no_selector('.component-guide-preview-page .preview-title')
expect(page).to have_selector('.component-guide-preview-page .test-component-with-params', text: 'A different value')
expect(page).to have_selector('.component-guide-preview-page [data-module="test-a11y"]')
end
end
136 changes: 136 additions & 0 deletions spec/javascripts/govuk_publishing_components/AccessibilityTestSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/* global describe, afterEach, it, expect */

var TEST_SELECTOR = '.js-test-a11y'
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need updating too?

Copy link
Contributor Author

@NickColley NickColley Aug 24, 2017

Choose a reason for hiding this comment

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

I had some troubles with using a consistent data attribute, I don't think axe likes it as much when outputting the targeting.

Since this spec test the behaviour rather than how it works in the guide, this is fine.


var AccessibilityTest = window.GOVUK.AccessibilityTest

function addToDom (html, style) {
var div = document.createElement('div')
var htmlToInject = ''
htmlToInject += '<div class="' + TEST_SELECTOR.replace('.', '') + '">'
if (style) {
htmlToInject += '<style>' + style + '</style>'
}
htmlToInject += html + '</div>'
div.innerHTML = htmlToInject
document.getElementsByTagName('body')[0].appendChild(div)
}

function removeFromDom (selector) {
var nodeToRemove = document.querySelector(selector)
if (nodeToRemove) {
nodeToRemove.parentNode.removeChild(nodeToRemove)
}
}

function renderErrorMessage (option) {
var url = window.location.href
var message = ''
if (!option.skipHeader) {
message += '\nAccessibility issues at ' + url + '\n\n'
}
message += (
'Problem: ' + option.problem + '\n' +
'\n' +
'\n' +
' Check the HTML:\n' +
' `' + option.html + '`\n' +
' found with the selector:\n' +
' "' + option.selector + '"\n' +
'\n' +
'\n' +
'Try fixing it with this help: ' + option.helpUrl
)
return message
}

describe('AccessibilityTest', function () {
afterEach(function () {
removeFromDom(TEST_SELECTOR)
})

it('should do nothing if no callback is specified', function () {
AccessibilityTest(TEST_SELECTOR)
})

it('should error if no selector is found', function (done) {
AccessibilityTest(TEST_SELECTOR, function (err, result) {
expect(result).toBe(undefined)
expect(err).toBe('No selector "' + TEST_SELECTOR + '" found')
done()
})
})

it('should throw if there\'s a contrast issue', function (done) {
addToDom('<a href="#">Low contrast</a>', 'a { background: white; color: #ddd }')
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👍


AccessibilityTest(TEST_SELECTOR, function (err, result) {
if (err) {
throw err
}

var errorMessage = renderErrorMessage({
problem: 'Elements must have sufficient color contrast',
html: '<a href="#" style="">Low contrast</a>',
selector: TEST_SELECTOR + ' > a',
helpUrl: 'https://dequeuniversity.com/rules/axe/2.3/color-contrast?application=axeAPI'
})

expect(result).toBe(errorMessage)

done()
})
})

it('should throw if there\'s a alt tag issue', function (done) {
addToDom('<img src="">')

AccessibilityTest(TEST_SELECTOR, function (err, result) {
if (err) {
throw err
}

var errorMessage = renderErrorMessage({
problem: 'Images must have alternate text',
html: '<img src="">',
selector: TEST_SELECTOR + ' > img',
helpUrl: 'https://dequeuniversity.com/rules/axe/2.3/image-alt?application=axeAPI'
})

expect(result).toBe(errorMessage)

done()
})
})

it('should throw on multiple issues', function (done) {
addToDom('<img src=""><a href="#">Low contrast</a>', 'a { background: white; color: #ddd }')

AccessibilityTest(TEST_SELECTOR, function (err, result) {
if (err) {
throw err
}

var errorMessage = (
renderErrorMessage({
problem: 'Elements must have sufficient color contrast',
html: '<a href="#" style="">Low contrast</a>',
selector: TEST_SELECTOR + ' > a',
helpUrl: 'https://dequeuniversity.com/rules/axe/2.3/color-contrast?application=axeAPI'
}) +
'\n\n- - -\n\n' +
renderErrorMessage({
skipHeader: true,
problem: 'Images must have alternate text',
html: '<img src="">',
selector: TEST_SELECTOR + ' > img',
helpUrl: 'https://dequeuniversity.com/rules/axe/2.3/image-alt?application=axeAPI'
})
)

expect(result).toBe(errorMessage)

done()
})
})
})
Loading