Implement better validation and rendering between dialpad and destination number field

[ayyrickay]

The bug:

1. Fill out the destination number field with a valid phone number
2. Press a number on the dialpad
3. Make a call

You'll see that the destination number field doesn't change in the UI, but the dialpad number is added to the actual value and submitted in the phone call.

There should be a single method for validating a number being added that gets applied to input from the keyboard OR the dialpad.

The following is the expected behavior:

1. Fill out the destination number field with a valid phone number
2. Press a number on the dialpad
3. Destination number field updates to show the correct number

Note that, absent a phone number validation library that handles international numbers, we should err on the side of optimism for phone number input (otherwise we risk, for example, calling a phone number from Switzerland invalid.

[hatealgebra]

Hi @ayyrickay ,

I'm looking into the issue and I can reproduce it. As I understand this issue should solve:

1. Correct rendering of phone number when there is combination of dialpad and keyboard input (This one I understand)
2. Validation of phone numbers based on the region? If so, can you please elaborate more on the last paragraph?

Thanks!

[ayyrickay]

Hey @hatealgebra. The first is correct. I would reframe the second issue as: what is a valid phone number?

There's some amount of form validation we should do. For example, if there are letters or special characters, we shouldn't risk breaking the SDK. But once we get past the basic validation, I think it becomes hard to be very clear about what constitutes a phone number.

We could try and make the Dev Phone very paranoid about phone numbers and reject calls to numbers that don't seem valid. This is the more pessimistic design (e.g., if the input doesn't look valid, we won't bother making a call.) But because there's a lot of variation in the length of phone numbers, maybe it's better to design optimistically, and just assume that users will put in valid numbers.

Still, there could be some cool opportunities if we used a number validator. One fun example might be identifying the country code and rendering it out to the user to show what country that number belongs to in an emoji, for example.

[hatealgebra]

Hi @ayyrickay ,

I think the first issue is fixed and I've tested it.

About the second issue/problem, here are some points that I thought/came up with, let me know what you think about it:

Masking the input/validation

I've tried to sort of "mask" the input within the reducer and it's specific action. This way user can only type digits and special chars such as [*#+]:

  case SET_DESTINATION_NUMBER:
            const regex = /[*#+\d]/i;
            const numberLength = action.number.length;
            const lastAddedChar = action.number[numberLength -1];

            if(!regex.test(lastAddedChar) && numberLength !== 0) return state;
            return { ...state, destinationNumber: action.number }

Optimistic approach & flag API

I thought about the optimistic approach and suggestion with the country code recognition and did a bit of exploring. Actually, I've started with Twilio resources, where I've actually stumbled on the solution with int-tel-input plugin, which is also mentioned in this Twilio labs template.

I tried to work it out, but is kind of hacky to set it up in the react and this package would provide easier usage.

Conclusion

I hope that I've actually comprehended your original thought on the topic of the optimistic approach and did not tweak it that much.

My idea is kinda guide the user with entering the phone number, where we would not to worry about the letters in the input the user would just pick up the states code, and then it would be up to him what he types.

Looking forward to hearing your feedback and wish you a nice start to the week.

[hatealgebra]

Hi @ayyrickay,

Did you have time to check my last comment? I'm waiting for you (dis)approval. Looking forward for your feedback.

[hatealgebra]

@ayyrickay Sending a reminder about the next steps, thanks.

[ayyrickay]

@hatealgebra Hey - I'm really sorry for the delay on this, Marius mentioned that you were still waiting on feedback and I didn't realize it was in GH Issues 😱.

Ultimately, this is good. Two questions:

Should the regular expression for input logic should be on the reducer? Performance-wise, it's definitely not optimized. But I don't think that the dev phone has major performance needs, so maybe the benefit of putting it in the reducer is that you can debug a little more easily (people can see that there is input, it's just not getting added to the number.)

A more atomic solution (if we keep this pattern) would be to put the regex logic in a helper function, and it can go in the reducer for now. But that also makes the code easier to reason about (there's a dedicated function in SET_PHONE_NUMBER that, for example, stripPnInput(), and if we feel like there's performance issues, it can easily be switched to the component without affecting tests as significantly.

Second, I'm usually hesitant to use pre-built components. I don't know if that's me being silly, because this one seems pretty well maintained, but if we did use it, it seems like we would need to use the SubComponents API to maintain the dev phone styling.

At that point, it does seem like the int-tel-input package is less opinionated and exposes a lot more configuration, which seems nice. For example, it seems like you can get pretty fine grained control over plugin input, and that becomes configuration instead of testing.

As always, curious about your thoughts here - and this time, I can respond more quickly...