Skip to main content

Bug Tracker

Side navigation

#8926 closed bug (invalid)

Opened April 20, 2011 06:52PM UTC

Closed April 25, 2011 03:52PM UTC

Last modified March 09, 2012 02:20PM UTC

Bay Area jQuery Conference T-Shirts do not follow the style guide.

Reported by: ralphholzmann Owned by: john
Priority: blocker Milestone: 1.6
Component: unfiled Version: 1.6b1
Keywords: Cc:
Blocked by: Blocking:
Description

Hello,

I attended the Bay Area jQuery Conference last weekend and was delighted to receive a stylish and high quality jQuery t-shirt. The front of the t-shirt says $("body").wrap(this);, while the back has the jQuery logo close to the top.

Upon further reflection, it dawned on me that the t-shirt does not follow the jQuery Core Style Guidelines. Per http://docs.jquery.com/JQuery_Core_Style_Guidelines#Function_Calls, it states "Always include extra spaces around the arguments." Considering that both the $ and wrap are function calls, the t-shirt should read $( "body" ).wrap( this );

This bug is a blocker for all future jQuery Conferences and I expect a patch to go out within the next couple of days. A sewable patch is preferred.

Thank you,

Ralph Holzmann

Attachments (0)
Change History (13)

Changed April 20, 2011 06:54PM UTC by miketaylr comment:1

I can't reproduce, but I wasn't at the conference.

Changed April 20, 2011 06:57PM UTC by timmywil comment:2

component: unfiledbuild
milestone: 1.next1.6
owner: → john
priority: undecidedblocker
status: newassigned
version: git1.6b1

Actually, spaces are not required when double quotes are present (since they provide a visible space by themselves), so it would be

$("body").wrap( this );

Changed April 20, 2011 07:04PM UTC by rwaldron comment:3

Confirmed.

Additionally, throws an uncaught type error exception, see the test case:

http://jsfiddle.net/rwaldron/RSdAg/

Changed April 20, 2011 07:08PM UTC by dmethvin comment:4

In the meantime, here is a workaround: Wear a size "Small" t-shirt and it will stretch to add some more space between the characters.

Changed April 20, 2011 07:20PM UTC by Nikola comment:5

Replying to [comment:4 dmethvin]:

In the meantime, here is a workaround: Wear a size "Small" t-shirt and it will stretch to add some more space between the characters.

Confirmed, this works. It may not be pretty but it is functional.

Changed April 20, 2011 09:03PM UTC by john comment:6

resolution: → patchwelcome
status: assignedclosed

Unfortunately the release is already out the door so it's a bit late. Let's bring this up during the next t-shirt roadmap discussion. In the meantime, patches welcome.

Changed April 21, 2011 02:50AM UTC by anonymous comment:7

saw this from twitter. Brilliant

Changed April 21, 2011 06:01PM UTC by dougneiner comment:8

The code did follow the style guide, but it was minified for production. We also recommend storing the shirts in a gzip-lock bag to keep the size small.

Additionally, after evaluating where this code would normally run, we realized we didn't need it to run in a closure. Unfortunately this means it is not compatible with jQuery running in .noConflict mode as $ is expected to equal jQuery. We do not recommend layering this shirt on top of a MooTools or Dojo shirt if possible. If you want to combine them, you will probably want to put the jQuery conference shirt on first.

Changed April 21, 2011 08:03PM UTC by cowboy comment:9

resolution: patchwelcome
status: closedreopened

I'm going to release this revised t-shirt first as a plugin, and if you guys want me to add it into core, I have no problem with that. Just bring it up at the next roadmap meeting.

[[Image(http://benalman.com/grab/5c2e39.png)]]

Changed April 21, 2011 09:05PM UTC by rkatic comment:10

No need to wait DOM ready - it is obviously a one page application, where tshirt was dynamically injected.

Changed April 22, 2011 03:35AM UTC by jswartwood@gmail.com comment:11

@rkatic, likely that guy in @cowboy's plugin snapshot is named Dominic... I would also recommend waiting for Dom ready before trying wrap. Otherwise he would not allow it I.e. "operation aborted".

Changed April 23, 2011 05:53PM UTC by john comment:12

component: buildunfiled

Changed April 25, 2011 03:52PM UTC by john comment:13

resolution: → invalid
status: reopenedclosed

Ok, we're closing bugs for 1.6, to make sure we have no blockers. We can have more fun later ;)