Fix issue 3541, Can't complete polygon after getting an interception error popup#3606
Fix issue 3541, Can't complete polygon after getting an interception error popup#3606hassan-nsubuga wants to merge 22 commits intomasterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3606 +/- ##
============================================
+ Coverage 68.81% 68.93% +0.12%
- Complexity 1826 1831 +5
============================================
Files 355 355
Lines 8577 8578 +1
Branches 1076 1076
============================================
+ Hits 5902 5913 +11
+ Misses 2037 2031 -6
+ Partials 638 634 -4
🚀 New features to boost your workflow:
|
Summary of ChangesHello, 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 addresses a critical bug preventing users from completing a polygon after a self-intersection error. By ensuring the 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. Changelog
Activity
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.
Code Review
The DrawAreaTaskViewModel was updated to ensure that when a self-intersecting polygon's last vertex is removed, the hasSelfIntersection flag is explicitly reset to false within the updateVertices method, which is now consistently used for all vertex list modifications.
01e9e74 to
9ece050
Compare
…nd refactor checkVertexIntersection to call updateVertices
…fIntersection if it causes self intersection non completion when self intersection happened
9ece050 to
5cb0e85
Compare
…3610) Bumps [com.google.maps.android:android-maps-utils](https://github.com/googlemaps/android-maps-utils) from 4.1.0 to 4.1.1. - [Release notes](https://github.com/googlemaps/android-maps-utils/releases) - [Changelog](https://github.com/googlemaps/android-maps-utils/blob/main/CHANGELOG.md) - [Commits](googlemaps/android-maps-utils@v4.1.0...v4.1.1) --- updated-dependencies: - dependency-name: com.google.maps.android:android-maps-utils dependency-version: 4.1.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andreia Ferreira <51242456+andreia-ferreira@users.noreply.github.com>
* chore: cleanup and tooling updates * feat: add HomeDrawer component and ViewModel logic * chore: remove build output file * Clean up PR * refactor(home): migrate drawer to Compose and remove XML * Restore deleted files * fix(home): add system bars padding to drawer * refactor: clean up unused imports and reformat Compose state observation calls. * feat: move version text display from a clickable drawer item to a static row at the bottom of the drawer * Update app/src/main/java/org/groundplatform/android/ui/home/HomeDrawer.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Update app/src/main/java/org/groundplatform/android/ui/home/HomeDrawer.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Fix checkCode issues: Refactor HomeDrawer/HomeScreenFragment, fix formatting and unused resources * Refactor HomeDrawer and fix HomeScreenFragmentTest - Refactor HomeScreenFragment to reduce method length - Refactor HomeDrawer to extract composables - Migrate HomeScreenViewModel dialog state to StateFlow - Initialize WorkManager in HomeScreenFragmentTest - Ignore flaky sign out dialog test in Robolectric - Fix static analysis issues (detekt, ktfmt) * Fix checkCode issues and test regressions * Fix ktfmt formatting issues * Refactor HomeDrawer to use HomeDrawerAction interface - Introduce `HomeDrawerAction` sealed interface to represent navigation and sign-out events. - Update `HomeDrawer` to expose a single `onAction` callback instead of multiple parameters. - Consolidate navigation logic in `HomeScreenFragment` within the `onAction` handler. * Make user parameter non-null in HomeDrawer * refactor: remove unnecessary comments from `openSignOutWarningDialog` helper. * Refactor HomeDrawerState into ViewModel * test: assert build version string with BuildConfig.VERSION_NAME in HomeScreenFragmentTest. * Replace Glide with Coil AsyncImage in HomeDrawer * Fix duplicate coil-compose in libs.versions.toml * Address PR #3554 feedback * Fix UI Home Drawer regressions - Restored original icons - Reordered items - Fixed typography for App title and list items - Fixed AppTheme import issue * Fix UI Home Drawer dividers - Apply 24dp end padding to drawer divider to match original design * Remove unused style * Refactor version constant * Fix UI Home Drawer typography and padding - Apply 24dp horizontal padding to drawer divider - Use titleSmall typography for drawer navigation items to match original NavigationView styles * Fix exact CSS layout spacings and typography - Matched AppInfoHeader paddings and SurveySelector gaps exactly to FIGMA CSS design specs - Replaced titleSmall with labelLarge (Manrope 600) for navigation items * Complete typography with explicit Google Sans and Manrope fonts per CSS * Suppress LongMethod detekt check on SurveySelector * Restore user photo size to 32dp * Restore Font Weights for Survey Selector and Nav Items per original CSS * Use typography instead of font literals * Use existing Typography styles --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… hasSelfIntersection to StateFlow and refactor code to use the new state, create two new methods to reset the variables. Refactor DrawAreaTaskFragment to show the dialog after screen rotation and call dismissSelfIntersectionDialog appropriately.
…nges created in DrawAreaTaskFragment and DrawAreaTaskViewModel on the new state and new methods
| // var hasSelfIntersection: Boolean = false | ||
| // private set |
There was a problem hiding this comment.
unused code can be removed
| refreshMap() | ||
| } | ||
|
|
||
| fun resetHasSelfIntersection() { |
There was a problem hiding this comment.
this is only used inside the VM so it can be private
| @@ -203,7 +208,12 @@ internal constructor( | |||
| @VisibleForTesting fun getLastVertex() = vertices.lastOrNull() | |||
|
|
|||
| private fun onSelfIntersectionDetected() { | |||
There was a problem hiding this comment.
suggestion: not related to this PR, so feel free to resolve this commen, but maybe a clearer name for this method would be showSelfIntersectionDialog
|
|
||
| override fun setUp() { | ||
| super.setUp() | ||
| Dispatchers.setMain(UnconfinedTestDispatcher()) |
There was a problem hiding this comment.
when overriding the main test dispatcher, it should be reset otherwise it may leak into other tests. I suggest adding the following to the test file to prevent it:
@After
fun tearDown() {
Dispatchers.resetMain()
}
- Refactor data collection task fragments to use Compose-based layout - Replace `TaskView` and `TaskViewFactory` with a new `TaskViewLayout` Compose component. - Remove `onCreateTaskView` from `AbstractTaskFragment` and subclasses, replacing it with a `taskHeader` property. - Update `AbstractTaskFragment` to use `createComposeView` for the entire fragment view, integrating header, body, and footer into a unified `TaskViewLayout`. - Migrated all task fragments (Text, Date, Number, Photo, MultipleChoice, Time, Instruction, DrawArea, DropPin, CaptureLocation) to the new layout system. - Introduce `Header` data class to handle task labels and optional icons in a standardized way. - Refactor LoiNameDialog to be a Composable function within AbstractTaskFragment - Move `LoiNameDialog` logic from `renderComposableDialog` into a dedicated `@Composable` function. - Update `onCreateView` to include `LoiNameDialog` when the task is an "Add LOI" task. - Simplify `handleNext` to trigger the dialog via the `loiNameDialogOpen` state. - Consolidate dialog state and value handling within the new `LoiNameDialog` component. - Refactor instructions dialog to use Compose state and centralized layout - Refactor the self-intersection dialog in `DrawAreaTaskFragment` to use Compose state - Refactor PhotoTaskFragment to handle permission dialogs using Compose state - Refactor location permission dialog in CaptureLocationTaskFragment to use Compose - Refactor `InstructionsDialog` logic into a dedicated Composable in `AbstractTaskFragment` - Remove unused xml layouts and associated views - Use compose to reposition the progress bar - Update `AbstractTaskFragment` to track footer position when keyboard visibility changes --------- Co-authored-by: Andreia Ferreira <51242456+andreia-ferreira@users.noreply.github.com> # Conflicts: # app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskFragment.kt # app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt
- Replace hardcoded text styles (font size, weight, family, line height) with standard `MaterialTheme.typography` styles (`titleMedium`, `labelMedium`, `bodyMedium`, `labelLarge`). - Clean up imports
Bumps `kotlinVersion` from 2.3.10 to 2.3.20. Updates `org.jetbrains.kotlin:kotlin-gradle-plugin` from 2.3.10 to 2.3.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v2.3.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v2.3.10...v2.3.20) Updates `org.jetbrains.kotlin:kotlin-reflect` from 2.3.10 to 2.3.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v2.3.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v2.3.10...v2.3.20) Updates `org.jetbrains.kotlin:kotlin-serialization` from 2.3.10 to 2.3.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v2.3.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v2.3.10...v2.3.20) Updates `org.jetbrains.kotlin:kotlin-stdlib` from 2.3.10 to 2.3.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v2.3.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v2.3.10...v2.3.20) Updates `org.jetbrains.kotlin:kotlin-test` from 2.3.10 to 2.3.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v2.3.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v2.3.10...v2.3.20) Updates `org.jetbrains.kotlin.plugin.compose` from 2.3.10 to 2.3.20 Updates `org.jetbrains.kotlin.android` from 2.3.10 to 2.3.20 Updates `org.jetbrains.kotlin.plugin.serialization` from 2.3.10 to 2.3.20 Updates `org.jetbrains.kotlin.multiplatform` from 2.3.10 to 2.3.20 --- updated-dependencies: - dependency-name: org.jetbrains.kotlin:kotlin-gradle-plugin dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin:kotlin-reflect dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin:kotlin-serialization dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin:kotlin-stdlib dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin:kotlin-test dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin.plugin.compose dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin.android dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin.plugin.serialization dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin.multiplatform dependency-version: 2.3.20 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andreia Ferreira <51242456+andreia-ferreira@users.noreply.github.com>
Bumps androidx.compose:compose-bom from 2026.02.01 to 2026.03.00. --- updated-dependencies: - dependency-name: androidx.compose:compose-bom dependency-version: 2026.03.00 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andreia Ferreira <51242456+andreia-ferreira@users.noreply.github.com>
Bumps gradle-wrapper from 9.4.0 to 9.4.1. --- updated-dependencies: - dependency-name: gradle-wrapper dependency-version: 9.4.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andreia Ferreira <51242456+andreia-ferreira@users.noreply.github.com>
… hasSelfIntersection to StateFlow and refactor code to use the new state, create two new methods to reset the variables. Refactor DrawAreaTaskFragment to show the dialog after screen rotation and call dismissSelfIntersectionDialog appropriately. # Conflicts: # app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskFragment.kt # app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt
…e, remove un-used code to clean file
… Update the ConfirmationDialog to call viewModel.dismissSelfIntersectionDialog and clean unused code
complete.polygon.after.inter.section.error.mp4
Fixes #3541
Can't complete polygon after getting an interception error popup, this happens after getting the intersection error and it's not possible to complete a polygon unless a new point is added.
Reset hasSelfIntersection to false after an intersection error to enable the completion button in order to complete a polygon without adding a new point
@... PTAL?