Skip to content
This repository

Shorthand syntax #41

Closed
wants to merge 6 commits into from

3 participants

Steve Love Johan Ronsse Scott Jehl
Steve Love

The media query recommendation defines a shorthand syntax in Example V.

Essentially, the following are equivalent:
@media all and (min-width:400px) { ... }
@media (min-width:400px) { ... }

Using the shorthand syntax with Respond caused incorrect matches with the regex used for media type.

Tests:
I modified the last test in the visual test cases (test/test.css) to use shorthand syntax, and it is passing. I also added tests that expose a bug in the way Respond injects style blocks grouped by media type rather than by query expression. I'll file an issue for that one.

Steve Love stevelove commented on the diff July 09, 2011
test/test.css
@@ -37,7 +37,7 @@ a {
37 37
 
38 38
 
39 39
 /*styles for 620px and up! (let's try this one minified )*/
40  
-@media screen and (min-width: 620px),only print,projector{body{background:red;}}
  40
+@media screen and (min-width: 620px),only print,projection{body{background:red;}}
1
Steve Love
stevelove added a note July 09, 2011

"projector" is not a valid media type. Changed it to "projection" instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Steve Love stevelove commented on the diff July 10, 2011
respond.src.js
@@ -115,8 +115,10 @@
115 115
 					
116 116
 				for( ; j < eql; j++ ){
117 117
 					thisq	= eachq[ j ];
  118
+					type = thisq.match( /(only\s+)?((?:\()|([a-zA-Z]+))(?:\sand)?/ );
  119
+					type = (!type[3]) ? "all" : (type[1]) ? type[1] + type[2] : type[2];
1
Steve Love
stevelove added a note July 10, 2011

Example: @media (min-width: 900px)
Regex matches: ["(", undefined, "(", undefined]
type = "all"

Example: @media screen and (min-width: 900px)
Regex matches: ["screen and", undefined, "screen", "screen"]
type = "screen"

Example: @media only screen and (min-width: 900px)
Regex matches: ["only screen and", "only", "screen", "screen"]
type = "only screen"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Johan Ronsse
Wolfr commented July 27, 2011

+1
Just encountered this issue.

Scott Jehl scottjehl closed this August 14, 2011
Scott Jehl
Owner

Thanks, Steve!
I've moved this into a branch https://github.com/scottjehl/Respond/tree/shorthand-syntax

and added a unit test, updated the min file. I don't have IE on me right now, so I can't test if it's passing or not. Mind giving it a look?

Otherwise, looks good and I should be able to bring this in this week.
Thanks!!

Steve Love

Excellent. The tests are 100% passing in IE7 and IE8.

A couple of notes:

  • I tested IE by changing the browser mode in IE9, and ran into a type mismatch error when loading qunit.js from GitHub. I had to save a local copy first to get it to work. The error is discussed here: http://stackoverflow.com/questions/5986772/ie9-script-response-blocked-due-to-mime-type-mismatch

  • Looks like the Ajax factory was refactored recently, and the "new" keyword was removed from XMLHttpRequest(). This causes an error in Firefox 2 (haven't checked others). With "new" in there it works fine on visual inspection. The unit tests are only 25% passing in Firefox 2, but I wonder if that's a QUnit issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
6  respond.src.js
@@ -81,7 +81,7 @@
81 81
 				useMedia	= !ql && media,
82 82
 				//vars used in loop
83 83
 				i			= 0,
84  
-				j, fullq, thisq, eachq, eql;
  84
+				j, fullq, thisq, eachq, eql, type;
85 85
 
86 86
 			//if path exists, tack on trailing slash
87 87
 			if( href.length ){ href += "/"; }	
@@ -115,8 +115,10 @@
115 115
 					
116 116
 				for( ; j < eql; j++ ){
117 117
 					thisq	= eachq[ j ];
  118
+					type = thisq.match( /(only\s+)?((?:\()|([a-zA-Z]+))(?:\sand)?/ );
  119
+					type = (!type[3]) ? "all" : (type[1]) ? type[1] + type[2] : type[2];
118 120
 					mediastyles.push( { 
119  
-						media	: thisq.match( /(only\s+)?([a-zA-Z]+)(\sand)?/ ) && RegExp.$2,
  121
+						media	: type,
120 122
 						rules	: rules.length - 1,
121 123
 						minw	: thisq.match( /\(min\-width:[\s]*([\s]*[0-9]+)px[\s]*\)/ ) && parseFloat( RegExp.$1 ), 
122 124
 						maxw	: thisq.match( /\(max\-width:[\s]*([\s]*[0-9]+)px[\s]*\)/ ) && parseFloat( RegExp.$1 )
35  test/test.css
@@ -37,7 +37,7 @@ a {
37 37
 
38 38
 
39 39
 /*styles for 620px and up! (let's try this one minified )*/
40  
-@media screen and (min-width: 620px),only print,projector{body{background:red;}}
  40
+@media screen and (min-width: 620px),only print,projection{body{background:red;}}
41 41
 
42 42
 /*styles for 800px and up!*/
43 43
 @media screen and (min-width: 800px){
@@ -46,15 +46,44 @@ a {
46 46
 	}
47 47
 }
48 48
 
  49
+/*styles for 900px and up
  50
+ * Test for query order in conjuction with test that follows
  51
+*/
  52
+@media all and (min-width:900px){
  53
+    #attribute-test {
  54
+        font-size:1.5em;
  55
+    }
  56
+}
  57
+
49 58
 /*styles for 1100px and up!*/
50 59
 @media screen and (min-width: 1100px){
51 60
 	body {
52 61
 		background: orange;
53 62
 	}
  63
+
  64
+    /* Test for query order. Currently fails.
  65
+     * This should override the style for 900px but fails
  66
+     * because style blocks are grouped by media type
  67
+    */
  68
+    #attribute-test {
  69
+        font-size:1em;
  70
+    }
54 71
 }
55 72
 
56  
-/*styles for 1300px and up!*/
57  
-@media screen and (min-width: 1300px){
  73
+/* Test that invalid media type is ignored by browser
  74
+ * and not mistakenly set to "all" by Respond
  75
+*/
  76
+@media fake and (min-width: 1300px){
  77
+    body {
  78
+        color: red;
  79
+    }
  80
+}
  81
+
  82
+/*styles for 1300px and up!
  83
+ * Test for shorthand syntax
  84
+ * See Example V: http://www.w3.org/TR/css3-mediaqueries/#media0
  85
+*/
  86
+@media (min-width: 1300px){
58 87
 	body {
59 88
 		background: navy;
60 89
 	}
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.