Skip to content

Update international code as user types phone#1568

Merged
jmhooper merged 1 commit intomasterfrom
jmhooper-phone-number-format
Jul 26, 2017
Merged

Update international code as user types phone#1568
jmhooper merged 1 commit intomasterfrom
jmhooper-phone-number-format

Conversation

@jmhooper
Copy link
Contributor

Why: It is confusing that the international code selection and the
number in the phone input can be out of sync. This commit keeps both in
sync by updating them as the user types. This also helps make sure the
format of phone number makes a little more sense.

@jmhooper jmhooper changed the title [WIP] Update intl code as user types phone Update intl code as user types phone Jul 21, 2017
@jmhooper jmhooper changed the title Update intl code as user types phone Update international code as user types phone Jul 21, 2017
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: Internaional -- instead of Interna__t__ional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops 😳

@monfresh
Copy link
Contributor

I'm testing this locally and it's not doing what I think it should be doing.
For example:

  • Select Morocco from the dropdown -> this automatically adds +212 in the text field. Then, when I start entering more digits, it's still formatting them as if it were a US number, i.e. it encloses the first 3 digits I type within parentheses, etc.
  • If I enter as many digits as the text field will let me, which is one too many in the case of Morocco, when I submit the form, it tells me the number is invalid, but the text field now is no longer formatted, and the country code is duplicated. For example, if originally the text field was +212 (333) 333-3333, after submitting the form, it becomes +212 21 2333 3333333

Copy link
Contributor

Choose a reason for hiding this comment

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

This line is missing a closing square bracket after the closing single quote. It's what's causing the tests to fail. Interestingly, Chrome doesn't complain about this, but Safari does.

@jmhooper jmhooper changed the title Update international code as user types phone [WIP] Update international code as user types phone Jul 24, 2017
@jmhooper
Copy link
Contributor Author

Select Morocco from the dropdown -> this automatically adds +212 in the text field. Then, when I start entering more digits, it's still formatting them as if it were a US number, i.e. it encloses the first 3 digits I type within parentheses, etc.

I've pulled in libphonenumber-js and re-written the phone number formatter so it's a bit smarter about international numbers. There are some drawbacks. For example, it's significantly more complicated to figure out how to position the cursor when international numbers are in play. Some of the awkward behavior is on display in our other formatted fields though, and was on display in the old formatted phone field so I'm not too concerned about it.

If I enter as many digits as the text field will let me, which is one too many in the case of Morocco, when I submit the form, it tells me the number is invalid, but the text field now is no longer formatted, and the country code is duplicated. For example, if originally the text field was +212 (333) 333-3333, after submitting the form, it becomes +212 21 2333 3333333

This is not introduced by the changes in this PR. I plan on addressing that issue in #1571 since that will touch the form code where the bug lives.

I've also changed some of the JS to get this to work on the update user form phone.

Everything should be ready for another round of 👀s

@jmhooper jmhooper changed the title [WIP] Update international code as user types phone Update international code as user types phone Jul 25, 2017
@jmhooper
Copy link
Contributor Author

Failing specs 🙃. Flipping back to WIP. Once sec

@jmhooper jmhooper changed the title Update international code as user types phone [WIP] Update international code as user types phone Jul 25, 2017
@jmhooper
Copy link
Contributor Author

Specs are ✅. Now this is ready for another set of 👀s

@jmhooper jmhooper changed the title [WIP] Update international code as user types phone Update international code as user types phone Jul 25, 2017
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

**Why**: It is confusing that the international code selection and the
number in the phone input can be out of sync. This commit keeps both in
sync by updating them as the user types. This also helps make sure the
format of phone number makes a little more sense.

Use libphonenumber-js to format intl numbers

**Why**: The phone number formatter built into field-kit does not know
how to format international numbers. This commit abandons that phone
formatter in favor of a custom formatter built on top of
libphonenumber-js which does know how to format international numbers.

Format phone numbers on update user phone form

**Why**: This change gives users the same experience typing numbers and
selecting international codes that they get from the 2FA setup form.
@jmhooper jmhooper force-pushed the jmhooper-phone-number-format branch from ff1a97e to 0733a6d Compare July 26, 2017 16:20
@jmhooper jmhooper merged commit 16fef6e into master Jul 26, 2017
@jmhooper jmhooper deleted the jmhooper-phone-number-format branch July 27, 2017 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants