Skip to content

Conversation

@maiatoday
Copy link
Collaborator

Refactors the detail view to utilize a WizardViewModel for managing state, improving separation of concerns and testability.

Key changes:

  • Introduces WizardViewModel to handle UI state, including bitmap processing, color reduction, and embroidery creation.
  • Updates PatchableDetail composable to use WizardViewModel and collect state using collectAsStateWithLifecycle.
  • Moves bitmap reduction and embroidery creation logic into the ViewModel.
  • Adds reset functionality to WizardViewModel to clear the state when a different patchable is selected.
  • Introduces PatchablePreviewMode enum to handle preview modes.

@maiatoday maiatoday self-assigned this Sep 6, 2025
Copy link
Member

@mariobodemann mariobodemann left a comment

Choose a reason for hiding this comment

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

Amazing work, thank you. 👍

One naming stood out to me, but otherwise, I really enjoy your work.

🤓👍

patchable
onBitmapUpdated = viewModel::updateBitmap,
onColorCountUpdated = viewModel::updateColorCount,
computeReducedBitmap = viewModel::computeReducedBitmap,
Copy link
Member

Choose a reason for hiding this comment

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

Nit pick: should that also be onComputeReducedBitmap? Consistency with the other names, or does work differently, hence the difference in naming?

Copy link
Collaborator Author

@maiatoday maiatoday Sep 7, 2025

Choose a reason for hiding this comment

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

for onBitmapUpdated and onColorCountUpdated we ask the composable to do something and it calls these methods when it completes the task.

on the other hand the computeReducedBitmap is an imperative action which we are asking and external function to perform, in this case the ViewModel.

BUT I see a nit myself, I am not consistently using color vs colour. very irritating, I will fix it. Fake news it is ok

import kotlinx.coroutines.launch

/**
* ViewModel to manage the Wizard screen state.
Copy link
Member

Choose a reason for hiding this comment

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

🧙‍♀️👍

maiatoday and others added 8 commits September 7, 2025 06:19
Allow SafeArea to Capture the Bitmap
Introduces `WizardViewModel` to manage the state and logic of the `WizardScreen`.
This change centralizes state management and improves the testability of the screen.

Refactor WizardScreen to use ViewModel

Refactor WizardScreen to use ViewModel

Introduces `WizardViewModel` to manage the state and logic of the `WizardScreen`.
This change centralizes state management and improves the testability of the screen.
@maiatoday maiatoday force-pushed the work/detail-view-model branch from 4971c5e to 45f1c63 Compare September 7, 2025 06:34
@maiatoday maiatoday merged commit cc930df into work/better-safe-area Sep 7, 2025
1 check 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.

3 participants