Skip to content

Enable deck option target selection in study screen #19963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lukstbit wants to merge 1 commit into
base: main
Choose a base branch
from
+142 −10
Open

Enable deck option target selection in study screen #19963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions AnkiDroid/src/main/java/com/ichi2/anki/pages/DeckOptionsDestination.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,15 @@ import com.ichi2.anki.FilteredDeckOptions
import com.ichi2.anki.libanki.DeckId
import com.ichi2.anki.utils.Destination

class DeckOptionsDestination(
private val deckId: DeckId,
private val isFiltered: Boolean,
/**
* @param options the list of deck options to present to the user before going to deck options. This
* will contain the current deck target([deckId]) plus any other possible deck targets(ex: decks of
* the current studied card)
*/
data class DeckOptionsDestination(
val deckId: DeckId,
val isFiltered: Boolean,
val options: List<DeckOptionEntry> = emptyList(),
) : Destination {
override fun toIntent(context: Context): Intent =
if (isFiltered) {
Expand All @@ -52,3 +58,19 @@ class DeckOptionsDestination(
}
}
}

/**
* True if we need to show to the user a list of decks before going to the deck options, false otherwise.
*/
val DeckOptionsDestination.haMultipleOptions: Boolean
Copy link
Member

@BrayanDSO BrayanDSO Jan 3, 2026

Choose a reason for hiding this comment

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

typo: ha -> has

Also, you could have this property inside the class instead of being a extension function on the same file. That way, we don't need to make an extra import on other packages

get() = options.isNotEmpty() && options.size > 1

/**
* Information about a deck that appears in the list of possible deck targets when deck options are
* requested from the study screen.
*/
data class DeckOptionEntry(
val deckId: DeckId,
val name: String?,
val isFiltered: Boolean,
)
70 changes: 66 additions & 4 deletions AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@ import android.text.style.UnderlineSpan
import android.view.KeyEvent
import android.view.MenuItem
import android.view.View
import android.view.ViewGroup
import android.view.ViewGroup.MarginLayoutParams
import android.view.WindowManager
import android.view.inputmethod.EditorInfo
import android.view.inputmethod.InputMethodManager
import android.webkit.WebView
import android.widget.ArrayAdapter
import android.widget.FrameLayout
import android.widget.LinearLayout
import android.widget.TextView
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.widget.ActionMenuView
import androidx.constraintlayout.widget.ConstraintSet
Expand All @@ -51,19 +54,24 @@ import androidx.lifecycle.flowWithLifecycle
import androidx.lifecycle.lifecycleScope
import androidx.lifecycle.repeatOnLifecycle
import anki.scheduler.CardAnswer.Rating
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import com.google.android.material.shape.ShapeAppearanceModel
import com.ichi2.anki.CollectionManager
import com.ichi2.anki.CollectionManager.TR
import com.ichi2.anki.DispatchKeyEventListener
import com.ichi2.anki.Flag
import com.ichi2.anki.R
import com.ichi2.anki.cardviewer.Gesture
import com.ichi2.anki.common.utils.android.isRobolectric
import com.ichi2.anki.databinding.DialogDeckOptionsSelectionBinding
import com.ichi2.anki.databinding.Reviewer2Binding
import com.ichi2.anki.dialogs.tags.TagsDialog
import com.ichi2.anki.dialogs.tags.TagsDialogFactory
import com.ichi2.anki.dialogs.tags.TagsDialogListener
import com.ichi2.anki.libanki.sched.Counts
import com.ichi2.anki.model.CardStateFilter
import com.ichi2.anki.pages.DeckOptionEntry
import com.ichi2.anki.pages.DeckOptionsDestination
import com.ichi2.anki.pages.haMultipleOptions
import com.ichi2.anki.preferences.reviewer.ViewerAction
import com.ichi2.anki.previewer.CardViewerActivity
import com.ichi2.anki.previewer.CardViewerFragment
Expand All @@ -86,12 +94,15 @@ import com.ichi2.anki.utils.ext.collectIn
import com.ichi2.anki.utils.ext.collectLatestIn
import com.ichi2.anki.utils.ext.sharedPrefs
import com.ichi2.anki.utils.ext.showDialogFragment
import com.ichi2.anki.utils.ext.usingStyledAttributes
import com.ichi2.anki.utils.ext.window
import com.ichi2.anki.workarounds.SafeWebViewLayout
import com.ichi2.themes.Themes
import com.ichi2.utils.customView
import com.ichi2.utils.dp
import com.ichi2.utils.show
import com.ichi2.utils.stripHtml
import com.ichi2.utils.title
import com.squareup.seismic.ShakeDetector
import dev.androidbroadcast.vbpd.viewBinding
import kotlinx.coroutines.Job
Expand Down Expand Up @@ -207,6 +218,21 @@ class ReviewerFragment :
}

viewModel.destinationFlow.collectIn(lifecycleScope) { destination ->
if (destination is DeckOptionsDestination && destination.haMultipleOptions) {
if (destination.options.any { it.name == null }) {
Copy link
Member

@david-allison david-allison Jan 4, 2026

Choose a reason for hiding this comment

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

Is there any other way to handle a 'bad' deck as a non-representable state

for example: making the option itself null, rather than relying on the implicit assumption of name == null => invalid'.

showSnackbar(R.string.something_wrong)
return@collectIn
}
showDeckOptionsTargetDialog(destination.options) { option ->
Copy link
Member

@david-allison david-allison Jan 4, 2026

Choose a reason for hiding this comment

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

option -> selectedOption

val updatedDestination =
destination.copy(
deckId = option.deckId,
isFiltered = option.isFiltered,
)
startActivity(updatedDestination.toIntent(requireContext()))
}
return@collectIn
}
startActivity(destination.toIntent(requireContext()))
}

Expand All @@ -232,6 +258,42 @@ class ReviewerFragment :
}
}

private fun showDeckOptionsTargetDialog(
options: List<DeckOptionEntry>,
onDeckSelected: (DeckOptionEntry) -> Unit,
) {
val binding = DialogDeckOptionsSelectionBinding.inflate(layoutInflater)
Copy link
Member

@david-allison david-allison Jan 4, 2026

Choose a reason for hiding this comment

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

nit: consider extracting this, it's a lot of code added to a very lean class (given its responsibility)

Copy link
Member

@david-allison david-allison Jan 4, 2026

Choose a reason for hiding this comment

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

Please add some high-level logs at the i level

val normalDeckNameColor: Int =
requireContext().usingStyledAttributes(null, intArrayOf(android.R.attr.textColor)) {
getColor(0, 0)
}
val dynamicDeckNameColor: Int =
requireContext().usingStyledAttributes(null, intArrayOf(R.attr.dynDeckColor)) {
getColor(0, 0)
}
binding.deckOptionsList.adapter =
object : ArrayAdapter<String>(
requireContext(),
R.layout.item_deck_option_selection,
options.map { it.name },
) {
override fun getView(
position: Int,
convertView: View?,
parent: ViewGroup,
): View {
val rowView = super.getView(position, convertView, parent) as TextView
rowView.setTextColor(if (options[position].isFiltered) dynamicDeckNameColor else normalDeckNameColor)
rowView.setOnClickListener { onDeckSelected(options[position]) }
return rowView
}
}
MaterialAlertDialogBuilder(requireContext()).show {
title(text = TR.deckConfigWhichDeck())
customView(binding.root)
}
}

private fun setupTypeAnswer() {
binding.typeAnswerEditText.apply {
setOnEditorActionListener { _, actionId, _ ->
Expand Down Expand Up @@ -559,16 +621,16 @@ class ReviewerFragment :
viewModel.stopAutoAdvance()

val minutes = (timebox.secs / 60f).roundToInt()
val message = CollectionManager.TR.studyingCardStudiedIn(timebox.reps) + " " + CollectionManager.TR.studyingMinute(minutes)
val message = TR.studyingCardStudiedIn(timebox.reps) + " " + TR.studyingMinute(minutes)

AlertDialog.Builder(requireContext()).show {
setTitle(R.string.timebox_reached_title)
setMessage(message)
setPositiveButton(CollectionManager.TR.studyingContinue()) { _, _ ->
setPositiveButton(TR.studyingContinue()) { _, _ ->
Timber.i("ReviewerFragment: Timebox 'Continue'")
viewModel.onPageFinished(false)
}
setNegativeButton(CollectionManager.TR.studyingFinish()) { _, _ ->
setNegativeButton(TR.studyingFinish()) { _, _ ->
Timber.i("ReviewerFragment: Timebox 'Finish'")
requireActivity().finish()
}
Expand Down
31 changes: 28 additions & 3 deletions AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,23 @@ import com.ichi2.anki.Reviewer
import com.ichi2.anki.asyncIO
import com.ichi2.anki.browser.BrowserDestination
import com.ichi2.anki.cardviewer.SingleCardSide
import com.ichi2.anki.common.annotations.NeedsTest
import com.ichi2.anki.common.time.TimeManager
import com.ichi2.anki.launchCatchingIO
import com.ichi2.anki.libanki.Card
import com.ichi2.anki.libanki.CardId
import com.ichi2.anki.libanki.Collection
import com.ichi2.anki.libanki.DeckId
import com.ichi2.anki.libanki.NoteId
import com.ichi2.anki.libanki.redoLabel
import com.ichi2.anki.libanki.sched.Counts
import com.ichi2.anki.libanki.sched.CurrentQueueState
import com.ichi2.anki.libanki.undoLabel
import com.ichi2.anki.noteeditor.NoteEditorLauncher
import com.ichi2.anki.observability.ChangeManager
import com.ichi2.anki.observability.undoableOp
import com.ichi2.anki.pages.AnkiServer
import com.ichi2.anki.pages.CardInfoDestination
import com.ichi2.anki.pages.DeckOptionEntry
import com.ichi2.anki.pages.DeckOptionsDestination
import com.ichi2.anki.pages.PostRequestUri
import com.ichi2.anki.pages.StatisticsDestination
Expand Down Expand Up @@ -284,14 +286,37 @@ class ReviewerViewModel(
destinationFlow.emit(destination)
}

@NeedsTest("verify that we show the expected deck options for the current card")
private suspend fun emitDeckOptionsDestination() {
val deckId = withCol { decks.getCurrentId() }
val isFiltered = withCol { decks.isFiltered(deckId) }
val destination = DeckOptionsDestination(deckId, isFiltered)
val card = currentCard.await()
// https://github.com/lukstbit/anki/blob/d24d2e33943af2361b5a9880572b30887efcf3ee/qt/aqt/deckoptions.py#L83-L100
Copy link
Member

@david-allison david-allison Jan 4, 2026

Choose a reason for hiding this comment

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

unusual to link to your fork

val extraDeckIds = mutableListOf<DeckId>()
if (card.oDid != 0L && card.oDid != deckId) {
extraDeckIds.add(card.oDid)
}
if (card.did != deckId) {
extraDeckIds.add(card.did)
}
val options = getDeckSelectionOptions(listOf(deckId) + extraDeckIds)
val destination = DeckOptionsDestination(deckId, options[0].isFiltered, options)
Timber.i("Launching 'deck options' for deck %d", deckId)
destinationFlow.emit(destination)
}

// backend sorts on dyn
private suspend fun getDeckSelectionOptions(dids: List<DeckId>): List<DeckOptionEntry> =
Copy link
Member

@david-allison david-allison Jan 4, 2026

Choose a reason for hiding this comment

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

explain or test the ordering here, it's implicit based on the order of the input parameter other than filtered decks

withCol {
dids
.map { deckId ->
DeckOptionEntry(
deckId = deckId,
name = decks.nameIfExists(deckId),
isFiltered = decks.isFiltered(deckId),
)
}.sortedBy { it.isFiltered }
}

private suspend fun emitBrowseDestination() {
val deckId = withCol { decks.getCurrentId() }
val cardId = currentCard.await().id
Expand Down
10 changes: 10 additions & 0 deletions AnkiDroid/src/main/res/layout/dialog_deck_options_selection.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="utf-8"?>
<ListView xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:id="@+id/deck_options_list"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:paddingHorizontal="24dp"
android:paddingVertical="16dp"
tools:listitem="@layout/item_deck_option_selection"
/>
13 changes: 13 additions & 0 deletions AnkiDroid/src/main/res/layout/item_deck_option_selection.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="utf-8"?>
<TextView xmlns:android="http://schemas.android.com/apk/res/android"
Copy link
Member

@david-allison david-allison Jan 4, 2026

Choose a reason for hiding this comment

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

FixedTextView

xmlns:tools="http://schemas.android.com/tools"
android:id="@+id/deck_option"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:minHeight="?attr/listPreferredItemHeight"
android:paddingVertical="8dp"
android:textAppearance="?attr/textAppearanceListItemSecondary"
android:gravity="center_vertical|start"
android:background="?attr/selectableItemBackground"
tools:text="Deck option target"
/>