Moving APIs in PreviewViewModel to Controller classes#482
Moving APIs in PreviewViewModel to Controller classes#482davidjiagoogle wants to merge 11 commits intomainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request successfully refactors the PreviewViewModel by extracting various UI event handling logic into dedicated callback data classes: QuickSettingsCallbacks, CaptureCallbacks, SnackBarCallbacks, and DebugCallbacks. This significantly simplifies the PreviewViewModel and improves modularity, making the code easier to understand and maintain. The changes are well-implemented, and the new callback structures enhance code organization and reusability. The test file PreviewViewModelTest.kt has also been updated to reflect these changes, ensuring continued test coverage.
feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt
Outdated
Show resolved
Hide resolved
feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt
Outdated
Show resolved
Hide resolved
feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt
Outdated
Show resolved
Hide resolved
...nts/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureCallbacks.kt
Outdated
Show resolved
Hide resolved
...ts/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/SnackBarCallbacks.kt
Outdated
Show resolved
Hide resolved
...capture/src/main/java/com/google/jetpackcamera/ui/components/capture/debug/DebugCallbacks.kt
Outdated
Show resolved
Hide resolved
.../java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsCallbacks.kt
Outdated
Show resolved
Hide resolved
|
/gemini review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a significant and well-executed refactoring that decomposes the large PreviewViewModel into several smaller, single-responsibility controller classes. This greatly improves the architecture by enhancing modularity, testability, and overall code clarity. The changes are consistently applied across the UI components, ViewModels, and tests. My review includes a few suggestions: fixing a minor copy-paste bug in CaptureControllerImpl, adding KDoc to the new controller APIs to improve documentation, and a couple of minor stylistic improvements in PreviewScreen.kt to make the code more idiomatic.
...main/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureControllerImpl.kt
Outdated
Show resolved
Hide resolved
feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt
Outdated
Show resolved
Hide resolved
feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt
Outdated
Show resolved
Hide resolved
...src/main/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureController.kt
Show resolved
Hide resolved
temcguir
left a comment
There was a problem hiding this comment.
This is a good start. There are definitely a few things that could be cleaned up later (such as the zoom controller). If you can address the comments I've added we could merge this and make smaller clean-up PRs to address those later.
Interfaces QuickSettingsController, CaptureController, CaptureScreenController, SnackBarController, ZoomController, and DebugController are created. They contain APIs that were originally implemented in the view models. A default implementation of their APIs is made for each interface so that view model can be simplified
PreviewViewModel now only contains instances of these controller and has no APIs itself.