-
Notifications
You must be signed in to change notification settings - Fork 121
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
Cleanup remaining usages of getDetailsText() from TaskData #2950
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2950 +/- ##
============================================
+ Coverage 61.90% 62.25% +0.35%
+ Complexity 1204 1200 -4
============================================
Files 268 265 -3
Lines 6441 6419 -22
Branches 897 890 -7
============================================
+ Hits 3987 3996 +9
+ Misses 1898 1867 -31
Partials 556 556
|
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.
Ah, finally!!! I've dreamt of this moment for 2+ years 😆
Would it be possible to add tests to the ViewModelTests to check this presentation logic while you're at it?
…e hilt injection problem in unit tests as it allows sharing the navcontroller
b944ca7
to
0938f68
Compare
…user visible text
…ing the coordinates and not the geometry
… capture location task)
…n TimeTaskFragmentTest
Done. PTAL? |
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.
LGTM, just a few more nits. Thanks!
...src/main/java/com/google/android/ground/ui/datacollection/tasks/point/DropPinTaskFragment.kt
Show resolved
Hide resolved
.../main/java/com/google/android/ground/ui/datacollection/tasks/polygon/DrawAreaTaskFragment.kt
Show resolved
Hide resolved
ground/src/main/java/com/google/android/ground/ui/map/gms/ConverterExt.kt
Outdated
Show resolved
Hide resolved
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.
🥳
Fixes #1733
getDetailsText() was previously used for displaying value of response for the current task fragment. This method is extended by all response model classes (TaskData). All text should be created on view layer and not is model due to i18n.
In this PR, I've migrated all remaining usages of getDetailsText() with appropriate values and cleaned up all unused methods.
This is a pure refactor and doesn't change any existing user behavior.
Verified that this doesn't introduce any regressions via unit tests and manually running the app in an emulator.
@gino-m PTAL?