refactor: Use com.google.android.material.appbar.MaterialToolbar
The previous code used `androidx.appcompat.widget.Toolbar` in a several places. It's better to use `MaterialToolbar` as that plays better with other Material components. Update the usage throughout the project. In addition, implement a lint check that will prevent any future use from creeping back in. Fixes #28
This commit is contained in:
parent
f8877909ca
commit
4b7eb2419e
|
@ -38,7 +38,7 @@ jobs:
|
||||||
run: ./gradlew app:lintOrangeDebug
|
run: ./gradlew app:lintOrangeDebug
|
||||||
|
|
||||||
- name: Test
|
- name: Test
|
||||||
run: ./gradlew app:testOrangeDebugUnitTest
|
run: ./gradlew app:testOrangeDebugUnitTest checks:test
|
||||||
|
|
||||||
- name: Build
|
- name: Build
|
||||||
run: ./gradlew app:buildOrangeDebug
|
run: ./gradlew app:buildOrangeDebug
|
||||||
|
|
|
@ -217,4 +217,6 @@ dependencies {
|
||||||
androidTestImplementation libs.androidx.test.junit
|
androidTestImplementation libs.androidx.test.junit
|
||||||
androidTestImplementation libs.hilt.android.testing
|
androidTestImplementation libs.hilt.android.testing
|
||||||
androidTestImplementation libs.androidx.test.core.ktx
|
androidTestImplementation libs.androidx.test.core.ktx
|
||||||
|
|
||||||
|
lintChecks project(":checks")
|
||||||
}
|
}
|
||||||
|
|
|
@ -427,7 +427,7 @@
|
||||||
</androidx.constraintlayout.widget.ConstraintLayout>
|
</androidx.constraintlayout.widget.ConstraintLayout>
|
||||||
|
|
||||||
<!-- top margin equal to statusbar size will be set programmatically -->
|
<!-- top margin equal to statusbar size will be set programmatically -->
|
||||||
<androidx.appcompat.widget.Toolbar
|
<com.google.android.material.appbar.MaterialToolbar
|
||||||
android:id="@+id/accountToolbar"
|
android:id="@+id/accountToolbar"
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="wrap_content"
|
android:layout_height="wrap_content"
|
||||||
|
|
|
@ -6,7 +6,7 @@
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="match_parent">
|
android:layout_height="match_parent">
|
||||||
|
|
||||||
<androidx.appcompat.widget.Toolbar
|
<com.google.android.material.appbar.MaterialToolbar
|
||||||
android:id="@+id/toolbar"
|
android:id="@+id/toolbar"
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="wrap_content"
|
android:layout_height="wrap_content"
|
||||||
|
@ -71,7 +71,7 @@
|
||||||
app:tint="@color/tusky_orange_light"
|
app:tint="@color/tusky_orange_light"
|
||||||
app:tooltipText="@string/hint_media_description_missing"
|
app:tooltipText="@string/hint_media_description_missing"
|
||||||
android:visibility="invisible" />
|
android:visibility="invisible" />
|
||||||
</androidx.appcompat.widget.Toolbar>
|
</com.google.android.material.appbar.MaterialToolbar>
|
||||||
|
|
||||||
<androidx.core.widget.NestedScrollView
|
<androidx.core.widget.NestedScrollView
|
||||||
android:id="@+id/composeMainScrollView"
|
android:id="@+id/composeMainScrollView"
|
||||||
|
|
|
@ -95,7 +95,7 @@
|
||||||
</LinearLayout>
|
</LinearLayout>
|
||||||
</ScrollView>
|
</ScrollView>
|
||||||
|
|
||||||
<androidx.appcompat.widget.Toolbar
|
<com.google.android.material.appbar.MaterialToolbar
|
||||||
android:id="@+id/toolbar"
|
android:id="@+id/toolbar"
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="?attr/actionBarSize"
|
android:layout_height="?attr/actionBarSize"
|
||||||
|
|
|
@ -9,7 +9,7 @@
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="wrap_content">
|
android:layout_height="wrap_content">
|
||||||
|
|
||||||
<androidx.appcompat.widget.Toolbar
|
<com.google.android.material.appbar.MaterialToolbar
|
||||||
android:id="@+id/loginToolbar"
|
android:id="@+id/loginToolbar"
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="wrap_content" />
|
android:layout_height="wrap_content" />
|
||||||
|
|
|
@ -13,7 +13,7 @@
|
||||||
app:liftOnScrollTargetViewId="@id/pages"
|
app:liftOnScrollTargetViewId="@id/pages"
|
||||||
app:layout_collapseMode="pin">
|
app:layout_collapseMode="pin">
|
||||||
|
|
||||||
<androidx.appcompat.widget.Toolbar
|
<com.google.android.material.appbar.MaterialToolbar
|
||||||
android:id="@+id/toolbar"
|
android:id="@+id/toolbar"
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="?attr/actionBarSize"
|
android:layout_height="?attr/actionBarSize"
|
||||||
|
|
|
@ -11,7 +11,7 @@
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="match_parent" />
|
android:layout_height="match_parent" />
|
||||||
|
|
||||||
<androidx.appcompat.widget.Toolbar
|
<com.google.android.material.appbar.MaterialToolbar
|
||||||
android:id="@+id/toolbar"
|
android:id="@+id/toolbar"
|
||||||
android:layout_width="match_parent"
|
android:layout_width="match_parent"
|
||||||
android:layout_height="?attr/actionBarSize"
|
android:layout_height="?attr/actionBarSize"
|
||||||
|
|
|
@ -0,0 +1 @@
|
||||||
|
/build
|
|
@ -0,0 +1,36 @@
|
||||||
|
plugins {
|
||||||
|
id "java-library"
|
||||||
|
id "kotlin"
|
||||||
|
id "com.android.lint"
|
||||||
|
}
|
||||||
|
|
||||||
|
lintOptions {
|
||||||
|
htmlReport(true)
|
||||||
|
htmlOutput(file("lint-report.html"))
|
||||||
|
textReport(true)
|
||||||
|
absolutePaths(false)
|
||||||
|
ignoreTestSources(true)
|
||||||
|
}
|
||||||
|
|
||||||
|
jar {
|
||||||
|
manifest {
|
||||||
|
attributes("Lint-Registry-v2": "app.pachli.lint.checks.LintRegistry")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
dependencies {
|
||||||
|
compileOnly("org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.3.61")
|
||||||
|
|
||||||
|
// Derived from the AGP version. If the AGP version is X.Y.Z the version
|
||||||
|
// here must by X+23.Y.Z.
|
||||||
|
// See https://github.com/googlesamples/android-custom-lint-rules#lint-version
|
||||||
|
def lintVersion = "31.1.2" // = AGP 8.1.2
|
||||||
|
|
||||||
|
// For a description of the below dependencies, see the main project README
|
||||||
|
compileOnly("com.android.tools.lint:lint-api:$lintVersion")
|
||||||
|
compileOnly("com.android.tools.lint:lint-checks:$lintVersion")
|
||||||
|
testImplementation("com.android.tools.lint:lint:$lintVersion")
|
||||||
|
testImplementation("com.android.tools.lint:lint-tests:$lintVersion")
|
||||||
|
|
||||||
|
testImplementation("junit:junit:4.13.2")
|
||||||
|
}
|
|
@ -0,0 +1,58 @@
|
||||||
|
package app.pachli.lint.checks
|
||||||
|
|
||||||
|
import com.android.tools.lint.detector.api.Category
|
||||||
|
import com.android.tools.lint.detector.api.Detector
|
||||||
|
import com.android.tools.lint.detector.api.Implementation
|
||||||
|
import com.android.tools.lint.detector.api.Issue
|
||||||
|
import com.android.tools.lint.detector.api.LintFix
|
||||||
|
import com.android.tools.lint.detector.api.Scope
|
||||||
|
import com.android.tools.lint.detector.api.Severity
|
||||||
|
import com.android.tools.lint.detector.api.XmlContext
|
||||||
|
import com.android.tools.lint.detector.api.XmlScanner
|
||||||
|
import org.w3c.dom.Element
|
||||||
|
|
||||||
|
class AndroidxToolbarDetector : Detector(), XmlScanner {
|
||||||
|
override fun getApplicableElements(): Collection<String> {
|
||||||
|
return listOf(ANDROIDX_TOOLBAR)
|
||||||
|
}
|
||||||
|
|
||||||
|
override fun visitElement(context: XmlContext, element: Element) {
|
||||||
|
val quickFixData = LintFix.create()
|
||||||
|
.name("Replace with `com.google.android.material.appbar.MaterialToolbar`")
|
||||||
|
.replace()
|
||||||
|
.text(element.localName)
|
||||||
|
.with("com.google.android.material.appbar.MaterialToolbar")
|
||||||
|
.independent(true)
|
||||||
|
.build()
|
||||||
|
|
||||||
|
context.report(
|
||||||
|
issue = ISSUE,
|
||||||
|
scope = element,
|
||||||
|
location = context.getElementLocation(element),
|
||||||
|
message = "Use `com.google.android.material.appbar.MaterialToolbar` instead of `androidx.appcompat.widget.Toolbar`",
|
||||||
|
quickfixData = quickFixData,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
companion object {
|
||||||
|
private const val ANDROIDX_TOOLBAR = "androidx.appcompat.widget.Toolbar"
|
||||||
|
|
||||||
|
@JvmField
|
||||||
|
val ISSUE: Issue = Issue.create(
|
||||||
|
id = "AndroidxToolbarDetector",
|
||||||
|
briefDescription = "Don't use `androidx.appcompat.widget.Toolbar` in this project",
|
||||||
|
explanation = """
|
||||||
|
Use `com.google.android.material.appbar.MaterialToolbar` instead of
|
||||||
|
`androidx.appcompat.widget.Toolbar` to ensure it works as expected with
|
||||||
|
other Material components. See https://developer.android.com/reference/com/google/android/material/appbar/MaterialToolbar
|
||||||
|
""",
|
||||||
|
category = Category.CORRECTNESS,
|
||||||
|
priority = 6,
|
||||||
|
severity = Severity.WARNING,
|
||||||
|
implementation = Implementation(
|
||||||
|
AndroidxToolbarDetector::class.java,
|
||||||
|
Scope.RESOURCE_FILE_SCOPE,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,14 @@
|
||||||
|
package app.pachli.lint.checks
|
||||||
|
|
||||||
|
import com.android.tools.lint.client.api.IssueRegistry
|
||||||
|
import com.android.tools.lint.detector.api.CURRENT_API
|
||||||
|
import com.android.tools.lint.detector.api.Issue
|
||||||
|
|
||||||
|
@Suppress("UnstableApiUsage")
|
||||||
|
class LintRegistry : IssueRegistry() {
|
||||||
|
override val issues: List<Issue>
|
||||||
|
get() = listOf(AndroidxToolbarDetector.ISSUE)
|
||||||
|
|
||||||
|
override val api: Int
|
||||||
|
get() = CURRENT_API
|
||||||
|
}
|
|
@ -0,0 +1,89 @@
|
||||||
|
package app.pachli.lint.checks
|
||||||
|
|
||||||
|
import com.android.tools.lint.checks.infrastructure.TestFiles.xml
|
||||||
|
import com.android.tools.lint.checks.infrastructure.TestLintTask.lint
|
||||||
|
import org.junit.Test
|
||||||
|
|
||||||
|
class AndroidxToolbarDetectorTest {
|
||||||
|
@Test
|
||||||
|
fun testError() {
|
||||||
|
lint().files(
|
||||||
|
xml(
|
||||||
|
"res/layout/test.xml",
|
||||||
|
"""<?xml version="1.0" encoding="utf-8"?>
|
||||||
|
<androidx.coordinatorlayout.widget.CoordinatorLayout xmlns:android="http://schemas.android.com/apk/res/android"
|
||||||
|
xmlns:app="http://schemas.android.com/apk/res-auto"
|
||||||
|
xmlns:tools="http://schemas.android.com/tools"
|
||||||
|
android:layout_width="match_parent"
|
||||||
|
android:layout_height="match_parent"
|
||||||
|
tools:context="app.pachli.components.search.SearchActivity">
|
||||||
|
|
||||||
|
<com.google.android.material.appbar.AppBarLayout
|
||||||
|
android:layout_width="match_parent"
|
||||||
|
android:layout_height="wrap_content"
|
||||||
|
app:liftOnScroll="true"
|
||||||
|
app:liftOnScrollTargetViewId="@id/pages"
|
||||||
|
app:layout_collapseMode="pin">
|
||||||
|
|
||||||
|
<androidx.appcompat.widget.Toolbar
|
||||||
|
android:id="@+id/toolbar"
|
||||||
|
android:layout_width="match_parent"
|
||||||
|
android:layout_height="?attr/actionBarSize"
|
||||||
|
app:contentInsetStartWithNavigation="0dp"
|
||||||
|
app:layout_scrollFlags="scroll|snap|enterAlways"
|
||||||
|
app:navigationIcon="?attr/homeAsUpIndicator" />
|
||||||
|
|
||||||
|
</com.google.android.material.appbar.AppBarLayout>
|
||||||
|
|
||||||
|
</androidx.coordinatorlayout.widget.CoordinatorLayout>
|
||||||
|
""",
|
||||||
|
).indented(),
|
||||||
|
).issues(AndroidxToolbarDetector.ISSUE).allowMissingSdk().run().expectWarningCount(1)
|
||||||
|
.expect(
|
||||||
|
"""res/layout/test.xml:16: Warning: Use com.google.android.material.appbar.MaterialToolbar instead of androidx.appcompat.widget.Toolbar [AndroidxToolbarDetector]
|
||||||
|
<androidx.appcompat.widget.Toolbar
|
||||||
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||||
|
0 errors, 1 warnings
|
||||||
|
""",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun testNoError() {
|
||||||
|
lint().files(
|
||||||
|
xml(
|
||||||
|
"res/layout/test.xml",
|
||||||
|
"""<?xml version="1.0" encoding="utf-8"?>
|
||||||
|
<androidx.coordinatorlayout.widget.CoordinatorLayout xmlns:android="http://schemas.android.com/apk/res/android"
|
||||||
|
xmlns:app="http://schemas.android.com/apk/res-auto"
|
||||||
|
xmlns:tools="http://schemas.android.com/tools"
|
||||||
|
android:layout_width="match_parent"
|
||||||
|
android:layout_height="match_parent"
|
||||||
|
tools:context="app.pachli.components.search.SearchActivity">
|
||||||
|
|
||||||
|
<com.google.android.material.appbar.AppBarLayout
|
||||||
|
android:layout_width="match_parent"
|
||||||
|
android:layout_height="wrap_content"
|
||||||
|
app:layout_collapseMode="pin"
|
||||||
|
app:liftOnScroll="true"
|
||||||
|
app:liftOnScrollTargetViewId="@id/pages">
|
||||||
|
|
||||||
|
<com.google.android.material.appbar.MaterialToolbar
|
||||||
|
android:id="@+id/toolbar"
|
||||||
|
android:layout_width="match_parent"
|
||||||
|
android:layout_height="?attr/actionBarSize"
|
||||||
|
app:contentInsetStartWithNavigation="0dp"
|
||||||
|
app:layout_scrollFlags="scroll|snap|enterAlways"
|
||||||
|
app:navigationIcon="?attr/homeAsUpIndicator" />
|
||||||
|
|
||||||
|
</com.google.android.material.appbar.AppBarLayout>
|
||||||
|
|
||||||
|
</androidx.coordinatorlayout.widget.CoordinatorLayout>
|
||||||
|
""",
|
||||||
|
).indented(),
|
||||||
|
).issues(AndroidxToolbarDetector.ISSUE)
|
||||||
|
.allowMissingSdk()
|
||||||
|
.run()
|
||||||
|
.expectWarningCount(0)
|
||||||
|
}
|
||||||
|
}
|
|
@ -20,3 +20,4 @@ enableFeaturePreview("STABLE_CONFIGURATION_CACHE")
|
||||||
|
|
||||||
include ':app'
|
include ':app'
|
||||||
include ':tools:mklanguages'
|
include ':tools:mklanguages'
|
||||||
|
include ':checks'
|
||||||
|
|
Loading…
Reference in New Issue