Add JS behavior to automatically trigger the addthis toolbar for ajax content (e.g. dialog)

1. +++ b/js/sharemessage.js
   @@ -0,0 +1,24 @@
   +      // This is used for special cases when the scripts are added using AJAX, for example if sharemessage is rendered
   

   80 col wrapping. :-)

2. +++ b/js/sharemessage.js
   @@ -0,0 +1,24 @@
   +      // in a dialog. In that case the adthis library is loaded after sharemessage.
   

   adthis => addthis... nice typo!
   sharemessage => ShareMessage
   And similar with the other line.

3. +++ b/js/sharemessage.js
   @@ -0,0 +1,24 @@
   +      var interval = setInterval(function(){
   ...
   +      }, 50);
   

   That's really an ugly race condition problem/ workaround with this polling. We should try to avoid this kind of dirty fix as much as possible.

   Is there no other way?

   If not, note that this Interval triggering is also fired and eating up browser performance for the non-ajax cases. And in case .addthis_toolbox isn't found, it keeps firing forever... At least fix the checks and sequence.

That's really an ugly race condition problem/ workaround with this polling. We should try to avoid this kind of dirty fix as much as possible.

Is there no other way?

In core/misc/dialog.js in openDialog() method code is triggering couple of events, the one we probably can reuse are dialog:beforecreate and dialog:aftercreate. So the code can look like this

$(window).on('dialog:beforecreate', function (e, dialog, $element, settings) {
  // do the stuff from interval function...
}

Not sure which event is better to use, probably both are OK.

Temporary fix until we find a better solution...

OK I misunderstood original problem which is actually a case that we can load addthis js dynamically later on some ajax event and then we need to execute it when actual js is loaded.

So addthis API is triggering event when it is ready (http://support.addthis.com/customer/portal/articles/1365497-addthis-java...), and it looks like this

// Listen for the ready event
addthis.addEventListener('addthis.ready', addthisReady);

And maybe we should use this also, but this again needs global addthis var to be initialized.

I have checked quickly addthis js code but could not found any place where addthis is triggering some event on load.

There is a way to detect when some js code is loaded, for example like explained on http://stackoverflow.com/questions/8718303/javascript-to-detect-when-ext..., but two problems here
1. Drupal is controling loading of JS code so not sure can we use this at all
2. Then not sure how much browsers support this script load event

So on the end using interval is not that bad but as Miro already said we should first check is addthis exist and if not then start that interval.

Ok, moved also the check for the addthis up.

+++ b/js/sharemessage.js
@@ -0,0 +1,25 @@
+            clearInterval(interval);

Still needs to be outside the .each().

Moved...

1. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +//
   

   Empty comment line? This should be just empty line.

2. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +  "use strict";
   

   Missing empty like after "use strict" and Drupal.behaviors.sharemessage should have doc comment.

3. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +    attach: function (context, addthis) {
   

   What is this addthis doing here?

4. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +      if ($(context).find('.addthis_toolbox') && window.addthis !== undefined) {
   +        var interval = setInterval(function(){
   

   Hmm maybe I am missing something obvious here but as I see it we should do two separate if here.

   First if should check if there is some element in context which has addthis_toolbox class. If there are no such elements we don't have nothing to do. But if there is such a element we then continue to second if.

   Second if should then check if window.addthis exist, if it exist then we can call toolbox for that element. If it does not exist then we can use setInterval approach and wait that addthis global is initialized.

   Also current if is not OK for a case when addthis_toolbox elements exist but addthis is undefined - basically you will never enter inside and get to the setInterval code.

5. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +        var interval = setInterval(function(){
   

   Space is missing after function().

6. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +          $(context).find('.addthis_toolbox').each(function () {
   

   Here again we are doing $(context).find('.addthis_toolbox') like in outer if, not optimal.

7. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +            window.addthis.toolbox('.addthis_toolbox');
   

   No need to write window.addthis.toolbox, global vars you can always access directly in any scope, so addthis.toolbox is enough.

8. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +          clearInterval(interval);
   

   Not sure currently where this call should be but we should call clearInterval(interval) only when we detect that addthis is defined.

9. +++ b/js/sharemessage.js
   @@ -0,0 +1,25 @@
   +})(jQuery);
   

   Because we are using Drupal global var inside you should add it here also in call and before in function definition, something like

   (function ($, Drupal) {
   // ....
   })(jQuery, Drupal);

+++ b/js/sharemessage.js
@@ -0,0 +1,25 @@
+            window.addthis.toolbox('.addthis_toolbox');

Can't we just say here

addthis.toolbox(this);

When you use class selector I guess addthis will first select all elements on page, and we should limit this to current context, and then try to init toolbox on them.

Check API doc on http://support.addthis.com/customer/portal/articles/1365325-rendering-to... which says that for target(s) we can also use HTML element references.

Made the changes for the comments above. I am only not sure about how to change the $(context).find('.addthis_toolbox') for comment #6. Any suggestion?

No point 4. i was thinking something else. OK maybe it will be faster to create a patch ;)

Didn't test this at all just rewrite it how i guess it should work.

@sasanikolic can you do full test of this and check that everything works properly?

Now it looks pretty solid. ;-)

Tested with IE, Firefox, Chrome and Safari. Works fine. I think we're good to go with this finally. :)