Skip to content

Commit fc7bb43

Browse files
david-allisonmikehardy
authored andcommitted
fix(image-occlusion): support updating a note's deck
Once a note had been created, the deck couldn't be updated. Fixed by: * Enable editing the deck in NoteEditorFragment if in edit mode * Disable editing the deck in ImageOcclusion if in edit mode In future, we can handle this in ImageOcclusion, BUT: There are 3 cases: * A user wants to edit a card when studying * A user wants to edit a selection of cards from the same note [Card Browser - CARDS] * A user wants to edit a note [Card Browser - NOTES] * What does it mean to edit the deck of a note, is a user aware of this? And a user can remove the cards they are editing (by removing masks). Since we don't yet have full control over the process of adding an occlusion note, I decided it was a risk to take the time to implement this Fixes 19605
1 parent 3c9a950 commit fc7bb43

File tree

4 files changed

+288
-33
lines changed

4 files changed

+288
-33
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2423,10 +2423,11 @@ class NoteEditorFragment :
24232423
populateEditFields(changeType, false)
24242424
updateFieldsFromStickyText()
24252425

2426-
// Showing the deck selection parts is not needed for Image Occlusion notetypes
2427-
// as deck selection is handled by the backend page
2428-
requireView().findViewById<TextView>(R.id.CardEditorDeckText).isVisible = !currentNotetypeIsImageOcclusion()
2429-
requireView().findViewById<View>(R.id.note_deck_name).isVisible = !currentNotetypeIsImageOcclusion()
2426+
// When adding a note, ImageOcclusion handles the deck selection
2427+
// as a user can reach this screen directly from an intent
2428+
val disableDeckEditing = addNote && currentNotetypeIsImageOcclusion()
2429+
requireView().findViewById<TextView>(R.id.CardEditorDeckText).isVisible = !disableDeckEditing
2430+
requireView().findViewById<View>(R.id.note_deck_name).isVisible = !disableDeckEditing
24302431
}
24312432

24322433
private fun addClozeButton(
@@ -2764,12 +2765,11 @@ class NoteEditorFragment :
27642765
ImageOcclusionArgs.Add(
27652766
noteTypeId = noteTypeId,
27662767
imagePath = imagePath,
2767-
deckId = deckId,
2768+
originalDeckId = deckId,
27682769
)
27692770
} else {
27702771
ImageOcclusionArgs.Edit(
27712772
noteId = editorNote!!.id,
2772-
deckId = deckId,
27732773
)
27742774
}
27752775

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import android.webkit.WebView
2424
import android.widget.TextView
2525
import androidx.activity.addCallback
2626
import androidx.core.os.bundleOf
27+
import androidx.core.view.isVisible
2728
import androidx.fragment.app.viewModels
2829
import androidx.lifecycle.Lifecycle
2930
import androidx.lifecycle.lifecycleScope
@@ -53,6 +54,8 @@ import timber.log.Timber
5354
* ([docs](https://docs.ankiweb.net/editing.html#image-occlusion) and
5455
* [source](https://github.com/ankitects/anki/blob/main/proto/anki/image_occlusion.proto)).
5556
*
57+
* When adding, a user may select the deck of the note
58+
*
5659
* **Paths**
5760
* `/image-occlusion/$PATH`
5861
* `/image-occlusion/$NOTE_ID`
@@ -132,7 +135,9 @@ class ImageOcclusion :
132135
deckNameView.text = name
133136
}
134137

135-
viewModel.deckNameFlow.launchCollectionInLifecycleScope(::onDeckNameChanged)
138+
viewModel.deckNameFlow?.launchCollectionInLifecycleScope(::onDeckNameChanged) ?: run {
139+
deckNameView.isVisible = false
140+
}
136141
}
137142

138143
// TODO: Move this to an extension method once we have context parameters

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

Lines changed: 46 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -47,26 +47,18 @@ sealed class ImageOcclusionArgs : Parcelable {
4747
data class Add(
4848
val imagePath: String,
4949
val noteTypeId: NoteTypeId,
50-
val deckId: DeckId,
50+
/**
51+
* The ID of the deck that was selected when the editor was opened.
52+
* Used to restore the deck after saving a note to prevent unexpected deck changes.
53+
*/
54+
val originalDeckId: DeckId,
5155
) : ImageOcclusionArgs()
5256

5357
@Parcelize
5458
data class Edit(
5559
val noteId: NoteId,
56-
val deckId: DeckId,
5760
) : ImageOcclusionArgs()
5861

59-
/**
60-
* The ID of the deck that was selected when the editor was opened.
61-
* Used to restore the deck after saving a note to prevent unexpected deck changes.
62-
*/
63-
val originalDeckId: DeckId
64-
get() =
65-
when (this) {
66-
is Add -> this.deckId
67-
is Edit -> this.deckId
68-
}
69-
7062
/**
7163
* A [JSONObject] containing options for loading the [image occlusion page][ImageOcclusion].
7264
* This includes the type of operation ("add" or "edit"), and relevant IDs and paths.
@@ -98,18 +90,30 @@ class ImageOcclusionViewModel(
9890
val args: ImageOcclusionArgs =
9991
checkNotNull(savedStateHandle[IO_ARGS_KEY]) { "$IO_ARGS_KEY required" }
10092

101-
var selectedDeckIdFlow = MutableStateFlow(args.originalDeckId)
93+
private val originalDeckId: DeckId? = (args as? ImageOcclusionArgs.Add)?.originalDeckId
94+
95+
/**
96+
* The currently selected deck ID
97+
*
98+
* Only valid in 'ADD' mode
99+
*/
100+
val selectedDeckIdFlow: MutableStateFlow<DeckId>? =
101+
originalDeckId?.let { MutableStateFlow(it) }
102102

103+
/**
104+
* The currently selected deck name
105+
*
106+
* Only valid in 'ADD' mode
107+
*/
103108
val deckNameFlow =
104-
selectedDeckIdFlow
105-
.map { did -> withCol { decks.name(did) } }
109+
selectedDeckIdFlow?.map { did -> withCol { decks.name(did) } }
106110

107111
init {
108-
// if we are in 'add' mode, thr current deck is used to add the note.
109-
// This is reverted in 'onSaveOperationCompleted'
112+
// if we are in 'add' mode, the current deck is used to add the note.
113+
// This is reverted in 'resetTemporaryDeckOverride'
110114
selectedDeckIdFlow
111-
.onEach { withCol { decks.select(it) } }
112-
.launchIn(viewModelScope)
115+
?.onEach { withCol { decks.select(it) } }
116+
?.launchIn(viewModelScope)
113117
}
114118

115119
/**
@@ -118,21 +122,37 @@ class ImageOcclusionViewModel(
118122
* @param deckId The [DeckId] object representing the selected deck. Can be null if no deck is selected.
119123
*/
120124
fun handleDeckSelection(deckId: DeckId) {
121-
selectedDeckIdFlow.value = deckId
125+
selectedDeckIdFlow?.let { it.value = deckId } ?: run {
126+
Timber.w("deck selection is unavailable")
127+
}
122128
}
123129

124130
/**
125131
* Executed when the 'save' operation is completed, before the UI receives the response
126132
*/
127133
fun onSaveOperationCompleted() {
128134
Timber.i("save operation completed")
129-
if (args.originalDeckId == selectedDeckIdFlow.value) return
135+
if (originalDeckId != null) {
136+
resetTemporaryDeckOverride(originalDeckId)
137+
}
138+
}
139+
140+
/**
141+
* Resets the current deck to the deck the screen was opened with
142+
*
143+
* Only for [ImageOcclusionArgs.Add] mode
144+
*/
145+
private fun resetTemporaryDeckOverride(originalDeckId: DeckId) {
146+
// no need to reset if the DeckId was unchanged
147+
if (originalDeckId == selectedDeckIdFlow?.value) return
148+
130149
// reset to the previous deck that the backend "saw" as selected, this
131-
// avoids other screens unexpectedly having their working decks modified(
132-
// most important being the Reviewer where the user would find itself
133-
// studying another deck after editing a note with changing the deck)
150+
// avoids other screens unexpectedly having their working decks modified
151+
// For example, the study screen: if a user backgrounds the screen, then
152+
// adds an occluded image via 'Share'
134153
viewModelScope.launch {
135-
withCol { backend.setCurrentDeck(args.originalDeckId) }
154+
Timber.i("resetting temporary deck override")
155+
withCol { backend.setCurrentDeck(originalDeckId) }
136156
}
137157
}
138158

0 commit comments

Comments
 (0)