-
Notifications
You must be signed in to change notification settings - Fork 19
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
chore: simplify passing arguments to scoped ViewModels #2023
Conversation
Build 758 failed. |
APKs built during tests are available here. Scroll down to Artifacts! |
Build 759 succeeded. The build produced the following APK's: |
Codecov Report
@@ Coverage Diff @@
## develop #2023 +/- ##
=============================================
+ Coverage 38.64% 39.03% +0.38%
- Complexity 899 914 +15
=============================================
Files 288 303 +15
Lines 11409 11150 -259
Branches 1473 1492 +19
=============================================
- Hits 4409 4352 -57
+ Misses 6575 6374 -201
+ Partials 425 424 -1
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
APKs built during tests are available here. Scroll down to Artifacts! |
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.
Wow, nice improvement 👏 Can you just add documentation how to use app/src/main/kotlin/com/wire/android/di/ViewModelScoped.kt in composable and in view model? Also we can update documentation ADR https://wearezeta.atlassian.net/wiki/spaces/AR/pages/812449806/24.05.2023+Divide+big+view+models+to+smaller
APKs built during tests are available here. Scroll down to Artifacts! |
…' into chore/simplify_scoped_viewmodels
Build 814 failed. |
…ped_viewmodels # Conflicts: # app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButton.kt # gradle/libs.versions.toml
APKs built during tests are available here. Scroll down to Artifacts! |
Build 849 succeeded. The build produced the following APK's: |
…ped_viewmodels # Conflicts: # app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButton.kt # app/src/main/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModel.kt # app/src/main/kotlin/com/wire/android/ui/home/conversations/CompositeMessageViewModel.kt # app/src/test/kotlin/com/wire/android/ui/connection/ConnectionActionButtonViewModelTest.kt # app/src/test/kotlin/com/wire/android/ui/home/conversations/CompositeMessageViewModelTest.kt
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.
Really cool and useful staff!!!!
But will we need it after navigation refactoring?
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.
Well done I would suggest to submit a PR or something to the official library
APKs built during tests are available here. Scroll down to Artifacts! |
Build 867 succeeded. The build produced the following APK's: |
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
Issues
Currently we need to pass arguments like key-value pairs into
Bundle
and then retrieve them manually fromSavedStateHandle
in theViewModel
, which is also not type-safe.Solutions
With the help from the Bundlizer library, we can make a
Bundle
out ofkotlinx.serialization Serializable
, so we can create arguments data class for the specific scopedViewModel
, and using implemented customhiltViewModelScoped
just pass the instance of this arguments class, and then in theViewModel
retrieve it type-safely using extension functionSavedStateHandle.scopedArgs()
. There is also aScopedArgs
interface with thekey
argument, so building the key for thehiltScopedViewModel
is moved to the arguments class, to make it even easier.So it's like this:
ViewModel
:ViewModel
:For testing purposes,
ScopedArgsTestExtension
is added, which allows for mockingscopedArgs
like:every { savedStateHandle.scopedArgs<CompositeMessageArgs>() } returns CompositeMessageArgs(MESSAGE_ID)
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764
.