Skip to content

Conversation

@dkhawk
Copy link
Contributor

@dkhawk dkhawk commented Nov 4, 2025

This pull request represents the initial phase of a broader migration effort to move onClick handlers out of XML layout files and into the corresponding Activity/Fragment code. This change aims to improve maintainability, testability, and adherence to modern Android development practices.

Crucially, this pull request also introduces testing to the project.

Key changes in this phase include:

  • Test Introduction & Refactoring: This PR introduces the first set of unit tests. The GroundOverlayDemoActivityTest has been refactored to extend the common MapDemoActivityTest base class, centralizing test setup, teardown, and map interaction logic.
  • Improved Synchronization: Replaced Thread.sleep() calls in CircleDemoActivityTest and GroundOverlayDemoActivityTest with idlingResource.waitForIdle() for more reliable and robust test execution.
  • MapProvider Implementation: GroundOverlayDemoActivity now implements the MapProvider interface to correctly expose the map instance and its ready state to the test framework.
  • Enhanced Test Readability: CircleDemoActivityTest has been improved with detailed comments explaining test flow and the rationale behind camera movements for accurate click event simulation.
  • New LatLng Assertion: Introduced a LatLngSubject extension with an isWith(tolerance).of(target) assertion for more precise and readable verification of spherical distances in tests.

This is an ongoing effort, and further work will continue in subsequent pull requests to complete the onClick migration and expand test coverage across the application.

This commit introduces a suite of Espresso tests for several demo activities and refactors numerous activities to use ViewBinding, eliminating `findViewById` and `onClick` XML attributes.

- **Testing Infrastructure:**
    - Adds `MapIdlingResource` for both Java and Kotlin test sources to synchronize Espresso tests with map camera movements, preventing flaky tests.
    - Introduces custom Truth subjects (`LatLngSubject`, `LatLngBoundsSubject`) for more readable and precise assertions on map-related objects, with built-in tolerance for floating-point comparisons.
    - Adds new dependencies for testing: `espresso-idling-resource`, `truth`, and `uiautomator`.
    - Increases Gradle daemon heap size to support test execution.

- **New Espresso Tests:**
    - Adds comprehensive tests for `GroundOverlayDemoActivity`, `CameraClampingDemoActivity`, `IndoorDemoActivity`, `CameraDemoActivity`, and `VisibleRegionDemoActivity` in both Java and Kotlin modules.
    - Tests verify UI interactions, camera behavior, and state changes within the demo activities.

- **Code Refactoring:**
    - Enables ViewBinding in the `java-app`, `kotlin-app`, and `common-ui` modules.
    - Refactors all major demo activities (e.g., `GroundOverlayDemoActivity`, `MarkerDemoActivity`, `CircleDemoActivity`, `UiSettingsDemoActivity`) to use ViewBinding.
    - Replaces `android:onClick` XML attributes with programmatic `setOnClickListener` calls, improving code organization and maintainability.
    - Enhances Javadoc and KDoc in `GroundOverlayDemoActivity` to better explain the concepts and implementation details, improving its educational value.
…Provider

Refactors the GroundOverlayDemoActivityTest to improve reliability and readability. This commit introduces a `MapProvider` interface and a base `MapDemoActivityTest` class to abstract away common map initialization logic.

Key changes include:
- Add a `MapProvider` interface to ensure activities provide a `GoogleMap` object.
- Create an abstract `MapDemoActivityTest` to handle common test setup, including map initialization and idling resources (WIP).
- Update `GroundOverlayDemoActivityTest` to extend `MapDemoActivityTest`, simplifying its setup logic.
- Replace `Thread.sleep()` calls with `idlingResource.waitForIdle()`.
- Update `GroundOverlayDemoActivity` to use `lateinit` for overlays, preventing nullable types.
- Add `maps-utils-ktx` dependency for idiomatic spherical offset calculations.
Enhances the `testLongClickAddsNewCircle` in `CircleDemoActivityTest` by adding
detailed comments that explain the test's flow and the critical reason for
camera movements: to minimize precision loss during LatLng to screen coordinate
conversion for accurate click event simulation.

This commit also includes the necessary `LatLngSubject` extension, introducing
a `isWith(tolerance).of(target)` assertion. This new assertion allows for
more precise and readable verification of spherical distances between `LatLng`
objects, directly supporting the improved test logic in `CircleDemoActivityTest`.
Refactors the java version of `GroundOverlayDemoActivityTest` to extend the common `MapDemoActivityTest` base class. This change centralizes test setup, teardown, and map interaction logic, removing redundant code and improving maintainability.

- `GroundOverlayDemoActivity` now implements the `MapProvider` interface to correctly expose the map instance and its ready state to the test framework.
- Replaces `Thread.sleep()` calls in `CircleDemoActivityTest` and `GroundOverlayDemoActivityTest` with `idlingResource.waitForIdle()`. This creates more reliable tests by waiting for map events to complete rather than using fixed delays.
- Adds a `waitForIdle(long)` overload to `MapIdlingResource` to handle cases requiring a brief, controlled pause after the map has settled.
This commit applies several small refactorings to the Kotlin demo activities, focusing on code modernization and resolving lint issues.

- In `MarkerDemoActivity`, replaces `Math.max()` with the more idiomatic Kotlin function `coerceAtLeast()`.
- Simplifies trigonometric function calls by using top-level `sin` and `cos` from the Kotlin standard library instead of `Math`.
- Removes unused imports from `MarkerDemoActivity`.
- In `LayersDemoActivity`, adds `@SuppressLint("MissingPermission")` to the `onMyLocationToggled` method to address a lint warning.
@dkhawk dkhawk requested a review from kikoso November 5, 2025 01:11
This commit updates the layout files to use the `android:name` attribute instead of the `class` attribute for `FragmentContainerView` to declare the fragment class. This is the modern, recommended practice.

It also removes minor whitespace and `onClick` attributes that are no longer necessary.

- Replaces the `class` attribute with `android:name` for all instances of `SupportMapFragment` and `SupportStreetViewPanoramaFragment` within `FragmentContainerView` tags across multiple demo layouts.
- Removes unnecessary `onClick` attributes from checkboxes in `layers_demo.xml`.
- Cleans up minor whitespace in `camera_clamping_demo.xml`, `street_view_panorama_navigation_demo.xml`, `indoor_demo.xml`, and `ground_overlay_demo.xml`.
@dkhawk dkhawk enabled auto-merge (squash) November 5, 2025 13:15
@RunWith(AndroidJUnit4.class)
public class CircleDemoActivityTest extends MapDemoActivityTest<CircleDemoActivity> {

private static final LatLng HERE_BE_DRAGONS = new LatLng(-34.91459615762562, 138.60572619793246);
Copy link
Collaborator

Choose a reason for hiding this comment

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

:)

@Test
public void testVisibleLevelInfo() {
try {
Thread.sleep(200);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a bit of an anti-pattern, but even using the IdlingResource, if the Thread.sleep() gets removed doesn't always work. I don't think there is a better solution.

private float mMaxZoom;

private TextView mCameraTextView;
private com.example.common_ui.databinding.CameraClampingDemoBinding binding;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can probably move this into an import.

@kikoso
Copy link
Collaborator

kikoso commented Nov 5, 2025

One small comment regarding some classes on the PR: we can probably use imports instead of FQDN when importing a class or attribute.

@kikoso
Copy link
Collaborator

kikoso commented Nov 6, 2025

Looks good. We were discussing whether it could be worth moving some components to a common module, but for now we can merge this PR.

@dkhawk dkhawk merged commit dfa18d2 into main Nov 6, 2025
12 checks passed
@dkhawk dkhawk deleted the fix/move-some-onclicks-out-of-xml-files branch November 6, 2025 15:09
googlemaps-bot pushed a commit that referenced this pull request Nov 6, 2025
# [1.19.0](v1.18.0...v1.19.0) (2025-11-06)

### Features

* migrate onClick out of the layout files, add testing ([#2328](#2328)) ([dfa18d2](dfa18d2))
@googlemaps-bot
Copy link
Contributor

🎉 This PR is included in version 1.19.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants