Moving APIs in PreviewViewModel to Controller classes by davidjiagoogle · Pull Request #482 · google/jetpack-camera-app

davidjiagoogle · 2026-02-20T01:57:17Z

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.

gemini-code-assist · 2026-02-20T01:57:21Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

temcguir · 2026-02-20T19:01:36Z

/gemini review

gemini-code-assist

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

...nts/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureCallbacks.kt

...ts/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/SnackBarCallbacks.kt

...capture/src/main/java/com/google/jetpackcamera/ui/components/capture/debug/DebugCallbacks.kt

.../java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsCallbacks.kt

temcguir · 2026-02-25T07:19:16Z

/gemini review

gemini-code-assist · 2026-02-25T07:19:19Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

temcguir · 2026-02-25T08:34:34Z

/gemini review

gemini-code-assist

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

feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt

...src/main/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureController.kt

temcguir

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.

temcguir · 2026-02-27T23:25:04Z

...ain/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt

    }
 }
+
+class MockQuickSettingsController : QuickSettingsController {


nit: Call this "NoOpQuickSettingsController". We should follow this naming convention for any of the @Preview composables..

temcguir · 2026-02-27T23:28:37Z

...nts/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/controller/Utils.kt

+    fun postCurrentMediaToMediaRepository(
+        viewModelScope: CoroutineScope,


Instead of passing in a viewModelScope, you should just make this a suspend function and have the viewModelScope.launch within the ViewModel wrapping this suspend function.

temcguir · 2026-02-27T23:29:44Z

...in/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureScreenController.kt

+    /**
+     * Pauses or resumes video recording.
+     *
+     * @param shouldBePaused Whether the recording should be paused.
+     */
+    fun setPaused(shouldBePaused: Boolean)


This should be in CaptureController.

temcguir · 2026-02-27T23:29:59Z

...in/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureScreenController.kt

+    /**
+     * Enables or disables audio recording.
+     *
+     * @param shouldEnableAudio Whether audio should be enabled.
+     */
+    fun setAudioEnabled(shouldEnableAudio: Boolean)


This should be in CaptureController.

temcguir · 2026-02-27T23:30:14Z

...in/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureScreenController.kt

+
+    /**
+     * Updates the UI with the last captured media.
+     */
+    fun updateLastCapturedMedia()
+
+    /**
+     * Transfers the media from the image well to the repository.
+     */
+    fun imageWellToRepository()


Can these be moved to an ImageWellController?

temcguir · 2026-02-27T23:40:02Z

...main/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureControllerImpl.kt

+    private val saveMode: SaveMode,
+    private val externalCaptureMode: ExternalCaptureMode,
+    private val externalCapturesCallback: () -> Pair<SaveLocation, IntProgress?>,
+    private val captureEvents: Channel<CaptureEvent>,


Does this need to be an input to the constructor? Can it just be a val on CaptureController? (likely as a ReceiveChannel in the interface).

temcguir · 2026-02-27T23:50:11Z

...main/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureControllerImpl.kt

+ */
+class CaptureControllerImpl(
+    private val trackedCaptureUiState: MutableStateFlow<TrackedCaptureUiState>,
+    private val viewModelScope: CoroutineScope,


It's generally considered bad practice to pass in CoroutineScope to constructors. At a minimum this should be a CoroutineContext from which we create a scope within the controller. See go/android-api-guidelines-async#coroutines-constructor-context for more info.

temcguir · 2026-02-27T23:55:45Z

...in/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureScreenController.kt

+    /**
+     * Sets the display rotation.
+     *
+     * @param deviceRotation The device rotation to set.
+     */
+    fun setDisplayRotation(deviceRotation: DeviceRotation)
+
+    /**
+     * Initiates a tap-to-focus action at the given coordinates.
+     *
+     * @param x The x-coordinate of the tap.
+     * @param y The y-coordinate of the tap.
+     */
+    fun tapToFocus(x: Float, y: Float)


These can be moved into CameraController.

temcguir · 2026-02-27T23:57:39Z

...main/java/com/google/jetpackcamera/ui/components/capture/controller/CaptureControllerImpl.kt

+    private val trackedCaptureUiState: MutableStateFlow<TrackedCaptureUiState>,
+    private val viewModelScope: CoroutineScope,
+    private val cameraSystem: CameraSystem,
+    private val mediaRepository: MediaRepository,


It would be good if we could remove the dependency on MediaRepository here. We should only have a MediaRepository dependency in the ViewModel. Maybe this can be replaced with a callback that can be supplied at creation time which will call the MediaRepository within the ViewModel instead of within the controller.

init

5b223fe

davidjiagoogle requested review from Kimblebee and temcguir February 20, 2026 01:57

gemini-code-assist bot reviewed Feb 20, 2026

View reviewed changes

davidjiagoogle added 4 commits February 24, 2026 13:43

controllers

65ea97a

camera, capturescreen, capture controllers

07f7ba3

Merge branch 'main' into david/previewViewModelCallbacks

759b6a6

spotless

998582a

davidjiagoogle changed the title ~~Moving APIs in PreviewViewModel to Callbacks data classes~~ Moving APIs in PreviewViewModel to Controller classes Feb 25, 2026

gemini-code-assist bot reviewed Feb 25, 2026

View reviewed changes

davidjiagoogle and others added 4 commits February 25, 2026 10:15

Merge branch 'main' into david/previewViewModelCallbacks

1aa1a97

add kdocs

3913eb1

Merge branch 'main' into david/previewViewModelCallbacks

5a7425c

spotless

231f11a

temcguir requested changes Feb 27, 2026

View reviewed changes

		fun postCurrentMediaToMediaRepository(
		viewModelScope: CoroutineScope,

Conversation

davidjiagoogle commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Feb 20, 2026

Uh oh!

temcguir commented Feb 20, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

temcguir commented Feb 25, 2026

Uh oh!

gemini-code-assist bot commented Feb 25, 2026

Uh oh!

temcguir commented Feb 25, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

temcguir left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

davidjiagoogle commented Feb 20, 2026 •

edited

Loading