Conversation
3eb7aaa to
8c6b89b
Compare
8c6b89b to
293149c
Compare
293149c to
00469c2
Compare
00469c2 to
37ae58f
Compare
There was a problem hiding this comment.
Android Lint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
37ae58f to
98c117d
Compare
| layer.addLayerToMap() | ||
| // [END maps_android_utils_kml_add_layer] | ||
|
|
||
| // [START maps_android_utils_kml_remove_layer] |
There was a problem hiding this comment.
Same class of issue I flagged on #2426, smaller in scope here. This PR drops 3 devsite region tags from the repo entirely:
| tag | was in |
|---|---|
maps_android_utils_kml_remove_layer |
this file, line 52 |
maps_android_ktx_install_snippet |
snippets/app-ktx/build.gradle.kts:74 |
maps_android_utils_ktx_install_snippet |
snippets/app-utils-ktx/build.gradle.kts:73 |
I verified by diffing every [START ...] tag between feat/snippets-java-app and feat/snippets-kotlin-app, then grepping each missing one across the whole tree to rule out relocation.
The two *_install_snippet tags are the Gradle dependency snippets for the KTX libraries, which are exactly the sort of thing an installation page includes by tag. Worth noting that snippets/app-ktx/build.gradle.kts is detected as a rename to snippets/kotlin-app/build.gradle.kts (R059), so the file survives but the tag inside it does not.
Could you carry these three across to the new module, or confirm they are safe to retire?
Separately, nice work on the androidTest suites here. SnippetDiscoveryTest plus the per-capability tests is a good pattern, and it's a real step up from what the legacy modules had.
There was a problem hiding this comment.
Addressed in commit bc534e92. All 3 dropped devsite region tags have been restored and verified:
maps_android_ktx_install_snippetandmaps_android_utils_ktx_install_snippethave been carried over tosnippets/kotlin-app/build.gradle.kts.maps_android_utils_kml_remove_layerhas been added toUtilsSnippets.kt(removeKmlLayer).- Also forwarded missing
GoogleMapmethods onTrackedMap.ktand eliminated.delegatecalls across snippet tags, and sanitized API key logging inMapActivity.kt.
98c117d to
bc534e9
Compare
bc534e9 to
25476e6
Compare
8aace02 to
65c7503
Compare
65c7503 to
d3c82f0
Compare
…est suites - Create :snippets:kotlin-app with 14 snippet categories, KTX extensions, and documentation region tags - Add Kotlin snippet infrastructure (KotlinSnippetsActivity, MapActivity, SnippetRegistry, TrackedMap) - Add Kotlin capabilities test suite (CatalogCapabilitiesTestSuite, CameraControl, Events, MapInit, Marker) - Add Kotlin visual test suite (BaseVisualTest, VisualTests) - Remove legacy Kotlin snippet modules (snippets/app-ktx, snippets/app-utils-ktx) - Update root settings.gradle.kts
… calls, and sanitize logging
…map state and apply live feedback
d3c82f0 to
5ecabb3
Compare
|
|
||
| // Initialize the manager with the context and the map. | ||
| // (Activity extends context, so we can pass 'this' in the constructor.) | ||
| val manager = ClusterManager<MyItem>(context, map.delegate) |
There was a problem hiding this comment.
TrackedMap.delegate & test-harness casts are still inside active DevSite region tags Because ClusterManager, GeoJsonLayer, KmlLayer, and the *Manager classes require a real GoogleMap instance, 12 lines inside 7 active [START ...] blocks in UtilsSnippets.kt (L96, L233, L243, L341, L354, L512–530) still pass map.delegate—and EventsSnippets.kt:L42–47 / DatasetLayerSnippets.kt:L125–146 include internal MapActivity casts and R.id.custom_controls_container wiring inside published tags. Root cause: map is typed as TrackedMap rather than GoogleMap, so anything inside [START ...] that needs GoogleMap leaks .delegate verbatim onto developers.google.com, which will not compile when developers copy-paste it. Fix: Either alias val map: GoogleMap = this.map.delegate outside the [START ...] tag (like KtxSnippets.kt:L71 does) or wrap internal harness setup in // [START_EXCLUDE silent] ... // [END_EXCLUDE].
| groupTitle = groupAnnotation.title, | ||
| action = { context, map, scope -> | ||
| try { | ||
| val trackedMap = TrackedMap(map, addedElements) |
There was a problem hiding this comment.
this section silently swallows all snippet crashes and SnippetDiscoveryTest closes before onMapReady runs Unlike origin/feat/snippets-java-app (where SnippetRegistry.java:L116–124 was updated to unwrap InvocationTargetException and rethrow RuntimeException), SnippetRegistry.kt:L125–127 still does catch (e: Exception) { e.printStackTrace() }, making MapActivity.kt:L297 dead code. Root cause: Reflection exceptions are swallowed inside SnippetRegistry, and SnippetDiscoveryTest.verifyAllSnippetsLaunchWithoutCrash() (L102–112) only checks activity.mapView != null in onCreate before closing ActivityScenario.use { ... }—destroying the Activity before getMapAsync / onMapReady ever executes snippet.action. Fix: Unwrap InvocationTargetException and rethrow in SnippetRegistry.kt (matching SnippetRegistry.java), and await onMapReady / snippet execution in SnippetDiscoveryTest.kt before closing the scenario.
|
Missing MapsObject.kt drops the Kotlin counterparts for 4 getting-started DevSite region tags In origin/feat/snippets-java-app, MapsObject.java was added to :snippets:java-app to preserve maps_android_on_create_set_content_view, maps_android_on_map_ready_callback, maps_android_on_map_ready_add_marker, and maps_android_get_map_async, but its Kotlin counterpart (MapsObject.kt from snippets/app/.../kotlin/MapsObject.kt on main) was never ported to :snippets:kotlin-app. Root cause: snippet-bot only checks whether a tag exists somewhere in the repo (which MapsObject.java satisfied), masking that the Kotlin definitions were deleted with snippets/app—and leaving snippets/kotlin-app/src/main/res/layout/main.xml orphaned (flagged as UnusedResources by Android Lint). Fix: Port MapsObject.kt into snippets/kotlin-app/src/main/java/com/example/snippets/kotlin/MapsObject.kt to maintain Java/Kotlin DevSite tab parity. |
| title = "3c. Remove Single Cluster Item", | ||
| description = "What it does: Removes a specified single item from the active ClusterManager collection.\nHow to see the effect: The target marker pin is removed and surrounding cluster count numbers decrement.", | ||
| ) | ||
| fun removeSingleClusterItem() { |
There was a problem hiding this comment.
removeSingleClusterItem & infoWindow: MyItem does not override equals/hashCode and addItems() uses "Title 0", so clusterManager?.removeItem(MyItem(..., "Title to remove", ...)) fails reference-equality lookup and removes nothing; meanwhile infoWindow() calls it.addItem(infoWindowItem) after setUpClusterer() without calling it.cluster(), so the added marker never renders.
| title = "2. Map Fragment Transaction", | ||
| description = "What it does: Dynamically adds a SupportMapFragment into the Activity view hierarchy programmatically.\nHow to see the effect: A new map fragment view is instantiated and rendered into the container layout frame.", | ||
| ) | ||
| fun mapFragment() { |
There was a problem hiding this comment.
Commits a SupportMapFragment into R.id.map_container (the root RelativeLayout), whereas MapActivity.recreateMapView() only clears R.id.map_view_holder—permanently covering the screen and breaking all subsequent Next / Previous snippet navigations.
| title = "2. Set Panorama Location", | ||
| description = "What it does: Sets the panorama view geographic coordinates, search radius, and outdoor source filter.\nHow to see the effect: The Street View camera jumps directly to target coordinates.", | ||
| ) | ||
| fun setLocation() { |
There was a problem hiding this comment.
setLocation(), zoomPanorama(), and animatePanorama() allocate unused local LatLng/StreetViewPanoramaCamera variables and launch StreetViewActivity with no Intent extras (so all 4 items do the exact same thing), while ktxAddMarker() and polylinePolygonDsl() add shapes in Sydney / Mountain View without moving the camera from (0, 0).
Summary
Stacked Base
Stacked on #2426 (
feat/snippets-java-app).Reviewers
@kikoso