Skip to content
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
<div class="mfa-selection">
<%= check_box_tag(
'two_factor_options_form[selection][]',
option.type,
Expand All @@ -7,16 +6,25 @@
checked: option.disabled?,
class: 'usa-checkbox__input usa-checkbox__input--tile',
id: "two_factor_options_form_selection_#{option.type}",
aria: {
describedby: [
"two_factor_options_form_selection_description_#{option.type}",
"two_factor_options_form_selection_info_#{option.type}",
],
},
) %>
<%= label_tag(
"two_factor_options_form_selection_#{option.type}",
class: 'usa-checkbox__label usa-checkbox__label--illustrated',
) do %>
<%= image_tag(asset_url("mfa-options/#{option.type}.svg"), alt: "#{option.label} icon", class: 'usa-checkbox__image') %>
<div class="usa-checkbox__label--text"><%= option.label %> <%= option.mfa_configuration_description %>
<span class="usa-checkbox__label-description">
<%= image_tag(asset_url("mfa-options/#{option.type}.svg"), alt: '', class: 'usa-checkbox__image') %>
<div class="usa-checkbox__label--text">
<%= option.label %>
<span id="two_factor_options_form_selection_description_<%= option.type %>" aria-hidden="true">
<%= option.mfa_configuration_description %>
</span>
<span id="two_factor_options_form_selection_info_<%= option.type %>" aria-hidden="true" class="usa-checkbox__label-description">
<%= option.info %>
</span>
</div>
<% end %>
</div>
2 changes: 1 addition & 1 deletion app/views/users/mfa_selection/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<% @presenter.options.each do |option| %>
<div id="<%= "select_#{option.type}" %>" class="<%= option.html_class %>">
<%= render partial: 'partials/multi_factor_authentication/mfa_selection',
locals: { form: f, option: option } %>
locals: { option: option } %>
</div>
<% end %>
</fieldset>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<% @presenter.options.each do |option| %>
<div id="<%= "select_#{option.type}" %>" class="<%= option.html_class %>">
<%= render partial: 'partials/multi_factor_authentication/mfa_selection',
locals: { form: f, option: option } %>
locals: { option: option } %>
</div>
<% end %>
</fieldset>
Expand Down
7 changes: 7 additions & 0 deletions spec/features/accessibility/user_pages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@

expect(current_path).to eq(authentication_methods_setup_path)
expect_page_to_have_no_accessibility_violations(page)
phone_checkbox = page.find_field('two_factor_options_form_selection_phone', visible: :all)
expect(phone_checkbox).to have_name(
t('two_factor_authentication.two_factor_choice_options.phone'),
)
expect(phone_checkbox).to have_description(
t('two_factor_authentication.two_factor_choice_options.phone_info'),
)
end

scenario 'phone setup page' do
Expand Down
100 changes: 100 additions & 0 deletions spec/support/matchers/accessibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,106 @@ def descriptors(element)
end
end

RSpec::Matchers.define :have_name do |name|

@aduth aduth May 3, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note: I realized this was a deeper rabbit hole than I imagined it would be after already pretty far down the rabbit hole 😅 (sunk cost fallacy and all that)

The reason for this is that Capybara's "label" checks are pretty forgiving, and don't require exactness. They also don't account for things like aria-hidden elements within the label. So a naive check for a label containing just "Text or voice message" would pass on main and not be sufficient for protecting against regressions.

# Implements a best effort approximation of W3C Accessible Name and Description Computation 1.2
# See: https://www.w3.org/TR/accname-1.2/#mapping_additional_nd_te
#
# In the future, consider implementing using Chrome DevTools Accessibility features. At time of
# writing, these are experimental and return a `nil` value in local testing.
#
# See: https://chromedevtools.github.io/devtools-protocol/tot/Accessibility/
#
# We can use the Capybara driver "bridge" to call these commands.
#
# Example: https://github.com/18F/identity-idp/blob/3c7d3be/spec/support/features/browser_emulation_helper.rb

def hidden_name(element)
# "If the current node is hidden [...] return the empty string."
#
# Note: This should also check page visibility, but Capybara's Element#visible? considers off-
# screen elements as non-visible, which is not the same as how it's considered for names.
'' if element['aria-hidden'] == 'true'
end

def aria_labelledby_name(element)
# "if computing a name, and the current node has an aria-labelledby attribute that contains at
# least one valid IDREF, and the current node is not already part of an aria-labelledby
# traversal, process its IDREFs in the order they occur"
valid_labels = element['aria-labelledby']&.
split(' ')&.
map { |label_id| page.find("##{label_id}")&.text }&.
compact

valid_labels.join('') if valid_labels.present?
end

def aria_label_name(element)
# "Otherwise, if computing a name, and if the current node has an aria-label attribute whose
# value is not the empty string, nor, when trimmed of white space, is not the empty string:"
element['aria-label']
end

def referenced_label_name(element)
# "Otherwise, if the current node's native markup provides an attribute (e.g. title) or element
# (e.g. HTML label) that defines a text alternative, return that alternative in the form of a
# flat string as defined by the host language"
descendent_name(page.find("label[for='#{element['id']}']"))
rescue Capybara::ElementNotFound
nil
end

def ancestor_label_text_name(element)
# "Otherwise, if the current node is a control embedded within the label"
descendent_name(element.find(:xpath, 'ancestor::label'))
rescue Capybara::ElementNotFound
nil
end

def descendent_name(element)
# "Otherwise, if the current node is a descendant of an element whose Accessible Name or
# Accessible Description is being computed, and contains descendants, proceed to 2F.i."
#
# Note: There should probably be some recursion here with the other methods, but Capybara only
# allows us to work with elements and not text nodes, which presents a challenge for computing
# the descendent name.
visible_text_js = <<~JS
(function getVisibleChildNodeText(childNodes) {
return Array.from(childNodes).flatMap((child) => {
switch (child.nodeType) {
case Node.TEXT_NODE:
return child.nodeValue;
case Node.ELEMENT_NODE:
if (child.getAttribute('aria-hidden') !== 'true') {
return child.nodeName === 'IMG' ?
child.getAttribute('alt') :
getVisibleChildNodeText(child.childNodes);
}
}
});
})(arguments[0].childNodes).filter(Boolean).join('').trim();
JS

page.evaluate_script(visible_text_js, element)
end

def computed_name(element)
hidden_name(element) ||
aria_labelledby_name(element) ||
aria_label_name(element) ||
referenced_label_name(element) ||
ancestor_label_text_name(element)
end

match { |element| computed_name(element) == name }

failure_message do |element|
<<-STR.squish
Expected element would have computed name "#{name}".
Found #{computed_name(element)}.
STR
end
end

RSpec::Matchers.define :be_uniquely_titled do
# Attempts to validate conformance to WCAG Success Criteria 2.4.2: Page Titled
#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@
}
end

it 'renders a field with mfa-selection class' do
expect(rendered).to have_css('.mfa-selection')
it 'renders an unchecked, enabled checkbox field' do
expect(rendered).to have_field(
'two_factor_options_form[selection][]',
checked: false,
disabled: false,
)
end
end

Expand Down