LG-5333: Improve error state for countries where phone verification disabled#5619
LG-5333: Improve error state for countries where phone verification disabled#5619
Conversation
aduth
force-pushed
the
aduth-lg-5333-phone-country-disabled
branch
from
d4ead59 to
f4094f3
Compare
**Why**: - For improved consistency of presentation of error messages in the application - Favoring design system over redundant, ad-hoc implementations - Unblocking new behaviors in #5619 where form-level `has-error` class error styling would take precedence over field-level validity state styling
…5626) **Why**: - For improved consistency of presentation of error messages in the application - Favoring design system over redundant, ad-hoc implementations - Unblocking new behaviors in #5619 where form-level `has-error` class error styling would take precedence over field-level validity state styling
aduth
force-pushed
the
aduth-lg-5333-phone-country-disabled
branch
from
b9f2bb4 to
f62ef50
Compare
**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.
Save for LG-5362
Per Rubocop. TIL!
This reverts commit a0c29c96d7ce22e8dcf307c1a14be691e6f08d10.
aduth
force-pushed
the
aduth-lg-5333-phone-country-disabled
branch
from
57a0493 to
432d152
Compare
intl-tel-input already applies a class to key off for dropdown vs. no-dropdown
Coverage for different combinations of changing values, and hidden delivery preferences panel
**Why**: Because we need this validation on the IAL2 hybrid step, which doesn't offer choices for delivery method
aduth
changed the title
|
This is ready for review now. I've updated the original comment to reflect that the pull request now encompasses the entirety of the ticket LG-5333, including revised behaviors for existing component usage on phone setup / hybrid screens. Screenshots are also included in the original comment. Despite pulling out a few changes to separate pull requests (#5626, #5627, #5628, #5631), this is still a bit larger than I'd hoped for. Let me know if it's a problem for review, and I can split off work between validation for existing phone inputs vs. new support for single country selection on the IAL2 address verification step. |
| width: min-content; | ||
| } | ||
|
|
||
| .validated-field__input-wrapper { | ||
| width: max-content; |
There was a problem hiding this comment.
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.
|
|
||
| def error_messages | ||
| { | ||
| valueMissing: t('simple_form.required.text'), |
There was a problem hiding this comment.
honestly surprised this camelCase symbol didn't make any of our rubocop linters complain 🤷
There was a problem hiding this comment.
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:
anniehirshman-gsa
left a comment
anniehirshman-gsa
left a comment
There was a problem hiding this comment.
Aside from Slack conversation about the specific wording for text messages sent to unsupported countries - screenshots LGTM
| @@ -0,0 +1,28 @@ | |||
| <lg-validated-field> | |||
| <%= content_tag( | |||
There was a problem hiding this comment.
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.
Updated texts in 8bb6434. |
3 participants
Why: As a user, I expect that if neither SMS nor Phone Call is supported in my country, I am given clear indication of what to do next, so that I can set up MFA / phone for my account.
This also brings the PhoneInputComponent to the IAL2 address verification step, since phone input fields should use consistent styling and validation behavior.
Screenshot: