Skip to content

Commit

Permalink
feat(browse): implement progress bar
Browse files Browse the repository at this point in the history 
Added bonus:
This means we can remove 'initCompleted' and don't need
to handle flow re-subscription if the user moves from dark
to light mode

Fixes a bug in the Columns where the preview didn't remain
if switching dark/light mode

* make searchForCards non-blocking
* stop refreshing searches unconditionally on state restore
  • Loading branch information
@david-allison
david-allison committed Jan 21, 2025
1 parent 959d627 commit be67a3b
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 79 deletions.
56 changes: 19 additions & 37 deletions AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ import androidx.annotation.VisibleForTesting
import androidx.appcompat.widget.SearchView
import androidx.appcompat.widget.ThemeUtils
import androidx.core.content.ContextCompat
import androidx.core.view.isVisible
import androidx.lifecycle.ViewModelProvider
import androidx.lifecycle.lifecycleScope
import androidx.recyclerview.widget.DividerItemDecoration
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import anki.collection.OpChanges
import com.google.android.material.progressindicator.LinearProgressIndicator
import com.google.android.material.snackbar.Snackbar
import com.ichi2.anim.ActivityTransitionAnimation.Direction
import com.ichi2.anki.CollectionManager.TR
Expand All @@ -61,6 +63,8 @@ import com.ichi2.anki.browser.CardBrowserColumn
import com.ichi2.anki.browser.CardBrowserLaunchOptions
import com.ichi2.anki.browser.CardBrowserViewModel
import com.ichi2.anki.browser.CardBrowserViewModel.SearchState
import com.ichi2.anki.browser.CardBrowserViewModel.SearchState.Initializing
import com.ichi2.anki.browser.CardBrowserViewModel.SearchState.Searching
import com.ichi2.anki.browser.CardOrNoteId
import com.ichi2.anki.browser.PreviewerIdsFile
import com.ichi2.anki.browser.RepositionCardsRequest.ContainsNonNewCardsError
Expand Down Expand Up @@ -255,13 +259,6 @@ open class CardBrowser :
ChangeManager.subscribe(this)
}

@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
fun changeCardOrder(sortType: SortType) =
launchCatchingTask {
// TODO: remove withProgress and replace with search progress bar
withProgress { viewModel.changeCardOrder(sortType)?.join() }
}

@VisibleForTesting
internal val mySearchesDialogListener: MySearchesDialogListener =
object : MySearchesDialogListener {
Expand Down Expand Up @@ -527,9 +524,13 @@ open class CardBrowser :
fun searchStateChanged(searchState: SearchState) {
Timber.d("search state: %s", searchState)
notifyDataSetChanged()

findViewById<LinearProgressIndicator>(R.id.browser_progress).isVisible =
searchState == Initializing ||
searchState == Searching
when (searchState) {
SearchState.Initializing -> { }
SearchState.Searching -> {
Initializing -> { }
Searching -> {
if ("" != viewModel.searchTerms && searchView != null) {
searchView!!.setQuery(viewModel.searchTerms, false)
searchItem!!.expandActionView()
Expand All @@ -542,12 +543,6 @@ open class CardBrowser :
}
}

fun initCompletedChanged(completed: Boolean) {
if (!completed) return

searchCards()
}

viewModel.flowOfIsTruncated.launchCollectionInLifecycleScope(::onIsTruncatedChanged)
viewModel.flowOfSearchQueryExpanded.launchCollectionInLifecycleScope(::onSearchQueryExpanded)
viewModel.flowOfSelectedRows.launchCollectionInLifecycleScope(::onSelectedRowsChanged)
Expand All @@ -558,15 +553,13 @@ open class CardBrowser :
viewModel.flowOfIsInMultiSelectMode.launchCollectionInLifecycleScope(::isInMultiSelectModeChanged)
viewModel.flowOfCardsUpdated.launchCollectionInLifecycleScope(::cardsUpdatedChanged)
viewModel.flowOfSearchState.launchCollectionInLifecycleScope(::searchStateChanged)
viewModel.flowOfInitCompleted.launchCollectionInLifecycleScope(::initCompletedChanged)
}

// Finish initializing the activity after the collection has been correctly loaded
override fun onCollectionLoaded(col: Collection) {
super.onCollectionLoaded(col)
Timber.d("onCollectionLoaded()")
registerReceiver()
cards.reset()

window.setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_ALWAYS_HIDDEN)
deckSpinnerSelection.apply {
Expand Down Expand Up @@ -898,7 +891,7 @@ open class CardBrowser :
viewModel.setSearchQueryExpanded(false)
// SearchView doesn't support empty queries so we always reset the search when collapsing
searchView!!.setQuery("", false)
searchCards("")
viewModel.launchSearchForCards("")
return true
}
},
Expand All @@ -918,7 +911,7 @@ open class CardBrowser :
}

override fun onQueryTextSubmit(query: String): Boolean {
searchCards(query)
viewModel.launchSearchForCards(query)
searchView!!.clearFocus()
return true
}
Expand Down Expand Up @@ -1264,7 +1257,7 @@ open class CardBrowser :
// TODO: move this into the ViewModel
CardBrowserOrderDialog.newInstance { dialog: DialogInterface, which: Int ->
dialog.dismiss()
changeCardOrder(SortType.fromCardBrowserLabelIndex(which))
viewModel.changeCardOrder(SortType.fromCardBrowserLabelIndex(which))
},
)
}
Expand Down Expand Up @@ -1562,22 +1555,17 @@ open class CardBrowser :

public override fun onRestoreInstanceState(savedInstanceState: Bundle) {
super.onRestoreInstanceState(savedInstanceState)
searchCards(savedInstanceState.getString("mSearchTerms", ""))
viewModel.launchSearchForCards(
savedInstanceState.getString("mSearchTerms", ""),
forceRefresh = false,
)
}

private fun forceRefreshSearch(useSearchTextValue: Boolean = false) {
if (useSearchTextValue && searchView != null) {
searchCards(searchView!!.query.toString())
viewModel.launchSearchForCards(searchView!!.query.toString())
} else {
searchCards()
}
}

@VisibleForTesting
fun searchCards() {
launchCatchingTask {
// TODO: Move this to a LinearProgressIndicator and remove withProgress
withProgress { viewModel.launchSearchForCards()?.join() }
viewModel.launchSearchForCards()
}
}

Expand Down Expand Up @@ -1799,12 +1787,6 @@ open class CardBrowser :
filterByTags(tags.toList(), CardStateFilter.ALL_CARDS)
}

@VisibleForTesting
fun searchCards(searchQuery: String) =
launchCatchingTask {
withProgress { viewModel.launchSearchForCards(searchQuery)?.join() }
}

override fun opExecuted(
changes: OpChanges,
handler: Any?,
Expand Down
80 changes: 46 additions & 34 deletions AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ class CardBrowserViewModel(

if (!manualInit) {
flowOfInitCompleted.update { true }
launchSearchForCards()
}
}
}
Expand Down Expand Up @@ -492,18 +493,18 @@ class CardBrowserViewModel(

fun hasSelectedAllDecks(): Boolean = lastDeckId == ALL_DECKS_ID

suspend fun changeCardOrder(which: SortType): Job? {
fun changeCardOrder(which: SortType) {
val changeType =
when {
which != order -> ChangeCardOrder.OrderChange(which)
// if the same element is selected again, reverse the order
which != SortType.NO_SORTING -> ChangeCardOrder.DirectionChange
else -> null
} ?: return null
} ?: return

Timber.i("updating order: %s", changeType)

return when (changeType) {
when (changeType) {
is ChangeCardOrder.OrderChange -> {
sortTypeFlow.update { which }
reverseDirectionFlow.update { ReverseDirection(orderAsc = false) }
Expand All @@ -512,8 +513,7 @@ class CardBrowserViewModel(
ChangeCardOrder.DirectionChange -> {
reverseDirectionFlow.update { ReverseDirection(orderAsc = !orderAsc) }
cards.reverse()
flowOfSearchState.emit(SearchState.Completed)
null
viewModelScope.launch { flowOfSearchState.emit(SearchState.Completed) }
}
}
}
Expand Down Expand Up @@ -854,45 +854,57 @@ class CardBrowserViewModel(
*/
fun endMultiSelectMode() = selectNone()

suspend fun launchSearchForCards(searchQuery: String): Job? {
/**
* @param forceRefresh if `true`, perform a search even if the search query is unchanged
*/
fun launchSearchForCards(
searchQuery: String,
forceRefresh: Boolean = true,
) {
if (!forceRefresh && searchTerms == searchQuery) {
Timber.d("skipping duplicate search: forceRefresh is false")
return
}
searchTerms = searchQuery
return launchSearchForCards()
launchSearchForCards()
}

/**
* @see com.ichi2.anki.searchForRows
*/
@NeedsTest("Invalid searches are handled. For instance: 'and'")
suspend fun launchSearchForCards(): Job? {
if (!initCompleted) return null
// update the UI while we're searching
clearCardsList()

val query: String =
if (searchTerms.contains("deck:")) {
"($searchTerms)"
} else {
if ("" != searchTerms) "$restrictOnDeck($searchTerms)" else restrictOnDeck
}
fun launchSearchForCards() {
if (!initCompleted) return

searchJob?.cancel()
searchJob =
launchCatchingIO(
errorMessageHandler = { error -> flowOfSearchState.emit(SearchState.Error(error)) },
) {
flowOfSearchState.emit(SearchState.Searching)
Timber.d("performing search: '%s'", query)
val cards = com.ichi2.anki.searchForRows(query, order.toSortOrder(), cardsOrNotes)
Timber.d("Search returned %d card(s)", cards.size)

ensureActive()
this@CardBrowserViewModel.cards.replaceWith(cardsOrNotes, cards)
flowOfSearchState.emit(SearchState.Completed)
}
return searchJob!!
viewModelScope.launch {
// update the UI while we're searching
clearCardsList()

val query: String =
if (searchTerms.contains("deck:")) {
"($searchTerms)"
} else {
if ("" != searchTerms) "$restrictOnDeck($searchTerms)" else restrictOnDeck
}

searchJob?.cancel()
searchJob =
launchCatchingIO(
errorMessageHandler = { error -> flowOfSearchState.emit(SearchState.Error(error)) },
) {
flowOfSearchState.emit(SearchState.Searching)
Timber.d("performing search: '%s'", query)
val cards = com.ichi2.anki.searchForRows(query, order.toSortOrder(), cardsOrNotes)
Timber.d("Search returned %d card(s)", cards.size)

ensureActive()
this@CardBrowserViewModel.cards.replaceWith(cardsOrNotes, cards)
flowOfSearchState.emit(SearchState.Completed)
}
}
}

private suspend fun refreshSearch() = launchSearchForCards()?.join()
private fun refreshSearch() = launchSearchForCards()

private suspend fun clearCardsList() {
cards.reset()
Expand Down
10 changes: 8 additions & 2 deletions AnkiDroid/src/main/res/layout/card_browser.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@

</LinearLayout>

<com.google.android.material.progressindicator.LinearProgressIndicator
android:id="@+id/browser_progress"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:indeterminate="true"
android:visibility="gone"
/>

<androidx.recyclerview.widget.RecyclerView
android:id="@+id/card_browser_list"
android:layout_width="match_parent"
Expand All @@ -40,6 +48,4 @@

</LinearLayout>

<include layout="@layout/anki_progress"/>

</androidx.coordinatorlayout.widget.CoordinatorLayout>
20 changes: 14 additions & 6 deletions AnkiDroid/src/test/java/com/ichi2/anki/CardBrowserTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ class CardBrowserTest : RobolectricTest() {
)

// reverse
b.changeCardOrder(SortType.SORT_FIELD)
b.viewModel.changeCardOrder(SortType.SORT_FIELD)

b.replaceSelectionWith(intArrayOf(0))
val intentAfterReverse = b.viewModel.queryPreviewIntentData()
Expand Down Expand Up @@ -704,7 +704,7 @@ class CardBrowserTest : RobolectricTest() {
)

// Change the display order of the card browser
cardBrowserController.get().changeCardOrder(SortType.EASE)
cardBrowserController.get().viewModel.changeCardOrder(SortType.EASE)

// Kill and restart the activity and ensure that display order is preserved
val outBundle = Bundle()
Expand Down Expand Up @@ -749,8 +749,7 @@ class CardBrowserTest : RobolectricTest() {
}

val cardBrowser = browserWithNoNewCards
cardBrowser.searchCards("Hello").join()
waitForAsyncTasksToComplete()
cardBrowser.searchCards("Hello")
assertThat(
"Card browser should have Test Deck as the selected deck",
cardBrowser.selectedDeckNameForUi,
Expand Down Expand Up @@ -786,8 +785,8 @@ class CardBrowserTest : RobolectricTest() {
@Test
fun checkDisplayOrderAfterTogglingCardsToNotes() {
browserWithNoNewCards.apply {
changeCardOrder(SortType.EASE) // order no. 7 corresponds to "cardEase"
changeCardOrder(SortType.EASE) // reverse the list
viewModel.changeCardOrder(SortType.EASE) // order no. 7 corresponds to "cardEase"
viewModel.changeCardOrder(SortType.EASE) // reverse the list

viewModel.setCardsOrNotes(NOTES)
searchCards()
Expand Down Expand Up @@ -1248,3 +1247,12 @@ val CardBrowser.columnHeadingViews
val CardBrowser.columnHeadings
get() =
columnHeadingViews.map { it.text.toString() }

fun CardBrowser.searchCards(search: String? = null) {
if (search != null) {
viewModel.launchSearchForCards(search)
} else {
viewModel.launchSearchForCards()
}
runBlocking { viewModel.searchJob?.join() }
}

0 comments on commit be67a3b

Please sign in to comment.