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 comment:1
Changed April 20, 2011 06:57PM UTC by comment:2
component: | unfiled → build |
---|---|
milestone: | 1.next → 1.6 |
owner: | → john |
priority: | undecided → blocker |
status: | new → assigned |
version: | git → 1.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 comment:3
Confirmed.
Additionally, throws an uncaught type error exception, see the test case:
Changed April 20, 2011 07:08PM UTC by 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 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 comment:6
resolution: | → patchwelcome |
---|---|
status: | assigned → closed |
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 comment:7
saw this from twitter. Brilliant
Changed April 21, 2011 06:01PM UTC by 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 comment:9
resolution: | patchwelcome |
---|---|
status: | closed → reopened |
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 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 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 comment:12
component: | build → unfiled |
---|
Changed April 25, 2011 03:52PM UTC by comment:13
resolution: | → invalid |
---|---|
status: | reopened → closed |
Ok, we're closing bugs for 1.6, to make sure we have no blockers. We can have more fun later ;)
I can't reproduce, but I wasn't at the conference.