tmort / Socialite

Other
1.68k stars 163 forks source link

Twitter conflict #22

Closed SBoudrias closed 10 years ago

SBoudrias commented 12 years ago

I had a case where different twitter widget caused trouble because one included a global window.twttr without the .ready() method used in Socialite.

The problem is on this line https://github.com/dbushell/Socialite/blob/master/socialite.js#L549

You test if window.twttr exist, and not if it contain the functions we need. To workaround this problem, I've been using jQuery extend method to make sure my object contain all the function I needed. Like so:

// Replace this
if (notwttr) {
    window.twttr = (t = { _e: [], ready: function(f) { t._e.push(f); } });
}

// By that
if (notwttr) {
    window.twttr = (t = { _e: [], ready: function(f) { t._e.push(f); } });
} else {
    window.twttr = (t = $.extend( { _e: [], ready: function(f) { t._e.push(f); } }, window.twttr ));
}

As socialite is dependencie free, I prefer not to commit any code as I'm not sure how you'd prefer to implement such solution.

Hope this help !

dbushell commented 12 years ago

Interesting case :)

I guess the ideal solution would be to extend Socialite to support additional Twitter widgets, obviously not a quick fix I appreciate.

In this instances you're losing one of the main benefits: late/asynchronous loading (I assume window.twttr exists because the other widget has loaded the Twitter script library already).

Do you mind sharing the code you're using to embed the other widget? I'm surprised it doesn't set up window.twttr in the same way.

It may be slightly quicker for you to do:

if (!notwttr && typeof window.twttr.ready !== 'function') {
    return false;
} else if (notwttr) {
    window.twttr = (t = { _e: [], ready: function(f) { t._e.push(f); } });
}

straight after notwttr is defined, the line before your change. This will effectively tell Socialite to activate the instances immediately. I believe that will work in your situation.

Anyway, I'll have to explore further how other widgets are embedded to understand how Twitter creates its object.

Thanks for the bug report, glad you could hack around it for now!

SBoudrias commented 12 years ago

Hi, the other twitter widget is async loaded too with Yepnope:

Modernizr.load([
            {
                load: "http://widgets.twimg.com/j/2/widget.js",
                complete: function() {
                    new TWTR.Widget({
                        version: 2,
                        type: 'search',
                        search: 'bbfoot2012',
                        interval: 30000,
                        title: 'Tournoi au profit du BEC',
                        subject: '#bbfoot2012',
                        width: 'auto',
                        height: 200,
                        id: 'twitter-widget',
                        theme: {
                            shell: {
                                  background: '#dd262f',
                                  color: '#ffffff'
                            },
                            tweets: {
                                  background: '#ffffff',
                                  color: '#444444',
                                  links: '#dd262f'
                             }
                        },
                        features: {
                            scrollbar: false,
                            loop: true,
                            live: true,
                            behavior: 'default'
                        }
                    }).render().start();
                }
            }
        ]);

Of course, ideally one loader for everything should be better, but I was a little rushed to ship a site.

dbushell commented 12 years ago

Thanks. Awesome job using Yesnope :) I was mistaken into thinking you were just loading the script normally.

You could have defined the window.twttr object (t = { _e: [], ready: function(f) { t._e.push(f); } }) before both Modernizr.load() and Socialite.load() and avoided the need to modify socialite entirely. Twitter defines it on the javascript events page. Not to matter now though, your solution is perfectly sound.

I will endeavour to extend Socialite further to support all of Twitters buttons and widgets.

Maybe I should allow other script loading to be done via Socialite too, or combine something like Yesnope with Socialite so a second script loader isn't needed.

SBoudrias commented 12 years ago

I think this would be a nice idea. I used Socialite.js to have fast and simple social media buttons set up.

But, in most of my projets, I'm using loader like yepnope.js on small project and require.js on more complex one. I don't know what's up with other user of socialite, but for me, it's redundant with other script loader.

Maybe you should offer simple functionality like hooks point to load plugins:

This could be pretty flexible and be used with any loader:

yepnope({
    load: socialite('twitter-share').src,
    complete: function() {
        socialite('twitter-share').init({
            configOption: '1'
        })
    }
});

(I'm making simple example here, but something more AMD friendly could be pretty neat too - although maybe too much for the scope of this kind of project)

And you offer as a dropin your own loader API to be use like Socialite work right now.