Skip to content

Commit

Permalink
Fix color scheme for empty cards report dialog
Browse files Browse the repository at this point in the history 
This removes the initial implementation using an WebView in favor of a
TextView and parsing the report with HtmlCompat. The nids links are
now implemented through ClickSpans.

Also adds a test to check the html backend report structure to make
sure the report parsing code present in EmptyCardsFragment stays
relevant.
  • Loading branch information
@lukstbit
lukstbit committed Feb 16, 2025
1 parent 296c4d0 commit 41cdcad
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 47 deletions.
1 change: 1 addition & 0 deletions .idea/dictionaries/android.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<w>ENOENT</w>
<w>FILESIZE</w>
<w>ONEPLUS</w>
<w>allempty</w>
<w>apkgfileprovider</w>
<w>asynctasks</w>
<w>backreference</w>
Expand Down
141 changes: 97 additions & 44 deletions AnkiDroid/src/main/java/com/ichi2/anki/dialogs/EmptyCardsDialogFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,27 @@ package com.ichi2.anki.dialogs

import android.app.Activity
import android.app.Dialog
import android.content.Context
import android.content.Intent
import android.content.res.Configuration
import android.content.res.Configuration.ORIENTATION_LANDSCAPE
import android.graphics.Insets
import android.os.Build
import android.os.Bundle
import android.text.SpannableStringBuilder
import android.text.Spanned
import android.text.method.LinkMovementMethod
import android.text.style.ClickableSpan
import android.util.DisplayMetrics
import android.view.View
import android.view.ViewGroup
import android.view.WindowInsets.Type.displayCutout
import android.view.WindowInsets.Type.navigationBars
import android.webkit.WebResourceRequest
import android.webkit.WebView
import android.webkit.WebViewClient
import android.widget.CheckBox
import android.widget.ScrollView
import android.widget.TextView
import androidx.appcompat.app.AlertDialog
import androidx.constraintlayout.widget.ConstraintLayout
import androidx.core.text.HtmlCompat
import androidx.core.view.isVisible
import androidx.fragment.app.DialogFragment
import androidx.fragment.app.viewModels
Expand All @@ -53,6 +57,7 @@ import com.ichi2.anki.dialogs.EmptyCardsUiState.SearchingForEmptyCards
import com.ichi2.anki.launchCatchingTask
import com.ichi2.anki.ui.internationalization.toSentenceCase
import com.ichi2.anki.withProgress
import com.ichi2.libanki.NoteId
import com.ichi2.libanki.emptyCids
import com.ichi2.utils.message
import com.ichi2.utils.positiveButton
Expand All @@ -77,36 +82,18 @@ class EmptyCardsDialogFragment : DialogFragment() {
get() = dialog?.findViewById(R.id.empty_report_label)
private val keepNotesWithNoValidCards: CheckBox?
get() = dialog?.findViewById(R.id.preserve_notes)
private val reportWebView: WebView?
private val reportScrollView: ScrollView?
get() = dialog?.findViewById(R.id.reportScrollView)
private val reportView: TextView?
get() = dialog?.findViewById(R.id.report)

// TODO handle WebViewClient.onRenderProcessGone()
private val customWebViewClient =
object : WebViewClient() {
override fun shouldOverrideUrlLoading(
view: WebView?,
request: WebResourceRequest?,
): Boolean {
val searchQuery = request?.url?.toString()
return if (searchQuery != null && searchQuery.startsWith("nid:")) {
val browserSearchIntent = Intent(requireContext(), CardBrowser::class.java)
browserSearchIntent.putExtra("search_query", searchQuery)
browserSearchIntent.putExtra("all_decks", true)
startActivity(browserSearchIntent)
true
} else {
super.shouldOverrideUrlLoading(view, request)
}
}
}

override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
bindToState()
viewModel.searchForEmptyCards()
val dialogView = layoutInflater.inflate(R.layout.dialog_empty_cards, null)
dialogView.findViewById<CheckBox>(R.id.preserve_notes)?.text =
TR.emptyCardsPreserveNotesCheckbox()
dialogView.findViewById<WebView>(R.id.report).webViewClient = customWebViewClient
dialogView.findViewById<TextView>(R.id.report).movementMethod = LinkMovementMethod.getInstance()

return AlertDialog
.Builder(requireContext())
Expand Down Expand Up @@ -166,11 +153,10 @@ class EmptyCardsDialogFragment : DialogFragment() {
(dialog as? AlertDialog)?.positiveButton?.text =
getString(R.string.dialog_ok)
} else {
reportWebView?.updateWebViewHeight()
reportWebView?.loadData(
reportScrollView?.updateViewHeight()
reportView?.setText(
state.emptyCardsReport.asActionableReport(),
"text/html",
null,
TextView.BufferType.SPANNABLE,
)
keepNotesWithNoValidCards?.isVisible = true
emptyReportMessage?.isVisible = false
Expand Down Expand Up @@ -198,24 +184,91 @@ class EmptyCardsDialogFragment : DialogFragment() {
}

/**
* Replaces the anki format [anki:nid:#nid](ex: [anki:nid:234783924354]) with "nid:#id"(
* ex. nid:234783924354) to be used as a query text in [CardBrowser].
* Replaces the anki format [anki:nid:#nid](ex: [anki:nid:234783924354]) from the report with
* just the nid as a [ClickableSpan] which will trigger a [CardBrowser] search for that nid.
*/
// https://github.com/ankitects/anki/blob/de7a693465ca302e457a4767c7f213c76478f0ee/qt/aqt/emptycards.py#L56-L60
private fun EmptyCardsReport.asActionableReport(): String {
@Suppress("RegExpRedundantEscape")
return Regex("\\[anki:nid:(\\d+)\\]")
.replace(
report,
"<a href=\"nid:$1\">$1</a>",
private fun EmptyCardsReport.asActionableReport(): SpannableStringBuilder {
val spannableReport =
SpannableStringBuilder(HtmlCompat.fromHtml(report, HtmlCompat.FROM_HTML_MODE_LEGACY))
AnkiNidTag.parseFromReport(spannableReport).forEach { tag ->
// make nid clickable
spannableReport.setSpan(
BrowserSearchByNidSpan(requireContext(), tag.nid),
tag.matchedNid.range.first,
tag.matchedNid.range.last + 1,
Spanned.SPAN_EXCLUSIVE_EXCLUSIVE,
)
// remove suffix
spannableReport.delete(tag.matchedSuffix)
// remove prefix
spannableReport.delete(tag.matchedPrefix)
}
return spannableReport
}

private fun SpannableStringBuilder.delete(group: MatchGroup) {
delete(group.range.first, group.range.last + 1)
}

/**
* Represents a Regex over `[anki:nid:1234]`.
*
* @param matchedPrefix `[anki:nid:
* @param matchedNid `1234`
* @param nid `1234`
* @param matchedSuffix `]`
*/
private class AnkiNidTag(
val matchedPrefix: MatchGroup,
val matchedNid: MatchGroup,
val nid: NoteId,
val matchedSuffix: MatchGroup,
) {
companion object {
// https://github.com/ankitects/anki/blob/de7a693465ca302e457a4767c7f213c76478f0ee/qt/aqt/emptycards.py#L56-L60
@Suppress("RegExpRedundantEscape")
private val ankiNidPattern = Regex("(\\[anki:nid:)(\\d+)(\\])")

fun parseFromReport(report: SpannableStringBuilder): Sequence<AnkiNidTag> {
return ankiNidPattern.findAll(report).mapNotNull { result ->
// for an entry like [anki:nid:1234] we should have 4 groups: the entire match(
// [anki:nid:1234]) and the three groups we defined: '[anki:nid:', '1234', ']'
if (result.groups.size != 4) return@mapNotNull null
val matchedPrefix = result.groups[1] ?: return@mapNotNull null
val matchedNid = result.groups[2] ?: return@mapNotNull null
val nid = matchedNid.value.toLongOrNull() ?: return@mapNotNull null
val matchedSuffix = result.groups[3] ?: return@mapNotNull null

AnkiNidTag(matchedPrefix, matchedNid, nid, matchedSuffix)
}
}
}
}

/**
* A specialized [ClickableSpan] that on click will open the [CardBrowser] and initiate a
* search with the passed [nid].
*
* @see CardBrowser
*/
private class BrowserSearchByNidSpan(
val context: Context,
val nid: Long,
) : ClickableSpan() {
override fun onClick(widget: View) {
val browserSearchIntent = Intent(context, CardBrowser::class.java)
browserSearchIntent.putExtra("search_query", "nid:$nid")
browserSearchIntent.putExtra("all_decks", true)
context.startActivity(browserSearchIntent)
}
}

/**
* The [WebView] doesn't properly fit the allocated space in the dialog so this method manually
* updates the [WebView]'s height to a value that fits, depending on orientation and screen size.
* The [ScrollView] in this dialog's custom layout doesn't properly fit the allocated height so
* this method manually updates the [ScrollView]'s height to a value that fits, depending on
* orientation and screen size.
*/
private fun WebView.updateWebViewHeight() {
private fun View.updateViewHeight() {
val currentOrientation = requireContext().resources.configuration.orientation
val targetPercent = if (currentOrientation == ORIENTATION_LANDSCAPE) 0.25 else 0.5
val screenHeight =
Expand All @@ -239,14 +292,14 @@ class EmptyCardsDialogFragment : DialogFragment() {
displayMetrics.heightPixels
}
val calculatedHeight = (screenHeight * targetPercent).toInt()
(layoutParams as ConstraintLayout.LayoutParams).height = calculatedHeight
(layoutParams as ViewGroup.LayoutParams).height = calculatedHeight
layoutParams = layoutParams
requestLayout()
}

override fun onConfigurationChanged(newConfig: Configuration) {
super.onConfigurationChanged(newConfig)
reportWebView?.updateWebViewHeight()
reportScrollView?.updateViewHeight()
}

companion object {
Expand Down
14 changes: 11 additions & 3 deletions AnkiDroid/src/main/res/layout/dialog_empty_cards.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,21 @@
android:paddingStart="@dimen/side_margin"
android:paddingEnd="@dimen/side_margin">

<WebView
android:id="@+id/report"
<ScrollView
android:id="@+id/reportScrollView"
android:layout_width="match_parent"
android:layout_height="wrap_content"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toTopOf="@+id/preserve_notes"/>
android:layout_marginTop="8dp"
android:paddingHorizontal="8dp"
app:layout_constraintBottom_toTopOf="@+id/preserve_notes">
<TextView
android:id="@+id/report"
android:layout_width="match_parent"
android:layout_height="wrap_content"/>
</ScrollView>

<CheckBox
android:id="@+id/preserve_notes"
Expand All @@ -86,6 +93,7 @@
android:checked="true"
tools:text="Keep notes with no valid cards"
android:layout_marginTop="8dp"
android:padding="4dp"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintBottom_toBottomOf="parent"/>
Expand Down
51 changes: 51 additions & 0 deletions AnkiDroid/src/test/java/com/ichi2/anki/dialogs/EmptyCardsReportTest.kt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/****************************************************************************************
* Copyright (c) 2025 lukstbit <52494258+lukstbit@users.noreply.github.com> *
* *
* This program is free software; you can redistribute it and/or modify it under *
* the terms of the GNU General Public License as published by the Free Software *
* Foundation; either version 3 of the License, or (at your option) any later *
* version. *
* *
* This program is distributed in the hope that it will be useful, but WITHOUT ANY *
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A *
* PARTICULAR PURPOSE. See the GNU General Public License for more details. *
* *
* You should have received a copy of the GNU General Public License along with *
* this program. If not, see <http://www.gnu.org/licenses/>. *
****************************************************************************************/
package com.ichi2.anki.dialogs

import androidx.test.ext.junit.runners.AndroidJUnit4
import com.ichi2.anki.CollectionManager.withCol
import com.ichi2.anki.RobolectricTest
import org.junit.Test
import org.junit.runner.RunWith
import kotlin.test.assertEquals
import kotlin.test.assertTrue

@RunWith(AndroidJUnit4::class)
class EmptyCardsReportTest : RobolectricTest() {
@Test
fun `backend report has expected html structure`() =
runTest {
addStandardNoteType("TestNotetype", arrayOf("f1", "f2"), "", "")
val note1 = addNoteUsingNoteTypeName("TestNotetype", "f1text1", "f2text1")
val note2 = addNoteUsingNoteTypeName("TestNotetype", "f1text2", "f2text2")
val emptyCardsReport = withCol { getEmptyCards() }
assertEquals(getExpectedEmptyCardsReport(note1.id, note2.id), emptyCardsReport.report)
}

private fun getExpectedEmptyCardsReport(
nid1: Long,
nid2: Long,
) =
"<div><b>Empty cards for \u2068TestNotetype\u2069:</b></div><ol><li class=allempty>[anki:nid:$nid1] \u20681\u2069 of \u20681\u2069 cards empty (\u2068Card 1\u2069).</li><li class=allempty>[anki:nid:$nid2] \u20681\u2069 of \u20681\u2069 cards empty (\u2068Card 1\u2069).</li></ol>"

@Test
fun `backend report is empty when there are no empty cards`() =
runTest {
addNotes(12)
val emptyCardsReport = withCol { getEmptyCards() }
assertTrue(emptyCardsReport.report.isEmpty())
}
}

0 comments on commit 41cdcad

Please sign in to comment.