Commit f92d685e authored by Lijin Shen's avatar Lijin Shen Committed by Commit Bot

[snowflake] Presubmit warning on android layout linespacing attribute

Encourage to use org.chromium.ui.widget.TextViewWithLeading instead of
android:lineSpacingExtra or android:lineSpacingMultiplier.

Bug: 1069805
Change-Id: I3eea084ac83034d99c27ecdb7b1c69c0877ff800
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2163427Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Commit-Queue: Lijin Shen <lazzzis@google.com>
Cr-Commit-Position: refs/heads/master@{#762022}
parent 95c73a95
...@@ -50,6 +50,7 @@ def _CommonChecks(input_api, output_api): ...@@ -50,6 +50,7 @@ def _CommonChecks(input_api, output_api):
result.extend(_CheckColorPaletteReferences(input_api, output_api)) result.extend(_CheckColorPaletteReferences(input_api, output_api))
result.extend(_CheckXmlNamespacePrefixes(input_api, output_api)) result.extend(_CheckXmlNamespacePrefixes(input_api, output_api))
result.extend(_CheckTextAppearance(input_api, output_api)) result.extend(_CheckTextAppearance(input_api, output_api))
result.extend(_CheckLineSpacingAttribute(input_api, output_api))
result.extend(_CheckButtonCompatWidgetUsage(input_api, output_api)) result.extend(_CheckButtonCompatWidgetUsage(input_api, output_api))
# Add more checks here # Add more checks here
return result return result
...@@ -400,6 +401,40 @@ def _CheckNewTextAppearance(input_api, output_api): ...@@ -400,6 +401,40 @@ def _CheckNewTextAppearance(input_api, output_api):
] ]
return [] return []
### unfavored layout attributes below ###
def _CheckLineSpacingAttribute(input_api, output_api):
"""
Encourage using TextViewWithLeading rather than android:lineSpacingExtra
and android:lineSpacingMultiplier.
"""
warnings = []
attributes = ['android:lineSpacingExtra', 'android:lineSpacingMultiplier']
for f in IncludedFiles(input_api):
for line_number, line in f.ChangedContents():
for attribute in attributes:
if attribute in line:
warnings.append(
' %s:%d\n \t%s' % (f.LocalPath(), line_number, line.strip()))
if warnings:
return [
output_api.PresubmitPromptWarning(
'''
Android Widget Check warning:
Your new code is using android:lineSpacingExtra
or android:lineSpacingMultiplier, listed below.
Use org.chromium.ui.widget.TextViewWithLeading instead of
using android:lineSpacingExtra or android:lineSpacingMultiplier if possible;
TextViewWithLeading is a TextView with the added leading property, which can
perform the calculation to setup leading correctly.
See https://crbug.com/1069805 for more information.
''', warnings)
]
return []
### unfavored android widgets below ### ### unfavored android widgets below ###
def _CheckButtonCompatWidgetUsage(input_api, output_api): def _CheckButtonCompatWidgetUsage(input_api, output_api):
......
...@@ -376,6 +376,32 @@ class NewTextAppearanceTest(unittest.TestCase): ...@@ -376,6 +376,32 @@ class NewTextAppearanceTest(unittest.TestCase):
self.assertEqual(0, len(errors)) self.assertEqual(0, len(errors))
class UnfavoredLayoutAttributesTest(unittest.TestCase):
def testLineSpacingAttributesUsage(self):
xmlChanges = [
'<TextView android:id="@+id/test"',
' android:lineSpacingExtra="42dp"',
' android:lineSpacingMultiplier="42dp"',
'/>',
'<TextViewWithLeading android:id="@+id/test2"',
' app:leading="42dp"',
'/>'
]
mock_input_api = MockInputApi()
mock_input_api.files = [
MockFile('ui/android/java/res/layout/new_textview.xml', xmlChanges)
]
result = checkxmlstyle._CheckLineSpacingAttribute(
mock_input_api, MockOutputApi())
self.assertEqual(1, len(result))
self.assertEqual(2, len(result[0].items))
self.assertEqual(' ui/android/java/res/layout/new_textview.xml:2',
result[0].items[0].splitlines()[0])
self.assertEqual(' ui/android/java/res/layout/new_textview.xml:3',
result[0].items[1].splitlines()[0])
class UnfavoredWidgetsTest(unittest.TestCase): class UnfavoredWidgetsTest(unittest.TestCase):
def testButtonCompatUsage(self): def testButtonCompatUsage(self):
......
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