From d4e1eba7a792160bfb5853b2b7fc790a5c860b52 Mon Sep 17 00:00:00 2001 From: Lewis Nyman Date: Fri, 1 Aug 2014 13:06:12 +0200 Subject: [PATCH 1/6] Added basic rule info --- src/rules/ltr-properties.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 src/rules/ltr-properties.js diff --git a/src/rules/ltr-properties.js b/src/rules/ltr-properties.js new file mode 100644 index 00000000..695acf1e --- /dev/null +++ b/src/rules/ltr-properties.js @@ -0,0 +1,18 @@ +/* + * Rule: Detects properties for LTR text styling the require equivilent RTL styling. + */ +/*global CSSLint*/ +CSSLint.addRule({ + //rule information + id: "ltr-properties", + name: "LTR properties", + desc: "Detects properties for LTR text styling the require equivilent RTL styling.", + browsers: "All", + + //initialization + init: function(parser, reporter){ + var rule = this; + + //rule initialization + } +}); From bd2ae06412e6fd32f965c430f0bebfea47a3cb5a Mon Sep 17 00:00:00 2001 From: Lewis Nyman Date: Fri, 1 Aug 2014 13:15:42 +0200 Subject: [PATCH 2/6] Added list of LTR rules --- src/rules/ltr-properties.js | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/rules/ltr-properties.js b/src/rules/ltr-properties.js index 695acf1e..e38d904c 100644 --- a/src/rules/ltr-properties.js +++ b/src/rules/ltr-properties.js @@ -11,8 +11,30 @@ CSSLint.addRule({ //initialization init: function(parser, reporter){ - var rule = this; - - //rule initialization + "use strict"; + var rule = this, + properties = { + "border": 1, + "border-left": 1, + "border-right": 1, + "border-radius": 1, + "border-top-right-radius": 1, + "border-top-left-radius": 1, + "border-bottom-right-radius": 1, + "border-bottom-left-radius": 1, + padding: 1, + "padding-left": 1, + "padding-right": 1, + margin: 1, + "margin-left": 1, + "margin-right": 1, + "text-align": 1, + float: 1, + clear: 1, + background: 1, + "background-position": 1, + left: 1, + right: 1 + }; } }); From a7ffef7d067a9ef8a1c056af9ef756b4d6445e14 Mon Sep 17 00:00:00 2001 From: Lewis Nyman Date: Fri, 1 Aug 2014 13:36:11 +0200 Subject: [PATCH 3/6] A rule that passes code standards --- src/rules/ltr-properties.js | 59 +++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/src/rules/ltr-properties.js b/src/rules/ltr-properties.js index e38d904c..0464e0cd 100644 --- a/src/rules/ltr-properties.js +++ b/src/rules/ltr-properties.js @@ -1,40 +1,47 @@ /* - * Rule: Detects properties for LTR text styling the require equivilent RTL styling. + * Rule: Detects properties for LTR text styling the require equivalent RTL styling. */ /*global CSSLint*/ CSSLint.addRule({ + //rule information id: "ltr-properties", name: "LTR properties", - desc: "Detects properties for LTR text styling the require equivilent RTL styling.", + desc: "Detects properties for LTR text styling the require equivalent RTL styling.", browsers: "All", //initialization init: function(parser, reporter){ "use strict"; - var rule = this, - properties = { - "border": 1, - "border-left": 1, - "border-right": 1, - "border-radius": 1, - "border-top-right-radius": 1, - "border-top-left-radius": 1, - "border-bottom-right-radius": 1, - "border-bottom-left-radius": 1, - padding: 1, - "padding-left": 1, - "padding-right": 1, - margin: 1, - "margin-left": 1, - "margin-right": 1, - "text-align": 1, - float: 1, - clear: 1, - background: 1, - "background-position": 1, - left: 1, - right: 1 - }; + var properties = { + "border": 1, + "border-left": 1, + "border-right": 1, + "border-radius": 1, + "border-top-right-radius": 1, + "border-top-left-radius": 1, + "border-bottom-right-radius": 1, + "border-bottom-left-radius": 1, + padding: 1, + "padding-left": 1, + "padding-right": 1, + margin: 1, + "margin-left": 1, + "margin-right": 1, + "text-align": 1, + float: 1, + clear: 1, + background: 1, + "background-position": 1, + left: 1, + right: 1 + }; + + parser.addListener("property", function(event){ + var name = event.property.toString().toLowerCase(); + if (properties[name]) { + reporter.report("Using " + name + "can require an equivalent rule for RTL styling."); + } + }); } }); From 7423564feda27cb1508eeb17c41741200737b559 Mon Sep 17 00:00:00 2001 From: Lewis Nyman Date: Fri, 1 Aug 2014 14:01:05 +0200 Subject: [PATCH 4/6] Almost passing the tests, still need to check potential properties --- src/rules/ltr-properties.js | 36 ++++++++++++++++++++++++++++------- tests/rules/ltr-properties.js | 22 +++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 tests/rules/ltr-properties.js diff --git a/src/rules/ltr-properties.js b/src/rules/ltr-properties.js index 0464e0cd..2eb0d9d4 100644 --- a/src/rules/ltr-properties.js +++ b/src/rules/ltr-properties.js @@ -1,7 +1,6 @@ /* * Rule: Detects properties for LTR text styling the require equivalent RTL styling. */ -/*global CSSLint*/ CSSLint.addRule({ //rule information @@ -14,7 +13,6 @@ CSSLint.addRule({ init: function(parser, reporter){ "use strict"; var properties = { - "border": 1, "border-left": 1, "border-right": 1, "border-radius": 1, @@ -22,25 +20,49 @@ CSSLint.addRule({ "border-top-left-radius": 1, "border-bottom-right-radius": 1, "border-bottom-left-radius": 1, - padding: 1, "padding-left": 1, "padding-right": 1, - margin: 1, "margin-left": 1, "margin-right": 1, "text-align": 1, - float: 1, clear: 1, background: 1, "background-position": 1, left: 1, right: 1 + }, + potentialProperties = { + padding: 1, + margin: 1, + border: 1, + float: 1, }; parser.addListener("property", function(event){ var name = event.property.toString().toLowerCase(); - if (properties[name]) { - reporter.report("Using " + name + "can require an equivalent rule for RTL styling."); + if (properties[name]){ + reporter.warning("Using " + name + " can require an equivalent rule for RTL styling."); + } + else if (potentialProperties[name]){ + // Check to see if they are actually affecting LTR. + switch(name){ + case "padding": + //do something + break; + + case "margin": + //do something + break; + + case "border": + //do something + break; + + case "float": + //do something + break; + + } } }); } diff --git a/tests/rules/ltr-properties.js b/tests/rules/ltr-properties.js new file mode 100644 index 00000000..2f4fb001 --- /dev/null +++ b/tests/rules/ltr-properties.js @@ -0,0 +1,22 @@ +(function(){ + "use strict"; + var Assert = YUITest.Assert; + + YUITest.TestRunner.add(new YUITest.TestCase({ + + name: "LTR property detection", + + "Using padding-right should result in a warning ": function(){ + var result = CSSLint.verify(".foo { padding-right: 10px; }", { "ltr-properties": 1 }); + Assert.areEqual(1, result.messages.length); + Assert.areEqual("warning", result.messages[0].type); + Assert.areEqual("Using padding-right can require an equivalent rule for RTL styling.", result.messages[0].message); + }, + + "Using font-size should result no warning ": function(){ + var result = CSSLint.verify(".foo { font-size: 10px; }", { "ltr-properties": 1 }); + Assert.areEqual(0, result.messages.length); + }, + })); + +})(); From c3c63e7087e5fb4e9065947fb3d37229013f6af5 Mon Sep 17 00:00:00 2001 From: Lewis Nyman Date: Fri, 1 Aug 2014 20:15:01 +0200 Subject: [PATCH 5/6] Passing tests! --- src/rules/ltr-properties.js | 10 ++++++++-- tests/rules/ltr-properties.js | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/rules/ltr-properties.js b/src/rules/ltr-properties.js index 2eb0d9d4..9297cad7 100644 --- a/src/rules/ltr-properties.js +++ b/src/rules/ltr-properties.js @@ -39,13 +39,19 @@ CSSLint.addRule({ }; parser.addListener("property", function(event){ - var name = event.property.toString().toLowerCase(); + var rule = this, + name = event.property.toString().toLowerCase(), + // value = event.value.toString(), + line = event.line, + col = event.col; + if (properties[name]){ - reporter.warning("Using " + name + " can require an equivalent rule for RTL styling."); + reporter.report("Using " + name + " could require an equivalent rule for RTL styling.", line, col, rule); } else if (potentialProperties[name]){ // Check to see if they are actually affecting LTR. switch(name){ + case "padding": //do something break; diff --git a/tests/rules/ltr-properties.js b/tests/rules/ltr-properties.js index 2f4fb001..edc5e3c2 100644 --- a/tests/rules/ltr-properties.js +++ b/tests/rules/ltr-properties.js @@ -10,7 +10,7 @@ var result = CSSLint.verify(".foo { padding-right: 10px; }", { "ltr-properties": 1 }); Assert.areEqual(1, result.messages.length); Assert.areEqual("warning", result.messages[0].type); - Assert.areEqual("Using padding-right can require an equivalent rule for RTL styling.", result.messages[0].message); + Assert.areEqual("Using padding-right could require an equivalent rule for RTL styling.", result.messages[0].message); }, "Using font-size should result no warning ": function(){ From e22b218698cd0c0c12b2c01f995ed0f3fd01b473 Mon Sep 17 00:00:00 2001 From: Lewis Nyman Date: Sun, 3 Aug 2014 17:44:49 +0200 Subject: [PATCH 6/6] Added checks for properties that could be LTR specific with certain values --- src/rules/ltr-properties.js | 46 +++++++++++++++++------------------ tests/rules/ltr-properties.js | 38 +++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/src/rules/ltr-properties.js b/src/rules/ltr-properties.js index 9297cad7..5dd16b97 100644 --- a/src/rules/ltr-properties.js +++ b/src/rules/ltr-properties.js @@ -15,7 +15,6 @@ CSSLint.addRule({ var properties = { "border-left": 1, "border-right": 1, - "border-radius": 1, "border-top-right-radius": 1, "border-top-left-radius": 1, "border-bottom-right-radius": 1, @@ -25,49 +24,48 @@ CSSLint.addRule({ "margin-left": 1, "margin-right": 1, "text-align": 1, - clear: 1, background: 1, "background-position": 1, left: 1, right: 1 }, potentialProperties = { + float: { + left: 1, + right: 1, + }, + clear: { + left: 1, + right: 1, + }, + }, + potentialMultipleValueProperties = { padding: 1, margin: 1, border: 1, - float: 1, + "border-radius": 1, }; parser.addListener("property", function(event){ var rule = this, name = event.property.toString().toLowerCase(), - // value = event.value.toString(), + value = event.value.toString(), line = event.line, col = event.col; + // These properties always affect RTL. if (properties[name]){ reporter.report("Using " + name + " could require an equivalent rule for RTL styling.", line, col, rule); } - else if (potentialProperties[name]){ - // Check to see if they are actually affecting LTR. - switch(name){ - - case "padding": - //do something - break; - - case "margin": - //do something - break; - - case "border": - //do something - break; - - case "float": - //do something - break; - + // These properties could affect RTL, if they have four values. + else if (potentialMultipleValueProperties[name] && event.value.parts.length === 4){ + reporter.report("Using " + name + " in this way could require an equivalent rule for RTL styling.", line, col, rule); + } + // These properties could affect RTL, if they have certain values. + else if(potentialProperties[name]) { + var property = potentialProperties[name]; + if(property[value]) { + reporter.report("Using " + name + " in this way could require an equivalent rule for RTL styling.", line, col, rule); } } }); diff --git a/tests/rules/ltr-properties.js b/tests/rules/ltr-properties.js index edc5e3c2..3661130d 100644 --- a/tests/rules/ltr-properties.js +++ b/tests/rules/ltr-properties.js @@ -6,17 +6,51 @@ name: "LTR property detection", - "Using padding-right should result in a warning ": function(){ + "Using padding-right should result in a warning": function(){ var result = CSSLint.verify(".foo { padding-right: 10px; }", { "ltr-properties": 1 }); Assert.areEqual(1, result.messages.length); Assert.areEqual("warning", result.messages[0].type); Assert.areEqual("Using padding-right could require an equivalent rule for RTL styling.", result.messages[0].message); }, - "Using font-size should result no warning ": function(){ + "Using padding: 0 1px 0 2px should result in a warning": function(){ + var result = CSSLint.verify(".foo { padding: 0 1px 0 2px; }", { "ltr-properties": 1 }); + Assert.areEqual(1, result.messages.length); + Assert.areEqual("warning", result.messages[0].type); + Assert.areEqual("Using padding in this way could require an equivalent rule for RTL styling.", result.messages[0].message); + }, + + "Using float: left should result in a warning": function(){ + var result = CSSLint.verify(".foo { float: left; }", { "ltr-properties": 1 }); + Assert.areEqual(1, result.messages.length); + Assert.areEqual("warning", result.messages[0].type); + Assert.areEqual("Using float in this way could require an equivalent rule for RTL styling.", result.messages[0].message); + }, + + "Using font-size should result no warning": function(){ var result = CSSLint.verify(".foo { font-size: 10px; }", { "ltr-properties": 1 }); Assert.areEqual(0, result.messages.length); }, + + "Using padding: 10px should result no warning": function(){ + var result = CSSLint.verify(".foo { padding: 0; }", { "ltr-properties": 1 }); + Assert.areEqual(0, result.messages.length); + }, + + "Using border-radius: 2px should result no warning": function(){ + var result = CSSLint.verify(".foo { padding: 0; }", { "ltr-properties": 1 }); + Assert.areEqual(0, result.messages.length); + }, + + "Using margin: 0 1px should result no warning": function(){ + var result = CSSLint.verify(".foo { margin: 0 1px; }", { "ltr-properties": 1 }); + Assert.areEqual(0, result.messages.length); + }, + + "Using clear: none should result no warning": function(){ + var result = CSSLint.verify(".foo { float: none; }", { "ltr-properties": 1 }); + Assert.areEqual(0, result.messages.length); + }, })); })();