rvera / image-picker

Image Picker is a simple jQuery plugin that transforms a select element into a more user friendly graphical interface.
http://rvera.github.com/image-picker
Other
906 stars 216 forks source link

Modified to provide scope for jQuery instance. #33

Closed ctwardell closed 10 years ago

ctwardell commented 10 years ago

Modified to provide scope for jQuery instance per http://learn.jquery.com/plugins/basic-plugin-creation/#protecting-the-alias-and-adding-scope

This is required when used on pages with multiple versions of jQuery.

rvera commented 10 years ago

Can you explain the benefits of this PR? it seems to me it does exactly the same it does now.

ctwardell commented 10 years ago

Rodrigo,

Good to hear from you, thanks for sharing this really nice plug-in with the jQuery community.

As far as your question, here is the long answer:

It allows you to use image-picker.js on a page that has more than one version of jQuery loaded.

It looks at first like this is just a change to using $, but it is really about scope.

The current image-picker.js uses the jQuery global variable to access jQuery. The modified version creates a locally scoped variable $ that is assigned the value of the jQuery variable at the time image-picker.js is loaded.

When a page only has one version of jQuery loaded there is no difference, but when a page has more than one version of jQuery there is a big difference.

The issue with using the global jQuery reference instead of a locally scoped reference is that image-picker.js can only be used when the version of jQuery that is using the jQuery global variable is compatible with image-picker.js.

So…an example:

I want to use image-picker.js on a Volusion store. Unfortunately Volusion depends on jQuery 1.4.2 and modifies user templates to inject that specific version. Image-picker.js does not work with 1.4.2, so an alternate solution must be found.

The solution is to add a second version of jQuery to the page that is compatible with image-picker.js. Since the Volusion page depends on jQuery 1.4.2 and expects it to use the $ and jQuery global variables, we need to use a difference variable for this new version.

JQuery provides the .noConflict() function to support this.

Here is what happens:

1 - Page loads jQuery 1.4.2, $ and jQuery reference 1.4.2 2 - Page loads some plug-ins that are compatible with 1.4.2 3 - Page loads jQuery 1.6.4, $ and jQuery reference 1.6.4, the previous references are backed up internal to the 1.6.4 copy 4 - Page loads some plug-ins that are compatible with 1.6.4 (like image-picker.js) 5 - Problem! Other existing script on the remainder of the page expects $ and jQuery to reference 1.4.2… 6 - Solution! var jq164 = $; // jq164 is now how we access 1.6.4 $.noConflict(true); // $ and jQuery now reference 1.4.2 again.

The issue with the current version of image-picker.js is that since it uses the global jQuery variable it will now try to use 1.4.2 instead of 1.6.4 that it was loaded under.

With the changes I suggested it has its own locally scoped reference to 1.6.4. In order to add the image picker to a select we need to use jq164(“someSelector”).imagepicker();

By making the changes I can use image-picker.js on the Volusion site.

Here is some code from the site

// begin script injected by Volusion, out of my control

// end script injected by Volusion, out of my control

// begin script later in the file that I have control over

// end script later in the file that I have control over

// begin script later in the file that I have control over // here is where I setup the image-picker $(function() { jq164("#customSelectColor").imagepicker({hide_select: false}); }); // end script later in the file that I have control over

Here is some reference info: https://api.jquery.com/jQuery.noConflict/ http://blog.nemikor.com/2009/10/03/using-multiple-versions-of-jquery/

Thanks,

Chris Wardell

From: Rodrigo Vera [mailto:notifications@github.com] Sent: Tuesday, April 15, 2014 12:42 PM To: rvera/image-picker Cc: Chris Wardell Subject: Re: [image-picker] Modified to provide scope for jQuery instance. (#33)

Can you explain the benefits of this PR? it seems to me it does exactly the same it does now.

— Reply to this email directly or view it on GitHubhttps://github.com/rvera/image-picker/pull/33#issuecomment-40504705.

rvera commented 10 years ago

Thanks for the response.

I get that part but assigning the scope to "the current value of jQuery when it was loaded" seems like bad design to me, of you were passing jQuery as an argument and/or replace it on the fly that seems like a much better alternative.

Still, why would you have multiple versions of jQuery on your page at the same time?

ctwardell commented 10 years ago

Rodrigo,

“I get that part but assigning the scope to "the current value of jQuery when it was loaded" seems like bad design to me, of you were passing jQuery as an argument and/or replace it on the fly that seems like a much better alternative.”

It is the pattern suggested by jQuery :

http://learn.jquery.com/plugins/basic-plugin-creation/#protecting-the-alias-and-adding-scope

You could make the plug-in be a named function instead of anonymous, that would let you explicitly pass a reference when you instantiate the plug-in, but people don’t expect to instantiate plug-ins.

“Still, why would you have multiple versions of jQuery on your page at the same time?” Normally I would avoid it, but as I mentioned in the previous reply the system I am working with injects script to load jQuery 1.4.2 and there is no way to change that. It is one of those cases of working within a fairly limited system and needing to do whatever it takes to add the features the customer is requesting.

http://blog.nemikor.com/2009/10/03/using-multiple-versions-of-jquery/

I guess ultimately it would come down to:

Thanks,

Chris Wardell

From: Rodrigo Vera [mailto:notifications@github.com] Sent: Tuesday, April 15, 2014 2:30 PM To: rvera/image-picker Cc: Chris Wardell Subject: Re: [image-picker] Modified to provide scope for jQuery instance. (#33)

Thanks for the response.

I get that part but assigning the scope to "the current value of jQuery when it was loaded" seems like bad design to me, of you were passing jQuery as an argument and/or replace it on the fly that seems like a much better alternative.

Still, why would you have multiple versions of jQuery on your page at the same time?

— Reply to this email directly or view it on GitHubhttps://github.com/rvera/image-picker/pull/33#issuecomment-40516949.

rvera commented 10 years ago

Welp, if it's the suggested way of doing it I guess I'll modify the library.

The PR you created is for the JS file, the source is actually in coffeescript.

ctwardell commented 10 years ago

Rodrigo,

Sorry about that, I just jumped right into the JS.

C.W.

From: Rodrigo Vera [mailto:notifications@github.com] Sent: Tuesday, April 15, 2014 7:42 PM To: rvera/image-picker Cc: Chris Wardell Subject: Re: [image-picker] Modified to provide scope for jQuery instance. (#33)

Welp, if it's the suggested way of doing it I guess I'll modify the library.

The PR you created is for the JS file, the source is actually in coffeescript.

— Reply to this email directly or view it on GitHubhttps://github.com/rvera/image-picker/pull/33#issuecomment-40547795.

rvera commented 10 years ago

I'm closing now, will work on migrating this change into coffee code since the original PR was never updated for it,