Skip to content

Commit

Permalink
[css-logical] Replace uses of webkit-prefixed logical properties with…
Browse files Browse the repository at this point in the history
… standard ones in styleguide, and add check to avoid future uses

Bug 850000 added standard logical properties and aliased prefixed ones
to them. The prefixed properties are still used in various places, but
the standard ones should be used instead. This patch updates the style
guide so that it refers to the standard properties, and adds a new CSS
presubmit check to ensure that the prefixed properties are not added
back anywhere.

Spec: https://drafts.csswg.org/css-logical/#box

BUG=862141

Change-Id: I50166e5bf13259302eb02aaa16ec299c9b2a4b0a
Reviewed-on: https://chromium-review.googlesource.com/1147524
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#577715}
  • Loading branch information
Loirooriol authored and Commit Bot committed Jul 24, 2018
1 parent 8c26ef3 commit c0446e3
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 30 deletions.
12 changes: 4 additions & 8 deletions styleguide/web/web.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ compatibility issues are less relevant for Chrome-only code).
.raw-button:hover,
.raw-button:active {
--sky-color: blue;
-webkit-margin-start: 0;
-webkit-margin-collapse: discard;
background-color: rgb(253, 123, 42);
background-repeat: no-repeat;
border: none;
Expand Down Expand Up @@ -241,10 +241,6 @@ compatibility issues are less relevant for Chrome-only code).
* Use scalable `font-size` units like `%` or `em` to respect users' default font
size

* Use `*-top/bottom` instead of `-webkit-*-before/after`
* `-top/bottom` are easier to understand (`before/after` is confusingly
similar to `start/end`)
* `-webkit-*-before/after` has far less advantage than `-webkit-*-start/end`

### Color

Expand Down Expand Up @@ -277,7 +273,7 @@ if `flattenhtml="true"` is specified in your .grd file.

```css
.suboption {
-webkit-margin-start: 16px;
margin-inline-start: 16px;
}

#save-button {
Expand All @@ -292,8 +288,8 @@ html[dir='rtl'] #save-button {

Use RTL-friendly versions of things like `margin` or `padding` where possible:

* `margin-left` -> `-webkit-margin-start`
* `padding-right` -> `-webkit-padding-end`
* `margin-left` -> `margin-inline-start`
* `padding-right` -> `padding-inline-end`
* `text-align: left` -> `text-align: start`
* `text-align: right` -> `text-align: end`
* set both `left` for `[dir='ltr']` and `right` for `[dir='rtl']`
Expand Down
49 changes: 38 additions & 11 deletions tools/web_dev_style/css_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,38 @@ def suggest_short_hex(line):
h = hex_reg.search(line).group(1)
return ' (replace with #%s)' % (h[0] + h[2] + h[4])

webkit_before_or_after_reg = re.compile(r'-webkit-(\w+-)(after|before):')

def suggest_top_or_bottom(line):
prop, pos = webkit_before_or_after_reg.search(line).groups()
top_or_bottom = 'top' if pos == 'before' else 'bottom'
return ' (replace with %s)' % (prop + top_or_bottom)
prefixed_logical_axis_reg = re.compile(r"""
-webkit-(min-|max-|)logical-(height|width):
""", re.VERBOSE)

def suggest_unprefixed_logical_axis(line):
preffix, prop = prefixed_logical_axis_reg.search(line).groups()
block_or_inline = 'block' if prop == 'height' else 'inline'
return ' (replace with %s)' % (preffix + block_or_inline + '-size')

def prefixed_logical_axis(line):
return prefixed_logical_axis_reg.search(line)

prefixed_logical_side_reg = re.compile(r"""
-webkit-(margin|padding|border)-(before|after|start|end)
(?!-collapse)(-\w+|):
""", re.VERBOSE)

def suggest_unprefixed_logical_side(line):
prop, pos, suffix = prefixed_logical_side_reg.search(line).groups()
if pos == 'before' or pos == 'after':
block_or_inline = 'block'
else:
block_or_inline = 'inline'
if pos == 'start' or pos == 'before':
start_or_end = 'start'
else:
start_or_end = 'end'
return ' (replace with %s)' % (
prop + '-' + block_or_inline + '-' + start_or_end + suffix)

def webkit_before_or_after(line):
return webkit_before_or_after_reg.search(line)
def prefixed_logical_side(line):
return prefixed_logical_side_reg.search(line)

def zero_width_lengths(contents):
hsl_reg = re.compile(r"""
Expand Down Expand Up @@ -370,9 +393,13 @@ def zero_width_lengths(contents):
'test': rgb_if_not_gray,
'after': suggest_rgb_from_hex,
},
{ 'desc': 'Use *-top/bottom instead of -webkit-*-before/after.',
'test': webkit_before_or_after,
'after': suggest_top_or_bottom,
{ 'desc': 'Unprefix logical axis property.',
'test': prefixed_logical_axis,
'after': suggest_unprefixed_logical_axis,
},
{ 'desc': 'Unprefix logical side property.',
'test': prefixed_logical_side,
'after': suggest_unprefixed_logical_side,
},
{ 'desc': 'Use "0" for zero-width lengths (i.e. 0px -> 0)',
'test': zero_width_lengths,
Expand Down
87 changes: 76 additions & 11 deletions tools/web_dev_style/css_checker_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def testCssStringWithAt(self):
}
div {
-webkit-margin-start: 5px;
margin-inline-start: 5px;
}
.stuff1 {
Expand All @@ -124,11 +124,11 @@ def testCssAlphaWithNonStandard(self):
div {
/* A hopefully safely ignored comment and @media statement. /**/
color: red;
-webkit-margin-start: 5px;
-webkit-margin-before-collapse: discard;
}""", """
- Alphabetize properties and list vendor specific (i.e. -webkit) above standard.
color: red;
-webkit-margin-start: 5px;""")
-webkit-margin-before-collapse: discard;""")

def testCssAlphaWithLongerDashedProps(self):
self.VerifyContentsProducesOutput("""
Expand Down Expand Up @@ -388,18 +388,83 @@ def testCssRgbIfNotGray(self):
color: #bad; (replace with rgb(187, 170, 221))
color: #bada55; (replace with rgb(186, 218, 85))""")

def testWebkitBeforeOrAfter(self):
def testPrefixedLogicalAxis(self):
self.VerifyContentsProducesOutput("""
.test {
-webkit-margin-before: 10px;
-webkit-margin-start: 20px;
-webkit-padding-after: 3px;
-webkit-padding-end: 5px;
-webkit-logical-height: 50%;
-webkit-logical-width: 50%;
-webkit-max-logical-height: 200px;
-webkit-max-logical-width: 200px;
-webkit-min-logical-height: 100px;
-webkit-min-logical-width: 100px;
}
""", """
- Use *-top/bottom instead of -webkit-*-before/after.
-webkit-margin-before: 10px; (replace with margin-top)
-webkit-padding-after: 3px; (replace with padding-bottom)""")
- Unprefix logical axis property.
-webkit-logical-height: 50%; (replace with block-size)
-webkit-logical-width: 50%; (replace with inline-size)
-webkit-max-logical-height: 200px; (replace with max-block-size)
-webkit-max-logical-width: 200px; (replace with max-inline-size)
-webkit-min-logical-height: 100px; (replace with min-block-size)
-webkit-min-logical-width: 100px; (replace with min-inline-size)""")

def testPrefixedLogicalSide(self):
self.VerifyContentsProducesOutput("""
.test {
-webkit-border-after: 1px solid blue;
-webkit-border-after-color: green;
-webkit-border-after-style: dotted;
-webkit-border-after-width: 10px;
-webkit-border-before: 2px solid blue;
-webkit-border-before-color: green;
-webkit-border-before-style: dotted;
-webkit-border-before-width: 20px;
-webkit-border-end: 3px solid blue;
-webkit-border-end-color: green;
-webkit-border-end-style: dotted;
-webkit-border-end-width: 30px;
-webkit-border-start: 4px solid blue;
-webkit-border-start-color: green;
-webkit-border-start-style: dotted;
-webkit-border-start-width: 40px;
-webkit-margin-after: 1px;
-webkit-margin-after-collapse: discard;
-webkit-margin-before: 2px;
-webkit-margin-before-collapse: discard;
-webkit-margin-end: 3px;
-webkit-margin-end-collapse: discard;
-webkit-margin-start: 4px;
-webkit-margin-start-collapse: discard;
-webkit-padding-after: 1px;
-webkit-padding-before: 2px;
-webkit-padding-end: 3px;
-webkit-padding-start: 4px;
}
""", """
- Unprefix logical side property.
-webkit-border-after: 1px solid blue; (replace with border-block-end)
-webkit-border-after-color: green; (replace with border-block-end-color)
-webkit-border-after-style: dotted; (replace with border-block-end-style)
-webkit-border-after-width: 10px; (replace with border-block-end-width)
-webkit-border-before: 2px solid blue; (replace with border-block-start)
-webkit-border-before-color: green; (replace with border-block-start-color)
-webkit-border-before-style: dotted; (replace with border-block-start-style)
-webkit-border-before-width: 20px; (replace with border-block-start-width)
-webkit-border-end: 3px solid blue; (replace with border-inline-end)
-webkit-border-end-color: green; (replace with border-inline-end-color)
-webkit-border-end-style: dotted; (replace with border-inline-end-style)
-webkit-border-end-width: 30px; (replace with border-inline-end-width)
-webkit-border-start: 4px solid blue; (replace with border-inline-start)
-webkit-border-start-color: green; (replace with border-inline-start-color)
-webkit-border-start-style: dotted; (replace with border-inline-start-style)
-webkit-border-start-width: 40px; (replace with border-inline-start-width)
-webkit-margin-after: 1px; (replace with margin-block-end)
-webkit-margin-before: 2px; (replace with margin-block-start)
-webkit-margin-end: 3px; (replace with margin-inline-end)
-webkit-margin-start: 4px; (replace with margin-inline-start)
-webkit-padding-after: 1px; (replace with padding-block-end)
-webkit-padding-before: 2px; (replace with padding-block-start)
-webkit-padding-end: 3px; (replace with padding-inline-end)
-webkit-padding-start: 4px; (replace with padding-inline-start)""")

def testCssZeroWidthLengths(self):
self.VerifyContentsProducesOutput("""
Expand Down

0 comments on commit c0446e3

Please sign in to comment.