Skip to content
This repository has been archived by the owner on Mar 7, 2022. It is now read-only.

Fixing issue #166: cant update the socialshareText attr on whatsapp #174

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jherencia
Copy link

No description provided.

@45kb
Copy link
Member

45kb commented Dec 15, 2016

@jherencia Hi, sorry for the delay, i checked the PR but it seems to me that it should work here https://github.com/mrmilu/angular-socialshare/blob/f554baa12ffd59eece190c1c8d0d68d48fad1dcf/dist/angular-socialshare.js#L1061

So i actually don't understand why binding again inside the method itself... shouldn't be better to check what's happening at that file line?

@mebibou
Copy link
Contributor

mebibou commented Dec 17, 2016

Hey so I think @jherencia is on the right track, let me explain what's going on: so for whatsapp, we use a normal link <a href="whatsapp://..."></a> to trigger a call to action on iOs or Android, which will open the app and parse the params passed in the href. So this is different from most of the other providers as they pop up a window when clicked on it, which can be done by JavaScript, but in some cases it doesn't work at all for Whatsapp (even if you try the trick window.open('whatsapp://...', '_blank'); it doesn't work on Safari iOs if I remember correctly).

Anyways, the right idea is to update the href when one of the attributes used to construct the href changes, but this is not very generic, i.e. the code will work for whatsapp because only url and text are used to construct the href, but if another provider is similar and uses different attributes, then it doesn't work. So I would suggest adding a new config param for each provider that can specify the attributes to watch for change, so like now we have trigger: 'click', another option for whatsapp could we attrsToWatch: 'url, text' and code could look like:

angular.forEach(attrs. attrsToWatch, function(attr) {
  attrs.$observe(attr, onEventTriggered);
});

@45kb
Copy link
Member

45kb commented Dec 17, 2016

@mebibou that is a cool idea! btw would you try something for that? i'll be absolutely glad to merge it

something like socialshare-watch="['url','text','media']" absolutely legit 👍

@mebibou
Copy link
Contributor

mebibou commented Dec 20, 2016

@45kb well not an attribute of the DOM element since it is specific to each provider

@45kb
Copy link
Member

45kb commented Dec 20, 2016

@mebibou you have the provider from socialshare-provider="" which is mandatory for any provider... isn't it enough!?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants