Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Phone number auto detection and rendering as a hyperlink #2780

Closed
arman-g opened this issue Dec 30, 2019 · 7 comments · Fixed by #2787
Closed

Phone number auto detection and rendering as a hyperlink #2780

arman-g opened this issue Dec 30, 2019 · 7 comments · Fixed by #2787
Assignees
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Required for internal reporting. Do not delete. customer-reported Required for internal Azure reporting. Do not delete.

Comments

@arman-g
Copy link

arman-g commented Dec 30, 2019

Feature Request

Is your feature request related to a problem? Please describe.

Currently webchat does not detect phone numbers in a message and relies on a browser auto detection which might be a browser configuration setting. It also not consistent from browser to browser.

Describe the suggestion or request in detail

A better solution and It will be desired to have auto detection on webchat which will detect a phone number and convert it to a hyperlink.
This could be a setting option giving the developer to control it.

Describe alternatives you have considered

None

[Enhancement]

@arman-g arman-g added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. Enhancement customer-reported Required for internal Azure reporting. Do not delete. labels Dec 30, 2019
@cwhitten
Copy link
Member

cwhitten commented Dec 30, 2019

Hi @arman-g. If we added auto-detection to WebChat's core logic and did what you suggested, what would the hyperlink link to? Keep in mind the need to support desktop browsers and mobile environments.

We have the activity middleware mechanism to allow consumers to consume and render activities in any way they would wish, and as it's designed this would be the way to support what you are looking to do: write your own phone number detection algorithm and run it inside the activity middleware pipeline.

@cwhitten cwhitten added the customer-replied-to Required for internal reporting. Do not delete. label Dec 30, 2019
@arman-g
Copy link
Author

arman-g commented Dec 30, 2019

@cwhitten I was just thinking about using middleware for this. Thanks for confirming it.
In general it would be good to have it as a build in setting option.
For example (321) 789-4623 would be rendered as <a href="tel:3217894623">(321) 789-4623</a>
This will be shown as an auto call under all browsers and mobile devices.

@arman-g
Copy link
Author

arman-g commented Dec 30, 2019

@cwhitten I can't find how to effect the rendering of the message from activity middleware. I don't think it is even possible unless it is just a simple text change. If I change (321) 789-4623 to <a href="tel:3217894623">(321) 789-4623</a> it shows error.

TS Code:

 const attachmentMiddleware = () => next => card => {
      if (card.attachment.contentType === 'application/vnd.microsoft.card.adaptive' ||
        card.attachment.contentType === 'application/vnd.microsoft.card.hero') {
        card.attachment = this.phoneHyperlink(card.attachment);
        $('div.main').hide();
      } else if (card.attachment.contentType === 'text/markdown' && card.activity.from.role === 'bot') {
        $('div.main').show();
      }
      return next(card);
    };
private phoneHyperlink(attachment): any {
    let str = JSON.stringify(attachment);
    str = str.replace(/\((\d{3})\)\s(\d{3})-(\d{4})/, '&lt;a href="tel:$1$2$3"&gt;($1) $2-$3&lt/a&gt;');
    return JSON.parse(str);
  }

Edit*
The error is due to JSON parsing since some characters are not valid for JSON format. I got rid of converting object to json string and back to object and instead replaced the phone number in the text of field directly. The problem is still there and I don't think it is possible to effect directly to the rendering process. I think this feature is desirable.
Capture

@tdurnford
Copy link
Contributor

Web Chat uses the Markdown-it npm package to render markdown into HTML which by default supports the tel protocol. However, Web Chat also uses the sanitize-html npm package to sanitize the resulting HTML from the markdown renderer, and we currently don't have the tel protocol listed as an allowed schema. As a result, the sanitizer removes the link.

There are two options to enable the tel protocol in Web Chat and remedy this issue:

Option 1

Add the tel protocol to the list of allowed schema's

const SANITIZE_HTML_OPTIONS = {
  ...
  allowedSchemes: ['data', 'http', 'https', 'ftp', 'mailto', 'sip', 'tel'],
  ...
};

const SANITIZE_HTML_OPTIONS = {

Option 2

Developers can currently pass their own markdown renderer to Web Chat that does not sanitize the HTML.

<script src="https://cdnjs.cloudflare.com/ajax/libs/markdown-it/10.0.0/markdown-it.js"></script>
<script>
  (async function() {
    const res = await fetch('https://webchat-mockbot.azurewebsites.net/directline/token', { method: 'POST' });
    const { token } = await res.json();

    const markdownIt = window.markdownit();

    window.WebChat.renderWebChat(
      {
        directLine: window.WebChat.createDirectLine({ token }),
        renderMarkdown: text => markdownIt.render(text)
      },
      document.getElementById('webchat')
    );

    document.querySelector('#webchat > *').focus();
  })().catch(err => console.error(err));
</script>

Result

Bot Framework SDK v4 (Node)

context.sendActivity("[(505)503-4455](tel:(505)503-4455));

Screenshots

image

image

@arman-g
Copy link
Author

arman-g commented Dec 31, 2019

@tdurnford thank you for the information. I am using the CDN and not sure if I can add 'tel' to sanitize options. Can this option be added in the next release of the webchat?

@tdurnford
Copy link
Contributor

tdurnford commented Dec 31, 2019

The tel protocol would have to be added to the ‘allowedSchema’ in a PR.

@compulim @corinagum @cwhitten Thoughts?

@tdurnford tdurnford self-assigned this Dec 31, 2019
@cwhitten
Copy link
Member

cwhitten commented Dec 31, 2019

@tdurnford that makes sense to me - it seems tel may have been an oversight in the original list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bot Services Required for internal Azure reporting. Do not delete. Do not change color. customer-replied-to Required for internal reporting. Do not delete. customer-reported Required for internal Azure reporting. Do not delete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants