Patch on the way

CommentFileSizeAuthor
#12 add_extension_support-1922130-12.patch1.97 KBcweaver
#11 add_tel_link-1922130-11.patch1.62 KBcweaver
#5 1922130.patch1.93 KBBTMash
#1 i1922130.patch1.93 KBattiks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

attiks’s picture

attiks CreditAttribution: attiks commented
Status: Active » Needs review
FileSize
i1922130.patch1.93 KB

Attached patch adds a new option so you can output the phone number as a link

attiks’s picture

attiks CreditAttribution: attiks commented

Still applies, can anybody review?

TommyK’s picture

TommyK CreditAttribution: TommyK commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Still applies. Works great!

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Reviewed & tested by the community » Needs work

Reviewed the patch, looking good except for a single copy/paste artifact:

+++ b/phone.module
@@ -186,11 +186,48 @@ function phone_field_formatter_info() {
+  // Display this setting only if image is linked.

This is not an "image" field :)

BTMash’s picture

BTMash CreditAttribution: BTMash commented
FileSize
1922130.patch1.93 KB

Changed the comment to be a more generic 'field' ;)

BTMash’s picture

BTMash CreditAttribution: BTMash commented
Status: Needs work » Needs review
pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Needs review » Reviewed & tested by the community

Great, thanks!

minorOffense’s picture

minorOffense CreditAttribution: minorOffense commented
Status: Reviewed & tested by the community » Needs work

The link display doesn't take into account any extensions which have been applied to the number. You just get one long list of digits.

<a href="tel://888555333345">(888) 555-3333 x45</a>

Based on the spec I think it should read:

<a href="tel://8885553333;ext=45">(888) 555-3333 x45</a>

See http://stackoverflow.com/questions/9838775/problems-using-extensions-in-... for reference.

minorOffense’s picture

minorOffense CreditAttribution: minorOffense commented

Or it could just be a 'p' character. Tough to say.

http://demosthenes.info/blog/536/Adding-Phone-Numbers-To-Web-Pages-With-...

cweaver’s picture

cweaver CreditAttribution: cweaver commented

Revised preg_replace for including extensions in telephone links. Based on RFC 3966...

extension = ";ext=" 1*phonedigit"

$text = '<a href="tel://' . preg_replace('/([0-9]+)x([0-9]+)/', '$1;ext=$2', preg_replace('/[^0-9x]/', '', $text)) . '">' . $text . '</a>';
cweaver’s picture

cweaver CreditAttribution: cweaver commented
FileSize
add_tel_link-1922130-11.patch1.62 KB
cweaver’s picture

cweaver CreditAttribution: cweaver commented
FileSize
add_extension_support-1922130-12.patch1.97 KB

Ignore patch #11. Patch #12 incorporates 1922130.patch and extends it with phone number extension support.

cweaver’s picture

cweaver CreditAttribution: cweaver commented
Status: Needs work » Needs review
kruser’s picture

kruser CreditAttribution: kruser commented

I would suggest a href="sms:// option as well if you wanted to send a Text.

akolahi’s picture

akolahi CreditAttribution: akolahi commented

I don't think the two forward slashes are necessary. Simply 'tel:' should suffice, i believe.