feat: introduce a view model in ManageNotetypes #17804
Open
+585
−149
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Encapsulates a loading dialog like we use in withProgress {} into a
fragment so it can be handled in transactions and react to restores.
Improves the documentation. Removes the internal keyword, only useful if we would actually used modules/libraries. The rename was done to better integrate with the changes for transitioning to a view model in ManageNotetypes.
The implemented viewmodel follows the official recommendations by implementing UDF(unidirectional data flow): user events go to the viewmodel and the viewmodel exposes states consumed by the ui. Not all actions in ManageNotetypes are done in the style of the reactive ManageNotetypeViewModel(delete/rename + the actions in AddNewNotetype).
david-allison
requested changes
Jan 13, 2025
Comment on lines
+36
to
+37
| private val _uiState = MutableStateFlow(NotetypesUiState()) | ||
| val uiState: StateFlow<NotetypesUiState> = _uiState.asStateFlow() |
There was a problem hiding this comment.
[no action needed, comment only]
I prefer exposing the mutable Flow, instead of the additional boilerplate
| ) | ||
| } | ||
| } | ||
| _uiState.update { state -> |
There was a problem hiding this comment.
Either an error in this method, or if callers don't call this method
AnkiDroid/src/main/java/com/ichi2/anki/notetype/ManageNotetypeViewModel.kt
Show resolved
Hide resolved
| } | ||
|
|
||
| /** | ||
| * Refreshes the list of notetypes, additionally filtering the list. Also calls to emit the new |
There was a problem hiding this comment.
'emit the new' feels like it's a truncated sentence
AnkiDroid/src/main/java/com/ichi2/anki/notetype/ManageNotetypeViewModel.kt
Show resolved
Hide resolved
lukstbit
added
the
Needs Author Reply
|
I'll use a LinearProgressDialog for this comment: the padding on the dialog should be a little larger |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose / Description
A bit more info in the commit message. The line count is big but it's mostly comments, copyright etc the actual code should be easy to read. Did this in one go as changing step by step into a view model would have introduced even more useless changes to review.
How the loading indicator looks:
How Has This Been Tested?
Ran the tests, manually verified the app.
Checklist