-
Notifications
You must be signed in to change notification settings - Fork 139
Migrate MapType bottom sheet dialog to Jetpack Compose #3508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The `MapTypeDialogFragment` has been refactored to use Compose instead of XML view binding. Key changes include: - Replaced the view-based layout and `MapTypeAdapter` with `MapTypeScreen` in `MapTypeDialogFragment`. - Updated `MapTypeViewModel` to expose `mapTypeFlow` and `offlineImageryEnabledFlow` as `StateFlow` for Compose consumption. - Updated `MapTypeDialogFragmentTest` to use Compose testing APIs instead of Espresso. - Renamed `offlineImageryPreferenceUpdated` to `updateOfflineImageryPreference` in the ViewModel.
…ed XML layouts Deletes the RecyclerView-based map type selection components, including: - `AdaptiveSpacingItemDecorator.kt` - `MapTypeAdapter.kt` - `map_type_dialog_fragment.xml` - `map_type_dialog_item.xml`
Replaces the Fragment-based `MapTypeDialogFragment` with a Compose-based `MapTypeScreen` using Material3 `ModalBottomSheet`.
Deleted `MapTypeDialogFragment` and its associated tests, removing the fragment-based implementation of the map type selector. References to the dialog were also removed from the navigation graph. The `MapTypeScreen` Composable now serves as the primary UI component for selecting map types.
…ComposeView` utility helper, removing redundant `ComposeView` boilerplate and `AppTheme` wrapping.
Adds `verticalScroll` to the main `Column` in `MapTypeScreen` to ensure content remains accessible on smaller screens or when scaled.
Extract `BasemapGallery` and `OfflineImageryToggle` into separate private composable functions to improve code readability and maintainability.
Ensures map type updates and dialog dismissals are processed by calling `composeTestRule.waitForIdle()` after click actions in tests.
Summary of ChangesHello @shobhitagarwal1612, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring of the map type selection feature, transitioning its user interface from a traditional Android Fragment and XML layout approach to a modern Jetpack Compose implementation. This change simplifies the UI codebase, improves component reusability, and aligns the project with contemporary Android development practices. The underlying logic for map type and offline imagery selection has been adapted to integrate seamlessly with the new Compose UI, utilizing reactive state flows for efficient data management. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully migrates the MapType bottom sheet dialog from XML and Fragments to Jetpack Compose. The changes are well-executed, resulting in cleaner, more maintainable code that aligns with modern Android development practices. The removal of the old MapTypeDialogFragment, its associated layouts, adapter, and navigation graph entries simplifies the codebase. The new BasemapSelectorScreen is a great example of building UI with Compose, and the accompanying BasemapSelectorViewModel correctly uses StateFlow to manage state. The addition of unit tests for the new composable is also a valuable contribution. I have one minor suggestion regarding modifier ordering for better correctness.
app/src/main/java/org/groundplatform/android/ui/basemapselector/BasemapSelectorScreen.kt
Outdated
Show resolved
Hide resolved
…pSelectorScreen` Refactor the map type selection logic into a standalone function within `BasemapSelectorScreen` to improve readability of the `ModalBottomSheet` content.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3508 +/- ##
============================================
+ Coverage 69.82% 70.03% +0.21%
- Complexity 1599 1600 +1
============================================
Files 322 322
Lines 8678 8783 +105
Branches 949 962 +13
============================================
+ Hits 6059 6151 +92
- Misses 2049 2054 +5
- Partials 570 578 +8
🚀 New features to boost your workflow:
|
Fixes #3507
map.type.dialog.-.before.webm
map.type.dialog.-.after.webm
@andreia-ferreira @gino-m PTAL?