Skip to content

feat(demo): edge-to-edge insets, editable fields, mock location, and UX polish - #118

Open
dkhawk wants to merge 1 commit into
feat/places-usage-attributionfrom
demo/ux-edge-to-edge-polish
Open

feat(demo): edge-to-edge insets, editable fields, mock location, and UX polish#118
dkhawk wants to merge 1 commit into
feat/places-usage-attributionfrom
demo/ux-edge-to-edge-polish

Conversation

@dkhawk

@dkhawk dkhawk commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR refactors and polishes the demo application (:places-compose-demo):

Highlights:

  1. Edge-to-Edge & System Insets: Updated MainActivity, PlacesAutocompleteMinimalActivity, LandmarkSelectionActivity, and CommonScreen for full edge-to-edge rendering with Scaffold padding and Material 3 TopAppBar styling.
  2. Editable Form Fields & Soft Keyboard: Wired bidirectional state updates on address forms so users can edit fields and trigger the virtual keyboard smoothly.
  3. Mock Location Fallback: Added a 1-tap "Use Mock Location Instead" fallback to the location permissions screen, enabling rapid emulator/device verification without needing GPS fixes.
  4. Address Descriptors & Landmark UX: Added descriptive empty-state guidance and quick-jump Bangalore KTPO demo shortcut for testing Address Descriptors in supported regions.
  5. Lint Cleanup: Fixed PluralsCandidate, removed redundant activity labels, and moved legacy drawable assets.

📚 Stacked PR Chain

# PR Description Base Status
1 #115 ci/workflow-security-hardening (Workflow permissions & action pinning) main ✅ CI Passing
2 #116 test/library-coverage-baseline (Test suite reaching 93.8% coverage) #115 ✅ CI Passing
3 #117 build/dependency-modernization (AGP 9.3.2, Gradle 9.5.1, Places 5.3.0) #116 ✅ CI Passing
4 #119 feat/places-usage-attribution (Usage attribution & remove :library) #117 ✅ CI Passing
5 👉 #118 demo/ux-edge-to-edge-polish (Demo UX, edge-to-edge, mock location) #119 ✅ CI Passing

Umbrella PR: #109

@dkhawk
dkhawk requested a review from LoyalAbbas September 3, 2026 19:49
@googlemaps-bot

googlemaps-bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Code Coverage

Overall Project 86.5% 🍏

There is no coverage information present for the Files changed

NextLocationButton(
isSelected = commonViewState.buttonStates.mockLocation == ButtonState.SELECTED
) {
onEvent(CommonEvent.OnNextMockLocation)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

onEvent(CommonEvent.OnNextMockLocation) updates the state asynchronously in CommonViewModel, but commonViewState.locationLabel is read synchronously in the same click frame before Compose recomposes. As a result, the Snackbar displays the previous location's name (or "Mock Location" on first tap) instead of the newly selected location.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated! Resolved by decoupling the Snackbar display from synchronous Compose state reads:

  1. MergedLocationRepository.nextMockLocation() now returns the selected mock location label directly.
  2. CommonViewModel emits the message via snackbarMessage: SharedFlow<String>.
  3. CommonScreen collects this flow in a LaunchedEffect(Unit) to show the Snackbar, eliminating the race condition with recomposition and always displaying the newly selected location label immediately.

@dkhawk
dkhawk requested a review from a team as a code owner September 9, 2026 21:21
@dkhawk
dkhawk force-pushed the demo/ux-edge-to-edge-polish branch from 85c6c96 to fcc3170 Compare September 9, 2026 22:26
Comment on lines +16 to +45
<vector xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:aapt="http://schemas.android.com/aapt"
android:width="108dp"
android:height="108dp"
android:viewportWidth="108"
android:viewportHeight="108">
<path android:pathData="M31,63.928c0,0 6.4,-11 12.1,-13.1c7.2,-2.6 26,-1.4 26,-1.4l38.1,38.1L107,108.928l-32,-1L31,63.928z">
<aapt:attr name="android:fillColor">
<gradient
android:endX="85.84757"
android:endY="92.4963"
android:startX="42.9492"
android:startY="49.59793"
android:type="linear">
<item
android:color="#44000000"
android:offset="0.0" />
<item
android:color="#00000000"
android:offset="1.0" />
</gradient>
</aapt:attr>
</path>
<path
android:fillColor="#FFFFFF"
android:fillType="nonZero"
android:pathData="M65.3,45.828l3.8,-6.6c0.2,-0.4 0.1,-0.9 -0.3,-1.1c-0.4,-0.2 -0.9,-0.1 -1.1,0.3l-3.9,6.7c-6.3,-2.8 -13.4,-2.8 -19.7,0l-3.9,-6.7c-0.2,-0.4 -0.7,-0.5 -1.1,-0.3C38.8,38.328 38.7,38.828 38.9,39.228l3.8,6.6C36.2,49.428 31.7,56.028 31,63.928h46C76.3,56.028 71.8,49.428 65.3,45.828zM43.4,57.328c-0.8,0 -1.5,-0.5 -1.8,-1.2c-0.3,-0.7 -0.1,-1.5 0.4,-2.1c0.5,-0.5 1.4,-0.7 2.1,-0.4c0.7,0.3 1.2,1 1.2,1.8C45.3,56.528 44.5,57.328 43.4,57.328L43.4,57.328zM64.6,57.328c-0.8,0 -1.5,-0.5 -1.8,-1.2s-0.1,-1.5 0.4,-2.1c0.5,-0.5 1.4,-0.7 2.1,-0.4c0.7,0.3 1.2,1 1.2,1.8C66.5,56.528 65.6,57.328 64.6,57.328L64.6,57.328z"
android:strokeWidth="1"
android:strokeColor="#00000000" />
</vector> No newline at end of file
)
queue.add(stringRequest)
return try {
suspendCancellableCoroutine { cont ->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Coroutine Cancellation & RequestQueue Leak in GeocoderRepository.reverseGeocode()

Issues:
Thread/Memory Leak: Volley.newRequestQueue(context) creates a new thread pool (CacheDispatcher + 4 NetworkDispatcher threads) on every single geocoding request and never stops them. RequestQueue should be initialized once as an instance property on GeocoderRepository (e.g. private val requestQueue by lazy { Volley.newRequestQueue(context.applicationContext) }).

Missing Cancellation Hook: Add cont.invokeOnCancellation { stringRequest.cancel() } inside suspendCancellableCoroutine so cancelled coroutines cancel the underlying HTTP request.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated! Both issues resolved:

  1. Initialized RequestQueue once on GeocoderRepository via private val requestQueue by lazy { Volley.newRequestQueue(context.applicationContext) }, avoiding creating thread pools on each geocode request.
  2. Added cont.invokeOnCancellation { stringRequest.cancel() } inside suspendCancellableCoroutine so cancelled coroutines properly cancel the underlying Volley request.

@dkhawk
dkhawk force-pushed the demo/ux-edge-to-edge-polish branch from fcc3170 to 50fde82 Compare September 10, 2026 19:43

@LoyalAbbas LoyalAbbas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

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.

4 participants