Moving APIs in PreviewViewModel to Controller classes

[davidjiagoogle]

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]

Warning

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

[temcguir]

/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.

[temcguir]

/gemini review

[gemini-code-assist]

Warning

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

[temcguir]

/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.

[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]

Sorry I just realized this when looking at removing Hilt dependencies from the non-app libraries.

This dependency on :core:camera highlights an architectural issue that makes testing our UI components difficult.

Right now, :ui:components contains not just Composables, but also the Controller implementations, which pull in heavyweight dependencies like :core:camera and Hilt. This forces our UI tests for simple components to deal with a larger, more complex dependency graph just to provide fakes.

To improve testability, I propose we separate our UI into three distinct modules:

1. :ui:api (renamed from :ui:uistate): Contains the UI state data classes and Controller interfaces.
2. :ui:components (this module): Contains only the @Composable functions.
3. :ui:controller (new module): Contains the Controller implementations.

This decouples our Composables from their production logic, allowing us to write lean UI tests against simple, fake controller implementations without needing the entire dependency graph.

[davidjiagoogle]

I would prefer keeping the existing ui:components & ui:uistate modules, and additionally create:

1. ui:controller (Controller interfaces, minimal deps)
2. ui:controller:impl (Controller impls, dep to camera:core, and ui:uistates for the time being)

[temcguir]

That works as well.