Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XmlWebViewInsideScrollView Detector #37

Merged

Conversation

vickyramachandra
Copy link
Contributor

This PR adds a lint check to make sure that Scrollview which has WebView as its child sets fillViewport to true

  • Add XmlWebViewInsideScrollViewDetector

  • Add Unit tests

Copy link
Collaborator

@ShaishavGandhi ShaishavGandhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the Issue in the IssueRegistry? Only then will it be enabled.

.value("true")
.build()
context.report(ISSUE,
context.getElementLocation(parentScrollView), LINT_ERROR_MESSAGE,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Move the LINT_ERROR_MESSAGE to new line for consistency?

Copy link
Contributor Author

@vickyramachandra vickyramachandra Nov 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do

TestLintTask.lint()
.files(xmlSource("res/layout/webview_inside_scrollview_wo_fill_viewport.xml",
"""<?xml version="1.0" encoding="utf-8"?>
<ScrollView
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: These are indented weird. and <?xml version> should be on the same indent level

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, let me format them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShaishavGandhi i ran a reformat on the file. Still this indent remained

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Manually formatted

@ShaishavGandhi
Copy link
Collaborator

ShaishavGandhi commented Nov 28, 2019

Can you run spotless again? It's failing the build

@vickyramachandra
Copy link
Contributor Author

Can you run spotless again? It's failing the build

Yeah fixed

@ShaishavGandhi ShaishavGandhi merged commit f1928f6 into uber:master Nov 28, 2019
@vickyramachandra vickyramachandra deleted the webview_inside_scrollview_detector branch November 28, 2019 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants