Skip to content

Commit

Permalink
improvement(custom-study): disable unusable items
Browse files Browse the repository at this point in the history 
Instead of removing them: better for UX

Issue 17666
  • Loading branch information
@david-allison
david-allison committed Jan 3, 2025
1 parent a842143 commit 77f3750
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 53 deletions.
163 changes: 116 additions & 47 deletions AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,16 @@ package com.ichi2.anki.dialogs.customstudy
import android.annotation.SuppressLint
import android.app.Dialog
import android.content.res.Resources
import android.graphics.Color
import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.view.WindowManager
import android.view.inputmethod.EditorInfo
import android.widget.ArrayAdapter
import android.widget.EditText
import android.widget.ListView
import android.widget.TextView
import androidx.annotation.VisibleForTesting
import androidx.appcompat.app.AlertDialog
Expand Down Expand Up @@ -51,6 +57,8 @@ import com.ichi2.anki.preferences.sharedPrefs
import com.ichi2.anki.showThemedToast
import com.ichi2.anki.ui.internationalization.toSentenceCase
import com.ichi2.anki.utils.ext.dismissAllDialogFragments
import com.ichi2.anki.utils.ext.dp
import com.ichi2.anki.utils.ext.setPaddingRelative
import com.ichi2.anki.utils.ext.sharedPrefs
import com.ichi2.anki.utils.ext.showDialogFragment
import com.ichi2.anki.withProgress
Expand All @@ -65,7 +73,6 @@ import com.ichi2.utils.BundleUtils.getNullableInt
import com.ichi2.utils.KotlinCleanup
import com.ichi2.utils.cancelable
import com.ichi2.utils.customView
import com.ichi2.utils.listItems
import com.ichi2.utils.negativeButton
import com.ichi2.utils.positiveButton
import com.ichi2.utils.textAsIntOrNull
Expand Down Expand Up @@ -167,44 +174,99 @@ class CustomStudyDialog(
}
}

/**
* Handles selecting an item from the main menu of the dialog
*/
private fun onMenuItemSelected(item: ContextMenuOption) =
when (item) {
STUDY_TAGS -> {
/*
* This is a special Dialog for CUSTOM STUDY, where instead of only collecting a
* number, it is necessary to collect a list of tags. This case handles the creation
* of that Dialog.
*/
val dialogFragment =
TagsDialog().withArguments(
context = requireContext(),
type = TagsDialog.DialogType.CUSTOM_STUDY_TAGS,
checkedTags = ArrayList(),
allTags = ArrayList(collection.tags.byDeck(dialogDeckId)),
)
requireActivity().showDialogFragment(dialogFragment)
}
STUDY_NEW,
STUDY_REV,
STUDY_FORGOT,
STUDY_AHEAD,
STUDY_PREVIEW,
-> {
// User asked for a standard custom study option
val d =
CustomStudyDialog(collection, customStudyListener)
.withArguments(dialogDeckId, item)
requireActivity().showDialogFragment(d)
}
}

private fun buildContextMenu(): AlertDialog {
val listIds = getListIds()
val menuItems = buildMenuItems()

val adapter =
object : ArrayAdapter<ContextMenuData>(requireContext(), android.R.layout.simple_list_item_1, menuItems) {
override fun getView(
position: Int,
convertView: View?,
parent: ViewGroup,
): View {
val view = convertView ?: LayoutInflater.from(context).inflate(android.R.layout.simple_list_item_1, parent, false)
val textView = view.findViewById<TextView>(android.R.id.text1)

val (item, isEnabled) = getItem(position)!!
textView.text = item.getTitle(resources)
textView.isEnabled = isEnabled
textView.setTextAppearance(android.R.style.TextAppearance_Material_Body1)
return view
}
}

// we use a custom view, because we want 'disabled' items, where clicking them performs
// a no-op rather than closing the dialog
val listView =
ListView(context).apply {
this.adapter = adapter

divider = null
setPaddingRelative(9.dp)

this.setOnItemClickListener { _, _, index, _ ->
val selectedItem = menuItems[index]
if (!selectedItem.enabled) {
return@setOnItemClickListener.also {
Timber.d("disabled item '%s' clicked", selectedItem.item)
}
}

onMenuItemSelected(selectedItem.item)
}

// some items are disabled
val disabledChildItems =
menuItems
.withIndex()
.filter { !it.value.enabled }
.map { getChildAt(it.index) }
for (child in disabledChildItems) {
child?.isEnabled = false
child?.setBackgroundColor(Color.TRANSPARENT) // remove selectable background
}
}

return AlertDialog
.Builder(requireActivity())
.title(text = TR.actionsCustomStudy().toSentenceCase(this, R.string.sentence_custom_study))
.cancelable(true)
.listItems(items = listIds.map { it.getTitle(resources) }) { _, index ->
when (listIds[index]) {
STUDY_TAGS -> {
/*
* This is a special Dialog for CUSTOM STUDY, where instead of only collecting a
* number, it is necessary to collect a list of tags. This case handles the creation
* of that Dialog.
*/
val dialogFragment =
TagsDialog().withArguments(
context = requireContext(),
type = TagsDialog.DialogType.CUSTOM_STUDY_TAGS,
checkedTags = ArrayList(),
allTags = ArrayList(collection.tags.byDeck(dialogDeckId)),
)
requireActivity().showDialogFragment(dialogFragment)
}
STUDY_NEW,
STUDY_REV,
STUDY_FORGOT,
STUDY_AHEAD,
STUDY_PREVIEW,
-> {
// User asked for a standard custom study option
val d =
CustomStudyDialog(collection, customStudyListener)
.withArguments(dialogDeckId, listIds[index])
requireActivity().showDialogFragment(d)
}
}
}.create()
.customView(listView)
.create()
}

/**
Expand Down Expand Up @@ -336,21 +398,19 @@ class CustomStudyDialog(
}

/**
* Retrieve the list of ids to put in the context menu list
* @return the ids of which values to show
* @return the items and their enabled status
* @see ContextMenuData
*/
private fun getListIds(): List<ContextMenuOption> {
// Standard context menu
return mutableListOf(STUDY_FORGOT, STUDY_AHEAD, STUDY_PREVIEW, STUDY_TAGS).apply {
if (defaults.extendReview.isUsable) {
this.add(0, STUDY_REV)
}
// We want 'Extend new' above 'Extend review' if both appear
if (defaults.extendNew.isUsable) {
this.add(0, STUDY_NEW)
}
private fun buildMenuItems(): List<ContextMenuData> =
mutableListOf(STUDY_NEW, STUDY_REV, STUDY_FORGOT, STUDY_AHEAD, STUDY_PREVIEW, STUDY_TAGS).map { option ->
val enabled =
when (option) {
STUDY_NEW -> defaults.extendNew.isUsable
STUDY_REV -> defaults.extendReview.isUsable
else -> true
}
ContextMenuData(option, enabled)
}
}

/** Line 1 of the number entry dialog */
private val text1: String
Expand Down Expand Up @@ -460,7 +520,16 @@ class CustomStudyDialog(
}

/**
* Possible context menu options that could be shown in the custom study dialog.
* A [context menu item][ContextMenuOption], and whether it is usable.
* Example: [STUDY_NEW] would be unusable if there are no new cards
*/
data class ContextMenuData(
val item: ContextMenuOption,
val enabled: Boolean,
)

/**
* Context menu options shown in the custom study dialog.
*/
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
enum class ContextMenuOption(
Expand Down
12 changes: 6 additions & 6 deletions AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ import android.os.Bundle
import androidx.appcompat.app.AlertDialog
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.action.ViewActions.replaceText
import androidx.test.espresso.assertion.ViewAssertions.doesNotExist
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.RootMatchers.isDialog
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
import androidx.test.espresso.matcher.ViewMatchers.isEnabled
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.ext.junit.runners.AndroidJUnit4
Expand All @@ -47,6 +46,7 @@ import com.ichi2.testutils.isJsonEqual
import io.mockk.every
import io.mockk.mockk
import org.hamcrest.CoreMatchers.equalTo
import org.hamcrest.CoreMatchers.not
import org.hamcrest.MatcherAssert.assertThat
import org.intellij.lang.annotations.Language
import org.json.JSONObject
Expand Down Expand Up @@ -172,7 +172,7 @@ class CustomStudyDialogTest : RobolectricTest() {
) { dialogFragment: CustomStudyDialog ->
onView(withText(TR.customStudyIncreaseTodaysNewCardLimit()))
.inRoot(isDialog())
.check(matches(isDisplayed()))
.check(matches(isEnabled()))
}
}

Expand All @@ -187,7 +187,7 @@ class CustomStudyDialogTest : RobolectricTest() {
) { dialogFragment: CustomStudyDialog ->
onView(withText(TR.customStudyIncreaseTodaysNewCardLimit()))
.inRoot(isDialog())
.check(doesNotExist())
.check(matches(not(isEnabled())))
}
}

Expand All @@ -202,7 +202,7 @@ class CustomStudyDialogTest : RobolectricTest() {
) { dialogFragment: CustomStudyDialog ->
onView(withText(TR.customStudyIncreaseTodaysReviewCardLimit()))
.inRoot(isDialog())
.check(matches(isDisplayed()))
.check(matches(isEnabled()))
}
}

Expand All @@ -217,7 +217,7 @@ class CustomStudyDialogTest : RobolectricTest() {
) { dialogFragment: CustomStudyDialog ->
onView(withText(TR.customStudyIncreaseTodaysReviewCardLimit()))
.inRoot(isDialog())
.check(doesNotExist())
.check(matches(not(isEnabled())))
}
}

Expand Down

0 comments on commit 77f3750

Please sign in to comment.