Skip tag selection when no tags are available in the deck

[Amardurai]

Purpose / Description

Skip Tag Selection - In Custom Study options, if no tags are available, display 'OK' instead of 'Choose Tags' and start the custom study session directly, skipping the tag selection screen

Fixes

• Fixes Onboarding/UX: Handle 'Custom Study by card state or tag' when there are no tags #18479

Approach

We check whether the deck's tag list is empty. If it is, we skip the tag selection screen and proceed directly to the custom study session

How Has This Been Tested?

456150775-f7d9c81c-a7ac-4f1e-b7de-9a3073fb7474.mov

Checklist

Please, go through these checks before submitting the PR.

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[welcome]

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

[mikehardy]

I have some asynchronous-data-loading concern about deferredDefaults as mentioned directly in comments, but, assuming those are handled well, the feature itself looks like exactly what was settled on in the related issue. Nice!

[david-allison]

Thanks! Couple of architectural fixes, then good to go!

[david-allison]

We should block - requiring deferredDefaults to be completed before the screen is launched.

This is currently done:

Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt

Lines 188 to 196 in 60fb6ce

	// on a slow phone, 'extend limits' may be clicked before we know there's no new/review cards
	// show 'no cards due' if this occurs
	if (item.checkAvailability != null) {
	val defaults = withProgress { deferredDefaults.await() }
	if (!item.checkAvailability(defaults)) {
	showSnackbar(getString((R.string.studyoptions_no_cards_due)))
	return
	}
	}

Since this no longer just means checkAvailability, we should also have a requiresDefaults property on ContextMenuOption, and modify the above assertion:

require(deferredDefaults.isCompleted || selectedSubDialog!!.checkAvailability == null)

[Amardurai]

Understood, Added the new requiresDefaults field to ContextMenuOption and updated the assertion!

[david-allison]

⚠️ This doesn't seem to pass in the CramKind selection

Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt

Lines 153 to 164 in 60fb6ce

	if (selectedStatePosition == AdapterView.INVALID_POSITION) return@setFragmentResultListener
	val kind = CustomStudyCardState.entries[selectedStatePosition].kind
	val cardsAmount = userInputValue ?: 100 // the default value
	requireActivity().launchCatchingTask {
	withProgress {
	try {
	customStudy(option, cardsAmount, kind, tagsToInclude, tagsToExclude)
	} finally {
	requireActivity().dismissAllDialogFragments()
	}
	}
	}

[Amardurai]

We're already passing the CramKind as the kind parameter here:

val kind = CustomStudyCardState.entries[selectedStatePosition].kind
...
customStudy(option, cardsAmount, kind, tagsToInclude, tagsToExclude)

So the CramKind is already being handled. Let me know if I missed anything!

[david-allison]

If canChooseTags is false AND contextMenuOption == STUDY_TAGS: A user has selected a CramKind

The following code is then executed, which does not use the CramKind: customStudy(contextMenuOption, n)

customStudy is defined as the following, which uses a default value of cramKind instead.

Given that this bug was flagged in a TODO, it may be a good idea to create a sealed class which blocks this from happening again

    // TODO cram kind and the included/excluded tags lists are only relevant for STUDY_TAGS and
    //  should be included in the option to not leak in the method's api
    private suspend fun customStudy(
        contextMenuOption: ContextMenuOption,
        userEntry: Int,
        cramKind: CramKind = CramKind.CRAM_KIND_NEW,

[Amardurai]

Thanks for clarification!

I've updated the code to explicitly pass the CramKind, even when canChooseTags is false, like this:

requireActivity().launchCatchingTask {
    withProgress {
        try {
            val kind = CustomStudyCardState.entries[selectedStatePosition].kind
            customStudy(contextMenuOption, n, kind)
        } finally {
            requireActivity().dismissAllDialogFragments()
        }
    }
}

Looks good?

[david-allison]

Did you test it?

EDIT: It feels like I'm talking with an LLM right now... I can't see how that would work given the definition:

Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt

Lines 136 to 140 in 60fb6ce

	private val selectedStatePosition: Int
	get() =
	dialog
	?.findViewById<Spinner>(R.id.cards_state_selector)
	?.selectedItemPosition ?: AdapterView.INVALID_POSITION

[Amardurai]

Yes, I’ve tested with all four options, and it’s working as expected.

[david-allison]

How did you test it, it doesn't compile.

[Amardurai]

leaving for someone else