From 315f5a151c9ef6a0dbb0580dc5a890a465ff7608 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 18 Nov 2021 12:54:33 -0500 Subject: [PATCH 01/31] LG-5333: Use PhoneInputComponent on IAL2 address verification **Why**: Phone input fields should use consistent styling and validation behavior. This requires additional handling to constrain the input to support only specific countries, since the address verification only supports U.S. phone numbers. --- app/assets/stylesheets/components/_form.scss | 10 +- .../stylesheets/components/_phone-input.scss | 18 +++- app/components/accordion_component.html.erb | 4 +- app/components/accordion_component.rb | 4 - app/components/base_component.rb | 4 + app/components/phone_input_component.html.erb | 18 +++- app/components/phone_input_component.rb | 36 +++++--- app/forms/idv/phone_form.rb | 2 +- .../eslint-plugin/configs/recommended.js | 1 + app/javascript/packages/phone-input/index.js | 68 +++++++++----- app/javascript/packs/form-validation.js | 92 ++++++++++++++----- app/javascript/packs/ial2-consent-button.js | 2 +- .../packs/otp-delivery-preference.js | 31 +------ app/views/idv/phone/new.html.erb | 7 +- config/locales/errors/en.yml | 1 + spec/components/base_component_spec.rb | 17 ++++ spec/features/idv/doc_auth/ssn_step_spec.rb | 2 +- 17 files changed, 199 insertions(+), 118 deletions(-) diff --git a/app/assets/stylesheets/components/_form.scss b/app/assets/stylesheets/components/_form.scss index 5745cec0aeb..85fe650bc6a 100644 --- a/app/assets/stylesheets/components/_form.scss +++ b/app/assets/stylesheets/components/_form.scss @@ -52,15 +52,7 @@ input::-webkit-inner-spin-button { .display-if-invalid { display: none; - [aria-invalid="true"] ~ & { - display: block; - } - - [aria-invalid="value-missing"] ~ &.display-if-invalid--value-missing { - display: block; - } - - [aria-invalid="pattern-mismatch"] ~ &.display-if-invalid--pattern-mismatch { + &.display-if-invalid--invalid { display: block; } } diff --git a/app/assets/stylesheets/components/_phone-input.scss b/app/assets/stylesheets/components/_phone-input.scss index 9e5a226da03..7b81a89e254 100644 --- a/app/assets/stylesheets/components/_phone-input.scss +++ b/app/assets/stylesheets/components/_phone-input.scss @@ -1,3 +1,11 @@ +lg-phone-input { + display: block; + + .iti__flag { + background-image: image-url('intl-tel-input/build/img/flags.png'); + } +} + .phone-input__international-code-wrapper { display: none; @@ -6,8 +14,14 @@ } } -lg-phone-input .iti__flag { - background-image: image-url('intl-tel-input/build/img/flags.png'); +.phone-input--single-country .iti input { + padding-left: 36px; + padding-right: 6px; +} + +.phone-input--single-country .iti .iti__flag-container { + left: 0; + right: auto; } @media (-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi) { diff --git a/app/components/accordion_component.html.erb b/app/components/accordion_component.html.erb index ffcbf829341..a7ab18f200f 100644 --- a/app/components/accordion_component.html.erb +++ b/app/components/accordion_component.html.erb @@ -4,12 +4,12 @@ type="button" class="usa-accordion__button" aria-expanded="false" - aria-controls="<%= @target_id %>" + aria-controls="accordion-<%= unique_id %>" > <%= header %> -
+
<%= content %>
diff --git a/app/components/accordion_component.rb b/app/components/accordion_component.rb index fdc84507e6d..4d971faef69 100644 --- a/app/components/accordion_component.rb +++ b/app/components/accordion_component.rb @@ -1,7 +1,3 @@ class AccordionComponent < BaseComponent renders_one :header - - def initialize - @target_id = "accordion-#{SecureRandom.hex(4)}" - end end diff --git a/app/components/base_component.rb b/app/components/base_component.rb index 85edfc12eb7..81ba0421ccb 100644 --- a/app/components/base_component.rb +++ b/app/components/base_component.rb @@ -10,4 +10,8 @@ def before_render def self.scripts @scripts ||= _sidecar_files(['js']).map { |file| File.basename(file, '.js') } end + + def unique_id + @unique_id ||= SecureRandom.hex(4) + end end diff --git a/app/components/phone_input_component.html.erb b/app/components/phone_input_component.html.erb index 12c8043c826..3d77b3a4727 100644 --- a/app/components/phone_input_component.html.erb +++ b/app/components/phone_input_component.html.erb @@ -1,9 +1,10 @@ - +<%= content_tag('lg-phone-input', class: css_class) do %> <%= content_tag( :script, { country_code_label: t('components.phone_input.country_code_label'), invalid_phone: t('errors.messages.improbable_phone'), + country_constraint_usa: t('errors.messages.phone_country_constraint_usa'), }.to_json, { type: 'application/json', @@ -40,10 +41,21 @@ as: :tel, required: required, label: false, + wrapper_html: { + class: 'margin-bottom-0', + }, input_html: { - aria: { invalid: false }, + aria: { + invalid: false, + describedby: "phone-input-error-#{unique_id}", + }, class: 'phone-input__number', }, ) %> - + +<% end %> <%= stylesheet_link_tag 'intl-tel-input/build/css/intlTelInput' %> diff --git a/app/components/phone_input_component.rb b/app/components/phone_input_component.rb index 208dd93aa90..97efb2adc59 100644 --- a/app/components/phone_input_component.rb +++ b/app/components/phone_input_component.rb @@ -1,29 +1,37 @@ class PhoneInputComponent < BaseComponent - attr_reader :form, :required + attr_reader :form, :required, :allowed_countries alias_method :f, :form - def initialize(form:, required: false) + def initialize(form:, allowed_countries: nil, required: false) + @allowed_countries = allowed_countries @form = form @required = required end def supported_country_codes - PhoneNumberCapabilities::INTERNATIONAL_CODES.keys + allowed_countries || PhoneNumberCapabilities::INTERNATIONAL_CODES.keys end def international_phone_codes - codes = PhoneNumberCapabilities::INTERNATIONAL_CODES.map do |key, value| - [ - international_phone_code_label(value), - key, - { data: international_phone_codes_data(value) }, - ] - end + supported_country_codes. + map do |code_key| + code_data = PhoneNumberCapabilities::INTERNATIONAL_CODES[code_key] + [ + international_phone_code_label(code_data), + code_key, + { data: international_phone_codes_data(code_data) }, + ] + end. + sort_by do |label, code_key, _data| + # Sort alphabetically by label, but put the US first in the list + [code_key == 'US' ? -1 : 1, label] + end + end - # Sort alphabetically by label, but put the US first in the list - codes.sort_by do |label, key, _data| - [key == 'US' ? -1 : 1, label] - end + def css_class + classes = ['margin-bottom-4'] + classes << 'phone-input--single-country' if supported_country_codes.size == 1 + classes end private diff --git a/app/forms/idv/phone_form.rb b/app/forms/idv/phone_form.rb index fbbcaa0c99d..9a747856935 100644 --- a/app/forms/idv/phone_form.rb +++ b/app/forms/idv/phone_form.rb @@ -4,7 +4,7 @@ class PhoneForm ALL_DELIVERY_METHODS = [:sms, :voice].freeze - attr_reader :user, :phone, :allowed_countries, :delivery_methods + attr_reader :user, :phone, :allowed_countries, :delivery_methods, :international_code validate :validate_valid_phone_for_allowed_countries validate :validate_phone_delivery_methods diff --git a/app/javascript/packages/eslint-plugin/configs/recommended.js b/app/javascript/packages/eslint-plugin/configs/recommended.js index f9f6beb90d5..b66e6bbe992 100644 --- a/app/javascript/packages/eslint-plugin/configs/recommended.js +++ b/app/javascript/packages/eslint-plugin/configs/recommended.js @@ -21,6 +21,7 @@ const config = { 'max-len': 'off', 'max-classes-per-file': 'off', 'newline-per-chained-call': 'off', + 'no-cond-assign': 'off', 'no-console': 'error', 'no-empty': ['error', { allowEmptyCatch: true }], 'no-param-reassign': ['off', 'never'], diff --git a/app/javascript/packages/phone-input/index.js b/app/javascript/packages/phone-input/index.js index bec088c0691..61bc2462f7b 100644 --- a/app/javascript/packages/phone-input/index.js +++ b/app/javascript/packages/phone-input/index.js @@ -1,12 +1,15 @@ -import { isValidNumber } from 'libphonenumber-js'; +import { isValidNumber, isValidNumberForRegion } from 'libphonenumber-js'; import 'intl-tel-input/build/js/utils.js'; import intlTelInput from 'intl-tel-input'; +/** @typedef {import('libphonenumber-js').CountryCode} CountryCode */ + /** * @typedef PhoneInputStrings * * @prop {string=} country_code_label * @prop {string=} invalid_phone + * @prop {string=} country_constraint_usa */ /** @@ -119,42 +122,63 @@ export class PhoneInput extends HTMLElement { initializeIntlTelInput() { const { supportedCountryCodes } = this; + const allowDropdown = supportedCountryCodes && supportedCountryCodes.length > 1; const iti = intlTelInput(this.textInput, { preferredCountries: ['US', 'CA'], onlyCountries: supportedCountryCodes, autoPlaceholder: 'off', + allowDropdown, }); - // Remove duplicate items in the country list - /** @type {NodeListOf} */ - const preferred = iti.countryList.querySelectorAll('.iti__preferred'); - preferred.forEach((listItem) => { - const { countryCode } = listItem.dataset; + if (allowDropdown) { + // Remove duplicate items in the country list /** @type {NodeListOf} */ - const duplicates = iti.countryList.querySelectorAll( - `.iti__standard[data-country-code="${countryCode}"]`, - ); - duplicates.forEach((duplicateListItem) => { - duplicateListItem.parentNode?.removeChild(duplicateListItem); + const preferred = iti.countryList.querySelectorAll('.iti__preferred'); + preferred.forEach((listItem) => { + const { countryCode } = listItem.dataset; + /** @type {NodeListOf} */ + const duplicates = iti.countryList.querySelectorAll( + `.iti__standard[data-country-code="${countryCode}"]`, + ); + duplicates.forEach((duplicateListItem) => { + duplicateListItem.parentNode?.removeChild(duplicateListItem); + }); }); - }); - // Improve base accessibility of intl-tel-input - iti.flagsContainer.setAttribute('aria-label', this.strings.country_code_label); - iti.selectedFlag.setAttribute('aria-haspopup', 'true'); - iti.selectedFlag.setAttribute('role', 'button'); - iti.selectedFlag.removeAttribute('aria-owns'); + // Improve base accessibility of intl-tel-input + iti.flagsContainer.setAttribute('aria-label', this.strings.country_code_label); + iti.selectedFlag.setAttribute('aria-haspopup', 'true'); + iti.selectedFlag.setAttribute('role', 'button'); + iti.selectedFlag.removeAttribute('aria-owns'); + } return iti; } validate() { - const { textInput, codeInput } = this; - if (textInput && codeInput) { - const isValid = isPhoneValid(textInput.value, codeInput.value); - const validity = (!isValid && this.strings.invalid_phone) || ''; - textInput.setCustomValidity(validity); + const { textInput, codeInput, supportedCountryCodes } = this; + if (!textInput || !codeInput) { + return; + } + + const phoneNumber = textInput.value; + const countryCode = /** @type {CountryCode} */ (codeInput.value); + + textInput.setCustomValidity(''); + if (!phoneNumber) { + return; + } + + const isInvalidCountry = + supportedCountryCodes?.length === 1 && !isValidNumberForRegion(phoneNumber, countryCode); + if (isInvalidCountry && countryCode === 'US' && this.strings.country_constraint_usa) { + textInput.setCustomValidity(this.strings.country_constraint_usa); + } + + const isInvalidPhoneNumber = !isPhoneValid(phoneNumber, countryCode); + if (isInvalidPhoneNumber && this.strings.invalid_phone) { + textInput.setCustomValidity(this.strings.invalid_phone); } } diff --git a/app/javascript/packs/form-validation.js b/app/javascript/packs/form-validation.js index fa612fa074c..e55e57cd79c 100644 --- a/app/javascript/packs/form-validation.js +++ b/app/javascript/packs/form-validation.js @@ -23,37 +23,47 @@ function kebabCase(string) { return string.replace(/(.)([A-Z])/g, '$1-$2').toLowerCase(); } -function resetInput(input) { - if (input.hasAttribute('data-form-validation-message')) { - input.setCustomValidity(''); - input.removeAttribute('data-form-validation-message'); +/** + * Returns the error message elements associated with the given input. + * + * @param {HTMLInputElement} input Input field. + * + * @return {Element[]} Error message elements. + */ +function getInputMessages(input) { + const messages = []; + + const describedBy = input.getAttribute('aria-describedby'); + if (describedBy) { + const descriptors = /** @type {Element[]} */ (describedBy + .split(' ') + .map((id) => document.getElementById(id)) + .filter(Boolean)); + + messages.push(...descriptors); } - input.setAttribute('aria-invalid', 'false'); - input.classList.remove('usa-input--error'); + + /** @type {Element?} */ + let sibling = input; + while ((sibling = sibling.nextElementSibling)) { + if (sibling.classList.contains('display-if-invalid')) { + messages.push(sibling); + } + } + + return messages; } /** - * Given an `input` or `invalid` event, updates custom validity of the given input. + * Given an `input` event, updates custom validity of the given input. * * @param {Event} event Input or invalid event. */ - function checkInputValidity(event) { const input = /** @type {HTMLInputElement} */ (event.target); - resetInput(input); - if ( - event.type === 'invalid' && - !input.validity.valid && - input.parentNode?.querySelector('.display-if-invalid') - ) { - event.preventDefault(); - const errors = Object.keys(ValidityState.prototype) - .filter((key) => key !== 'valid') - .filter((key) => input.validity[key]); - - input.setAttribute('aria-invalid', errors.length ? kebabCase(errors[0]) : 'false'); - input.classList.add('usa-input--error'); - input.focus(); + if (input.hasAttribute('data-form-validation-message')) { + input.setCustomValidity(''); + input.removeAttribute('data-form-validation-message'); } const { I18n } = /** @type {typeof window & LoginGovGlobal} */ (window).LoginGov; @@ -71,6 +81,41 @@ function checkInputValidity(event) { } } +/** + * Given an `input` or `invalid` event, toggles visibility of custom error messages. + * + * @param {Event} event Input or invalid event. + */ +function toggleErrorMessages(event) { + const input = /** @type {HTMLInputElement} */ (event.target); + const messages = getInputMessages(input); + const errors = Object.keys(ValidityState.prototype) + .filter((key) => key !== 'valid') + .filter((key) => input.validity[key]); + const activeMessages = errors + .map((type) => `display-if-invalid--${kebabCase(type)}`) + .flatMap((className) => messages.filter((message) => message.classList.contains(className))); + + input.setAttribute('aria-invalid', 'false'); + input.classList.remove('usa-input--error'); + messages.forEach((message) => message.classList.remove('display-if-invalid--invalid')); + + const hasActiveMessages = !!activeMessages.length; + if (event.type === 'invalid' && hasActiveMessages) { + event.preventDefault(); + + input.setAttribute('aria-invalid', 'true'); + input.classList.add('usa-input--error'); + input.focus(); + + const firstActiveMessage = activeMessages[0]; + firstActiveMessage.classList.add('display-if-invalid--invalid'); + if (firstActiveMessage.classList.contains('display-if-invalid--custom-error')) { + firstActiveMessage.textContent = input.validationMessage; + } + } +} + /** * Binds validation to a given input. * @@ -78,7 +123,8 @@ function checkInputValidity(event) { */ function validateInput(input) { input.addEventListener('input', checkInputValidity); - input.addEventListener('invalid', checkInputValidity); + input.addEventListener('input', toggleErrorMessages); + input.addEventListener('invalid', toggleErrorMessages); } /** diff --git a/app/javascript/packs/ial2-consent-button.js b/app/javascript/packs/ial2-consent-button.js index 345108c0481..310e5623275 100644 --- a/app/javascript/packs/ial2-consent-button.js +++ b/app/javascript/packs/ial2-consent-button.js @@ -7,7 +7,7 @@ function toggleButton() { continueButton.classList.toggle('usa-button--disabled', !checkbox.checked); errorMessage.classList.toggle( 'display-none', - checkbox.getAttribute('aria-invalid') !== 'value-missing', + checkbox.getAttribute('aria-invalid') === 'false', ); } diff --git a/app/javascript/packs/otp-delivery-preference.js b/app/javascript/packs/otp-delivery-preference.js index afb3ab5f1c3..fa59c71972c 100644 --- a/app/javascript/packs/otp-delivery-preference.js +++ b/app/javascript/packs/otp-delivery-preference.js @@ -24,12 +24,6 @@ const getOTPDeliveryMethods = () => const isDeliveryOptionSupported = (delivery, selectedOption) => selectedOption.getAttribute(`data-supports-${delivery}`) !== 'false'; -/** - * @param {HTMLFormElement} form - * @return {HTMLButtonElement|HTMLInputElement|null} - */ -const getSubmitButton = (form) => form.querySelector('button:not([type]),[type="submit"]'); - /** * @param {string} delivery * @param {string} location @@ -121,28 +115,5 @@ function updateOTPDeliveryMethods(event) { document.querySelectorAll('lg-phone-input').forEach((node) => { const phoneInput = /** @type {PhoneInput} */ (node); - const form = /** @type {HTMLFormElement} */ (phoneInput.closest('form')); - - function setSubmitDisabled(isDisabled) { - const submitButton = getSubmitButton(form); - if (submitButton && submitButton.disabled !== isDisabled) { - submitButton.disabled = isDisabled; - } - } - - phoneInput.addEventListener('input', () => setSubmitDisabled(!form.checkValidity())); - phoneInput.addEventListener('change', (event) => { - setSubmitDisabled(!form.checkValidity()); - updateOTPDeliveryMethods(event); - }); - phoneInput.addEventListener( - 'invalid', - (event) => { - setSubmitDisabled(true); - event.preventDefault(); - }, - true, - ); - - setSubmitDisabled(!form.checkValidity()); + phoneInput.addEventListener('change', updateOTPDeliveryMethods); }); diff --git a/app/views/idv/phone/new.html.erb b/app/views/idv/phone/new.html.erb index 4fc16457d0c..3db833babfb 100644 --- a/app/views/idv/phone/new.html.erb +++ b/app/views/idv/phone/new.html.erb @@ -57,12 +57,7 @@ method: :put, class: 'margin-top-2', }) do |f| %> - <%= f.input :phone, required: true, - input_html: { aria: { invalid: false }, - class: 'sm-col-8' }, - label: t('idv.form.phone'), - wrapper: false, - label_html: { class: 'bold' } %> + <%= render(PhoneInputComponent.new(form: f, allowed_countries: ['US'], required: true)) %> <%= t('simple_form.required.text') %> diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 86ef50b0edf..1b9a531a03b 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -70,6 +70,7 @@ en: otp_failed: Your security code failed to send. password_incorrect: Incorrect password personal_key_incorrect: Incorrect personal key + phone_country_constraint_usa: Must be a U.S. phone number phone_duplicate: This account is already using the phone number you entered as an authenticator. Please use a different phone number. phone_unsupported: Sorry, we are unable to send SMS at this time. Please try the diff --git a/spec/components/base_component_spec.rb b/spec/components/base_component_spec.rb index acc806ce98f..a0f4534f118 100644 --- a/spec/components/base_component_spec.rb +++ b/spec/components/base_component_spec.rb @@ -45,4 +45,21 @@ def call render_inline(ExampleComponentWithScript.new) end end + + describe '#unique_id' do + it 'provides a unique id' do + first_component = ExampleComponentWithScript.new + second_component = ExampleComponentWithScript.new + + expect(first_component.unique_id).to be_present + expect(second_component.unique_id).to be_present + expect(first_component.unique_id).not_to eq(second_component.unique_id) + end + + it 'is memoized' do + component = ExampleComponentWithScript.new + + expect(component.unique_id).to eq(component.unique_id) + end + end end diff --git a/spec/features/idv/doc_auth/ssn_step_spec.rb b/spec/features/idv/doc_auth/ssn_step_spec.rb index 9f1cb4d9dbb..b6975e3c90c 100644 --- a/spec/features/idv/doc_auth/ssn_step_spec.rb +++ b/spec/features/idv/doc_auth/ssn_step_spec.rb @@ -29,7 +29,7 @@ fill_out_ssn_form_fail click_idv_continue - expect(page.find('#doc_auth_ssn')['aria-invalid']).to eq('value-missing') + expect(page.find('#doc_auth_ssn')['aria-invalid']).to eq('true') expect(page).to have_current_path(idv_doc_auth_ssn_step) end From 97149d9e635d5b0245508f14b1479d94c44ce24a Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 18 Nov 2021 15:23:17 -0500 Subject: [PATCH 02/31] Restore no-cond-assign enforcement with except-parens See: https://github.com/18F/identity-idp/pull/5619#discussion_r752504290 --- app/javascript/packages/eslint-plugin/configs/recommended.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/packages/eslint-plugin/configs/recommended.js b/app/javascript/packages/eslint-plugin/configs/recommended.js index b66e6bbe992..9bca4ccca33 100644 --- a/app/javascript/packages/eslint-plugin/configs/recommended.js +++ b/app/javascript/packages/eslint-plugin/configs/recommended.js @@ -21,7 +21,7 @@ const config = { 'max-len': 'off', 'max-classes-per-file': 'off', 'newline-per-chained-call': 'off', - 'no-cond-assign': 'off', + 'no-cond-assign': ['error', 'except-parens'], 'no-console': 'error', 'no-empty': ['error', { allowEmptyCatch: true }], 'no-param-reassign': ['off', 'never'], From 491e6684ef1168f7f9f96fc6aacb416e96aa5975 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 18 Nov 2021 15:24:16 -0500 Subject: [PATCH 03/31] Validate allowed_countries against actual codes --- app/components/phone_input_component.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/components/phone_input_component.rb b/app/components/phone_input_component.rb index 97efb2adc59..bb800b400f0 100644 --- a/app/components/phone_input_component.rb +++ b/app/components/phone_input_component.rb @@ -9,7 +9,9 @@ def initialize(form:, allowed_countries: nil, required: false) end def supported_country_codes - allowed_countries || PhoneNumberCapabilities::INTERNATIONAL_CODES.keys + codes = PhoneNumberCapabilities::INTERNATIONAL_CODES.keys + codes = codes & allowed_countries if allowed_countries + codes end def international_phone_codes From 5b6bb9445ba049d9187793b23d014688c0abda39 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 19 Nov 2021 11:15:03 -0500 Subject: [PATCH 04/31] Revert OTP delivery preference validation changes Save for LG-5362 --- .../packs/otp-delivery-preference.js | 31 ++++++++++++++++++- app/views/users/phone_setup/index.html.erb | 2 +- app/views/users/phones/add.html.erb | 2 +- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/app/javascript/packs/otp-delivery-preference.js b/app/javascript/packs/otp-delivery-preference.js index fa59c71972c..afb3ab5f1c3 100644 --- a/app/javascript/packs/otp-delivery-preference.js +++ b/app/javascript/packs/otp-delivery-preference.js @@ -24,6 +24,12 @@ const getOTPDeliveryMethods = () => const isDeliveryOptionSupported = (delivery, selectedOption) => selectedOption.getAttribute(`data-supports-${delivery}`) !== 'false'; +/** + * @param {HTMLFormElement} form + * @return {HTMLButtonElement|HTMLInputElement|null} + */ +const getSubmitButton = (form) => form.querySelector('button:not([type]),[type="submit"]'); + /** * @param {string} delivery * @param {string} location @@ -115,5 +121,28 @@ function updateOTPDeliveryMethods(event) { document.querySelectorAll('lg-phone-input').forEach((node) => { const phoneInput = /** @type {PhoneInput} */ (node); - phoneInput.addEventListener('change', updateOTPDeliveryMethods); + const form = /** @type {HTMLFormElement} */ (phoneInput.closest('form')); + + function setSubmitDisabled(isDisabled) { + const submitButton = getSubmitButton(form); + if (submitButton && submitButton.disabled !== isDisabled) { + submitButton.disabled = isDisabled; + } + } + + phoneInput.addEventListener('input', () => setSubmitDisabled(!form.checkValidity())); + phoneInput.addEventListener('change', (event) => { + setSubmitDisabled(!form.checkValidity()); + updateOTPDeliveryMethods(event); + }); + phoneInput.addEventListener( + 'invalid', + (event) => { + setSubmitDisabled(true); + event.preventDefault(); + }, + true, + ); + + setSubmitDisabled(!form.checkValidity()); }); diff --git a/app/views/users/phone_setup/index.html.erb b/app/views/users/phone_setup/index.html.erb index 80777ca1cc4..296e60123e0 100644 --- a/app/views/users/phone_setup/index.html.erb +++ b/app/views/users/phone_setup/index.html.erb @@ -16,7 +16,7 @@ <% end %>

-<%= validated_form_for( +<%= simple_form_for( @new_phone_form, html: { autocomplete: 'off', method: :patch }, data: { international_phone_form: true }, diff --git a/app/views/users/phones/add.html.erb b/app/views/users/phones/add.html.erb index 7c968eba98f..c68df34e43a 100644 --- a/app/views/users/phones/add.html.erb +++ b/app/views/users/phones/add.html.erb @@ -14,7 +14,7 @@ <% end %>

-<%= validated_form_for( +<%= simple_form_for( @new_phone_form, html: { autocomplete: 'off', method: :post }, data: { international_phone_form: true }, From fd0e6d3b976c7744252be118902bb1bf6722b84f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 19 Nov 2021 14:50:22 -0500 Subject: [PATCH 05/31] Use self-assignment operator Per Rubocop. TIL! --- app/components/phone_input_component.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/phone_input_component.rb b/app/components/phone_input_component.rb index bb800b400f0..64f7d1ff5f1 100644 --- a/app/components/phone_input_component.rb +++ b/app/components/phone_input_component.rb @@ -10,7 +10,7 @@ def initialize(form:, allowed_countries: nil, required: false) def supported_country_codes codes = PhoneNumberCapabilities::INTERNATIONAL_CODES.keys - codes = codes & allowed_countries if allowed_countries + codes &= allowed_countries if allowed_countries codes end From 5c8a634bf006390ef51ab2ca836be1bb0d8e09e5 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 19 Nov 2021 17:09:15 -0500 Subject: [PATCH 06/31] Revert "Revert OTP delivery preference validation changes" This reverts commit a0c29c96d7ce22e8dcf307c1a14be691e6f08d10. --- .../packs/otp-delivery-preference.js | 31 +------------------ app/views/users/phone_setup/index.html.erb | 2 +- app/views/users/phones/add.html.erb | 2 +- 3 files changed, 3 insertions(+), 32 deletions(-) diff --git a/app/javascript/packs/otp-delivery-preference.js b/app/javascript/packs/otp-delivery-preference.js index afb3ab5f1c3..fa59c71972c 100644 --- a/app/javascript/packs/otp-delivery-preference.js +++ b/app/javascript/packs/otp-delivery-preference.js @@ -24,12 +24,6 @@ const getOTPDeliveryMethods = () => const isDeliveryOptionSupported = (delivery, selectedOption) => selectedOption.getAttribute(`data-supports-${delivery}`) !== 'false'; -/** - * @param {HTMLFormElement} form - * @return {HTMLButtonElement|HTMLInputElement|null} - */ -const getSubmitButton = (form) => form.querySelector('button:not([type]),[type="submit"]'); - /** * @param {string} delivery * @param {string} location @@ -121,28 +115,5 @@ function updateOTPDeliveryMethods(event) { document.querySelectorAll('lg-phone-input').forEach((node) => { const phoneInput = /** @type {PhoneInput} */ (node); - const form = /** @type {HTMLFormElement} */ (phoneInput.closest('form')); - - function setSubmitDisabled(isDisabled) { - const submitButton = getSubmitButton(form); - if (submitButton && submitButton.disabled !== isDisabled) { - submitButton.disabled = isDisabled; - } - } - - phoneInput.addEventListener('input', () => setSubmitDisabled(!form.checkValidity())); - phoneInput.addEventListener('change', (event) => { - setSubmitDisabled(!form.checkValidity()); - updateOTPDeliveryMethods(event); - }); - phoneInput.addEventListener( - 'invalid', - (event) => { - setSubmitDisabled(true); - event.preventDefault(); - }, - true, - ); - - setSubmitDisabled(!form.checkValidity()); + phoneInput.addEventListener('change', updateOTPDeliveryMethods); }); diff --git a/app/views/users/phone_setup/index.html.erb b/app/views/users/phone_setup/index.html.erb index 296e60123e0..80777ca1cc4 100644 --- a/app/views/users/phone_setup/index.html.erb +++ b/app/views/users/phone_setup/index.html.erb @@ -16,7 +16,7 @@ <% end %>

-<%= simple_form_for( +<%= validated_form_for( @new_phone_form, html: { autocomplete: 'off', method: :patch }, data: { international_phone_form: true }, diff --git a/app/views/users/phones/add.html.erb b/app/views/users/phones/add.html.erb index c68df34e43a..7c968eba98f 100644 --- a/app/views/users/phones/add.html.erb +++ b/app/views/users/phones/add.html.erb @@ -14,7 +14,7 @@ <% end %>

-<%= simple_form_for( +<%= validated_form_for( @new_phone_form, html: { autocomplete: 'off', method: :post }, data: { international_phone_form: true }, From 9d337fc60d8fb250a28bc8907174006924a6e302 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 23 Nov 2021 10:28:50 -0500 Subject: [PATCH 07/31] Remove default CSS class, support custom class --- app/components/phone_input_component.rb | 10 +++-- app/views/idv/doc_auth/send_link.html.erb | 2 +- app/views/idv/phone/new.html.erb | 7 +++- app/views/users/phone_setup/index.html.erb | 2 +- app/views/users/phones/add.html.erb | 2 +- spec/components/phone_input_component_spec.rb | 40 +++++++++++++++++++ 6 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 spec/components/phone_input_component_spec.rb diff --git a/app/components/phone_input_component.rb b/app/components/phone_input_component.rb index 64f7d1ff5f1..965ea6a7b6a 100644 --- a/app/components/phone_input_component.rb +++ b/app/components/phone_input_component.rb @@ -1,11 +1,13 @@ class PhoneInputComponent < BaseComponent - attr_reader :form, :required, :allowed_countries + attr_reader :form, :required, :allowed_countries, :tag_options + alias_method :f, :form - def initialize(form:, allowed_countries: nil, required: false) + def initialize(form:, allowed_countries: nil, required: false, **tag_options) @allowed_countries = allowed_countries @form = form @required = required + @tag_options = tag_options end def supported_country_codes @@ -31,9 +33,9 @@ def international_phone_codes end def css_class - classes = ['margin-bottom-4'] + classes = [] classes << 'phone-input--single-country' if supported_country_codes.size == 1 - classes + [*classes, *tag_options[:class]] end private diff --git a/app/views/idv/doc_auth/send_link.html.erb b/app/views/idv/doc_auth/send_link.html.erb index a2191f476f0..8496423c539 100644 --- a/app/views/idv/doc_auth/send_link.html.erb +++ b/app/views/idv/doc_auth/send_link.html.erb @@ -23,7 +23,7 @@ method: 'PUT', html: { autocomplete: 'off', class: 'margin-top-4' }, ) do |f| %> - <%= render PhoneInputComponent.new(form: f, required: true) %> + <%= render PhoneInputComponent.new(form: f, required: true, class: 'margin-bottom-4') %>
- <%= f.input( - :phone, + <%= render ValidatedFieldComponent.new( + name: :phone, + form: f, as: :tel, required: required, label: false, @@ -45,17 +46,8 @@ class: 'margin-bottom-0', }, input_html: { - aria: { - invalid: false, - describedby: "phone-input-error-#{unique_id}", - }, class: 'phone-input__number', }, ) %> - <% end %> <%= stylesheet_link_tag 'intl-tel-input/build/css/intlTelInput' %> diff --git a/app/components/validated_field_component.html.erb b/app/components/validated_field_component.html.erb new file mode 100644 index 00000000000..befc7b92bbd --- /dev/null +++ b/app/components/validated_field_component.html.erb @@ -0,0 +1,27 @@ + + <%= content_tag( + :script, + error_messages.to_json, + { + type: 'application/json', + class: 'validated-field__error-strings', + }, + false, + ) %> + <%= f.input( + name, + tag_options.deep_merge( + error: false, + input_html: { + aria: { + invalid: false, + describedby: "validated-field-error-#{unique_id}", + }, + data: { + input: '', + }, + }, + ), + ) %> + <%= f.error(name, id: "validated-field-error-#{unique_id}") %> + diff --git a/app/components/validated_field_component.js b/app/components/validated_field_component.js new file mode 100644 index 00000000000..c235c0826a1 --- /dev/null +++ b/app/components/validated_field_component.js @@ -0,0 +1,5 @@ +import { loadPolyfills } from '@18f/identity-polyfill'; + +loadPolyfills(['custom-elements', 'classlist']) + .then(() => import('@18f/identity-validated-field')) + .then(({ ValidatedField }) => customElements.define('lg-validated-field', ValidatedField)); diff --git a/app/components/validated_field_component.rb b/app/components/validated_field_component.rb new file mode 100644 index 00000000000..e5c5e9e4b4b --- /dev/null +++ b/app/components/validated_field_component.rb @@ -0,0 +1,19 @@ +class ValidatedFieldComponent < BaseComponent + attr_reader :form, :name, :tag_options + + alias_method :f, :form + + def initialize(form:, name:, error_messages: {}, **tag_options) + @form = form + @name = name + @error_messages = error_messages + @tag_options = tag_options + end + + def error_messages + { + valueMissing: t('simple_form.required.text'), + **@error_messages, + } + end +end diff --git a/app/javascript/packages/phone-input/index.spec.js b/app/javascript/packages/phone-input/index.spec.js new file mode 100644 index 00000000000..a45f3f56875 --- /dev/null +++ b/app/javascript/packages/phone-input/index.spec.js @@ -0,0 +1,99 @@ +import { getByLabelText } from '@testing-library/dom'; +import userEvent from '@testing-library/user-event'; + +const MULTIPLE_OPTIONS_HTML = ` + +`; + +const SINGLE_OPTION_HTML = ` + +`; + +describe('PhoneInput', () => { + before(async () => { + await import('intl-tel-input/build/js/utils.js'); + window.intlTelInputUtils = global.intlTelInputUtils; + const { PhoneInput } = await import('./index.js'); + customElements.define('lg-phone-input', PhoneInput); + }); + + function createAndConnectElement({ isSingleOption = false } = {}) { + const element = document.createElement('lg-phone-input'); + element.innerHTML = ` + +
+ + ${isSingleOption ? SINGLE_OPTION_HTML : MULTIPLE_OPTIONS_HTML} +
+ +
+ Example: + +
+ + + + + `; + + document.body.appendChild(element); + + return element; + } + + it('initializes with dropdown', () => { + const input = createAndConnectElement(); + + expect(input.querySelector('.iti.iti--allow-dropdown')).to.be.ok(); + }); + + it('validates input', () => { + const input = createAndConnectElement(); + + /** @type {HTMLInputElement} */ + const phoneNumber = getByLabelText(input, 'Phone number'); + + expect(phoneNumber.validity.valueMissing).to.be.true(); + + userEvent.type(phoneNumber, '5'); + expect(phoneNumber.validationMessage).to.equal( + 'Invalid phone number. Please make sure you enter a valid phone number.', + ); + + userEvent.type(phoneNumber, '13-555-1234'); + expect(phoneNumber.validity.valid).to.be.true(); + }); + + context('with single option', () => { + it('initializes without dropdown', () => { + const input = createAndConnectElement({ isSingleOption: true }); + + expect(input.querySelector('.iti:not(.iti--allow-dropdown)')).to.be.ok(); + }); + + it('validates phone from region', () => { + const input = createAndConnectElement({ isSingleOption: true }); + + /** @type {HTMLInputElement} */ + const phoneNumber = getByLabelText(input, 'Phone number'); + + userEvent.type(phoneNumber, '306-555-1234'); + expect(phoneNumber.validationMessage).to.equal('Must be a U.S. phone number'); + }); + }); +}); diff --git a/app/javascript/packages/validated-field/index.js b/app/javascript/packages/validated-field/index.js new file mode 100644 index 00000000000..c9bb8b3807a --- /dev/null +++ b/app/javascript/packages/validated-field/index.js @@ -0,0 +1,90 @@ +export class ValidatedField extends HTMLElement { + /** @type {Partial} */ + errorStrings = {}; + + connectedCallback() { + /** @type {HTMLInputElement?} */ + this.input = this.querySelector('[data-input]'); + this.errorMessage = this.querySelector('.usa-error-message'); + this.descriptorId = this.input?.getAttribute('aria-describedby'); + try { + Object.assign( + this.errorStrings, + JSON.parse(this.querySelector('.validated-field__error-strings')?.textContent || ''), + ); + } catch {} + + this.input?.addEventListener('input', () => this.setErrorMessage()); + this.input?.addEventListener('invalid', (event) => this.toggleErrorMessage(event)); + } + + /** + * Handles an invalid event, rendering or hiding an error message based on the input's current + * validity. + * + * @param {Event} event Invalid event. + */ + toggleErrorMessage(event) { + event.preventDefault(); + this.setErrorMessage(this.getNormalizedValidationMessage(this.input)); + } + + /** + * Renders the given message as an error, if present. Otherwise, hides any visible error message. + * + * @param {string?=} message Error message to show, or empty to hide. + */ + setErrorMessage(message) { + if (message) { + this.getOrCreateErrorMessageElement().textContent = message; + this.input?.focus(); + } else if (this.errorMessage) { + this.removeChild(this.errorMessage); + this.errorMessage = null; + } + + this.input?.classList.toggle('usa-input--error', !!message); + } + + /** + * Returns a validation message for the given input, normalized to use customized error strings. + * An empty string is returned for a valid input. + * + * @param {HTMLInputElement?=} input Input element. + * + * @return {string} Validation message. + */ + getNormalizedValidationMessage(input) { + if (!input || input.validity.valid) { + return ''; + } + + for (const type in input.validity) { + if (type !== 'valid' && input.validity[type] && this.errorStrings[type]) { + return this.errorStrings[type]; + } + } + + return input.validationMessage; + } + + /** + * Returns an error message element. If one doesn't already exist, it is created and appended to + * the root. + * + * @returns {Element} Error message element. + */ + getOrCreateErrorMessageElement() { + if (!this.errorMessage) { + this.errorMessage = this.ownerDocument.createElement('div'); + this.errorMessage.classList.add('usa-error-message'); + if (this.descriptorId) { + this.errorMessage.id = this.descriptorId; + } + + this.appendChild(this.errorMessage); + } + + return this.errorMessage; + } +} diff --git a/app/javascript/packages/validated-field/index.spec.js b/app/javascript/packages/validated-field/index.spec.js new file mode 100644 index 00000000000..1854707cf95 --- /dev/null +++ b/app/javascript/packages/validated-field/index.spec.js @@ -0,0 +1,105 @@ +import { getByRole, getByText } from '@testing-library/dom'; +import userEvent from '@testing-library/user-event'; +import { ValidatedField } from '.'; + +describe('ValidatedField', () => { + before(() => { + customElements.define('lg-validated-field', ValidatedField); + }); + + function createAndConnectElement({ hasInitialError = false } = {}) { + const element = document.createElement('lg-validated-field'); + element.innerHTML = ` + + + ${ + hasInitialError + ? '
Invalid value
' + : '' + } + `; + + const form = document.createElement('form'); + form.appendChild(element); + document.body.appendChild(form); + + return element; + } + + it('shows error state and focuses on form validation', () => { + const element = createAndConnectElement(); + + /** @type {HTMLInputElement} */ + const input = getByRole(element, 'textbox'); + + /** @type {HTMLFormElement} */ + const form = element.parentNode; + form.checkValidity(); + + expect(input.classList.contains('usa-input--error')).to.be.true(); + expect(document.activeElement).to.equal(input); + const message = getByText(element, 'This field is required'); + expect(message).to.be.ok(); + expect(message.id).to.equal(input.getAttribute('aria-describedby')); + }); + + it('shows custom validity as message content', () => { + const element = createAndConnectElement(); + + /** @type {HTMLInputElement} */ + const input = getByRole(element, 'textbox'); + input.value = 'a'; + input.setCustomValidity('custom validity'); + + /** @type {HTMLFormElement} */ + const form = element.parentNode; + form.checkValidity(); + + expect(getByText(element, 'custom validity')).to.be.ok(); + }); + + it('clears existing validation state on input', () => { + const element = createAndConnectElement(); + + /** @type {HTMLInputElement} */ + const input = getByRole(element, 'textbox'); + + /** @type {HTMLFormElement} */ + const form = element.parentNode; + form.checkValidity(); + + userEvent.type(input, '5'); + + expect(input.classList.contains('usa-input--error')).to.be.false(); + expect(() => getByText(element, 'This field is required')).to.throw(); + }); + + context('with initial error message', () => { + it('clears existing validation state on input', () => { + const element = createAndConnectElement(); + + /** @type {HTMLInputElement} */ + const input = getByRole(element, 'textbox'); + + /** @type {HTMLFormElement} */ + const form = element.parentNode; + form.checkValidity(); + + userEvent.type(input, '5'); + + expect(input.classList.contains('usa-input--error')).to.be.false(); + expect(() => getByText(element, 'Invalid value')).to.throw(); + }); + }); +}); diff --git a/app/javascript/packages/validated-field/package.json b/app/javascript/packages/validated-field/package.json new file mode 100644 index 00000000000..4838734149e --- /dev/null +++ b/app/javascript/packages/validated-field/package.json @@ -0,0 +1,5 @@ +{ + "name": "@18f/identity-validated-field", + "private": true, + "version": "1.0.0" +} diff --git a/app/javascript/packs/form-validation.js b/app/javascript/packs/form-validation.js index e55e57cd79c..fa612fa074c 100644 --- a/app/javascript/packs/form-validation.js +++ b/app/javascript/packs/form-validation.js @@ -23,47 +23,37 @@ function kebabCase(string) { return string.replace(/(.)([A-Z])/g, '$1-$2').toLowerCase(); } -/** - * Returns the error message elements associated with the given input. - * - * @param {HTMLInputElement} input Input field. - * - * @return {Element[]} Error message elements. - */ -function getInputMessages(input) { - const messages = []; - - const describedBy = input.getAttribute('aria-describedby'); - if (describedBy) { - const descriptors = /** @type {Element[]} */ (describedBy - .split(' ') - .map((id) => document.getElementById(id)) - .filter(Boolean)); - - messages.push(...descriptors); - } - - /** @type {Element?} */ - let sibling = input; - while ((sibling = sibling.nextElementSibling)) { - if (sibling.classList.contains('display-if-invalid')) { - messages.push(sibling); - } +function resetInput(input) { + if (input.hasAttribute('data-form-validation-message')) { + input.setCustomValidity(''); + input.removeAttribute('data-form-validation-message'); } - - return messages; + input.setAttribute('aria-invalid', 'false'); + input.classList.remove('usa-input--error'); } /** - * Given an `input` event, updates custom validity of the given input. + * Given an `input` or `invalid` event, updates custom validity of the given input. * * @param {Event} event Input or invalid event. */ + function checkInputValidity(event) { const input = /** @type {HTMLInputElement} */ (event.target); - if (input.hasAttribute('data-form-validation-message')) { - input.setCustomValidity(''); - input.removeAttribute('data-form-validation-message'); + resetInput(input); + if ( + event.type === 'invalid' && + !input.validity.valid && + input.parentNode?.querySelector('.display-if-invalid') + ) { + event.preventDefault(); + const errors = Object.keys(ValidityState.prototype) + .filter((key) => key !== 'valid') + .filter((key) => input.validity[key]); + + input.setAttribute('aria-invalid', errors.length ? kebabCase(errors[0]) : 'false'); + input.classList.add('usa-input--error'); + input.focus(); } const { I18n } = /** @type {typeof window & LoginGovGlobal} */ (window).LoginGov; @@ -81,41 +71,6 @@ function checkInputValidity(event) { } } -/** - * Given an `input` or `invalid` event, toggles visibility of custom error messages. - * - * @param {Event} event Input or invalid event. - */ -function toggleErrorMessages(event) { - const input = /** @type {HTMLInputElement} */ (event.target); - const messages = getInputMessages(input); - const errors = Object.keys(ValidityState.prototype) - .filter((key) => key !== 'valid') - .filter((key) => input.validity[key]); - const activeMessages = errors - .map((type) => `display-if-invalid--${kebabCase(type)}`) - .flatMap((className) => messages.filter((message) => message.classList.contains(className))); - - input.setAttribute('aria-invalid', 'false'); - input.classList.remove('usa-input--error'); - messages.forEach((message) => message.classList.remove('display-if-invalid--invalid')); - - const hasActiveMessages = !!activeMessages.length; - if (event.type === 'invalid' && hasActiveMessages) { - event.preventDefault(); - - input.setAttribute('aria-invalid', 'true'); - input.classList.add('usa-input--error'); - input.focus(); - - const firstActiveMessage = activeMessages[0]; - firstActiveMessage.classList.add('display-if-invalid--invalid'); - if (firstActiveMessage.classList.contains('display-if-invalid--custom-error')) { - firstActiveMessage.textContent = input.validationMessage; - } - } -} - /** * Binds validation to a given input. * @@ -123,8 +78,7 @@ function toggleErrorMessages(event) { */ function validateInput(input) { input.addEventListener('input', checkInputValidity); - input.addEventListener('input', toggleErrorMessages); - input.addEventListener('invalid', toggleErrorMessages); + input.addEventListener('invalid', checkInputValidity); } /** diff --git a/app/javascript/packs/ial2-consent-button.js b/app/javascript/packs/ial2-consent-button.js index 310e5623275..345108c0481 100644 --- a/app/javascript/packs/ial2-consent-button.js +++ b/app/javascript/packs/ial2-consent-button.js @@ -7,7 +7,7 @@ function toggleButton() { continueButton.classList.toggle('usa-button--disabled', !checkbox.checked); errorMessage.classList.toggle( 'display-none', - checkbox.getAttribute('aria-invalid') === 'false', + checkbox.getAttribute('aria-invalid') !== 'value-missing', ); } diff --git a/app/views/idv/phone/new.html.erb b/app/views/idv/phone/new.html.erb index 8c52b4285c3..ad00f8931bd 100644 --- a/app/views/idv/phone/new.html.erb +++ b/app/views/idv/phone/new.html.erb @@ -63,9 +63,6 @@ required: true, class: 'margin-bottom-4' ) %> - - <%= t('simple_form.required.text') %> -
<%= render('shared/spinner_button', action_message: t('doc_auth.info.verifying')) do %> <%= f.button :submit, t('forms.buttons.continue'), class: 'usa-button--big usa-button--wide' %> diff --git a/spec/components/phone_input_component_spec.rb b/spec/components/phone_input_component_spec.rb index 16c172045b1..95c6968cf19 100644 --- a/spec/components/phone_input_component_spec.rb +++ b/spec/components/phone_input_component_spec.rb @@ -37,4 +37,26 @@ expect(rendered).to have_css('lg-phone-input.example-class') end end + + context 'with allowed countries' do + let(:allowed_countries) { ['US'] } + + it 'limits the allowed countries' do + expect(rendered).to have_select( + t('components.phone_input.country_code_label'), + options: ['United States +1'], + ) + end + + context 'with invalid allowed countries' do + let(:allowed_countries) { ['US', 'ZZ'] } + + it 'limits the allowed countries to valid countries' do + expect(rendered).to have_select( + t('components.phone_input.country_code_label'), + options: ['United States +1'], + ) + end + end + end end diff --git a/spec/components/validated_field_component_spec.rb b/spec/components/validated_field_component_spec.rb new file mode 100644 index 00000000000..9c70e449504 --- /dev/null +++ b/spec/components/validated_field_component_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +RSpec.describe ValidatedFieldComponent, type: :component do + include SimpleForm::ActionViewExtensions::FormHelper + + let(:lookup_context) { ActionView::LookupContext.new(ActionController::Base.view_paths) } + let(:view_context) { ActionView::Base.new(lookup_context, {}, controller) } + let(:form_object) { User.new } + let(:form_builder) do + SimpleForm::FormBuilder.new(form_object.model_name.param_key, form_object, view_context, {}) + end + let(:name) { :uuid } + let(:error_messages) { nil } + let(:options) do + { + name: name, + form: form_builder, + error_messages: error_messages, + }.compact + end + + subject(:rendered) do + render_inline(described_class.new(**options)) + end + + it 'renders with error message texts' do + expect(rendered).to have_css( + 'script', + text: { valueMissing: t('simple_form.required.text') }.to_json, + visible: :all, + ) + end + + context 'custom error message texts' do + let(:error_messages) { { valueMissing: 'missing', tooLong: 'too long' } } + + it 'renders with error message texts' do + expect(rendered).to have_css( + 'script', + text: { valueMissing: 'missing', tooLong: 'too long' }.to_json, + visible: :all, + ) + end + end +end diff --git a/spec/features/idv/doc_auth/ssn_step_spec.rb b/spec/features/idv/doc_auth/ssn_step_spec.rb index b6975e3c90c..9f1cb4d9dbb 100644 --- a/spec/features/idv/doc_auth/ssn_step_spec.rb +++ b/spec/features/idv/doc_auth/ssn_step_spec.rb @@ -29,7 +29,7 @@ fill_out_ssn_form_fail click_idv_continue - expect(page.find('#doc_auth_ssn')['aria-invalid']).to eq('true') + expect(page.find('#doc_auth_ssn')['aria-invalid']).to eq('value-missing') expect(page).to have_current_path(idv_doc_auth_ssn_step) end diff --git a/spec/features/phone/add_phone_spec.rb b/spec/features/phone/add_phone_spec.rb index adb0a3a7c6b..ad9a4f5704f 100644 --- a/spec/features/phone/add_phone_spec.rb +++ b/spec/features/phone/add_phone_spec.rb @@ -41,22 +41,27 @@ hidden_select = page.find('[name="new_phone_form[international_code]"]', visible: :hidden) - find_button 'Continue', disabled: true - expect(hidden_select.value).to eq('US') - - input = fill_in :new_phone_form_phone, with: '5135558410' - expect(input.value).to eq('5135558410') - find_button 'Continue', disabled: false + click_continue + focused_input = page.find(':focus') + error_message = page.find_by_id(focused_input[:'aria-describedby']) + expect(focused_input).to match_css('.phone-input__number.usa-input--error') + expect(error_message).to have_content(t('simple_form.required.text')) expect(hidden_select.value).to eq('US') fill_in :new_phone_form_phone, with: 'abcd1234' - find_button 'Continue', disabled: true + click_continue + focused_input = page.find(':focus') + error_message = page.find_by_id(focused_input[:'aria-describedby']) + expect(focused_input).to match_css('.phone-input__number.usa-input--error') + expect(error_message).to have_content(t('errors.messages.improbable_phone')) expect(hidden_select.value).to eq('US') input = fill_in :new_phone_form_phone, with: '+81543543643' expect(input.value).to eq('+81 543543643') - find_button 'Continue', disabled: false expect(hidden_select.value).to eq('JP') + + click_continue + expect(page).to have_content(t('forms.two_factor.code')) end scenario 'adding a phone that is already on the user account shows error message' do From 9593ab9e491ac93184f8c6a86aca1e67249fdacf Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 23 Nov 2021 13:31:35 -0500 Subject: [PATCH 09/31] Add US-constrained phone number translations --- config/locales/errors/es.yml | 1 + config/locales/errors/fr.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index 818050a9406..c3d9a3e59d8 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -73,6 +73,7 @@ es: otp_failed: Se produjo un error al enviar el código de seguridad. password_incorrect: La contraseña es incorrecta personal_key_incorrect: La clave personal es incorrecta + phone_country_constraint_usa: Debe ser un número telefónico de los Estados Unidos phone_duplicate: Esta cuenta ya está utilizando el número de teléfono que ingresó como autenticador. Por favor, use un número de teléfono diferente. diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 8f689535b8e..720da0020c8 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -80,6 +80,7 @@ fr: otp_failed: Échec de l’envoi de votre code de sécurité. password_incorrect: Mot de passe incorrect personal_key_incorrect: Clé personnelle incorrecte + phone_country_constraint_usa: Dois être un numéro de téléphone américain phone_duplicate: Ce compte utilise déjà le numéro de téléphone que vous avez entré en tant qu’authentificateur. Veuillez utiliser un numéro de téléphone différent. From 3047d3eded9a501c0a027f95ff4e5f7623db19f1 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 23 Nov 2021 13:48:17 -0500 Subject: [PATCH 10/31] Validate non-US single country option --- app/javascript/packages/phone-input/index.js | 12 ++++--- .../packages/phone-input/index.spec.js | 31 +++++++++++++++---- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/app/javascript/packages/phone-input/index.js b/app/javascript/packages/phone-input/index.js index 61bc2462f7b..9dddc13ede0 100644 --- a/app/javascript/packages/phone-input/index.js +++ b/app/javascript/packages/phone-input/index.js @@ -172,13 +172,17 @@ export class PhoneInput extends HTMLElement { const isInvalidCountry = supportedCountryCodes?.length === 1 && !isValidNumberForRegion(phoneNumber, countryCode); - if (isInvalidCountry && countryCode === 'US' && this.strings.country_constraint_usa) { - textInput.setCustomValidity(this.strings.country_constraint_usa); + if (isInvalidCountry) { + if (countryCode === 'US') { + textInput.setCustomValidity(this.strings.country_constraint_usa || ''); + } else { + textInput.setCustomValidity(this.strings.invalid_phone || ''); + } } const isInvalidPhoneNumber = !isPhoneValid(phoneNumber, countryCode); - if (isInvalidPhoneNumber && this.strings.invalid_phone) { - textInput.setCustomValidity(this.strings.invalid_phone); + if (isInvalidPhoneNumber) { + textInput.setCustomValidity(this.strings.invalid_phone || ''); } } diff --git a/app/javascript/packages/phone-input/index.spec.js b/app/javascript/packages/phone-input/index.spec.js index a45f3f56875..7bb532006a0 100644 --- a/app/javascript/packages/phone-input/index.spec.js +++ b/app/javascript/packages/phone-input/index.spec.js @@ -5,14 +5,17 @@ const MULTIPLE_OPTIONS_HTML = ` -`; + `; const SINGLE_OPTION_HTML = ` -`; + `; + +const SINGLE_OPTION_SELECT_NON_US_HTML = ` + `; describe('PhoneInput', () => { before(async () => { @@ -22,7 +25,7 @@ describe('PhoneInput', () => { customElements.define('lg-phone-input', PhoneInput); }); - function createAndConnectElement({ isSingleOption = false } = {}) { + function createAndConnectElement({ isSingleOption = false, isNonUSSingleOption = false } = {}) { const element = document.createElement('lg-phone-input'); element.innerHTML = `
- ${isSingleOption ? SINGLE_OPTION_HTML : MULTIPLE_OPTIONS_HTML} + ${isSingleOption ? SINGLE_OPTION_HTML : ''} + ${isNonUSSingleOption ? SINGLE_OPTION_SELECT_NON_US_HTML : ''} + ${!isSingleOption && !isNonUSSingleOption ? MULTIPLE_OPTIONS_HTML : ''}
@@ -95,5 +100,19 @@ describe('PhoneInput', () => { userEvent.type(phoneNumber, '306-555-1234'); expect(phoneNumber.validationMessage).to.equal('Must be a U.S. phone number'); }); + + context('with non-U.S. single option', () => { + it('validates phone from region', () => { + const input = createAndConnectElement({ isNonUSSingleOption: true }); + + /** @type {HTMLInputElement} */ + const phoneNumber = getByLabelText(input, 'Phone number'); + + userEvent.type(phoneNumber, '513-555-1234'); + expect(phoneNumber.validationMessage).to.equal( + 'Invalid phone number. Please make sure you enter a valid phone number.', + ); + }); + }); }); }); From ef343ee1c017317675da5d96c85c5431a7e49692 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 23 Nov 2021 13:50:18 -0500 Subject: [PATCH 11/31] Add customized phone required messaging --- app/components/phone_input_component.html.erb | 5 ++++- config/locales/errors/en.yml | 1 + config/locales/errors/es.yml | 1 + config/locales/errors/fr.yml | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/components/phone_input_component.html.erb b/app/components/phone_input_component.html.erb index 0089b170fc1..b41d6cf969f 100644 --- a/app/components/phone_input_component.html.erb +++ b/app/components/phone_input_component.html.erb @@ -37,8 +37,11 @@
<%= render ValidatedFieldComponent.new( - name: :phone, form: f, + name: :phone, + error_messages: { + valueMissing: t('errors.messages.phone_required'), + }, as: :tel, required: required, label: false, diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 1b9a531a03b..710c4a5e405 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -73,6 +73,7 @@ en: phone_country_constraint_usa: Must be a U.S. phone number phone_duplicate: This account is already using the phone number you entered as an authenticator. Please use a different phone number. + phone_required: Phone number is required phone_unsupported: Sorry, we are unable to send SMS at this time. Please try the phone call option below, or use your personal key. premium_rate_phone: This appears to be a premium rate phone number. Please diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index c3d9a3e59d8..ad2bdbd72df 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -79,6 +79,7 @@ es: diferente. phone_unsupported: Lo sentimos, no podemos enviar SMS en este momento. Pruebe la opción de llamada telefónica a continuación o use su clave personal. + phone_required: Número de teléfono requerido premium_rate_phone: Parece que se trata de un número de teléfono de tarifa premium. Por favor, seleccione otro número e inténtelo de nuevo. pwned_password: La contraseña que ingresaste no es segura. Está en una lista de diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 720da0020c8..84994f80801 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -84,6 +84,7 @@ fr: phone_duplicate: Ce compte utilise déjà le numéro de téléphone que vous avez entré en tant qu’authentificateur. Veuillez utiliser un numéro de téléphone différent. + phone_required: Le numéro de téléphone est obligatoire phone_unsupported: Désolé, nous ne sommes pas en mesure d’envoyer des SMS pour le moment. S’il vous plaît essayez l’option d’appel téléphonique ci-dessous, ou utilisez votre clé personnelle. From dd42d944dea88a62ee83b9f58a2eb265d0b48f6e Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 23 Nov 2021 13:54:18 -0500 Subject: [PATCH 12/31] Update invalid phone number messaging --- app/components/phone_input_component.html.erb | 2 +- config/locales/errors/en.yml | 2 +- config/locales/errors/es.yml | 2 +- config/locales/errors/fr.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/components/phone_input_component.html.erb b/app/components/phone_input_component.html.erb index b41d6cf969f..ec2e1f6e798 100644 --- a/app/components/phone_input_component.html.erb +++ b/app/components/phone_input_component.html.erb @@ -3,7 +3,7 @@ :script, { country_code_label: t('components.phone_input.country_code_label'), - invalid_phone: t('errors.messages.improbable_phone'), + invalid_phone: t('errors.messages.invalid_phone_number'), country_constraint_usa: t('errors.messages.phone_country_constraint_usa'), }.to_json, { diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index 710c4a5e405..4ec171e1ed4 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -57,7 +57,7 @@ en: inclusion: is not included in the list invalid_calling_area: Calls to that phone number are not supported. Please try SMS if you have an SMS-capable phone. - invalid_phone_number: The phone number entered is not valid. + invalid_phone_number: Phone number is not valid invalid_sms_number: The phone number entered doesn’t support text messaging. Try the Phone call option. invalid_voice_number: Invalid phone number. Check that you’ve entered the diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index ad2bdbd72df..8f482c0b057 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -60,7 +60,7 @@ es: inclusion: No se incluye en la lista. invalid_calling_area: No se admiten llamadas a ese número de teléfono. Intenta enviar un SMS si tienes un teléfono que permita enviar SMS. - invalid_phone_number: El número de teléfono ingresado no está en el formato correcto. + invalid_phone_number: Número de teléfono no válido invalid_sms_number: El número de teléfono ingresado no admite mensajes de texto. Pruebe la opción de llamada telefónica. invalid_voice_number: Numero de telefono invalido. Verifique que haya ingresado diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 84994f80801..ff270623d53 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -67,7 +67,7 @@ fr: invalid_calling_area: Les appels vers ce numéro de téléphone ne sont pas pris en charge. Veuillez essayer par SMS si vous possédez un téléphone disposant de cette fonction. - invalid_phone_number: Le numéro de téléphone saisi n’est pas valide. + invalid_phone_number: Le numéro de téléphone n’est pas valide invalid_sms_number: Le numéro de téléphone saisi ne prend pas en charge les messages textuels. Veuillez essayer l’option d’appel téléphonique. invalid_voice_number: Numéro de téléphone invalide. Vérifiez que vous avez entré From 7d6e363af65b912966d92514e08e8266252fba60 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 23 Nov 2021 14:28:00 -0500 Subject: [PATCH 13/31] Update unsupported phone numbers translations --- config/locales/errors/es.yml | 2 +- config/locales/two_factor_authentication/en.yml | 2 +- config/locales/two_factor_authentication/es.yml | 3 +-- config/locales/two_factor_authentication/fr.yml | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index 8f482c0b057..fae221de6ef 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -77,9 +77,9 @@ es: phone_duplicate: Esta cuenta ya está utilizando el número de teléfono que ingresó como autenticador. Por favor, use un número de teléfono diferente. + phone_required: Número de teléfono requerido phone_unsupported: Lo sentimos, no podemos enviar SMS en este momento. Pruebe la opción de llamada telefónica a continuación o use su clave personal. - phone_required: Número de teléfono requerido premium_rate_phone: Parece que se trata de un número de teléfono de tarifa premium. Por favor, seleccione otro número e inténtelo de nuevo. pwned_password: La contraseña que ingresaste no es segura. Está en una lista de diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 5c778de14e4..d5cb3d1b937 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -85,7 +85,7 @@ en: otp_delivery_preference: instruction: You can change this selection the next time you sign in. If you entered a landline, please select “Phone call” below. - no_supported_options: We’re unable to verify phone numbers from %{location} at this time. + no_supported_options: We are unable to verify phone numbers from %{location} sms: Text message (SMS) sms_unsupported: We’re unable to send text messages to people in %{location} at this time. title: How should we send you a code? diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index c056ec7c8c2..2d744093f70 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -91,8 +91,7 @@ es: no_auth_option: No se pudo encontrar ninguna opción de autenticación para iniciar sesión otp_delivery_preference: instruction: Puede cambiar esta selección la próxima vez que inicie sesión. - no_supported_options: En estos momentos no podemos verificar números telefónicos - a personas de %{location}. + no_supported_options: No podemos verificar los números de teléfono de %{location} sms: Mensaje de texto (SMS, sigla en inglés) sms_unsupported: En estos momentos no podemos enviar mensajes de texto a personas de %{location}. diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 3937e717b4a..1a8d6e8eacb 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -92,7 +92,7 @@ fr: otp_delivery_preference: instruction: Vous pouvez modifier cette sélection lors de votre prochaine connexion. no_supported_options: Nous ne sommes pas en mesure de vérifier les numéros de - téléphone de %{location} pour le moment. + téléphone de %{location} sms: Message texte (SMS) sms_unsupported: Nous ne sommes pas en mesure d’envoyer des SMS aux personnes se trouvant en %{location} pour le moment. From 873e984bbe397dcabf56e4119e909130c90d8af1 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 23 Nov 2021 15:57:01 -0500 Subject: [PATCH 14/31] WIP: Toggle error on undeliverable countries --- app/javascript/packages/phone-input/index.js | 17 ++------------ .../packs/otp-delivery-preference.js | 23 +++++++++++++++++-- ...otp_delivery_preference_selection.html.erb | 2 +- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/app/javascript/packages/phone-input/index.js b/app/javascript/packages/phone-input/index.js index 9dddc13ede0..597cf5e57f7 100644 --- a/app/javascript/packages/phone-input/index.js +++ b/app/javascript/packages/phone-input/index.js @@ -23,8 +23,6 @@ const { intlTelInputUtils, } = /** @type {window & { intlTelInputUtils: IntlTelInputUtilsGlobal }} */ (window); -const INTERNATIONAL_CODE_REGEX = /^\+(\d+) |^1 /; - const isPhoneValid = (phone, countryCode) => { let phoneValid = isValidNumber(phone, countryCode); if (!phoneValid && countryCode === 'US') { @@ -33,14 +31,6 @@ const isPhoneValid = (phone, countryCode) => { return phoneValid; }; -const internationalCodeFromPhone = (phone) => { - const match = phone.match(INTERNATIONAL_CODE_REGEX); - if (match) { - return match[1] || match[2]; - } - return '1'; -}; - const updateInternationalCodeInPhone = (phone, newCode) => phone.replace(new RegExp(`^\\+?(\\d+\\s+|${newCode})?`), `+${newCode} `); @@ -193,12 +183,9 @@ export class PhoneInput extends HTMLElement { } const phone = textInput.value; - const inputInternationalCode = internationalCodeFromPhone(phone); const selectedInternationalCode = selectedOption.dataset.countryCode; - - if (inputInternationalCode !== selectedInternationalCode) { - textInput.value = updateInternationalCodeInPhone(phone, selectedInternationalCode); - } + textInput.value = updateInternationalCodeInPhone(phone, selectedInternationalCode); + textInput.dispatchEvent(new CustomEvent('input', { bubbles: true })); } /** diff --git a/app/javascript/packs/otp-delivery-preference.js b/app/javascript/packs/otp-delivery-preference.js index fa59c71972c..2e5f730f384 100644 --- a/app/javascript/packs/otp-delivery-preference.js +++ b/app/javascript/packs/otp-delivery-preference.js @@ -8,6 +8,8 @@ const { t } = /** @type {GlobalWithLoginGov} */ (window).LoginGov.I18n; +const getOTPDeliveryMethodContainer = () => document.querySelector('.js-otp-delivery-preferences'); + /** * @return {HTMLInputElement[]} */ @@ -62,6 +64,9 @@ const isAllDisabled = (inputs) => inputs.every((input) => input.disabled); */ const getFirstEnabledInput = (inputs) => inputs.find((input) => !input.disabled); +const toggleDeliveryPreferencesVisible = (isVisible) => + getOTPDeliveryMethodContainer()?.classList.toggle('display-none', !isVisible); + /** * @param {Event} event */ @@ -103,17 +108,31 @@ function updateOTPDeliveryMethods(event) { const hintText = t('two_factor_authentication.otp_delivery_preference.no_supported_options', { location, }); + toggleDeliveryPreferencesVisible(!isAllMethodsDisabled); if (isAllMethodsDisabled) { - setHintText(hintText); select.setCustomValidity(hintText); select.reportValidity(); } else if (!select.validity.valid) { + textInput.setCustomValidity(''); select.setCustomValidity(''); - select.reportValidity(); + } +} + +function syncSelectValidityToTextInput(phoneInput, event) { + const { target: select } = event; + const { textInput } = phoneInput; + if (select instanceof HTMLSelectElement && !select.validity.valid && textInput) { + textInput.setCustomValidity(select.validationMessage); + textInput.reportValidity(); } } document.querySelectorAll('lg-phone-input').forEach((node) => { const phoneInput = /** @type {PhoneInput} */ (node); phoneInput.addEventListener('change', updateOTPDeliveryMethods); + phoneInput.addEventListener( + 'invalid', + (event) => syncSelectValidityToTextInput(phoneInput, event), + true, + ); }); diff --git a/app/views/users/shared/_otp_delivery_preference_selection.html.erb b/app/views/users/shared/_otp_delivery_preference_selection.html.erb index 74603902862..f3368106833 100644 --- a/app/views/users/shared/_otp_delivery_preference_selection.html.erb +++ b/app/views/users/shared/_otp_delivery_preference_selection.html.erb @@ -5,7 +5,7 @@ form_name_tag_voice = form_name + "_otp_delivery_preference_voice" %> -
+
<%= t('two_factor_authentication.otp_delivery_preference.title') %> From 8342a3409820c7207d7f3a8901d9e19a76abb548 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 23 Nov 2021 16:04:08 -0500 Subject: [PATCH 15/31] WIP: Try to constrain width of error message to match input --- app/assets/stylesheets/components/_validated-field.scss | 8 ++++++++ app/assets/stylesheets/components/all.scss | 1 + app/components/validated_field_component.html.erb | 7 ++++--- app/javascript/packages/phone-input/index.spec.js | 2 +- app/javascript/packages/validated-field/index.js | 2 +- app/javascript/packages/validated-field/index.spec.js | 3 +-- 6 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 app/assets/stylesheets/components/_validated-field.scss diff --git a/app/assets/stylesheets/components/_validated-field.scss b/app/assets/stylesheets/components/_validated-field.scss new file mode 100644 index 00000000000..e2e306ee50c --- /dev/null +++ b/app/assets/stylesheets/components/_validated-field.scss @@ -0,0 +1,8 @@ +lg-validated-field { + display: block; + width: min-content; +} + +.validated-field__input-wrapper { + width: max-content; +} diff --git a/app/assets/stylesheets/components/all.scss b/app/assets/stylesheets/components/all.scss index 78864dcf600..a90df314012 100644 --- a/app/assets/stylesheets/components/all.scss +++ b/app/assets/stylesheets/components/all.scss @@ -25,3 +25,4 @@ @import 'step-indicator'; @import 'troubleshooting-options'; @import 'i18n-dropdown'; +@import 'validated-field'; diff --git a/app/components/validated_field_component.html.erb b/app/components/validated_field_component.html.erb index befc7b92bbd..52944728e56 100644 --- a/app/components/validated_field_component.html.erb +++ b/app/components/validated_field_component.html.erb @@ -12,14 +12,15 @@ name, tag_options.deep_merge( error: false, + wrapper_html: { + class: [*tag_options.dig(:wrapper_html, :class), 'validated-field__input-wrapper'], + }, input_html: { + class: [*tag_options.dig(:input_html, :class), 'validated-field__input'], aria: { invalid: false, describedby: "validated-field-error-#{unique_id}", }, - data: { - input: '', - }, }, ), ) %> diff --git a/app/javascript/packages/phone-input/index.spec.js b/app/javascript/packages/phone-input/index.spec.js index 7bb532006a0..1b2475391c6 100644 --- a/app/javascript/packages/phone-input/index.spec.js +++ b/app/javascript/packages/phone-input/index.spec.js @@ -52,7 +52,7 @@ describe('PhoneInput', () => { "valueMissing": "This field is required" } - + `; diff --git a/app/javascript/packages/validated-field/index.js b/app/javascript/packages/validated-field/index.js index c9bb8b3807a..dc9e4f10120 100644 --- a/app/javascript/packages/validated-field/index.js +++ b/app/javascript/packages/validated-field/index.js @@ -4,7 +4,7 @@ export class ValidatedField extends HTMLElement { connectedCallback() { /** @type {HTMLInputElement?} */ - this.input = this.querySelector('[data-input]'); + this.input = this.querySelector('.validated-field__input'); this.errorMessage = this.querySelector('.usa-error-message'); this.descriptorId = this.input?.getAttribute('aria-describedby'); try { diff --git a/app/javascript/packages/validated-field/index.spec.js b/app/javascript/packages/validated-field/index.spec.js index 1854707cf95..f92306de296 100644 --- a/app/javascript/packages/validated-field/index.spec.js +++ b/app/javascript/packages/validated-field/index.spec.js @@ -18,10 +18,9 @@ describe('ValidatedField', () => { ${ hasInitialError From 877c7a5df80549051d5f00ac05aac49922bde6b3 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 24 Nov 2021 08:10:54 -0500 Subject: [PATCH 16/31] Add inline code document for new otp-delivery-preference methods --- app/javascript/packs/otp-delivery-preference.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/app/javascript/packs/otp-delivery-preference.js b/app/javascript/packs/otp-delivery-preference.js index 2e5f730f384..8e81f17357a 100644 --- a/app/javascript/packs/otp-delivery-preference.js +++ b/app/javascript/packs/otp-delivery-preference.js @@ -8,7 +8,13 @@ const { t } = /** @type {GlobalWithLoginGov} */ (window).LoginGov.I18n; -const getOTPDeliveryMethodContainer = () => document.querySelector('.js-otp-delivery-preferences'); +/** + * Returns the OTP delivery preference element. + * + * @return {HTMLElement} + */ +const getOTPDeliveryMethodContainer = () => + /** @type {HTMLElement} */ (document.querySelector('.js-otp-delivery-preferences')); /** * @return {HTMLInputElement[]} @@ -64,8 +70,13 @@ const isAllDisabled = (inputs) => inputs.every((input) => input.disabled); */ const getFirstEnabledInput = (inputs) => inputs.find((input) => !input.disabled); +/** + * Toggles the delivery preferences selection visible or hidden. + * + * @param {boolean} isVisible Whether the selection element should be visible. + */ const toggleDeliveryPreferencesVisible = (isVisible) => - getOTPDeliveryMethodContainer()?.classList.toggle('display-none', !isVisible); + getOTPDeliveryMethodContainer().classList.toggle('display-none', !isVisible); /** * @param {Event} event From 78e57b43d1b87c12e8e722a209c8ac601ba898bc Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 24 Nov 2021 08:12:34 -0500 Subject: [PATCH 17/31] Update spec for revised phone required error text --- spec/features/phone/add_phone_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/phone/add_phone_spec.rb b/spec/features/phone/add_phone_spec.rb index ad9a4f5704f..cad32c80b2d 100644 --- a/spec/features/phone/add_phone_spec.rb +++ b/spec/features/phone/add_phone_spec.rb @@ -45,7 +45,7 @@ focused_input = page.find(':focus') error_message = page.find_by_id(focused_input[:'aria-describedby']) expect(focused_input).to match_css('.phone-input__number.usa-input--error') - expect(error_message).to have_content(t('simple_form.required.text')) + expect(error_message).to have_content(t('errors.messages.phone_required')) expect(hidden_select.value).to eq('US') fill_in :new_phone_form_phone, with: 'abcd1234' From 63d9bebe10a1c9c5e941f108da7818af7ce55803 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 24 Nov 2021 08:17:46 -0500 Subject: [PATCH 18/31] Simplify single country styling intl-tel-input already applies a class to key off for dropdown vs. no-dropdown --- app/assets/stylesheets/components/_phone-input.scss | 4 ++-- app/components/phone_input_component.html.erb | 4 ++-- app/components/phone_input_component.rb | 6 ------ 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/components/_phone-input.scss b/app/assets/stylesheets/components/_phone-input.scss index 7b81a89e254..630b4de1911 100644 --- a/app/assets/stylesheets/components/_phone-input.scss +++ b/app/assets/stylesheets/components/_phone-input.scss @@ -14,12 +14,12 @@ lg-phone-input { } } -.phone-input--single-country .iti input { +.iti:not(.iti--allow-dropdown) input { padding-left: 36px; padding-right: 6px; } -.phone-input--single-country .iti .iti__flag-container { +.iti:not(.iti--allow-dropdown) .iti__flag-container { left: 0; right: auto; } diff --git a/app/components/phone_input_component.html.erb b/app/components/phone_input_component.html.erb index ec2e1f6e798..3b1327b6cf4 100644 --- a/app/components/phone_input_component.html.erb +++ b/app/components/phone_input_component.html.erb @@ -1,4 +1,4 @@ -<%= content_tag('lg-phone-input', class: css_class) do %> + <%= content_tag( :script, { @@ -52,5 +52,5 @@ class: 'phone-input__number', }, ) %> -<% end %> + <%= stylesheet_link_tag 'intl-tel-input/build/css/intlTelInput' %> diff --git a/app/components/phone_input_component.rb b/app/components/phone_input_component.rb index 965ea6a7b6a..6c2e5d2e5e2 100644 --- a/app/components/phone_input_component.rb +++ b/app/components/phone_input_component.rb @@ -32,12 +32,6 @@ def international_phone_codes end end - def css_class - classes = [] - classes << 'phone-input--single-country' if supported_country_codes.size == 1 - [*classes, *tag_options[:class]] - end - private def international_phone_code_label(code_data) From b708fb9b4c2838570ecd4347c63d9c03656b1f6d Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 24 Nov 2021 09:33:13 -0500 Subject: [PATCH 19/31] Resolve console error on attempted hidden field focus --- app/javascript/packs/otp-delivery-preference.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/javascript/packs/otp-delivery-preference.js b/app/javascript/packs/otp-delivery-preference.js index 8e81f17357a..3a4953519d4 100644 --- a/app/javascript/packs/otp-delivery-preference.js +++ b/app/javascript/packs/otp-delivery-preference.js @@ -135,6 +135,10 @@ function syncSelectValidityToTextInput(phoneInput, event) { if (select instanceof HTMLSelectElement && !select.validity.valid && textInput) { textInput.setCustomValidity(select.validationMessage); textInput.reportValidity(); + + // Prevent default behavior, which may attempt to draw focus to the select input. Because it is + // hidden, the browser may throw an error. + event.preventDefault(); } } From d4f69efb6b681b25eeba88635fc7e5abf6b8b568 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 24 Nov 2021 09:33:21 -0500 Subject: [PATCH 20/31] Add more inline code commments --- app/javascript/packs/otp-delivery-preference.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/app/javascript/packs/otp-delivery-preference.js b/app/javascript/packs/otp-delivery-preference.js index 3a4953519d4..9d3fcd0b3fc 100644 --- a/app/javascript/packs/otp-delivery-preference.js +++ b/app/javascript/packs/otp-delivery-preference.js @@ -124,11 +124,22 @@ function updateOTPDeliveryMethods(event) { select.setCustomValidity(hintText); select.reportValidity(); } else if (!select.validity.valid) { - textInput.setCustomValidity(''); + // Reset previously-set custom validity. This may have been sync'd to the text input if the user + // tried to submit, and should be reset there as well if it had been. select.setCustomValidity(''); + if (textInput.validationMessage === hintText) { + textInput.setCustomValidity(''); + } } } +/** + * On an invalid event of the selected code (e.g. on form submission after selecting an unsupported + * country), sync invalid state to text input, so that an error message is shown. + * + * @param {PhoneInput} phoneInput PhoneInput element. + * @param {Event} event Invalid event. + */ function syncSelectValidityToTextInput(phoneInput, event) { const { target: select } = event; const { textInput } = phoneInput; From bad1ec0e9b02d558e128eafaeeb689d6e9a1267d Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 24 Nov 2021 09:34:18 -0500 Subject: [PATCH 21/31] Add more specs for add_phone_spec Coverage for different combinations of changing values, and hidden delivery preferences panel --- spec/features/phone/add_phone_spec.rb | 37 +++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/spec/features/phone/add_phone_spec.rb b/spec/features/phone/add_phone_spec.rb index cad32c80b2d..6cbc51aa847 100644 --- a/spec/features/phone/add_phone_spec.rb +++ b/spec/features/phone/add_phone_spec.rb @@ -41,6 +41,7 @@ hidden_select = page.find('[name="new_phone_form[international_code]"]', visible: :hidden) + # Required field should prompt as required on submit click_continue focused_input = page.find(':focus') error_message = page.find_by_id(focused_input[:'aria-describedby']) @@ -48,18 +49,50 @@ expect(error_message).to have_content(t('errors.messages.phone_required')) expect(hidden_select.value).to eq('US') + # Invalid number should prompt as invalid on submit fill_in :new_phone_form_phone, with: 'abcd1234' click_continue focused_input = page.find(':focus') error_message = page.find_by_id(focused_input[:'aria-describedby']) expect(focused_input).to match_css('.phone-input__number.usa-input--error') - expect(error_message).to have_content(t('errors.messages.improbable_phone')) + expect(error_message).to have_content(t('errors.messages.invalid_phone_number')) expect(hidden_select.value).to eq('US') + # Unsupported country should prompt as invalid and hide delivery options immediately + page.find('div[aria-label="Country code"]').click + within(page.find('.iti__flag-container', visible: :all)) do + find('span', text: 'Sri Lanka').click + end + focused_input = page.find('.phone-input__number:focus') + error_message = page.find_by_id(focused_input[:'aria-describedby']) + expect(error_message).to have_content( + t( + 'two_factor_authentication.otp_delivery_preference.no_supported_options', + location: 'Sri Lanka', + ), + ) + expect(page).to_not have_content(t('two_factor_authentication.otp_delivery_preference.title')) + expect(hidden_select.value).to eq('LK') + fill_in :new_phone_form_phone, with: '+94 071 234 5678' + click_continue + expect(page.find(':focus')).to match_css('.phone-input__number') + + # Switching to supported country should re-show delivery options, but prompt as invalid number + page.find('div[aria-label="Country code"]').click + within(page.find('.iti__flag-container', visible: :all)) do + find('span', text: 'United States').click + end + expect(page).to have_content(t('two_factor_authentication.otp_delivery_preference.title')) + expect(page).to_not have_css('.usa-error-message') + expect(hidden_select.value).to eq('US') + click_continue + expect(page.find(':focus')).to match_css('.phone-input__number') + expect(page).to have_content(t('errors.messages.invalid_phone_number')) + + # Entering valid number should allow submission input = fill_in :new_phone_form_phone, with: '+81543543643' expect(input.value).to eq('+81 543543643') expect(hidden_select.value).to eq('JP') - click_continue expect(page).to have_content(t('forms.two_factor.code')) end From d6a9c7349a6401e54b2ae59c76d2d54011fd68ab Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 24 Nov 2021 10:16:41 -0500 Subject: [PATCH 22/31] Move deliverability validation into PhoneInput implementation **Why**: Because we need this validation on the IAL2 hybrid step, which doesn't offer choices for delivery method --- app/components/phone_input_component.html.erb | 9 ++++- app/components/phone_input_component.rb | 11 +++++- app/javascript/packages/phone-input/index.js | 34 +++++++++++++++- .../packages/phone-input/index.spec.js | 29 ++++++++++---- .../packs/otp-delivery-preference.js | 39 ------------------- app/views/idv/doc_auth/send_link.html.erb | 7 +++- .../idv/doc_auth/send_link_step_spec.rb | 17 ++++++++ 7 files changed, 92 insertions(+), 54 deletions(-) diff --git a/app/components/phone_input_component.html.erb b/app/components/phone_input_component.html.erb index 3b1327b6cf4..d6be825ea92 100644 --- a/app/components/phone_input_component.html.erb +++ b/app/components/phone_input_component.html.erb @@ -1,10 +1,15 @@ - +<%= content_tag( + 'lg-phone-input', + class: tag_options[:class], + data: { delivery_methods: delivery_methods }, + ) do %> <%= content_tag( :script, { country_code_label: t('components.phone_input.country_code_label'), invalid_phone: t('errors.messages.invalid_phone_number'), country_constraint_usa: t('errors.messages.phone_country_constraint_usa'), + unsupported_country: t('two_factor_authentication.otp_delivery_preference.no_supported_options'), }.to_json, { type: 'application/json', @@ -52,5 +57,5 @@ class: 'phone-input__number', }, ) %> - +<% end %> <%= stylesheet_link_tag 'intl-tel-input/build/css/intlTelInput' %> diff --git a/app/components/phone_input_component.rb b/app/components/phone_input_component.rb index 6c2e5d2e5e2..5d8a1ddf5ee 100644 --- a/app/components/phone_input_component.rb +++ b/app/components/phone_input_component.rb @@ -1,12 +1,19 @@ class PhoneInputComponent < BaseComponent - attr_reader :form, :required, :allowed_countries, :tag_options + attr_reader :form, :required, :allowed_countries, :delivery_methods, :tag_options alias_method :f, :form - def initialize(form:, allowed_countries: nil, required: false, **tag_options) + def initialize( + form:, + allowed_countries: nil, + delivery_methods: [:sms, :voice], + required: false, + **tag_options + ) @allowed_countries = allowed_countries @form = form @required = required + @delivery_methods = delivery_methods @tag_options = tag_options end diff --git a/app/javascript/packages/phone-input/index.js b/app/javascript/packages/phone-input/index.js index 597cf5e57f7..3d054072d41 100644 --- a/app/javascript/packages/phone-input/index.js +++ b/app/javascript/packages/phone-input/index.js @@ -1,6 +1,7 @@ import { isValidNumber, isValidNumberForRegion } from 'libphonenumber-js'; import 'intl-tel-input/build/js/utils.js'; import intlTelInput from 'intl-tel-input'; +import { replaceVariables } from '@18f/identity-i18n'; /** @typedef {import('libphonenumber-js').CountryCode} CountryCode */ @@ -10,6 +11,7 @@ import intlTelInput from 'intl-tel-input'; * @prop {string=} country_code_label * @prop {string=} invalid_phone * @prop {string=} country_constraint_usa + * @prop {string=} unsupported_country */ /** @@ -38,6 +40,9 @@ export class PhoneInput extends HTMLElement { /** @type {PhoneInputStrings} */ #_strings; + /** @type {string[]} */ + deliveryMethods = []; + connectedCallback() { /** @type {HTMLInputElement?} */ this.textInput = this.querySelector('.phone-input__number'); @@ -45,6 +50,9 @@ export class PhoneInput extends HTMLElement { this.codeInput = this.querySelector('.phone-input__international-code'); this.codeWrapper = this.querySelector('.phone-input__international-code-wrapper'); this.exampleText = this.querySelector('.phone-input__example'); + try { + this.deliveryMethods = JSON.parse(this.dataset.deliveryMethods || ''); + } catch {} if (!this.textInput || !this.codeInput) { return; @@ -147,8 +155,8 @@ export class PhoneInput extends HTMLElement { } validate() { - const { textInput, codeInput, supportedCountryCodes } = this; - if (!textInput || !codeInput) { + const { textInput, codeInput, supportedCountryCodes, selectedOption } = this; + if (!textInput || !codeInput || !selectedOption) { return; } @@ -174,6 +182,18 @@ export class PhoneInput extends HTMLElement { if (isInvalidPhoneNumber) { textInput.setCustomValidity(this.strings.invalid_phone || ''); } + + if (!this.isSupportedCountry()) { + const validationMessage = replaceVariables(this.strings.unsupported_country || '', { + location: selectedOption.dataset.countryName, + }); + + textInput.setCustomValidity(validationMessage); + + // While most other validations can wait 'til submission to present user feedback, this one + // should notify immediately. + textInput.dispatchEvent(new CustomEvent('invalid')); + } } formatTextInput() { @@ -200,6 +220,16 @@ export class PhoneInput extends HTMLElement { return !!selectedOption && selectedOption.getAttribute(`data-supports-${delivery}`) !== 'false'; } + /** + * Returns true if the currently selected country can receive a supported delivery options, or + * false otherwise. + * + * @return {boolean} Whether selected country is supported. + */ + isSupportedCountry() { + return this.deliveryMethods.some((delivery) => this.isDeliveryOptionSupported(delivery)); + } + setExampleNumber() { const { exampleText, iti } = this; const { iso2 = 'us' } = iti.selectedCountryData; diff --git a/app/javascript/packages/phone-input/index.spec.js b/app/javascript/packages/phone-input/index.spec.js index 1b2475391c6..f39b9ae6bba 100644 --- a/app/javascript/packages/phone-input/index.spec.js +++ b/app/javascript/packages/phone-input/index.spec.js @@ -5,6 +5,7 @@ const MULTIPLE_OPTIONS_HTML = ` `; const SINGLE_OPTION_HTML = ` @@ -27,12 +28,14 @@ describe('PhoneInput', () => { function createAndConnectElement({ isSingleOption = false, isNonUSSingleOption = false } = {}) { const element = document.createElement('lg-phone-input'); + element.setAttribute('data-delivery-methods', '["sms","voice"]'); element.innerHTML = `
@@ -76,14 +79,26 @@ describe('PhoneInput', () => { expect(phoneNumber.validity.valueMissing).to.be.true(); userEvent.type(phoneNumber, '5'); - expect(phoneNumber.validationMessage).to.equal( - 'Invalid phone number. Please make sure you enter a valid phone number.', - ); + expect(phoneNumber.validationMessage).to.equal('Phone number is not valid'); userEvent.type(phoneNumber, '13-555-1234'); expect(phoneNumber.validity.valid).to.be.true(); }); + it('validates supported delivery method', () => { + const input = createAndConnectElement(); + + /** @type {HTMLInputElement} */ + const phoneNumber = getByLabelText(input, 'Phone number'); + /** @type {HTMLSelectElement} */ + const countryCode = getByLabelText(input, 'Country code', { selector: 'select' }); + + userEvent.selectOptions(countryCode, 'LK'); + expect(phoneNumber.validationMessage).to.equal( + 'We are unable to verify phone numbers from Sri Lanka', + ); + }); + context('with single option', () => { it('initializes without dropdown', () => { const input = createAndConnectElement({ isSingleOption: true }); @@ -109,9 +124,7 @@ describe('PhoneInput', () => { const phoneNumber = getByLabelText(input, 'Phone number'); userEvent.type(phoneNumber, '513-555-1234'); - expect(phoneNumber.validationMessage).to.equal( - 'Invalid phone number. Please make sure you enter a valid phone number.', - ); + expect(phoneNumber.validationMessage).to.equal('Phone number is not valid'); }); }); }); diff --git a/app/javascript/packs/otp-delivery-preference.js b/app/javascript/packs/otp-delivery-preference.js index 9d3fcd0b3fc..bc4feb706fb 100644 --- a/app/javascript/packs/otp-delivery-preference.js +++ b/app/javascript/packs/otp-delivery-preference.js @@ -116,49 +116,10 @@ function updateOTPDeliveryMethods(event) { }); const isAllMethodsDisabled = isAllDisabled(methods); - const hintText = t('two_factor_authentication.otp_delivery_preference.no_supported_options', { - location, - }); toggleDeliveryPreferencesVisible(!isAllMethodsDisabled); - if (isAllMethodsDisabled) { - select.setCustomValidity(hintText); - select.reportValidity(); - } else if (!select.validity.valid) { - // Reset previously-set custom validity. This may have been sync'd to the text input if the user - // tried to submit, and should be reset there as well if it had been. - select.setCustomValidity(''); - if (textInput.validationMessage === hintText) { - textInput.setCustomValidity(''); - } - } -} - -/** - * On an invalid event of the selected code (e.g. on form submission after selecting an unsupported - * country), sync invalid state to text input, so that an error message is shown. - * - * @param {PhoneInput} phoneInput PhoneInput element. - * @param {Event} event Invalid event. - */ -function syncSelectValidityToTextInput(phoneInput, event) { - const { target: select } = event; - const { textInput } = phoneInput; - if (select instanceof HTMLSelectElement && !select.validity.valid && textInput) { - textInput.setCustomValidity(select.validationMessage); - textInput.reportValidity(); - - // Prevent default behavior, which may attempt to draw focus to the select input. Because it is - // hidden, the browser may throw an error. - event.preventDefault(); - } } document.querySelectorAll('lg-phone-input').forEach((node) => { const phoneInput = /** @type {PhoneInput} */ (node); phoneInput.addEventListener('change', updateOTPDeliveryMethods); - phoneInput.addEventListener( - 'invalid', - (event) => syncSelectValidityToTextInput(phoneInput, event), - true, - ); }); diff --git a/app/views/idv/doc_auth/send_link.html.erb b/app/views/idv/doc_auth/send_link.html.erb index 8496423c539..769b7575bff 100644 --- a/app/views/idv/doc_auth/send_link.html.erb +++ b/app/views/idv/doc_auth/send_link.html.erb @@ -23,7 +23,12 @@ method: 'PUT', html: { autocomplete: 'off', class: 'margin-top-4' }, ) do |f| %> - <%= render PhoneInputComponent.new(form: f, required: true, class: 'margin-bottom-4') %> + <%= render PhoneInputComponent.new( + form: f, + required: true, + delivery_methods: [:sms], + class: 'margin-bottom-4', + ) %>