From 9fdb07d3c6cfd676fd3580a43dd00e733b70403c Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 17 Nov 2023 09:58:10 -0800 Subject: [PATCH 1/7] Update State ID hint, move HTML out of translation strings into template **Why**: Keeping complex HTML inside translation strings is error-prone and can be easy to accidentally mis-align across languages changelog: Internal, Source code, Reorganize hint text for in-person state ID --- app/views/idv/in_person/state_id.html.erb | 15 ++++++++++++++- config/locales/in_person_proofing/en.yml | 12 +++++------- config/locales/in_person_proofing/es.yml | 12 +++++------- config/locales/in_person_proofing/fr.yml | 12 +++++------- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/app/views/idv/in_person/state_id.html.erb b/app/views/idv/in_person/state_id.html.erb index 9c129515d07..4cb1a0a59d6 100644 --- a/app/views/idv/in_person/state_id.html.erb +++ b/app/views/idv/in_person/state_id.html.erb @@ -110,10 +110,23 @@ ) %>
+ <% state_id_hint = capture do %> + <%= t('in_person_proofing.form.state_id.state_id_number_hint') %> + <% [ + [t('in_person_proofing.form.state_id.state_id_number_hint_spaces'), ' '], + [t('in_person_proofing.form.state_id.state_id_number_hint_forward_slashes'), '/'], + [t('in_person_proofing.form.state_id.state_id_number_hint_asterisks'), '*'], + [t('in_person_proofing.form.state_id.state_id_number_hint_dashes'), '-', true], + ].each do |text, symbol, last| %> + <%= text %><%= ',' if !last %> + + <% end %> + <% end %> + <%= render ValidatedFieldComponent.new( name: :state_id_number, form: f, - hint: t('in_person_proofing.form.state_id.state_id_number_hint_html'), + hint: state_id_hint, hint_html: { class: 'tablet:grid-col-10' }, input_html: { value: pii[:state_id_number] }, label: t('in_person_proofing.form.state_id.state_id_number'), diff --git a/config/locales/in_person_proofing/en.yml b/config/locales/in_person_proofing/en.yml index 2f91302f5e5..4eda871d5be 100644 --- a/config/locales/in_person_proofing/en.yml +++ b/config/locales/in_person_proofing/en.yml @@ -131,13 +131,11 @@ en: state_id_jurisdiction_hint: This is the state that issued your ID state_id_jurisdiction_prompt: '- Select -' state_id_number: ID number - state_id_number_hint_html: "May include letters, numbers, and the following - symbols: spaces, forward - slashes, asterisks, - dashes" + state_id_number_hint: 'May include letters, numbers, and the following symbols:' + state_id_number_hint_asterisks: asterisks + state_id_number_hint_dashes: dashes + state_id_number_hint_forward_slashes: forward slashes + state_id_number_hint_spaces: spaces zipcode: ZIP Code headings: address: Enter your current residential address diff --git a/config/locales/in_person_proofing/es.yml b/config/locales/in_person_proofing/es.yml index 855cb080701..7660371e705 100644 --- a/config/locales/in_person_proofing/es.yml +++ b/config/locales/in_person_proofing/es.yml @@ -143,13 +143,11 @@ es: state_id_jurisdiction_hint: Este es el estado que emitió su identificación state_id_jurisdiction_prompt: '- Seleccione -' state_id_number: Número de identidad - state_id_number_hint_html: "Puede incluir letras, números y los siguientes - símbolos: espacios, barras - diagonales, asteriscos, guiones" + state_id_number_hint: 'Puede incluir letras, números y los siguientes símbolos:' + state_id_number_hint_asterisks: 'asteriscos' + state_id_number_hint_dashes: guiones + state_id_number_hint_forward_slashes: 'barras diagonal' + state_id_number_hint_spaces: 'espacios' zipcode: Código postal headings: address: Ingresa tu domicilio actual diff --git a/config/locales/in_person_proofing/fr.yml b/config/locales/in_person_proofing/fr.yml index 91657bdacb3..8f7ff60de71 100644 --- a/config/locales/in_person_proofing/fr.yml +++ b/config/locales/in_person_proofing/fr.yml @@ -143,13 +143,11 @@ fr: state_id_jurisdiction_hint: Il s’agit de l’État qui a émis votre pièce d’identité state_id_jurisdiction_prompt: '- Sélectionnez -' state_id_number: Numéro d’identification - state_id_number_hint_html: "Il peut s’agir de lettres, de chiffres et des - symboles suivants: des espaces, des barres - obliques, des astérisques, des - tirets" + state_id_number_hint: 'Il peut s’agir de lettres, de chiffres et des symboles suivants:' + state_id_number_hint_asterisks: 'des astérisques' + state_id_number_hint_dashes: 'des tirets' + state_id_number_hint_forward_slashes: 'des barres obliques' + state_id_number_hint_spaces: 'des espaces' zipcode: Code postal headings: address: Indiquez votre adresse résidentielle actuelle From c01cd61df76373c250c941dbcbdc6896f4c7520d Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 17 Nov 2023 10:21:44 -0800 Subject: [PATCH 2/7] variable rename --- app/views/idv/in_person/state_id.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/idv/in_person/state_id.html.erb b/app/views/idv/in_person/state_id.html.erb index 4cb1a0a59d6..a5ce2955b5b 100644 --- a/app/views/idv/in_person/state_id.html.erb +++ b/app/views/idv/in_person/state_id.html.erb @@ -110,7 +110,7 @@ ) %>
- <% state_id_hint = capture do %> + <% state_id_number_hint = capture do %> <%= t('in_person_proofing.form.state_id.state_id_number_hint') %> <% [ [t('in_person_proofing.form.state_id.state_id_number_hint_spaces'), ' '], @@ -126,7 +126,7 @@ <%= render ValidatedFieldComponent.new( name: :state_id_number, form: f, - hint: state_id_hint, + hint: state_id_number_hint, hint_html: { class: 'tablet:grid-col-10' }, input_html: { value: pii[:state_id_number] }, label: t('in_person_proofing.form.state_id.state_id_number'), From b0b839e6650cc2691d7a5e66e9e71d3b65134ae0 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 17 Nov 2023 12:29:20 -0800 Subject: [PATCH 3/7] Refactor to support toggling for state-specific hints - Add HTML view specs too --- app/javascript/packs/state-guidance.ts | 22 ++++------ app/views/idv/in_person/state_id.html.erb | 16 +++++++- spec/javascript/packs/state-guidance-spec.js | 43 ++++++++++++++++---- 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/app/javascript/packs/state-guidance.ts b/app/javascript/packs/state-guidance.ts index b3a3d443467..3b7301d7b57 100644 --- a/app/javascript/packs/state-guidance.ts +++ b/app/javascript/packs/state-guidance.ts @@ -1,17 +1,13 @@ -import { t } from '@18f/identity-i18n'; - -function jurisdictionExtrasHintText(jurisdiction) { - switch (jurisdiction) { - case 'TX': - return t('in_person_proofing.form.state_id.state_id_number_texas_hint'); - default: - return t('in_person_proofing.form.state_id.state_id_number_hint'); - } -} - export function showOrHideJurisdictionExtras(jurisdictionCode) { - document.querySelectorAll('.jurisdiction-extras').forEach((element) => { - element.textContent = jurisdictionExtrasHintText(jurisdictionCode); + const hasJurisdictionSpecificHint = + jurisdictionCode && + document.querySelectorAll(`.jurisdiction-extras [data-state=${jurisdictionCode}]`).length > 0; + + document.querySelectorAll(`.jurisdiction-extras [data-state]`).forEach((element) => { + const shouldShow = + element.dataset.state === jurisdictionCode || + (!hasJurisdictionSpecificHint && element.dataset.state === 'default'); + element.classList.toggle('display-none', !shouldShow); }); } diff --git a/app/views/idv/in_person/state_id.html.erb b/app/views/idv/in_person/state_id.html.erb index cb192a0d598..8a550f2f11a 100644 --- a/app/views/idv/in_person/state_id.html.erb +++ b/app/views/idv/in_person/state_id.html.erb @@ -111,7 +111,7 @@ ) %>
- <% state_id_number_hint = capture do %> + <% state_id_number_hint_default = capture do %> <%= t('in_person_proofing.form.state_id.state_id_number_hint') %> <% [ [t('in_person_proofing.form.state_id.state_id_number_hint_spaces'), ' '], @@ -124,6 +124,20 @@ <% end %> <% end %> + <% state_id_number_hint = capture do %> + <% [ + [:default, state_id_number_hint_default], + ['TX', t('in_person_proofing.form.state_id.state_id_number_texas_hint')], + ].each do |state, hint| %> + <%= content_tag( + :span, + hint, + class: state == :default ? nil : 'display-none', + data: { state: }, + ) %> + <% end %> + <% end %> + <%= render ValidatedFieldComponent.new( name: :state_id_number, form: f, diff --git a/spec/javascript/packs/state-guidance-spec.js b/spec/javascript/packs/state-guidance-spec.js index df2405a2a48..f1b420a1d1f 100644 --- a/spec/javascript/packs/state-guidance-spec.js +++ b/spec/javascript/packs/state-guidance-spec.js @@ -41,7 +41,11 @@ describe('state-guidance', () => {
-
+
+ Default help text + + +
`; }); @@ -49,17 +53,33 @@ describe('state-guidance', () => { it('includes Texas specific hint text when Texas is selected', () => { const jurisdictionCode = 'TX'; showOrHideJurisdictionExtras(jurisdictionCode); - const elementInnerHtml = document.querySelector('.jurisdiction-extras')?.textContent; - expect(elementInnerHtml).to.eq('in_person_proofing.form.state_id.state_id_number_texas_hint'); + const allHintTexts = document.querySelectorAll('.jurisdiction-extras [data-state]'); + const texasText = document.querySelectorAll('.jurisdiction-extras [data-state=TX]'); + const nonTexasText = document.querySelectorAll( + '.jurisdiction-extras [data-state].display-none', + ); + + expect(texasText.length).to.eq(1); + expect(texasText[0].classList.contains('display-none')).to.eq(false); + + expect(nonTexasText.length + texasText.length).to.eq(allHintTexts.length); }); it('includes default hint text when no state is selected', () => { - const jurisdictionCode = ' '; + const jurisdictionCode = ''; showOrHideJurisdictionExtras(jurisdictionCode); - const elementInnerHtml = document.querySelector('.jurisdiction-extras')?.textContent; - expect(elementInnerHtml).to.eq('in_person_proofing.form.state_id.state_id_number_hint'); + const allHintTexts = document.querySelectorAll('.jurisdiction-extras [data-state]'); + const defaultText = document.querySelectorAll('.jurisdiction-extras [data-state=default]'); + const nonDefaultText = document.querySelectorAll( + '.jurisdiction-extras [data-state].display-none', + ); + + expect(defaultText.length).to.eq(1); + expect(defaultText[0].classList.contains('display-none')).to.eq(false); + + expect(nonDefaultText.length + defaultText.length).to.eq(allHintTexts.length); }); it('includes default hint text when a state without a state specific hint is selected', () => { @@ -67,7 +87,16 @@ describe('state-guidance', () => { showOrHideJurisdictionExtras(jurisdictionCode); const elementInnerHtml = document.querySelector('.jurisdiction-extras')?.textContent; - expect(elementInnerHtml).to.eq('in_person_proofing.form.state_id.state_id_number_hint'); + const allHintTexts = document.querySelectorAll('.jurisdiction-extras [data-state]'); + const defaultText = document.querySelectorAll('.jurisdiction-extras [data-state=default]'); + const nonDefaultText = document.querySelectorAll( + '.jurisdiction-extras [data-state].display-none', + ); + + expect(defaultText.length).to.eq(1); + expect(defaultText[0].classList.contains('display-none')).to.eq(false); + + expect(nonDefaultText.length + defaultText.length).to.eq(allHintTexts.length); }); }); }); From 4aa077cfc861f9b27ee564c03f50d6437304d1a1 Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 17 Nov 2023 12:39:08 -0800 Subject: [PATCH 4/7] commit missing view spec --- .../idv/in_person/state_id.html.erb_spec.rb | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 spec/views/idv/in_person/state_id.html.erb_spec.rb diff --git a/spec/views/idv/in_person/state_id.html.erb_spec.rb b/spec/views/idv/in_person/state_id.html.erb_spec.rb new file mode 100644 index 00000000000..cd056247f3a --- /dev/null +++ b/spec/views/idv/in_person/state_id.html.erb_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +RSpec.describe 'idv/in_person/state_id.html.erb' do + let(:pii) { {} } + let(:form) { Idv::StateIdForm.new(pii) } + let(:parsed_dob) { Date.new(1970, 1, 1) } + + before do + allow(view).to receive(:url_for).and_return('https://example.com/') + end + + subject(:render_template) do + render template: 'idv/in_person/state_id', locals: { updating_state_id: true, form: form, pii: pii, parsed_dob: parsed_dob } + end + + it 'renders state ID hint text with correct screenreader tags', aggregate_failures: true do + render_template + + doc = Nokogiri::HTML(rendered) + + jurisdiction_extras = doc.at_css('.jurisdiction-extras') + + all_hints = jurisdiction_extras.css('[data-state]') + shown = jurisdiction_extras.css('[data-state]:not(.display-none)') + hidden = jurisdiction_extras.css('[data-state].display-none') + + expect(shown.size).to eq(1), 'only shows one hint' + expect(shown.size + hidden.size).to eq(all_hints.size) + + default_hint = jurisdiction_extras.at_css('[data-state=default]') + default_hint_screenreader_tags = default_hint.css('.usa-sr-only') + *first, last = default_hint_screenreader_tags.map(&:text) + expect(first).to all end_with(',') + expect(last).to_not end_with(',') + end +end From 8e71e5c0492a0c183d40035585fb6e7cd0f3f73c Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 17 Nov 2023 14:21:06 -0800 Subject: [PATCH 5/7] lint fixes --- app/javascript/packs/state-guidance.ts | 2 +- spec/javascript/packs/state-guidance-spec.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/javascript/packs/state-guidance.ts b/app/javascript/packs/state-guidance.ts index 3b7301d7b57..a317fd668da 100644 --- a/app/javascript/packs/state-guidance.ts +++ b/app/javascript/packs/state-guidance.ts @@ -1,7 +1,7 @@ export function showOrHideJurisdictionExtras(jurisdictionCode) { const hasJurisdictionSpecificHint = jurisdictionCode && - document.querySelectorAll(`.jurisdiction-extras [data-state=${jurisdictionCode}]`).length > 0; + document.querySelectorAll(`.jurisdiction-extras [data-state=${jurisdictionCode}]`).length > 0; document.querySelectorAll(`.jurisdiction-extras [data-state]`).forEach((element) => { const shouldShow = diff --git a/spec/javascript/packs/state-guidance-spec.js b/spec/javascript/packs/state-guidance-spec.js index f1b420a1d1f..6314f2a7f3d 100644 --- a/spec/javascript/packs/state-guidance-spec.js +++ b/spec/javascript/packs/state-guidance-spec.js @@ -85,7 +85,6 @@ describe('state-guidance', () => { it('includes default hint text when a state without a state specific hint is selected', () => { const jurisdictionCode = 'NY'; showOrHideJurisdictionExtras(jurisdictionCode); - const elementInnerHtml = document.querySelector('.jurisdiction-extras')?.textContent; const allHintTexts = document.querySelectorAll('.jurisdiction-extras [data-state]'); const defaultText = document.querySelectorAll('.jurisdiction-extras [data-state=default]'); From 2f24d13cd3985ffda97884a4e5b9c725aee1f47d Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 17 Nov 2023 14:23:02 -0800 Subject: [PATCH 6/7] rubocop --- spec/views/idv/in_person/state_id.html.erb_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/views/idv/in_person/state_id.html.erb_spec.rb b/spec/views/idv/in_person/state_id.html.erb_spec.rb index cd056247f3a..f9be82b9844 100644 --- a/spec/views/idv/in_person/state_id.html.erb_spec.rb +++ b/spec/views/idv/in_person/state_id.html.erb_spec.rb @@ -10,7 +10,8 @@ end subject(:render_template) do - render template: 'idv/in_person/state_id', locals: { updating_state_id: true, form: form, pii: pii, parsed_dob: parsed_dob } + render template: 'idv/in_person/state_id', + locals: { updating_state_id: true, form: form, pii: pii, parsed_dob: parsed_dob } end it 'renders state ID hint text with correct screenreader tags', aggregate_failures: true do From 2473e90415429fe3eceb0daccb663c15465be72a Mon Sep 17 00:00:00 2001 From: Zach Margolis Date: Fri, 17 Nov 2023 14:41:30 -0800 Subject: [PATCH 7/7] erblint --- app/views/idv/in_person/state_id.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/idv/in_person/state_id.html.erb b/app/views/idv/in_person/state_id.html.erb index 8a550f2f11a..a22d2a03829 100644 --- a/app/views/idv/in_person/state_id.html.erb +++ b/app/views/idv/in_person/state_id.html.erb @@ -126,9 +126,9 @@ <% state_id_number_hint = capture do %> <% [ - [:default, state_id_number_hint_default], - ['TX', t('in_person_proofing.form.state_id.state_id_number_texas_hint')], - ].each do |state, hint| %> + [:default, state_id_number_hint_default], + ['TX', t('in_person_proofing.form.state_id.state_id_number_texas_hint')], + ].each do |state, hint| %> <%= content_tag( :span, hint,