Factor out job cards into Composables for new data entry points #2892

sufyanAbbasi · 2024-12-02T20:51:58Z

Fixes #2446

Reimplements the entire add LOI/selection UX to match the new designs (see above issue).

Removes the old RecycleView and Cards UI and rebuilds them using Composables instead.
Handles multiple ad-hoc jobs by showing each job in a modal and does not show the modal if there is only one ad-hoc job.
Also does not show the "+" button or the "add data" button when the user is viewer only.
TODO: Get translations for add_data and add_site.

@gino-m PTAL?

gino-m

Hey @sufyanAbbasi so cool to see this is working! A few suggestions/tweaks. Once we do those, I think we can submit as is without too much UI churn for now. @rawbzz Is still finalizing the latest designs, but what you have here takes us much closer.

ground/src/main/res/layout/add_loi_card_item.xml

ground/src/main/java/com/google/android/ground/ui/theme/Color.kt

ground/src/main/res/values/strings.xml

ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/cards/JobSelectionDialog.kt

...c/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt

# Conflicts: # ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt

codecov · 2024-12-19T10:11:59Z

Codecov Report

Attention: Patch coverage is 23.07692% with 210 lines in your changes missing coverage. Please review.

Project coverage is 62.41%. Comparing base (a0975c3) to head (c4be97e).

Files with missing lines	Patch %	Lines
.../ground/ui/home/mapcontainer/jobs/JobMapAdapter.kt	20.67%	175 Missing and 13 partials ⚠️
...ome/mapcontainer/HomeScreenMapContainerFragment.kt	21.05%	10 Missing and 5 partials ⚠️
...me/mapcontainer/HomeScreenMapContainerViewModel.kt	50.00%	3 Missing and 4 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2892      +/-   ##
============================================
- Coverage     63.55%   62.41%   -1.14%     
- Complexity     1267     1283      +16     
============================================
  Files           270      271       +1     
  Lines          6560     6769     +209     
  Branches        927      950      +23     
============================================
+ Hits           4169     4225      +56     
- Misses         1786     1921     +135     
- Partials        605      623      +18

Files with missing lines	Coverage Δ
.../mapcontainer/jobs/DataCollectionEntryPointData.kt	`100.00% <100.00%> (ø)`
...n/java/com/google/android/ground/ui/theme/Color.kt	`100.00% <100.00%> (ø)`
...me/mapcontainer/HomeScreenMapContainerViewModel.kt	`51.94% <50.00%> (+6.11%)`	⬆️
...ome/mapcontainer/HomeScreenMapContainerFragment.kt	`15.60% <21.05%> (-1.07%)`	⬇️
.../ground/ui/home/mapcontainer/jobs/JobMapAdapter.kt	`20.67% <20.67%> (ø)`

... and 2 files with indirect coverage changes

gino-m

Great work, @sufyanAbbasi 🥳 A few nits, otherwise LGTM!

...c/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt

ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/jobs/MapUiData.kt

…ound-android into sufy/2446/new-loi-flow

Added "add_site" and two more strings (untranslated)

Remaining translations added ("unnamed_job" and "job_site_icon").

gino-m · 2024-12-20T14:15:01Z

Codecov Report

Attention: Patch coverage is 20.43796% with 218 lines in your changes missing coverage. Please review.

Project coverage is 61.23%. Comparing base (543795e) to head (55393d8).

Files with missing lines Patch % Lines
.../ground/ui/home/mapcontainer/jobs/JobMapAdapter.kt 20.58% 176 Missing and 13 partials ⚠️
...ome/mapcontainer/HomeScreenMapContainerFragment.kt 26.31% 9 Missing and 5 partials ⚠️
...me/mapcontainer/HomeScreenMapContainerViewModel.kt 7.14% 13 Missing ⚠️
...roid/ground/ui/home/mapcontainer/jobs/MapUiData.kt 0.00% 2 Missing ⚠️
Additional details and impacted files

Would you be able to add tests for modified lines? We can focus on the ones reported here by codecov if it helps.

…ound-android into sufy/2446/new-loi-flow

gino-m · 2025-01-26T19:10:33Z

@sufyanAbbasi RFAL?

sufyanAbbasi · 2025-01-27T06:46:54Z

Sorry I had to take a little time to merge in the ground --> app file change and added a test file to cover the core view model logic. I think we're good to go!

gino-m · 2025-01-27T15:02:24Z

Hi @shobhitagarwal1612! PTAL?

shobhitagarwal1612 · 2025-01-28T07:47:37Z

...ain/java/com/google/android/ground/ui/home/mapcontainer/jobs/DataCollectionEntryPointData.kt

@@ -13,15 +13,16 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-package com.google.android.ground.ui.home.mapcontainer.cards
+
+package com.google.android.ground.ui.home.mapcontainer.jobs

 import com.google.android.ground.model.job.Job
 import com.google.android.ground.model.locationofinterest.LocationOfInterest

 /** Data classes used to populate the Map cards (either an Loi card, or a Suggest Loi card). */


Please update the ktdoc

shobhitagarwal1612 · 2025-01-28T07:49:17Z

.../main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerViewModel.kt

+      if (loiCard == null && feature != null) {
+        // The feature is not in view anymore.
+        featureClicked.value = null


It feels like the resetting logic should live somewhere else. This method should only return the data and shouldn't do anything other than that.

app/src/main/java/com/google/android/ground/ui/home/mapcontainer/jobs/JobMapAdapter.kt

shobhitagarwal1612 · 2025-01-28T07:58:14Z

app/src/main/java/com/google/android/ground/ui/home/mapcontainer/jobs/JobMapAdapter.kt

+      basemapLayoutBinding.mapTypeBtn.show()
+      basemapLayoutBinding.locationLockBtn.show()
+      menuBinding.hamburgerBtn.show()


In order to make these class pure composable objects and remove coupling from existing layout elements, can we instead expose states which can then be handled in the caller class (HomescreenMapContainerFragment)?

Something like onDismissed or anything else

app/src/main/java/com/google/android/ground/ui/map/gms/GoogleMapsFragment.kt

app/src/main/res/drawable/ic_ring_marker.xml

sufyanAbbasi added 2 commits November 14, 2024 16:10

Add new LOI flow (unstyled)

1663746

Merge branch 'master' into sufy/2446/new-loi-flow

f8654dc

sufyanAbbasi requested a review from gino-m December 2, 2024 20:51

gino-m changed the title ~~[Draft - Needs Styling] Implements new, simplified LOI flow~~ [Draft - Needs Styling] Introduce new data collection entry points Dec 4, 2024

gino-m requested changes Dec 6, 2024

View reviewed changes

sufyanAbbasi added 3 commits December 6, 2024 16:23

Refactor out cards

f5fec71

Merge branch 'master' into sufy/2446/new-loi-flow

5858af2

# Conflicts: # ground/src/main/java/com/google/android/ground/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt

Refactor cards into Composables.

5aa9496

sufyanAbbasi changed the title ~~[Draft - Needs Styling] Introduce new data collection entry points~~ Factor our job cards into Composables for new data entry points Dec 19, 2024

Merge branch 'master' into sufy/2446/new-loi-flow

4d0d5e6

sufyanAbbasi marked this pull request as ready for review December 19, 2024 09:47

auto-assign bot requested a review from gino-m December 19, 2024 09:47

sufyanAbbasi requested a review from anandwana001 December 19, 2024 09:48

sufyanAbbasi added 2 commits December 19, 2024 01:50

Formatting.

34dc21e

Address checkCode issues.

5c420e8

sufyanAbbasi changed the title ~~Factor our job cards into Composables for new data entry points~~ Factor out job cards into Composables for new data entry points Dec 19, 2024

Merge branch 'master' into sufy/2446/new-loi-flow

55393d8

gino-m requested changes Dec 19, 2024

View reviewed changes

sufyanAbbasi and others added 11 commits December 19, 2024 12:03

Remove debug logs.

7e66573

Merge branch 'sufy/2446/new-loi-flow' of https://github.com/google/gr…

2581739

…ound-android into sufy/2446/new-loi-flow

Rename MapUIData and fields.

6630320

Merge branch 'master' into sufy/2446/new-loi-flow

f91494f

Update strings.xml

ac03e37

Update strings.xml

a6a5838

Update strings.xml

b0eb253

Added "add_site" and two more strings (untranslated)

Update strings.xml

cce58a5

Added "add_site" and two more strings (untranslated)

Update strings.xml

da4a801

Remaining translations added ("unnamed_job" and "job_site_icon").

Update strings.xml

d28e429

Remaining translations added ("unnamed_job" and "job_site_icon").

Update strings.xml

0e30070

Remaining translations added ("unnamed_job" and "job_site_icon").

sufyanAbbasi added 5 commits January 24, 2025 12:16

Merge branch 'master' into sufy/2446/new-loi-flow

fddb470

Add missing import

6a64f25

Merge branch 'sufy/2446/new-loi-flow' of https://github.com/google/gr…

4466139

…ound-android into sufy/2446/new-loi-flow

Formatting

2705df0

Merge branch 'master' into sufy/2446/new-loi-flow

818e354

Add tests for new job logic.

c4be97e

sufyanAbbasi requested a review from shobhitagarwal1612 January 27, 2025 15:54

shobhitagarwal1612 requested changes Jan 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Factor out job cards into Composables for new data entry points #2892

Factor out job cards into Composables for new data entry points #2892

sufyanAbbasi commented Dec 2, 2024 •

edited by gino-m

Loading

gino-m left a comment

codecov bot commented Dec 19, 2024 •

edited

Loading

gino-m left a comment

gino-m commented Dec 20, 2024

Codecov Report

gino-m commented Jan 26, 2025

sufyanAbbasi commented Jan 27, 2025

gino-m commented Jan 27, 2025

shobhitagarwal1612 Jan 28, 2025

shobhitagarwal1612 Jan 28, 2025

shobhitagarwal1612 Jan 28, 2025

Factor out job cards into Composables for new data entry points #2892

Are you sure you want to change the base?

Factor out job cards into Composables for new data entry points #2892

Conversation

sufyanAbbasi commented Dec 2, 2024 • edited by gino-m Loading

gino-m left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 19, 2024 • edited Loading

Codecov Report

gino-m left a comment

Choose a reason for hiding this comment

gino-m commented Dec 20, 2024

Codecov Report

gino-m commented Jan 26, 2025

sufyanAbbasi commented Jan 27, 2025

gino-m commented Jan 27, 2025

shobhitagarwal1612 Jan 28, 2025

Choose a reason for hiding this comment

shobhitagarwal1612 Jan 28, 2025

Choose a reason for hiding this comment

shobhitagarwal1612 Jan 28, 2025

Choose a reason for hiding this comment

sufyanAbbasi commented Dec 2, 2024 •

edited by gino-m

Loading

codecov bot commented Dec 19, 2024 •

edited

Loading