Skip to content

Comments

ScanSettings View model refactor#136

Merged
Chrisimx merged 16 commits intomainfrom
view-model-refactor
Feb 20, 2026
Merged

ScanSettings View model refactor#136
Chrisimx merged 16 commits intomainfrom
view-model-refactor

Conversation

@Chrisimx
Copy link
Owner

@Chrisimx Chrisimx commented Feb 20, 2026

Relates to #135

@Chrisimx Chrisimx force-pushed the view-model-refactor branch from 166bc03 to 491d5b7 Compare February 20, 2026 18:24
@Chrisimx Chrisimx changed the title View model refactor ScanSettings View model refactor Feb 20, 2026
@Chrisimx Chrisimx requested a review from Copilot February 20, 2026 22:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request implements a significant refactoring of the ScanSettings view model architecture to properly separate concerns between view models and UI state. The changes move away from Compose's mutable state patterns to a more traditional ViewModel approach using StateFlow, and introduces Koin for dependency injection.

Changes:

  • Refactored ScanSettingsComposableViewModel to use StateFlow instead of mutable Compose state
  • Introduced Koin dependency injection framework (version 4.2.0-RC1) for view model management
  • Created LocaleProvider interface and AndroidLocaleProvider implementation for locale management
  • Removed legacy mutable state classes (MutableESCLScanSettingsState, MutableScanRegionState) and consolidated into LegacyESCLScanSettings for backward compatibility
  • Updated string resources to support dynamic unit display (millimeters vs inches) based on locale
  • Moved getInputSourceCaps and getInputSourceOptions extension functions to the esclkt library

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
gradle/libs.versions.toml Added Koin dependencies and updated Kotlin to 2.3.20-Beta1
app/build.gradle.kts Added Koin plugin and implementation dependencies
app/src/main/res/values/strings.xml Added parameterized unit strings and new unit abbreviation strings
app/src/main/res/values-de/strings.xml Updated width/height strings (missing parameterization)
app/src/main/res/values-it/strings.xml Updated width/height strings (missing parameterization)
app/src/main/java/.../util/StateFlowExtensions.kt New utility for creating derived StateFlows
app/src/main/java/.../util/ESCLKtExtensions.kt Removed duplicate extension functions, now using esclkt library versions
app/src/main/java/.../uicomponents/ValidatedTextField.kt Refactored to use NumberValidationResult instead of local error state
app/src/main/java/.../stores/SessionsStore.kt Added ScannerCapabilities parameter for session loading
app/src/main/java/.../stores/DefaultScanSettingsStore.kt Updated to work with ScanSettings instead of StatelessImmutableESCLScanSettingsState
app/src/main/java/.../services/LocaleProvider.kt New interface for locale provision
app/src/main/java/.../services/AndroidLocaleProvider.kt Implementation of LocaleProvider with StateFlow
app/src/main/java/.../data/ui/ScanningScreenViewModel.kt Updated to use Koin scopes for ScanSettingsComposableViewModel creation
app/src/main/java/.../data/ui/ScanSettingsComposableViewModel.kt Major refactor to use StateFlow-based architecture with proper validation
app/src/main/java/.../data/ui/ScanSettingsComposableData.kt Simplified to single data class with NumberValidationResult
app/src/main/java/.../data/model/Session.kt Updated to handle ScanSettings migration from legacy format
app/src/main/java/.../data/model/ScanRegionState.kt Removed (functionality moved to LegacyESCLScanSettings.kt)
app/src/main/java/.../data/model/LegacyESCLScanSettings.kt New file for backward compatibility with old session format
app/src/main/java/.../data/model/ESCLScanSettingsState.kt Removed (replaced by direct ScanSettings usage)
app/src/main/java/.../ScanningScreen.kt Updated to remove context parameter from ScanSettingsUI
app/src/main/java/.../ScanSettings.kt Refactored to use koinViewModel and StateFlow-based state management
app/src/main/java/.../ScanBridgeApplication.kt Added Koin initialization with dependency modules
app/src/main/java/.../MainActivity.kt Updated to inject AndroidLocaleProvider via Koin
app/src/main/java/.../CropScreen.kt Updated SessionsStore.loadSession call with null capabilities parameter
Comments suppressed due to low confidence (1)

app/src/main/java/io/github/chrisimx/scanbridge/services/AndroidLocaleProvider.kt:17

  • This logging call uses Timber.Forest.d() instead of the standard Timber.d() that is used throughout the rest of the codebase. Timber.Forest is the container for Timber trees, not the typical logging interface. This should be changed to Timber.d() for consistency with the rest of the codebase.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (2)

app/src/main/java/io/github/chrisimx/scanbridge/data/ui/ScanningScreenViewModel.kt:70

  • The import org.koin.java.KoinJavaComponent.get appears unused (Koin access is done via getKoin().get { ... }). Removing it avoids confusion about which get is in use.
import org.koin.core.parameter.parametersOf
import org.koin.java.KoinJavaComponent.get
import org.koin.mp.KoinPlatform.getKoin

gradle/libs.versions.toml:42

  • The alias koin-androix-navigation looks like a typo (androidx). It works as long as the accessor usage matches, but it’s easy to propagate and makes the catalog harder to search/maintain. Consider renaming it to koin-androidx-navigation (and updating usages) for clarity.
koin-compose = { module = "io.insert-koin:koin-compose", version.ref = "koin"}

koin-androix-navigation = { module = "io.insert-koin:koin-androidx-compose-navigation", version.ref = "koin"}


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Chrisimx Chrisimx merged commit 2822732 into main Feb 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant