Skip to content

Commit 14741bd

Browse files
david-allisonmikehardy
authored andcommitted
refactor(image-occlusion): use discriminated union for args
* Removes nullable properties * Strongly types 'id' + 'deckId' * Properties are only available if the mode ('Add'/'Edit') is checked For issue 19605, we will want to know whether the page is adding or editing a note, this will read better if we are not assuming existence of properties
1 parent 4c0ef76 commit 14741bd

File tree

3 files changed

+87
-62
lines changed

3 files changed

+87
-62
lines changed

AnkiDroid/src/main/java/com/ichi2/anki/NoteEditorFragment.kt

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ import com.ichi2.anki.noteeditor.Toolbar.TextFormatListener
141141
import com.ichi2.anki.noteeditor.Toolbar.TextWrapper
142142
import com.ichi2.anki.observability.undoableOp
143143
import com.ichi2.anki.pages.ImageOcclusion
144+
import com.ichi2.anki.pages.viewmodel.ImageOcclusionArgs
144145
import com.ichi2.anki.preferences.sharedPrefs
145146
import com.ichi2.anki.previewer.TemplatePreviewerArguments
146147
import com.ichi2.anki.previewer.TemplatePreviewerPage
@@ -2751,22 +2752,28 @@ class NoteEditorFragment :
27512752
private fun currentNotetypeIsImageOcclusion() = currentlySelectedNotetype?.isImageOcclusion == true
27522753

27532754
private fun setupImageOcclusionEditor(imagePath: String = "") {
2754-
val kind: String
2755-
val id: Long
2756-
if (addNote) {
2757-
kind = "add"
2758-
// if opened from an intent, the selected note type may not be suitable for IO
2759-
id =
2760-
if (currentNotetypeIsImageOcclusion()) {
2761-
currentlySelectedNotetype!!.id
2762-
} else {
2763-
0
2764-
}
2765-
} else {
2766-
kind = "edit"
2767-
id = editorNote?.id!!
2768-
}
2769-
val intent = ImageOcclusion.getIntent(requireContext(), kind, id, imagePath, deckId)
2755+
val args =
2756+
if (addNote) {
2757+
// if opened from an intent, the selected note type may not be suitable for IO
2758+
val noteTypeId =
2759+
if (currentNotetypeIsImageOcclusion()) {
2760+
currentlySelectedNotetype!!.id
2761+
} else {
2762+
0
2763+
}
2764+
ImageOcclusionArgs.Add(
2765+
noteTypeId = noteTypeId,
2766+
imagePath = imagePath,
2767+
deckId = deckId,
2768+
)
2769+
} else {
2770+
ImageOcclusionArgs.Edit(
2771+
noteId = editorNote!!.id,
2772+
deckId = deckId,
2773+
)
2774+
}
2775+
2776+
val intent = ImageOcclusion.getIntent(requireContext(), args)
27702777
requestIOEditorCloser.launch(intent)
27712778
}
27722779

AnkiDroid/src/main/java/com/ichi2/anki/pages/ImageOcclusion.kt

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import com.ichi2.anki.common.annotations.NeedsTest
3434
import com.ichi2.anki.dialogs.DeckSelectionDialog
3535
import com.ichi2.anki.dialogs.DiscardChangesDialog
3636
import com.ichi2.anki.launchCatchingTask
37-
import com.ichi2.anki.libanki.DeckId
3837
import com.ichi2.anki.model.SelectableDeck
3938
import com.ichi2.anki.pages.viewmodel.ImageOcclusionArgs
4039
import com.ichi2.anki.pages.viewmodel.ImageOcclusionViewModel
@@ -103,7 +102,7 @@ class ImageOcclusion :
103102
url: String?,
104103
) {
105104
super.onPageFinished(view, url)
106-
viewModel.webViewOptions.let { options ->
105+
viewModel.args.toImageOcclusionMode().let { options ->
107106
view?.evaluateJavascript("globalThis.anki.imageOcclusion.mode = $options") {
108107
super.onPageFinished(view, url)
109108
}
@@ -137,24 +136,17 @@ class ImageOcclusion :
137136

138137
companion object {
139138
/**
140-
* @param editorWorkingDeckId the current deck id that [com.ichi2.anki.NoteEditorFragment] is using
139+
* @param args arguments for either adding or editing a note
141140
*/
142141
fun getIntent(
143142
context: Context,
144-
kind: String,
145-
noteOrNotetypeId: Long,
146-
imagePath: String?,
147-
editorWorkingDeckId: DeckId,
143+
args: ImageOcclusionArgs,
148144
): Intent {
149-
val suffix = if (kind == "edit") noteOrNotetypeId else Uri.encode(imagePath)
150-
151-
val args =
152-
ImageOcclusionArgs(
153-
kind = kind,
154-
id = noteOrNotetypeId,
155-
imagePath = imagePath,
156-
editorDeckId = editorWorkingDeckId,
157-
)
145+
val suffix =
146+
when (args) {
147+
is ImageOcclusionArgs.Add -> Uri.encode(args.imagePath)
148+
is ImageOcclusionArgs.Edit -> args.noteId
149+
}
158150

159151
val arguments =
160152
bundleOf(

AnkiDroid/src/main/java/com/ichi2/anki/pages/viewmodel/ImageOcclusionViewModel.kt

Lines changed: 56 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,52 +23,78 @@ import androidx.lifecycle.ViewModel
2323
import androidx.lifecycle.viewModelScope
2424
import com.ichi2.anki.CollectionManager
2525
import com.ichi2.anki.libanki.DeckId
26+
import com.ichi2.anki.libanki.NoteId
27+
import com.ichi2.anki.libanki.NoteTypeId
28+
import com.ichi2.anki.pages.ImageOcclusion
2629
import kotlinx.coroutines.launch
2730
import kotlinx.parcelize.Parcelize
2831
import org.json.JSONObject
2932
import timber.log.Timber
3033

31-
@Parcelize
32-
data class ImageOcclusionArgs(
33-
val kind: String,
34-
val id: Long,
35-
val imagePath: String?,
36-
val editorDeckId: Long,
37-
) : Parcelable
38-
3934
/**
40-
* ViewModel for the Image Occlusion fragment.
35+
* Arguments for either adding or editing an image occlusion note
36+
*
37+
* @see ImageOcclusionArgs.Add
38+
* @see ImageOcclusionArgs.Edit
4139
*/
42-
class ImageOcclusionViewModel(
43-
savedStateHandle: SavedStateHandle,
44-
) : ViewModel() {
45-
private val args: ImageOcclusionArgs =
46-
checkNotNull(savedStateHandle[IO_ARGS_KEY]) { "$IO_ARGS_KEY required" }
40+
@Parcelize
41+
sealed class ImageOcclusionArgs : Parcelable {
42+
@Parcelize
43+
data class Add(
44+
val imagePath: String,
45+
val noteTypeId: NoteTypeId,
46+
val deckId: DeckId,
47+
) : ImageOcclusionArgs()
4748

48-
var selectedDeckId: Long = args.editorDeckId
49+
@Parcelize
50+
data class Edit(
51+
val noteId: NoteId,
52+
val deckId: DeckId,
53+
) : ImageOcclusionArgs()
4954

5055
/**
51-
* The ID of the deck that was originally selected when the editor was opened.
52-
* This is used to restore the deck after saving a note to prevent unexpected deck changes.
56+
* The ID of the deck that was selected when the editor was opened.
57+
* Used to restore the deck after saving a note to prevent unexpected deck changes.
5358
*/
54-
val oldDeckId: Long = args.editorDeckId
59+
val originalDeckId: DeckId
60+
get() =
61+
when (this) {
62+
is Add -> this.deckId
63+
is Edit -> this.deckId
64+
}
5565

5666
/**
5767
* A [JSONObject] containing options for loading the [image occlusion page][ImageOcclusion].
5868
* This includes the type of operation ("add" or "edit"), and relevant IDs and paths.
5969
*
60-
* Defined in https://github.com/ankitects/anki/blob/main/ts/routes/image-occlusion/lib.ts
70+
* See 'IOMode' in https://github.com/ankitects/anki/blob/main/ts/routes/image-occlusion/lib.ts
6171
*/
62-
val webViewOptions: JSONObject =
63-
JSONObject().apply {
64-
put("kind", args.kind)
65-
if (args.kind == "add") {
66-
put("imagePath", args.imagePath)
67-
put("notetypeId", args.id)
68-
} else {
69-
put("noteId", args.id)
70-
}
72+
fun toImageOcclusionMode() =
73+
when (this) {
74+
is Add ->
75+
JSONObject().also {
76+
it.put("kind", "add")
77+
it.put("imagePath", this.imagePath)
78+
it.put("notetypeId", this.noteTypeId)
79+
}
80+
is Edit ->
81+
JSONObject().also {
82+
it.put("kind", "edit")
83+
it.put("noteId", this.noteId)
84+
}
7185
}
86+
}
87+
88+
/**
89+
* ViewModel for the Image Occlusion fragment.
90+
*/
91+
class ImageOcclusionViewModel(
92+
savedStateHandle: SavedStateHandle,
93+
) : ViewModel() {
94+
val args: ImageOcclusionArgs =
95+
checkNotNull(savedStateHandle[IO_ARGS_KEY]) { "$IO_ARGS_KEY required" }
96+
97+
var selectedDeckId: DeckId = args.originalDeckId
7298

7399
/**
74100
* Handles the selection of a new deck.
@@ -86,13 +112,13 @@ class ImageOcclusionViewModel(
86112
*/
87113
fun onSaveOperationCompleted() {
88114
Timber.i("save operation completed")
89-
if (oldDeckId == selectedDeckId) return
115+
if (args.originalDeckId == selectedDeckId) return
90116
// reset to the previous deck that the backend "saw" as selected, this
91117
// avoids other screens unexpectedly having their working decks modified(
92118
// most important being the Reviewer where the user would find itself
93119
// studying another deck after editing a note with changing the deck)
94120
viewModelScope.launch {
95-
CollectionManager.withCol { backend.setCurrentDeck(oldDeckId) }
121+
CollectionManager.withCol { backend.setCurrentDeck(args.originalDeckId) }
96122
}
97123
}
98124

0 commit comments

Comments
 (0)