diff --git a/app/views/users/totp_setup/new.html.erb b/app/views/users/totp_setup/new.html.erb index fa6e8228d61..df08d180fca 100644 --- a/app/views/users/totp_setup/new.html.erb +++ b/app/views/users/totp_setup/new.html.erb @@ -3,7 +3,7 @@ <%= render PageHeadingComponent.new.with_content(t('headings.totp_setup.new')) %>

- <%= t('forms.totp_setup.totp_intro') %> + <%= t('forms.totp_setup.totp_intro') %> <%= new_tab_link_to( t('links.what_is_totp'), help_center_redirect_path( @@ -17,46 +17,51 @@

<%= simple_form_for('', method: :patch, html: { class: 'margin-bottom-4' }) do |f| %> - <%= render ProcessListComponent.new(connected: true, class: 'margin-y-4') do |c| %> - <%= c.with_item(heading: t('forms.totp_setup.totp_step_1')) do %> -

<%= t('forms.totp_setup.totp_step_1a') %>

- <%= render ValidatedFieldComponent.new( - form: f, - name: :name, - required: true, - label: false, - wrapper_html: { class: 'margin-bottom-0' }, - input_html: { - aria: { label: t('forms.totp_setup.totp_step_1') }, - maxlength: 20, - }, - ) %> - <% end %> - <%= c.with_item(heading: t('forms.totp_setup.totp_step_2')) %> - <%= c.with_item(heading: t('forms.totp_setup.totp_step_3')) do %> -
- <%= image_tag @qrcode, size: 240, skip_pipeline: true, alt: t('image_description.totp_qrcode') %> -
-

<%= t('instructions.mfa.authenticator.manual_entry') %>

- - <%= @code %> - - <%= render ClipboardButtonComponent.new(clipboard_text: @code.upcase, outline: true) %> - <% end %> - <%= c.with_item(heading: t('forms.totp_setup.totp_step_4')) do %> - <%= render OneTimeCodeInputComponent.new( - form: f, - transport: nil, - code_length: TwoFactorAuthenticatable::OTP_LENGTH, - field_options: { +
+ <%= render ProcessListComponent.new(connected: true) do |c| %> + <%= c.with_item(heading: t('forms.totp_setup.totp_step_1')) do %> +

<%= t('forms.totp_setup.totp_step_1a') %>

+ <%= render ValidatedFieldComponent.new( + form: f, + name: :name, + required: true, label: false, + wrapper_html: { class: 'margin-bottom-0' }, input_html: { - aria: { label: t('forms.totp_setup.totp_step_4') }, + aria: { label: t('forms.totp_setup.totp_step_1') }, + maxlength: 20, + }, + ) %> + <% end %> + <%= c.with_item(heading: t('forms.totp_setup.totp_step_2')) %> + <%= c.with_item(heading: t('forms.totp_setup.totp_step_3')) do %> +
+ <%= image_tag @qrcode, size: 240, skip_pipeline: true, alt: t('image_description.totp_qrcode') %> +
+

<%= t('instructions.mfa.authenticator.manual_entry') %>

+ + <%= @code %> + + <%= render ClipboardButtonComponent.new(clipboard_text: @code.upcase, outline: true) %> + <% end %> + <%= c.with_item(heading: t('forms.totp_setup.totp_step_4')) do %> + <%= render OneTimeCodeInputComponent.new( + form: f, + transport: nil, + code_length: TwoFactorAuthenticatable::OTP_LENGTH, + field_options: { + label: false, + wrapper_html: { + class: 'margin-bottom-0', + }, + input_html: { + aria: { label: t('forms.totp_setup.totp_step_4') }, + }, }, - }, - ) %> + ) %> + <% end %> <% end %> - <% end %> +
<%= f.input( :remember_device, as: :boolean, diff --git a/spec/features/users/totp_management_spec.rb b/spec/features/users/totp_management_spec.rb index e02c2df3e3d..e7610030323 100644 --- a/spec/features/users/totp_management_spec.rb +++ b/spec/features/users/totp_management_spec.rb @@ -43,6 +43,13 @@ click_link t('account.index.auth_app_add'), href: authenticator_setup_url + expect( + [ + page.find_field(t('forms.totp_setup.totp_step_1')), + page.find_field(t('forms.totp_setup.totp_step_4')), + ], + ).to be_logically_grouped(t('forms.totp_setup.totp_intro')) + secret = find('#qr-code').text fill_in 'name', with: 'foo' fill_in 'code', with: generate_totp_code(secret) diff --git a/spec/support/matchers/accessibility.rb b/spec/support/matchers/accessibility.rb index 5e2eca1095b..d24c5b6e50d 100644 --- a/spec/support/matchers/accessibility.rb +++ b/spec/support/matchers/accessibility.rb @@ -50,6 +50,24 @@ end end +RSpec::Matchers.define :be_logically_grouped do |name| + match do |inputs| + accessible_name = AccessibleName.new(page:) + group_names = inputs.map do |input| + group = input.ancestor('fieldset,[role=group]') + accessible_name.computed_name(group) + end + + group_names.compact.uniq == [name] + end + + failure_message do + <<-STR.squish + Expected inputs to be logically grouped with as "#{name}" + STR + end +end + RSpec::Matchers.define :have_valid_markup do def page_html if page.driver.is_a?(Capybara::Selenium::Driver) @@ -112,8 +130,24 @@ def descriptors(element) end RSpec::Matchers.define :have_name do |name| + match { |element| AccessibleName.new(page:).computed_name(element) == name } + + failure_message do |element| + <<-STR.squish + Expected element would have computed name "#{name}". + Found #{computed_name(element)}. + STR + end +end + +class AccessibleName + attr_reader :page + # Implements a best effort approximation of W3C Accessible Name and Description Computation 1.2 + # and naming behaviors described in the HTML Accessibility API Mappings 1.0 + # # See: https://www.w3.org/TR/accname-1.2/#mapping_additional_nd_te + # See: https://w3c.github.io/html-aam/ # # 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. @@ -124,6 +158,21 @@ def descriptors(element) # # Example: https://github.com/18F/identity-idp/blob/3c7d3be/spec/support/features/browser_emulation_helper.rb + def initialize(page:) + @page = page + 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) || + fieldset_legend_name(element) + end + + private + def hidden_name(element) # "If the current node is hidden [...] return the empty string." # @@ -193,21 +242,19 @@ def descendent_name(element) 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 + def fieldset_legend_name(element) + # See: https://w3c.github.io/html-aam/#fieldset-element-accessible-name-computation + return if element.tag_name != 'fieldset' - match { |element| computed_name(element) == name } + # "If the accessible name is still empty, then: if the fieldset element has a child that is a + # legend element, then use the subtree of the first such element." + begin + return element.find('legend').text + rescue Capybara::ElementNotFound; end - failure_message do |element| - <<-STR.squish - Expected element would have computed name "#{name}". - Found #{computed_name(element)}. - STR + # "If the accessible name is still empty, then:, if the fieldset element has a title + # attribute, then use that attribute." + element['title'] end end