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

Check accessibility of previews with axe-core npm package. #1588

Merged
merged 27 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
22385d0
Don't skip color contrast rule
koddsson Nov 9, 2022
fb0ed82
Inject axe-core from node_modules into system tests
koddsson Nov 9, 2022
55a35ff
Fix rubocop violations
koddsson Nov 9, 2022
e376864
Fix `AccessibilityTest`
koddsson Nov 9, 2022
b456eaf
Match `aria-label` with element text contents
koddsson Nov 9, 2022
0189d13
Ensure that tooltip text includes element text content
koddsson Nov 9, 2022
0bf2066
Fix violations reporting
koddsson Nov 9, 2022
3cf50ef
Add surrounding text to link previews for contrast checks
koddsson Nov 9, 2022
8cb08c9
Output path when assertion fails
koddsson Nov 9, 2022
22987f7
Add a changeset
koddsson Nov 9, 2022
c00be25
Rescue error correctly
koddsson Nov 9, 2022
07bec9c
Explicitly install `axe-core@4.5`
koddsson Nov 9, 2022
ff171b9
Skip `aria-required-children` which is broken
koddsson Nov 9, 2022
bbcf293
Skip anouther rule
koddsson Nov 9, 2022
ec1ccd4
Revert some things
koddsson Nov 9, 2022
a7731f6
Remove variable from `assert_accessible` again.
koddsson Nov 9, 2022
77135a9
Remove `axe-core-api` gem
koddsson Nov 9, 2022
9e5d4b0
Remove old `require` statements
koddsson Nov 9, 2022
aa32f06
Fix formatting in system/test_case.rb
camertron Jan 19, 2023
07286a0
Merge upstream
camertron Jan 19, 2023
a9797c5
Make Rubocop happy
camertron Jan 19, 2023
0546173
Remove file re-introduced by bad merge
camertron Jan 19, 2023
9d40360
Generating component snapshots
camertron Jan 19, 2023
294f519
Fix clipboard copy preview
camertron Jan 19, 2023
57255c7
Merge branch 'system-tests' of github.com:primer/view_components into…
camertron Jan 19, 2023
d6b8e38
Use code from dotcom
camertron Jan 19, 2023
be3fafa
Fix rubocop violations
camertron Jan 19, 2023
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
5 changes: 5 additions & 0 deletions .changeset/healthy-lamps-bow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

Update `axe-core` scanning in system tests
33 changes: 0 additions & 33 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ GEM
allocation_tracer (0.6.3)
ansi (1.5.0)
ast (2.4.2)
axe-core-api (4.2.1)
capybara
dumb_delegator
selenium-webdriver
virtus
watir
axiom-types (0.1.1)
descendants_tracker (~> 0.0.4)
ice_nine (~> 0.11.0)
thread_safe (~> 0.3, >= 0.3.1)
benchmark-ips (2.8.4)
better_html (2.0.1)
actionview (>= 6.0)
Expand All @@ -71,11 +61,8 @@ GEM
rack-test (>= 0.6.3)
regexp_parser (>= 1.5, < 3.0)
xpath (~> 3.2)
childprocess (4.1.0)
cliver (0.3.2)
coderay (1.1.3)
coercible (1.0.0)
descendants_tracker (~> 0.0.1)
concurrent-ruby (1.1.10)
crack (0.4.5)
rexml
Expand All @@ -85,10 +72,7 @@ GEM
cuprite (0.13)
capybara (>= 2.1, < 4)
ferrum (~> 0.11.0)
descendants_tracker (0.0.4)
thread_safe (~> 0.3, >= 0.3.1)
docile (1.4.0)
dumb_delegator (1.0.0)
erb_lint (0.2.0)
activesupport
better_html (>= 2.0.1)
Expand All @@ -109,7 +93,6 @@ GEM
htmlentities (4.3.4)
i18n (1.12.0)
concurrent-ruby (~> 1.0)
ice_nine (0.11.2)
listen (3.7.1)
rb-fsevent (~> 0.10, >= 0.10.3)
rb-inotify (~> 0.9, >= 0.9.10)
Expand Down Expand Up @@ -204,12 +187,6 @@ GEM
rack (>= 1.1)
rubocop (>= 0.87.0)
ruby-progressbar (1.11.0)
rubyzip (2.3.2)
selenium-webdriver (4.4.0)
childprocess (>= 0.5, < 5.0)
rexml (~> 3.2, >= 3.2.5)
rubyzip (>= 1.2.2, < 3.0)
websocket (~> 1.0)
semantic_range (3.0.0)
simplecov (0.21.2)
docile (~> 1.1)
Expand All @@ -232,7 +209,6 @@ GEM
terminal-table (3.0.2)
unicode-display_width (>= 1.1.1, < 3)
thor (1.2.1)
thread_safe (0.3.6)
timecop (0.9.5)
tzinfo (2.0.5)
concurrent-ruby (~> 1.0)
Expand All @@ -241,13 +217,6 @@ GEM
activesupport (>= 5.2.0, < 8.0)
concurrent-ruby (~> 1.0)
method_source (~> 1.0)
virtus (2.0.0)
axiom-types (~> 0.1)
coercible (~> 1.0)
descendants_tracker (~> 0.0, >= 0.0.3)
watir (7.1.0)
regexp_parser (>= 1.2, < 3)
selenium-webdriver (~> 4.0)
webmock (3.18.1)
addressable (>= 2.8.0)
crack (>= 0.3.2)
Expand All @@ -258,7 +227,6 @@ GEM
railties (>= 5.2)
semantic_range (>= 2.3.0)
webrick (1.7.0)
websocket (1.2.9)
websocket-driver (0.7.5)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.5)
Expand All @@ -278,7 +246,6 @@ DEPENDENCIES
activesupport (= 7.0.3)
allocation_stats (~> 0.1)
allocation_tracer (~> 0.6.3)
axe-core-api (~> 4.2.0)
benchmark-ips (~> 2.8.4)
bootsnap (>= 1.4.2)
capybara (~> 3)
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"@rollup/plugin-typescript": "^8.3.3",
"@typescript-eslint/eslint-plugin": "^5.31.0",
"@typescript-eslint/parser": "^5.31.0",
"axe-core": "^4.5.1",
"chokidar-cli": "^3.0.0",
"cssnano": "^5.1.13",
"eslint": "^8.23.1",
Expand Down
16 changes: 8 additions & 8 deletions previews/primer/alpha/tooltip_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class TooltipPreview < ViewComponent::Preview
# @param type [Symbol] select [["Description", description], ["Label", label]]
# @param direction select [s, n, e, w, ne, nw, se, sw]
# @param tooltip_text text
def playground(type: :description, direction: :s, tooltip_text: "Tooltip text")
def playground(type: :description, direction: :s, tooltip_text: "You can press a button")
camertron marked this conversation as resolved.
Show resolved Hide resolved
render(Primer::Beta::Button.new(id: "button-with-tooltip")) do |component|
component.with_tooltip(text: tooltip_text, type: type, direction: direction)
"Button"
Expand All @@ -17,7 +17,7 @@ def playground(type: :description, direction: :s, tooltip_text: "Tooltip text")
# @param type [Symbol] select [["Description", description], ["Label", label]]
# @param direction select [s, n, e, w, ne, nw, se, sw]
# @param tooltip_text text
def default(type: :description, direction: :s, tooltip_text: "Tooltip text")
def default(type: :description, direction: :s, tooltip_text: "You can press a button")
render(Primer::Beta::Button.new(id: "button-with-tooltip")) do |component|
component.with_tooltip(text: tooltip_text, type: type, direction: direction)
"Button"
Expand All @@ -26,7 +26,7 @@ def default(type: :description, direction: :s, tooltip_text: "Tooltip text")

# @param direction select [s, n, e, w, ne, nw, se, sw]
# @param tooltip_text text
def label_tooltip_on_button_with_existing_labelledby(type: :label, direction: :s, tooltip_text: "Tooltip text")
def label_tooltip_on_button_with_existing_labelledby(type: :label, direction: :s, tooltip_text: "You can press a button")
render(Primer::Beta::Button.new(id: "button-with-existing-label", "aria-labelledby": "existing-label-id")) do |component|
component.with_tooltip(text: tooltip_text, type: type, direction: direction)
"Button"
Expand All @@ -35,7 +35,7 @@ def label_tooltip_on_button_with_existing_labelledby(type: :label, direction: :s

# @param direction select [s, n, e, w, ne, nw, se, sw]
# @param tooltip_text text
def description_tooltip_on_button_with_existing_describedby(type: :description, direction: :s, tooltip_text: "Tooltip text")
def description_tooltip_on_button_with_existing_describedby(type: :description, direction: :s, tooltip_text: "You can press a button")
render(Primer::Beta::Button.new(id: "button-with-existing-description", "aria-describedby": "existing-description-id")) do |component|
component.with_tooltip(text: tooltip_text, type: type, direction: direction)
"Button"
Expand All @@ -56,7 +56,7 @@ def with_right_most_position(type: :description, direction: :s, tooltip_text: "A

# @param direction select [s, n, e, w, ne, nw, se, sw]
# @param tooltip_text text
def with_multiple_on_a_page(type: :description, direction: :s, tooltip_text: "Tooltip text")
def with_multiple_on_a_page(type: :description, direction: :s, tooltip_text: "You can press a button")
render_with_template(
locals: {
type: type,
Expand All @@ -68,23 +68,23 @@ def with_multiple_on_a_page(type: :description, direction: :s, tooltip_text: "To

# @!group Tooltip enabled elements
# @label Tooltip with Primer::Beta::Button
def tooltip_with_button(type: :description, direction: :s, tooltip_text: "Tooltip text")
def tooltip_with_button(type: :description, direction: :s, tooltip_text: "You can press a button")
render(Primer::Beta::Button.new(id: "button-with-tooltip")) do |component|
component.with_tooltip(text: tooltip_text, type: type, direction: direction)
"Button"
end
end

# @label Tooltip with Primer::Beta::Link
def tooltip_with_link(type: :description, direction: :s, tooltip_text: "Tooltip text")
def tooltip_with_link(type: :description, direction: :s, tooltip_text: "You can press a button")
render(Primer::Beta::Link.new(href: "#link-with-tooltip", id: "link-with-tooltip")) do |component|
component.with_tooltip(text: tooltip_text, type: type, direction: direction)
"Button"
end
end

# @label Tooltip with Primer::IconButton
def tooltip_with_icon_button(direction: :s, tooltip_text: "Tooltip text")
def tooltip_with_icon_button(direction: :s, tooltip_text: "You can press a button")
render(Primer::Beta::IconButton.new(icon: :search, "aria-label": tooltip_text, tooltip_direction: direction))
end
# @!endgroup
Expand Down
1 change: 0 additions & 1 deletion primer_view_components.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ Gem::Specification.new do |spec|

spec.add_development_dependency "allocation_stats", "~> 0.1"
spec.add_development_dependency "allocation_tracer", "~> 0.6.3"
spec.add_development_dependency "axe-core-api", "~> 4.2.0"
spec.add_development_dependency "benchmark-ips", "~> 2.8.4"
spec.add_development_dependency "capybara", "~> 3"
spec.add_development_dependency "cuprite", "= 0.13"
Expand Down
2 changes: 1 addition & 1 deletion test/accessibility_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class AccessibilityTest < System::TestCase
component_previews.each do |preview|
define_method(:"test_#{component_uri.parameterize(separator: "_")}_#{preview}") do
visit("/rails/view_components/#{component_uri}/#{preview}")
assert_accessible(page)
assert_accessible
puts "#{component_uri}##{preview} passed check."
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/system/alpha/tooltip_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def test_renders
visit_preview(:default)

assert_selector("button[id='button-with-tooltip']")
assert_selector("tool-tip[for='button-with-tooltip'][data-view-component][role='tooltip']", text: "Tooltip text", visible: :hidden)
assert_selector("tool-tip[for='button-with-tooltip'][data-view-component][role='tooltip']", text: "You can press a button", visible: :hidden)
assert_equal(find("button")["aria-describedby"], find("tool-tip", visible: :hidden)["id"])
end

Expand Down
84 changes: 67 additions & 17 deletions test/system/test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
require "capybara/rails"
require "capybara/minitest"

require "axe/matchers/be_axe_clean"
require "axe/expectation"

require "test_helpers/cuprite_setup"
require "test_helpers/retry"

Expand All @@ -16,8 +13,15 @@ class TestCase < ActionDispatch::SystemTestCase

# Skip `:region` which relates to preview page structure rather than individual component.
# Skip `:color-contrast` which requires primer design-level change.
AXE_RULES_TO_SKIP = [:region, :"color-contrast"].freeze
AXE_WITHIN_SELECTOR = "body"
# Skip `:aria-required-children` is broken in 4.5: https://github.com/dequelabs/axe-core/issues/3758
# Skip `:link-in-text-block` which is new and seems broken.
AXE_RULES_TO_SKIP = %i[
region
color-contrast
color-contrast-enhanced
aria-required-children
link-in-text-block
].freeze

def visit_preview(preview_name, params = {})
component_name = self.class.name.gsub("Test", "").gsub("Integration", "")
Expand All @@ -33,34 +37,80 @@ def visit_preview(preview_name, params = {})

visit(url)

assert_accessible(page)
assert_accessible
end

def assert_accessible(page, within: AXE_WITHIN_SELECTOR, skipping: AXE_RULES_TO_SKIP, **options)
options[:within] = within
options[:skipping] = skipping

is_axe_clean = Axe::Matchers::BeAxeClean.new.tap do |a|
options.each do |option|
key, value = option
a.send(key, *value)
def format_accessibility_errors(violations)
# rubocop:disable Lint/UnusedBlockArgument
results = violations.flat_map.with_index do |summary, index|
summary["nodes"].map do |node|
violations = node["any"] + node["all"]
# rubocop:disable Security/Eval
eval(Erubi::Engine.new(<<~ERB, trim: true).src)
<%= index + 1 %>) <%= summary["id"] %>: <%= summary["description"] %> (<%= summary["impact"] %>)
<%= summary["helpUrl"] %>

The following <%= violations.size %> <%= violations.size == 1 ? "node violates" : "nodes violate" %> this rule:
<% violations.each do |_violation| %>
Selector: <%= node["target"].join(", ") %>
HTML: <%= node["html"] %>
<%= node["failureSummary"] %>
<% end %>
ERB
# rubocop:enable Security/Eval
end
end
# rubocop:enable Lint/UnusedBlockArgument

<<~OUTPUT
Found #{violations.size} accessibility violations:
#{results.join}
OUTPUT
end

def assert_accessible
axe_exists = page.driver.evaluate_script("!!window.axe")

results = page.driver.evaluate_async_script(<<~JS)
const callback = arguments[arguments.length - 1];
#{File.read('node_modules/axe-core/axe.min.js') unless axe_exists}

const rulesToRun = axe
.getRules(["wcag2a", "wcag21a", "wcag2aa", "wcag2aaa", "wcag21aa", "wcag21aaa"])
.map(rule => rule.ruleId)
.filter(id => ![#{AXE_RULES_TO_SKIP.map { |id| "'#{id}'" }.join(', ')}].includes(id))

const options = {
elementRef: true,
resultTypes: ['violations', 'incomplete'],
runOnly: {
type: 'rule',
values: rulesToRun
}
}
axe.run(document.body, options).then(res => JSON.parse(JSON.stringify(res))).then(callback);
JS

violations = results["violations"] + results["incomplete"].select do |item|
item["impact"] != "serious" && item["impact"] != "critical"
end

message = format_accessibility_errors(violations)

Axe::AccessibleExpectation.new.assert page, is_axe_clean
assert violations.empty?, message
end

# Capybara Overrides to run accessibility checks when UI changes.
def fill_in(locator = nil, **kwargs)
super

assert_accessible(page)
assert_accessible
end

def click_button(locator = nil, **kwargs)
super

assert_accessible(page)
assert_accessible
end
end
end