gutenberg icon indicating copy to clipboard operation
gutenberg copied to clipboard

Automatically add tel to phone number when linking url

Open devansh016 opened this issue 1 year ago • 3 comments

What?

The link field is automatically populated with tel: when the selected text is a phone number.

Why?

Solves: https://github.com/WordPress/gutenberg/issues/64804

How?

Testing Instructions

  1. Open a post or page.
  2. Add a phone number to a paragraph
  3. Select the phone number and click on link.

Screenshots or screencast

https://github.com/user-attachments/assets/6133cc1a-e800-492b-bc07-2bc36ad765ea

devansh016 avatar Aug 28 '24 10:08 devansh016

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: devansh016 <[email protected]>
Co-authored-by: jeryj <[email protected]>
Co-authored-by: ajlende <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions[bot] avatar Aug 28 '24 10:08 github-actions[bot]

:wave: Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @devansh016! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

github-actions[bot] avatar Aug 28 '24 10:08 github-actions[bot]

@devansh016, it was late last night when I was wrapping things up. I forgot to say that this is looking pretty good, all things considered! Phone numbers just happen to be more complicated than I realized 😅

I don't want this PR to get held up by those complexities, and I'm sure there's a "good enough" solution for this case. I'm asking a few folks today what they think the threshold should be for parsing a phone number.

ajlende avatar Sep 04 '24 14:09 ajlende

Hi @ajlende thanks for sharing your input.

@devansh016, it was late last night when I was wrapping things up. I forgot to say that this is looking pretty good, all things considered! Phone numbers just happen to be more complicated than I realized 😅

I don't want this PR to get held up by those complexities, and I'm sure there's a "good enough" solution for this case. I'm asking a few folks today what they think the threshold should be for parsing a phone number.

I have earlier made a wrong assumption that the phone number can be only 10 digits long excluding the country code will change it to include numbers upto 15 digits max. Also, minimum length should be 3 for phone number excluding the country code to include emergency number like 911 or 102?

There can be cases where the country code may not be provided. So will be making changes to the regex.

We probably want to add an optional tel: that handles the RFC3966 spec.

Good catch. Will add an optional tel:

False positives Dates in various formats: 2024.09.03.

There are low changes for user to add link to date or other text which are false positive but will check ways if I can avoid this.

devansh016 avatar Sep 04 '24 16:09 devansh016

I have earlier made a wrong assumption that the phone number can be only 10 digits long excluding the country code will change it to include numbers upto 15 digits max. Also, minimum length should be 3 for phone number excluding the country code to include emergency number like 911 or 102?

My personal opinion is that it's okay to exclude some valid phone numbers if there's a higher likelihood that the same pattern will result in a false positive.

I expect it's more likely that a string of 3 or 4 digits is NOT a phone number than it is a phone number which is why I was suggesting 5 or 6 as the minimum instead.

Take, for example, someone wanting to link the title of 1984 by George Orwell to a purchase page for the book or something. Turning 1984 into a tel: link by default is unexpected in this case, and I think those situations are more common than someone from Saint Helena trying to link a local phone number.

There can be cases where the country code may not be provided. So will be making changes to the regex.

The RFC3966 spec does say in section 5.1.5 that local numbers are required to have a phone-context parameter in the tel: URI. Whether or not Android or iOS follow the specification or do their own processing to append it based on the phone's locale is something that I haven't researched.

If you're up for the challenge, that's something you could look into.

If they follow the spec exactly and don't infer a context, then I would suggest an even simpler approach for now where we only handle phone numbers that start with + and include a country code.

There are low changes for user to add link to date or other text which are false positive but will check ways if I can avoid this.

The reason I thought about dates was for linking to iCal events or something. Again, I think it's okay for some false positives to slip through.

ajlende avatar Sep 04 '24 18:09 ajlende

@ajlende , updated the logic as below.

Assumptions:

  • Phone number can be maximum of length 15 and minimum length of 6 digits only including country code.
  • Number can start with/without country code.

PHONE_REGEXP is defined as /^(tel:)?(\+)?\d{6,15}$/

  • ^(tel:)?: Matches an optional "tel:" prefix at the start of the string.
  • (\+)?: Matches an optional + sign.
  • \d{6,15}$: Matches a sequence of 6 to 15 digits until the end of the string.

phoneNumber.replace(/[-.() ]/g, ''):

  • This verifies if the resulting string (after removing separators ) is a valid phone number according to the defined pattern.
  • Since there are many variations of separator it is better to remove separator before regex check and not include it in regex.

Things needed to be taken care:

  • Some cases which use tel: prefix liketel:879 may not be valid number should we can add a check if any phone number contains tel: as a prefix it should be a valid phone number. Should this be added?

devansh016 avatar Sep 05 '24 11:09 devansh016