Updating for external JS call [#705696]

Alan,

Thanks for starting this module out - really helpful!

I poked around through it and made a few updates: most importantly to include the external JS file, allowing for both http and https protocols and inserting in the 'right' place in the header (modeled after google_analytics module code). Also went through the admin include file and fixed spelling/capitalization of Typekit to make it more in line with their own usage. I've attached the whole module as I don't have permission to commit changes on the server. That said, I'd be happy to become a co-maintainer with you on it.

Cheers,

Jason

Comment	File	Size	Author
#2	externalJSupdate.patch (updates typekit.module)	1.03 KB	jpamental
#2	spellingCleanup.patch (updates includes/typekit.admin.inc)	1.73 KB	jpamental
	typekit.zip	12.4 KB	jpamental

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

zzolo CreditAttribution: zzolo commented 5 February 2010 at 15:03

Hey jpamental.

Thanks for doing this. Do you mind actually making a patch?
http://drupal.org/patch/create

Comment #2

jpamental CreditAttribution: jpamental commented 5 February 2010 at 16:03

File	Size
spellingCleanup.patch (updates includes/typekit.admin.inc)	1.73 KB
externalJSupdate.patch (updates typekit.module)	1.03 KB

Sure! Thanks for the link - never created a patch before.

Here are both. Would love to see this move out of alpha and help promote the module.

Cheers,

Jason

Comment #3

zzolo CreditAttribution: zzolo commented 7 February 2010 at 20:52

Hey @jpamental,

I am curious what would be the benefits of doing it this way? The one obvious thing is that it checks for https, but we can do that on the PHP side which would be more efficient. It seems like having JS includes it's own file could be not ideal, but I don't actually know a problem. You say it's the "right", but I am curious what that is. Thoughts? I don't think you are wrong, I just don't see the benefit.

--
zzolo

Comment #4

jpamental CreditAttribution: jpamental commented 8 February 2010 at 16:11

Hey @zzolo-

Primarily I think the important part of that patch is to get the JS call to the external script in the more appropriate place and use the Drupal function for adding JS rather than sticking it at the top of the stack. One advantage then could be that if the user does not have JS enabled, it won't even call the file. (so my 'rightness' test is really based on where/how the JS is brought into the header)

As for HTTP/HTTPS - that could certainly go on either end (JS or PHP). I just grabbed the whole code snippet from the GA module.

All in all, just wanted to help make the module as clean and in line with Drupal module conventions as possible. It'd be great to see it updated and listed as a 'stable' module release, and then work to get some exposure for it within the Typekit community.

Cheers,

Jason

Comment #5

zzolo CreditAttribution: zzolo commented 8 February 2010 at 22:01

@jpamental

This actually makes sense. This way, it allows the JS to go in the footer (or at the end of the HTML which is preferred by most).

On the topic of creating a stable release. I would be happy to release a 1.0 if the module is considered stable. Basically, I just wanted to only release an alpha because I was not sure where this module needed to go. Honestly, I don't even really use it except on a couple personal sites. But it seems stable and I don't really think there are any serious features it needs.

I am happy to have a co-maintainer, but you do need a CVS account and I probably need a little more evidence that you know how to develop in the Drupal contrib space. If you want, you can start another issue about you becoming a co-maintainer and we can try to work together to make it happen and I can try to point some resources at you to get going.

--
zzolo

Comment #6

jpamental CreditAttribution: jpamental commented 8 February 2010 at 23:03

@zzolo-

Thanks - I'll do that (separate issue).

I've been working with Drupal for a couple years now and want to start being more involved in the community. You can get a little more info on my background here: http://thinkinginpencil.com (if you're interested). Happy to do whatever is required!

Cheers,

Jason