Skip to content

Commit

Permalink
fix(browse): col reopen & language change
Browse files Browse the repository at this point in the history
* After a collection change, `setActiveBrowserColumns` is reset
* After a language change, column headers need to be updated

Due to the language change, the activity goes through `onCreate`,
so pick up this change on `onRestoreInstanceState`

This was a good opportunity to decouple the column data and the column
headings in the View

Fixes 17803
Fixes 17367
  • Loading branch information
david-allison committed Jan 17, 2025
1 parent e19d86f commit bd04738
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 41 deletions.
45 changes: 23 additions & 22 deletions AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,18 @@ import com.ichi2.anki.browser.BrowserColumnCollection
import com.ichi2.anki.browser.BrowserColumnSelectionFragment
import com.ichi2.anki.browser.BrowserMultiColumnAdapter
import com.ichi2.anki.browser.BrowserRowCollection
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.ColumnHeading
import com.ichi2.anki.browser.PreviewerIdsFile
import com.ichi2.anki.browser.RepositionCardsRequest.ContainsNonNewCardsError
import com.ichi2.anki.browser.RepositionCardsRequest.RepositionData
import com.ichi2.anki.browser.SaveSearchResult
import com.ichi2.anki.browser.SharedPreferencesLastDeckIdRepository
import com.ichi2.anki.browser.getLabel
import com.ichi2.anki.browser.toCardBrowserLaunchOptions
import com.ichi2.anki.dialogs.BrowserOptionsDialog
import com.ichi2.anki.dialogs.CardBrowserMySearchesDialog
Expand Down Expand Up @@ -455,27 +454,8 @@ open class CardBrowser :
fun onSelectedRowsChanged(rows: Set<Any>) = onSelectionChanged()

fun onColumnsChanged(columnCollection: BrowserColumnCollection) {
Timber.d("columns changed")
notifyDataSetChanged()

if (!viewModel.columnNamesLoaded) return

// reset headings
val headingsContainer =
browserColumnHeadings.apply {
removeAllViews()
}

val layoutInflater = LayoutInflater.from(headingsContainer.context)
for (column in columnCollection.columns) {
Timber.d("setting up column %s", column)
layoutInflater.inflate(R.layout.browse_column_heading, headingsContainer, false).apply {
headingsContainer.addView(this)
(this as TextView).text =
viewModel
.namedColumns[CardBrowserColumn.entries.indexOf(column)]
.getLabel(viewModel.cardsOrNotes)
}
}
}

fun onFilterQueryChanged(filterQuery: String) {
Expand Down Expand Up @@ -540,6 +520,25 @@ open class CardBrowser :
}
}

fun onColumnNamesChanged(columnCollection: List<ColumnHeading>) {
Timber.d("column names changed")
// reset headings
val headingsContainer =
browserColumnHeadings.apply {
removeAllViews()
}

// set up the new columns
val layoutInflater = LayoutInflater.from(headingsContainer.context)
for (column in columnCollection) {
Timber.d("setting up column %s", column)
layoutInflater.inflate(R.layout.browse_column_heading, headingsContainer, false).apply {
headingsContainer.addView(this)
(this as TextView).text = column.label
}
}
}

viewModel.flowOfIsTruncated.launchCollectionInLifecycleScope(::onIsTruncatedChanged)
viewModel.flowOfSearchQueryExpanded.launchCollectionInLifecycleScope(::onSearchQueryExpanded)
viewModel.flowOfSelectedRows.launchCollectionInLifecycleScope(::onSelectedRowsChanged)
Expand All @@ -550,6 +549,7 @@ open class CardBrowser :
viewModel.flowOfIsInMultiSelectMode.launchCollectionInLifecycleScope(::isInMultiSelectModeChanged)
viewModel.flowOfCardsUpdated.launchCollectionInLifecycleScope(::cardsUpdatedChanged)
viewModel.flowOfSearchState.launchCollectionInLifecycleScope(::searchStateChanged)
viewModel.flowOfColumnHeadings.launchCollectionInLifecycleScope(::onColumnNamesChanged)
}

// Finish initializing the activity after the collection has been correctly loaded
Expand Down Expand Up @@ -1552,6 +1552,7 @@ open class CardBrowser :

public override fun onRestoreInstanceState(savedInstanceState: Bundle) {
super.onRestoreInstanceState(savedInstanceState)
viewModel.onReinit()
viewModel.launchSearchForCards(
savedInstanceState.getString("mSearchTerms", ""),
forceRefresh = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.combineTransform
import kotlinx.coroutines.flow.filter
Expand Down Expand Up @@ -138,6 +139,13 @@ class CardBrowserViewModel(
private val reverseDirectionFlow = MutableStateFlow(ReverseDirection(orderAsc = false))
val orderAsc get() = reverseDirectionFlow.value.orderAsc

/**
* A map from column backend key to backend column definition
*
* @see [flowOfColumnHeadings]
*/
private val flowOfAllColumns = MutableSharedFlow<Map<String, BrowserColumns.Column>>()

val flowOfActiveColumns =
MutableStateFlow(
BrowserColumnCollection(
Expand All @@ -152,9 +160,6 @@ class CardBrowserViewModel(
val activeColumns
get() = flowOfActiveColumns.value.columns

/** Available columns with names */
lateinit var namedColumns: List<BrowserColumns.Column>

val flowOfSearchQueryExpanded = MutableStateFlow(false)

private val searchQueryInputFlow = MutableStateFlow<String?>(null)
Expand Down Expand Up @@ -241,8 +246,16 @@ class CardBrowserViewModel(

val flowOfInitCompleted = MutableStateFlow(false)

val columnNamesLoaded
get() = ::namedColumns.isInitialized
val flowOfColumnHeadings: StateFlow<List<ColumnHeading>> =
combine(flowOfActiveColumns, flowOfCardsOrNotes, flowOfAllColumns) { activeColumns, cardsOrNotes, allColumns ->
Timber.d("updated headings for %d columns", activeColumns.count)
activeColumns.columns.map {
ColumnHeading(
label = allColumns[it.ankiColumnKey]!!.getLabel(cardsOrNotes),
)
}
// stateIn is requires for tests
}.stateIn(viewModelScope, SharingStarted.Eagerly, initialValue = emptyList())

/**
* Whether the task launched from CardBrowserViewModel.init has completed.
Expand Down Expand Up @@ -305,19 +318,21 @@ class CardBrowserViewModel(
.onEach { sortType -> withCol { sortType.save(config, sharedPrefs()) } }
.launchIn(viewModelScope)

flowOfCardsOrNotes
.onEach { cardsOrNotes ->
Timber.d("loading columns for %s mode", cardsOrNotes)
updateActiveColumns(BrowserColumnCollection.load(sharedPrefs(), cardsOrNotes))
}.launchIn(viewModelScope)

viewModelScope.launch {
val initialDeckId = if (selectAllDecks) ALL_DECKS_ID else getInitialDeck()
// PERF: slightly inefficient if the source was lastDeckId
setDeckId(initialDeckId)
refreshBackendColumns()

val cardsOrNotes = withCol { CardsOrNotes.fromCollection(this@withCol) }
flowOfCardsOrNotes.update { cardsOrNotes }

val allColumns = withCol { allBrowserColumns() }.associateBy { it.key }
namedColumns =
CardBrowserColumn.entries.map { allColumns[it.ankiColumnKey]!! }

setupColumns(cardsOrNotes)

withCol {
sortTypeFlow.update { SortType.fromCol(config, cardsOrNotes, sharedPrefs()) }
reverseDirectionFlow.update { ReverseDirection.fromConfig(config) }
Expand All @@ -331,11 +346,41 @@ class CardBrowserViewModel(
}
}

private suspend fun setupColumns(cardsOrNotes: CardsOrNotes) {
Timber.d("loading columns columns for %s mode", cardsOrNotes)
val columns = BrowserColumnCollection.load(sharedPrefs(), cardsOrNotes)
flowOfActiveColumns.update { columns }
/**
* Called if `onCreate` is called again, which may be due to the collection being reopened
*
* If this is the case, the backend has lost the active columns state, which is required for
* [transformBrowserRow]
*/
fun onReinit() {
// this can occur after process death, if so, the ViewModel starts normally
if (!initCompleted) return

Timber.d("onReinit: executing")

// we currently have no way to test whether setActiveBrowserColumns was called
// so set it again. This needs to be done immediately to ensure that the RecyclerView
// gets correct values when initialized
CollectionManager
.getBackend()
.setActiveBrowserColumns(flowOfActiveColumns.value.backendKeys)

// if the language has changed, the backend column labels may have changed
viewModelScope.launch {
refreshBackendColumns()
}
}

/** Handles an update to the list of backend columns */
private suspend fun refreshBackendColumns() {
flowOfAllColumns.emit(withCol { allBrowserColumns() }.associateBy { it.key })
}

/** Handles an update of the visible columns */
private suspend fun updateActiveColumns(columns: BrowserColumnCollection) {
Timber.d("updating active columns")
withCol { backend.setActiveBrowserColumns(columns.backendKeys) }
flowOfActiveColumns.update { columns }
}

@VisibleForTesting
Expand Down Expand Up @@ -398,7 +443,6 @@ class CardBrowserViewModel(
newValue.saveToCollection(this@withCol)
}
flowOfCardsOrNotes.update { newValue }
setupColumns(newValue)
}

fun setTruncated(value: Boolean) {
Expand Down Expand Up @@ -553,10 +597,8 @@ class CardBrowserViewModel(
Timber.d("editing columns for current headings: %b", isEditingCurrentHeadings)

if (isEditingCurrentHeadings) {
Timber.d("updating active columns")
viewModelScope.launch {
withCol { backend.setActiveBrowserColumns(columnCollection.backendKeys) }
flowOfActiveColumns.update { columnCollection }
updateActiveColumns(columnCollection)
}
}

Expand Down Expand Up @@ -1074,3 +1116,7 @@ sealed class RepositionCardsRequest {
}

fun BrowserColumns.Column.getLabel(cardsOrNotes: CardsOrNotes): String = if (cardsOrNotes == CARDS) cardsModeLabel else notesModeLabel

data class ColumnHeading(
val label: String,
)

0 comments on commit bd04738

Please sign in to comment.