Commit 96ceea3a authored by John Lee's avatar John Lee Committed by Commit Bot

WebUI: Check for CSS mixin usage during presubmit

Bug: 973674
Change-Id: I124c385f33ebbe265392a2bfb4812475e3387c75
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2336014
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: default avatardpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795608}
parent fffb67dc
......@@ -82,10 +82,9 @@ class CSSChecker(object):
mixin_shim_reg = r'[\w-]+_-_[\w-]+'
def _remove_mixins_and_valid_vars(s):
valid_vars = r'--(?!' + mixin_shim_reg + r')[\w-]+:\s*'
mixin_or_value = r'({.*?}|[^;}]+);?\s*'
return re.sub(valid_vars + mixin_or_value, '', s, flags=re.DOTALL)
def _remove_valid_vars(s):
valid_vars = r'--(?!' + mixin_shim_reg + r')[\w-]+:\s*([^;{}}]+);\s*'
return re.sub(valid_vars, '', s, flags=re.DOTALL)
def _remove_disable(content, lstrip=False):
prefix_reg = ('\s*' if lstrip else '')
......@@ -383,6 +382,10 @@ class CSSChecker(object):
errors.append(' ' + first_line)
return errors
def mixins(line):
return re.search(r'--[\w-]+:\s*({.*?)', line) or re.search(
r'@apply', line)
# NOTE: Currently multi-line checks don't support 'after'. Instead, add
# suggestions while parsing the file so another pass isn't necessary.
added_or_modified_files_checks = [
......@@ -460,6 +463,10 @@ class CSSChecker(object):
'test': zero_width_lengths,
'multiline': True,
},
{ 'desc': 'Avoid using CSS mixins. Use CSS shadow parts, CSS ' \
'variables, or common CSS classes instead.',
'test': mixins,
},
]
results = []
......@@ -490,7 +497,7 @@ class CSSChecker(object):
if not is_html:
file_contents = _remove_comments_except_for_disables(file_contents)
file_contents = _remove_mixins_and_valid_vars(file_contents)
file_contents = _remove_valid_vars(file_contents)
file_contents = _remove_template_expressions(file_contents)
files.append((path, file_contents))
......
......@@ -94,7 +94,6 @@ class CssCheckerTest(unittest.TestCase):
body.alternate-logo #logo {
-webkit-mask-image: url(images/google_logo.png@2x);
background: none;
@apply(--some-variable);
}
div {
......@@ -133,13 +132,6 @@ div {
self.VerifyContentIsValid("""
#id {
--zzyxx-xylophone: 3px;
--ignore-me: {
/* TODO(dbeam): fix this by creating a "sort context". If we simply strip
* off the mixin, the inside contents will be compared to the outside
* contents, which isn't what we want. */
visibility: hidden;
color: black;
};
--aardvark-animal: var(--zzyxz-xylophone);
}
""")
......@@ -157,9 +149,6 @@ blah /* hey! */
.mixed-in {
display: none;
--css-mixin: {
color: red;
}; /* This should be ignored. */
}
.this.is { /* allowed */
......@@ -169,6 +158,18 @@ blah /* hey! */
div{
{""")
def testMixins(self):
self.VerifyContentsProducesOutput(
"""
.mixed-in {
--css-mixin: {
color: red;
}
}""", """
- Avoid using CSS mixins. Use CSS shadow parts, CSS variables, or common CSS \
classes instead.
--css-mixin: {""")
def testCssClassesUseDashes(self):
self.VerifyContentsProducesOutput("""
.className,
......@@ -193,15 +194,6 @@ blah /* hey! */
#id { /* $i18n{*} and $i18nRaw{*} should be ignored. */
rule: $i18n{someValue};
rule2: $i18nRaw{someValue};
--css-mixin: {
color: red;
};
}
.paper-wrapper {
--paper-thinger: {
background: blue;
};
}
#rule {
......@@ -310,12 +302,6 @@ div {
rule: value; /* rule: value; */
rule: value; rule: value;
}
.remix {
--dj: {
spin: that;
};
}
""", """
- One rule per line (what not to do: color: red; margin: 0;).
rule: value; rule: value;""")
......@@ -358,7 +344,8 @@ b:before,
""")
def testCssRgbIfNotGray(self):
self.VerifyContentsProducesOutput("""
self.VerifyContentsProducesOutput(
"""
#abc,
#aaa,
#aabbcc {
......@@ -368,7 +355,7 @@ b:before,
}""", """
- Use rgb() over #hex when not a shade of gray (like #333).
background: -webkit-linear-gradient(left, from(#abc), to(#def)); """
"""(replace with rgb(170, 187, 204), rgb(221, 238, 255))
"""(replace with rgb(170, 187, 204), rgb(221, 238, 255))
color: #bad; (replace with rgb(187, 170, 221))
color: #bada55; (replace with rgb(186, 218, 85))""")
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment