refactor(CardTemplateEditor): load notetype into ViewModel #20109
+377
−3
Conversation
This file contains hidden or 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
xenonnn4w
added
the
Blocked by dependency
xenonnn4w
commented
Jan 14, 2026
Comment on lines
+83
to
+96
| // During ViewPager tab transitions, TextWatcher may fire with a stale ordinal | ||
| // (e.g. when switching from a note type with 3 templates to one with 2). | ||
| // The legacy code path handles the actual update, so we can safely skip here. |
Contributor
Author
There was a problem hiding this comment.
While testing, I encountered a crash when switching between note types with different template counts. The TextWatcher fired during ViewPager tab transitions with a stale cardIndex, causing an IndexOutOfBoundsException i.e. Fixed by adding a bounds check that safely skips the ViewMode update when the ordinal is out of range.
Process: com.ichi2.anki.debug, PID: 21692
java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:603)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:932)
Caused by: java.lang.reflect.InvocationTargetException
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:593)
... 1 more
Caused by: org.json.JSONException: Index 2 out of range [0..2)
Member
There was a problem hiding this comment.
We should fix this in the view layer, it should be a bug in the ViewModel
Comment on lines
+83
to
+96
| // During ViewPager tab transitions, TextWatcher may fire with a stale ordinal | ||
| // (e.g. when switching from a note type with 3 templates to one with 2). | ||
| // The legacy code path handles the actual update, so we can safely skip here. |
Member
There was a problem hiding this comment.
We should fix this in the view layer, it should be a bug in the ViewModel
AnkiDroid/src/main/java/com/ichi2/anki/CardTemplateEditorViewModel.kt
Outdated
Show resolved
Hide resolved
Introduce CardTemplateEditorViewModel and CardTemplateEditorState as the foundation for migrating CardTemplateEditor to use the Rust backend's atomic updateNotetype() API. Part 1 of migration, infrastructure only, no behavioral changes yet.
Connect CardTemplateEditorViewModel to CardTemplateEditor Activity: - Add viewModel declaration using viewModels delegate - Add state collection with lifecycleScope + repeatOnLifecycle - Wire tab selection to viewModel.setCurrentTemplateOrd() - Handle sealed class states (Loading, Loaded, Error, Finished) Part 2 of migration, infrastructure wiring, no behavioral changes.
…emplate updates - Add tempNotetype property to Loaded state (sealed class) - Add loadNotetype() to load and deep clone notetype into ViewModel - Add updateTemplateContent() using EditorViewType enum - Add addTemplate() and removeTemplate() placeholder methods - Wire Fragment's TextWatcher to call ViewModel in parallel with legacy path - Share ViewModel between Activity and Fragment via activityViewModels() Part 3 of migration, infrastructure wiring, no behavioral changes.
xenonnn4w
force-pushed
the
pr3-migrateCTE
branch
from
1c4fc50 to
4e798f0
Compare
Comment on lines
+270
to
+272
| showSnackbar( | ||
| state.exception.source.localizedMessage ?: getString(R.string.something_wrong), | ||
| ) |
Member
There was a problem hiding this comment.
This feels like it needs a better abstraction.
- If it's a loading error, a snackbar is insufficient, as it'll be closed when the screen is closed
- If it's a loading error, this doesn't finish the screen, and it probably should
If it's a transient error (save failed), it may be displayed alongside the valid state
We probably want the user to copy/paste the error, so an error dialog is more friendly
| val tempNotetype = loadedState.tempNotetype | ||
| if (tempNotetype.templateCount < 2) { | ||
| Timber.w("Cannot delete last template") | ||
| updateLoadedState { it.copy(message = CardTemplateEditorState.UserMessage.CantDeleteLastTemplate) } |
Member
There was a problem hiding this comment.
Would be great to test some of this logic
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Third PR in the CardTemplateEditor migration series.
tempNotetypeproperty toCardTemplateEditorStateloadNotetype()in ViewModelupdateTemplateContent()addTemplate()andremoveTemplate()methodsViewModelwith Activity viaactivityViewModels()TextWatchercalls ViewModel in parallel with legacy code path during this transitional phaseLinks without closing (Part of)
Based on
How Has This Been Tested?
No regressions obtained, and all
TemplateEditorViewModelTestpasses.Checklist
Please, go through these checks before submitting the PR.