feat: extract on-device review app and tools to :ApiDemos:reviewer module by dkhawk · Pull Request #2428 · googlemaps-samples/android-samples

dkhawk · 2026-09-15T00:18:43Z

Summary

Extract on-device review tools into dedicated :ApiDemos:reviewer module
Add Room database persistence (SampleCatalogDatabase, SampleEvaluationDao, SampleEvaluationEntity)
Implement ReviewerActivity, ReviewEvaluationDialog, AnnotationCanvasView, and GrievanceReportExporter
Wire reviewer toolbar actions into SamplesBaseActivity (Java & Kotlin)
Add BasicMapReviewerUiTest for reviewer toolbar validation
Add verification tooling guides, manual verification dashboard, and generated snippet catalogs

Stacked Base

Stacked on #2427 (feat/snippets-kotlin-app).

Reviewers

@LoyalAbbas

snippet-bot · 2026-09-15T00:38:35Z

No region tags are edited in this PR.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

Refresh this comment

…dule - Extract on-device review tools into dedicated :ApiDemos:reviewer module - Add Room database persistence (SampleCatalogDatabase, SampleEvaluationDao, SampleEvaluationEntity) - Implement ReviewerActivity, ReviewEvaluationDialog, AnnotationCanvasView, and GrievanceReportExporter - Wire reviewer toolbar actions into SamplesBaseActivity (Java & Kotlin) - Add BasicMapReviewerUiTest for reviewer toolbar validation - Add verification tooling guides, manual verification dashboard, and generated snippet catalogs

kikoso · 2026-09-16T12:02:48Z

+    server_address = ("0.0.0.0", PORT)
+    httpd = http.server.ThreadingHTTPServer(server_address, CatalogRequestHandler)


Should this bind 127.0.0.1 instead? On 0.0.0.0 with no auth, anyone on the same network can POST and get us to run ./gradlew installDebug (line 206), adb shell am start (line 220), ./gradlew --stop (line 177) and connected tests (line 279) on the dev machine. The shell quoting itself looks fine — it's the bind address I'd worry about.

Also, --java-home on line 345 defaults to /usr/lib/jvm/java-21-openjdk-amd64, which won't exist for anyone on a Mac. Fall back to JAVA_HOME when it's not passed?

kikoso · 2026-09-16T12:04:43Z

    androidTestImplementation(libs.uiautomator)

    implementation(project(":ApiDemos:common-ui"))
+    implementation(project(":ApiDemos:reviewer"))


Should this be debugImplementation? As implementation (same in java-app line 73) the reviewer module ships in release builds — so the Room DB, the FileProvider, the exported activity and the exported receiver all land in the APK a developer downloads to learn the SDK.

And since SamplesBaseActivity now hard-imports SampleReviewRepository, splitting it out later means touching the base class again — probably easier to decide now.

kikoso · 2026-09-16T12:05:16Z

+    <external-files-path name="external_files" path="." />
+    <files-path name="internal_files" path="." />
+    <cache-path name="internal_cache" path="." />


Can we drop these three? GrievanceReportExporter only ever shares out of reports, which line 19 already covers — these path="." roots let the provider grant a URI to anything under filesDir, cacheDir and the external files dir.

Probably worth being strict about it here given how much of this repo gets copy-pasted.

kikoso · 2026-09-16T12:08:23Z

+    private fun applyImmersiveStickyMode() {
+        val insetsController = androidx.core.view.WindowCompat.getInsetsController(window, window.decorView)
+        insetsController.systemBarsBehavior =
+            androidx.core.view.WindowInsetsControllerCompat.BEHAVIOR_SHOW_TRANSIENT_BARS_BY_SWIPE
+        insetsController.hide(WindowInsetsCompat.Type.systemBars())
    }


Should this be gated on isReviewerMode? Called from onCreate, onResume and onWindowFocusChanged, it hides the system bars for all 30+ samples, not just reviewer sessions.

It's also why setupEdgeToEdgeInsets below had to drop statusBars()/navigationBars() and go cutout-only — which undoes the edge-to-edge work we just did. Big UX change to land in a PR about extracting the reviewer module, no?

kikoso · 2026-09-16T12:16:53Z

+    implementation(libs.activity.compose)
+    implementation(platform(libs.compose.bom))
+    implementation(libs.ui)
+    implementation(libs.ui.graphics)
+    implementation(libs.ui.tooling.preview)
+    implementation(libs.material3)
+    implementation(libs.material.icons.extended)
+    debugImplementation(libs.ui.tooling)


Can this go in its own commit? Adding the whole Compose stack to kotlin-app — plus dropping kotlin.stdlib, cardview, recyclerview, multidex, volley and the -Xopt-in compiler arg — is a fair-sized change to land under "extract review tools to :ApiDemos:reviewer".

It does all still build, so the removals look safe, I'd just rather they were visible on their own rather than buried in a 12k-line diff.

kikoso · 2026-09-16T12:17:19Z

+        val scenario = ActivityScenario.launch(BasicMapDemoActivity::class.java)
+
+        scenario.onActivity { activity ->
+            val toolbar = activity.findViewById<MaterialToolbar>(R.id.top_bar)
+            assertNotNull("Top bar must be present in layout", toolbar)
+
+            // Verify action bar is attached to the toolbar
+            val actionBar = activity.supportActionBar
+            assertNotNull("SupportActionBar must be attached to MaterialToolbar", actionBar)
+
+            // Verify menu items are present in toolbar
+            val menu = toolbar.menu
+            assertNotNull("Toolbar menu must not be null", menu)
+
+            val infoItem = menu.findItem(2001)
+            assertNotNull("Criteria & Purpose (id 2001) action button must exist in toolbar menu", infoItem)
+
+            val goodJobItem = menu.findItem(2003)
+            assertNotNull("Good Job (id 2003) action button must exist in toolbar menu", goodJobItem)
+
+            val wrongItem = menu.findItem(2004)
+            assertNotNull("Something's Wrong (id 2004) action button must exist in toolbar menu", wrongItem)


Shouldn't the launch intent carry extra_is_reviewer_mode? Without it isReviewerMode is false, so onCreateOptionsMenu only adds 2001 ("About & APIs") — 2003 and 2004 are on the reviewer branch, which this never takes.

I couldn't run it (no device here), so I may be missing something. Also assertTrue is imported but unused, and there's no Java counterpart.

LoyalAbbas · 2026-09-16T13:54:19Z

+ * Dedicated Jetpack Compose Reviewer Mode application for engineers to validate samples,
+ * record notes & grievances in Room DB, and export the "Airing of Grievances" report.
+ */
+open class ReviewerActivity : ComponentActivity() {


When we open this activity from CatalogActivity.kt, still we are calling with old name

onSwitchMode = {
val intent = Intent().setClassName(packageName, "com.example.common_ui.catalog.compose.ReviewerActivity")
startActivity(intent)
}

We need to change this with new ReviewerActivity name

LoyalAbbas · 2026-09-16T14:18:22Z

+                val javaActivity = metadata?.javaActivity
+                if (javaActivity != null) {
+                    finish()
+                    val intent = Intent().setClassName(packageName, javaActivity).apply {


ActivityNotFoundException Crash When Switching Between Kotlin & Java Samples
Please verify to other similar kind of Intent calling.

Bug: :ApiDemos:kotlin-app (com.example.kotlindemos) and :ApiDemos:java-app (com.example.mapdemo) are two separate APKs. In SamplesBaseActivity.kt, packageName is "com.example.kotlindemos", while metadata.javaActivity is "com.example.mapdemo.". Calling Intent().setClassName(packageName, javaActivity) looks for com.example.mapdemo.* inside the com.example.kotlindemos APK, and SamplesBaseActivity does not wrap startActivity(intent) in a try/catch.

Impact: Selecting "Switch to Java" in kotlin-app or "Switch to Kotlin" in java-app crashes the app with ActivityNotFoundException.

Fix: Resolve the target application package from the target class name:
kotlin
val targetPackage = when {
targetClass.startsWith("com.example.mapdemo.") -> "com.example.mapdemo"
targetClass.startsWith("com.example.kotlindemos.") -> "com.example.kotlindemos"
else -> packageName
}

And guard startActivity(intent) with try/catch before calling finish().

LoyalAbbas · 2026-09-16T14:19:31Z

+            if (currentSampleMetadata != null && currentSampleMetadata.getKotlinActivity() != null) {
+                finish();
+                Intent intent = new Intent();
+                intent.setClassName(getPackageName(), currentSampleMetadata.getKotlinActivity());


ActivityNotFoundException Crash When Switching Between Kotlin & Java Samples

Please verify to other similar kind of Intent calling.

Bug: :ApiDemos:kotlin-app (com.example.kotlindemos) and :ApiDemos:java-app (com.example.mapdemo) are two separate APKs. In SamplesBaseActivity.kt, packageName is "com.example.kotlindemos", while metadata.javaActivity is "com.example.mapdemo.". Calling Intent().setClassName(packageName, javaActivity) looks for com.example.mapdemo.* inside the com.example.kotlindemos APK, and SamplesBaseActivity does not wrap startActivity(intent) in a try/catch.

Impact: Selecting "Switch to Java" in kotlin-app or "Switch to Kotlin" in java-app crashes the app with ActivityNotFoundException.

Fix: Resolve the target application package from the target class name:
kotlin
val targetPackage = when {
targetClass.startsWith("com.example.mapdemo.") -> "com.example.mapdemo"
targetClass.startsWith("com.example.kotlindemos.") -> "com.example.kotlindemos"
else -> packageName
}

And guard startActivity(intent) with try/catch before calling finish().

LoyalAbbas · 2026-09-17T05:43:47Z

+                )
+            }
+        }
+        ContextCompat.registerReceiver(


EvaluationExportReceiver already handles com.google.maps.EXPORT_EVALUATIONS, do we really need this here ?

dkhawk force-pushed the feat/on-device-reviewer-module branch from 6179dc5 to 2f8a8c6 Compare September 15, 2026 00:26

dkhawk force-pushed the feat/on-device-reviewer-module branch from 2f8a8c6 to f7c29f5 Compare September 15, 2026 00:35

dkhawk marked this pull request as ready for review September 15, 2026 00:38

dkhawk requested a review from LoyalAbbas September 15, 2026 00:38

dkhawk added this pull request to stack #2429 September 15, 2026 00:43

dkhawk force-pushed the feat/on-device-reviewer-module branch from f7c29f5 to 7958e3a Compare September 15, 2026 19:08

dkhawk force-pushed the feat/on-device-reviewer-module branch from 7958e3a to 92f23bf Compare September 15, 2026 22:58

kikoso reviewed Sep 16, 2026

View reviewed changes

LoyalAbbas reviewed Sep 16, 2026

View reviewed changes

LoyalAbbas reviewed Sep 17, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat: extract on-device review app and tools to :ApiDemos:reviewer module - #2428

feat: extract on-device review app and tools to :ApiDemos:reviewer module#2428
dkhawk wants to merge 1 commit into
feat/snippets-kotlin-appfrom
feat/on-device-reviewer-module

dkhawk commented Sep 15, 2026

Uh oh!

snippet-bot Bot commented Sep 15, 2026 •

edited

Loading

Uh oh!

kikoso Sep 16, 2026

Uh oh!

kikoso Sep 16, 2026

Uh oh!

kikoso Sep 16, 2026

Uh oh!

kikoso Sep 16, 2026

Uh oh!

kikoso Sep 16, 2026

Uh oh!

kikoso Sep 16, 2026

Uh oh!

LoyalAbbas Sep 16, 2026

Uh oh!

LoyalAbbas Sep 16, 2026

Uh oh!

LoyalAbbas Sep 16, 2026 •

edited

Loading

Uh oh!

LoyalAbbas Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		server_address = ("0.0.0.0", PORT)
		httpd = http.server.ThreadingHTTPServer(server_address, CatalogRequestHandler)

Uh oh!

Conversation

dkhawk commented Sep 15, 2026

Summary

Stacked Base

Reviewers

Uh oh!

snippet-bot Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

No region tags are edited in this PR.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LoyalAbbas Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

snippet-bot Bot commented Sep 15, 2026 •

edited

Loading

LoyalAbbas Sep 16, 2026 •

edited

Loading