JS bug

The whole JS file is wrapped in the new conflict-avoiding function thingy:

(function($) {
...
})(jQuery)

But the jQuery window popupWindow plugin is wrapped like that too...which I think is superfluous, and in fact causing a bug where when I click the share link I get a whole window as opposed to the nice popup.

Patch on the way...

Comment	File	Size	Author
#1	simpleshare_js_1096398_1.patch	6.91 KB	stevetweeddale

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

stevetweeddale CreditAttribution: stevetweeddale commented 17 March 2011 at 17:59

File	Size
simpleshare_js_1096398_1.patch	6.91 KB

Patch made with git format-patch --no-prefix ...still working out the proper way of doing things in git land!

Comment #2

stevetweeddale CreditAttribution: stevetweeddale commented 17 March 2011 at 18:17

Status:

Active

» Needs review

Comment #3

Rob_Feature CreditAttribution: Rob_Feature commented 17 March 2011 at 19:14

Hmm...something wrong with the patch. Trying to see what you mean and make the change though...will roll into dev soon

Comment #4

Rob_Feature CreditAttribution: Rob_Feature commented 17 March 2011 at 19:31

Version:

6.x-1.x-dev

» 7.x-1.x-dev

A new patch against HEAD would be nifty if you see an issue (I was sorta guessing at what I was fixing since I don't know js well at all)

//Edit// Nope, that didn't do it. Would love a new patch.

Comment #5

stevetweeddale CreditAttribution: stevetweeddale commented 18 March 2011 at 10:51

Status:

Needs review

» Fixed

Yeah, sorry about patch weirdness - I'm still trying to get my head around the workflow. Theoretically, creating a patch with format-patch means that the maintainer can just apply it with git am, leaving the patch author details intact in the git history.

Looking at them, I don't think my IDE's white space fixing makes for easily readable patches though.

But anyway, the JS actually looks fine now. All that the code I quoted above does is allow you to use $ for jQuery anywhere between the {}, without potentially conflicting with other JS libraries that might give the $ special meaning. So you've wrapped your $(document).ready function and your popupWindow plugin in that function separately now, which is fine - all I was doing was wrapping the whole file once... either way is fine I believe. I think the problem before was nesting it.

Thanks!