Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
315f5a1
LG-5333: Use PhoneInputComponent on IAL2 address verification
aduth Nov 18, 2021
97149d9
Restore no-cond-assign enforcement with except-parens
aduth Nov 18, 2021
491e668
Validate allowed_countries against actual codes
aduth Nov 18, 2021
5b6bb94
Revert OTP delivery preference validation changes
aduth Nov 19, 2021
fd0e6d3
Use self-assignment operator
aduth Nov 19, 2021
5c8a634
Revert "Revert OTP delivery preference validation changes"
aduth Nov 19, 2021
9d337fc
Remove default CSS class, support custom class
aduth Nov 23, 2021
432d152
Add ValidatedFieldComponent for independent field validation
aduth Nov 19, 2021
9593ab9
Add US-constrained phone number translations
aduth Nov 23, 2021
3047d3e
Validate non-US single country option
aduth Nov 23, 2021
ef343ee
Add customized phone required messaging
aduth Nov 23, 2021
dd42d94
Update invalid phone number messaging
aduth Nov 23, 2021
7d6e363
Update unsupported phone numbers translations
aduth Nov 23, 2021
873e984
WIP: Toggle error on undeliverable countries
aduth Nov 23, 2021
8342a34
WIP: Try to constrain width of error message to match input
aduth Nov 23, 2021
877c7a5
Add inline code document for new otp-delivery-preference methods
aduth Nov 24, 2021
78e57b4
Update spec for revised phone required error text
aduth Nov 24, 2021
63d9beb
Simplify single country styling
aduth Nov 24, 2021
b708fb9
Resolve console error on attempted hidden field focus
aduth Nov 24, 2021
d4f69ef
Add more inline code commments
aduth Nov 24, 2021
bad1ec0
Add more specs for add_phone_spec
aduth Nov 24, 2021
d6a9c73
Move deliverability validation into PhoneInput implementation
aduth Nov 24, 2021
fdd0116
PhoneInput: Expand test coverage to include constrained delivery methods
aduth Nov 24, 2021
02e5850
PhoneInputComponent: Show delivery method-specific messaging
aduth Nov 24, 2021
5207e7b
Update SignInSpec for always-enabled submit button
aduth Nov 24, 2021
38e05ff
PhoneInput: Add spec for phone country switch formatting
aduth Nov 24, 2021
d65e43a
Remove unused translations
aduth Nov 24, 2021
ff818d0
Restart build
aduth Nov 24, 2021
4f1ba02
Fix spec for updated SMS deliverable method message
aduth Nov 24, 2021
354679c
Expand test coverage to include voice constraint
aduth Nov 29, 2021
8bb6434
Update unsupported delivery method error texts
aduth Nov 29, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,12 @@ Rails/Blank:
Rails/Delegate:
Enabled: false

Rails/DynamicFindBy:
Include:
- tests/features/**/*.rb
AllowedMethods:
- find_by_id

Rails/FilePath:
Enabled: false

Expand Down
18 changes: 16 additions & 2 deletions app/assets/stylesheets/components/_phone-input.scss
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -6,8 +14,14 @@
}
}

lg-phone-input .iti__flag {
background-image: image-url('intl-tel-input/build/img/flags.png');
.iti:not(.iti--allow-dropdown) input {
padding-left: 36px;
padding-right: 6px;
}

.iti:not(.iti--allow-dropdown) .iti__flag-container {
left: 0;
right: auto;
}

@media (-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi) {
Expand Down
8 changes: 8 additions & 0 deletions app/assets/stylesheets/components/_validated-field.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
lg-validated-field {
display: block;
width: min-content;
}

.validated-field__input-wrapper {
width: max-content;
Comment on lines +3 to +7
Copy link
Copy Markdown
Contributor Author

@aduth aduth Nov 24, 2021

Choose a reason for hiding this comment

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

FYI: IE11 doesn't support min-content or max-content, but I'd consider this under the realm of progressive enhancement / graceful degradation. Main difference is that the width of the error message won't be constrained to the width of the input.

}
1 change: 1 addition & 0 deletions app/assets/stylesheets/components/all.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@
@import 'step-indicator';
@import 'troubleshooting-options';
@import 'i18n-dropdown';
@import 'validated-field';
4 changes: 2 additions & 2 deletions app/components/accordion_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
type="button"
class="usa-accordion__button"
aria-expanded="false"
aria-controls="<%= @target_id %>"
aria-controls="accordion-<%= unique_id %>"
>
<%= header %>
</button>
</div>
<div id="<%= @target_id %>" class="usa-accordion__container">
<div id="accordion-<%= unique_id %>" class="usa-accordion__container">
<div class="usa-accordion__content usa-prose">
<%= content %>
</div>
Expand Down
4 changes: 0 additions & 4 deletions app/components/accordion_component.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
class AccordionComponent < BaseComponent
renders_one :header

def initialize
@target_id = "accordion-#{SecureRandom.hex(4)}"
end
end
4 changes: 4 additions & 0 deletions app/components/base_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
25 changes: 16 additions & 9 deletions app/components/phone_input_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
<lg-phone-input>
<%= 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.improbable_phone'),
}.to_json,
strings.to_json,
{
type: 'application/json',
class: 'phone-input__strings',
Expand Down Expand Up @@ -35,15 +36,21 @@
<%= t('forms.example') %>
<span class="phone-input__example"></span>
</div>
<%= f.input(
:phone,
<%= render ValidatedFieldComponent.new(
form: f,
name: :phone,
error_messages: {
valueMissing: t('errors.messages.phone_required'),
},
as: :tel,
required: required,
label: false,
wrapper_html: {
class: 'margin-bottom-0',
},
input_html: {
aria: { invalid: false },
class: 'phone-input__number',
},
) %>
</lg-phone-input>
<% end %>
<%= stylesheet_link_tag 'intl-tel-input/build/css/intlTelInput' %>
61 changes: 47 additions & 14 deletions app/components/phone_input_component.rb
Original file line number Diff line number Diff line change
@@ -1,33 +1,66 @@
class PhoneInputComponent < BaseComponent
attr_reader :form, :required
attr_reader :form, :required, :allowed_countries, :delivery_methods, :tag_options

alias_method :f, :form

def initialize(form:, required: false)
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

def supported_country_codes
PhoneNumberCapabilities::INTERNATIONAL_CODES.keys
codes = PhoneNumberCapabilities::INTERNATIONAL_CODES.keys
codes &= allowed_countries if allowed_countries
codes
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 strings
{
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: unsupported_country_string,
}
end

private

def unsupported_country_string
case delivery_methods.sort
when [:sms, :voice]
t('two_factor_authentication.otp_delivery_preference.no_supported_options')
when [:sms]
t('two_factor_authentication.otp_delivery_preference.sms_unsupported')
when [:voice]
t('two_factor_authentication.otp_delivery_preference.voice_unsupported')
end
end

def international_phone_code_label(code_data)
"#{code_data['name']} +#{code_data['country_code']}"
end
Expand Down
28 changes: 28 additions & 0 deletions app/components/validated_field_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<lg-validated-field>
<%= content_tag(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note for future consideration: Not sure if we should want to be populating strings like this, vs. leveraging our existing Webpack string extraction. On one hand, it improves portability of the component to think about how it might be used outside our toolchain, e.g. for design system consideration. On the other hand, it works differently, and loses out on ease-of-use of the Webpack approach.

Also worth noting that it probably won't work out of the box as-is unfortunately, because I've found the Webpack plugin is quite naive in how it identifies strings, and isn't very flexible to how Webpack would internally rewrite imports to import { t } from '@18f/identity-i18n'. Last time I looked, improving this should likely happen with/after the Webpacker 6 (Webpack 5) upgrade.

:script,
error_messages.to_json,
{
type: 'application/json',
class: 'validated-field__error-strings',
},
false,
) %>
<%= f.input(
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}",
},
},
),
) %>
<%= f.error(name, id: "validated-field-error-#{unique_id}") %>
</lg-validated-field>
5 changes: 5 additions & 0 deletions app/components/validated_field_component.js
Original file line number Diff line number Diff line change
@@ -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));
19 changes: 19 additions & 0 deletions app/components/validated_field_component.rb
Original file line number Diff line number Diff line change
@@ -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'),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

honestly surprised this camelCase symbol didn't make any of our rubocop linters complain 🤷

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

honestly surprised this camelCase symbol didn't make any of our rubocop linters complain 🤷

I see a handful of cops related to case naming where camelCase may be considered, though not for hash properties specifically:

**@error_messages,
}
end
end
2 changes: 1 addition & 1 deletion app/forms/idv/phone_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const config = {
'max-len': 'off',
'max-classes-per-file': 'off',
'newline-per-chained-call': 'off',
'no-cond-assign': ['error', 'except-parens'],
'no-console': 'error',
'no-empty': ['error', { allowEmptyCatch: true }],
'no-param-reassign': ['off', 'never'],
Expand Down
Loading