Skip to content

LG-5193: Extract PhoneInputComponent for reuse#5481

Merged
aduth merged 28 commits intomainfrom
aduth-lg-5193-phone-input
Oct 15, 2021
Merged

LG-5193: Extract PhoneInputComponent for reuse#5481
aduth merged 28 commits intomainfrom
aduth-lg-5193-phone-input

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Oct 7, 2021

Why: Toward supporting LG-5193, to display an international phone input on the hybrid "Send Link" screen.

@aduth aduth marked this pull request as ready for review October 12, 2021 16:04
@aduth aduth changed the title Extract PhoneInputComponent for reuse LG-5193: Extract PhoneInputComponent for reuse Oct 12, 2021
Comment on lines 5 to 6
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aduth aduth marked this pull request as draft October 12, 2021 19:55
Base automatically changed from aduth-component-scripts to main October 14, 2021 13:42
@aduth aduth force-pushed the aduth-lg-5193-phone-input branch from 1185076 to 9fb0f9b Compare October 14, 2021 13:52
@aduth aduth marked this pull request as ready for review October 14, 2021 14:57
@aduth
Copy link
Contributor Author

aduth commented Oct 14, 2021

Alright, after merging #5490, rebasing the branch, and adding final translations, I think this should be ready for review finally.

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

aduth added 9 commits October 15, 2021 08:19
**Why**: Toward supporting LG-5193, to display an international phone input on the hybrid "Send Link" screen.
querySelectorAll always returns an array, which is truthy
Only one per element, elements available from iti instance
**Why**: Not needed with no-js. Maybe helps make it be a bit more standalone, but still tied to USWDS. YAGNI for now
codeWrapper not called in constructor
More reliable than waiting on intlTelInput to set placeholder, e.g. in case of failed server-side validation where value is prepopulated, intlTelInput won't actually set the placeholder
aduth added 19 commits October 15, 2021 08:19
Because it's only used within the component
**Why**: So that it's less of a mystery when things break
**Why**: Need to sort out conflicts with Webpack dynamic imports

See: #5481 (comment)
More compatible with querySelector result
Address separately at: #5492

This reverts commit 631d27efd42b0e314cbc9de87844cf2d01e6b3ec.
**Why**: Because it creates scoped variables which aren't accessible at time of assertions
**Why**: Avoid conflicts with multiple custom validations on text input
**Why**: Avoid conflicts of validation handling between form-validation and otp-delivery-preference
**Why**: Regression spec for disabling submit on invalid phone numbers
**Why**: Hopefully fix issue with Webpacker root path being set with double leading slashes ("//packs-test")

See: #5481 (review)
This reverts commit c655059eac6728bb344f749a1a3eccf6fc51f1a9.
Because we don't set it, so it's more confusing to be visible
@aduth aduth force-pushed the aduth-lg-5193-phone-input branch from 5fcfffd to 3a8a971 Compare October 15, 2021 12:20
@aduth aduth merged commit c0eb968 into main Oct 15, 2021
@aduth aduth deleted the aduth-lg-5193-phone-input branch October 15, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants