From bd0473830aed2bcf738de5042468387af68fd8c8 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Wed, 15 Jan 2025 22:52:16 +0000 Subject: [PATCH] fix(browse): col reopen & language change * 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 --- .../main/java/com/ichi2/anki/CardBrowser.kt | 45 +++++----- .../anki/browser/CardBrowserViewModel.kt | 84 ++++++++++++++----- 2 files changed, 88 insertions(+), 41 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt index f276da1766b6..98e5683fd90f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt @@ -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 @@ -455,27 +454,8 @@ open class CardBrowser : fun onSelectedRowsChanged(rows: Set) = 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) { @@ -540,6 +520,25 @@ open class CardBrowser : } } + fun onColumnNamesChanged(columnCollection: List) { + 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) @@ -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 @@ -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, diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt index 1ab3a83723c3..0df0f50ebdd7 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/browser/CardBrowserViewModel.kt @@ -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 @@ -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>() + val flowOfActiveColumns = MutableStateFlow( BrowserColumnCollection( @@ -152,9 +160,6 @@ class CardBrowserViewModel( val activeColumns get() = flowOfActiveColumns.value.columns - /** Available columns with names */ - lateinit var namedColumns: List - val flowOfSearchQueryExpanded = MutableStateFlow(false) private val searchQueryInputFlow = MutableStateFlow(null) @@ -241,8 +246,16 @@ class CardBrowserViewModel( val flowOfInitCompleted = MutableStateFlow(false) - val columnNamesLoaded - get() = ::namedColumns.isInitialized + val flowOfColumnHeadings: StateFlow> = + 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. @@ -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) } @@ -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 @@ -398,7 +443,6 @@ class CardBrowserViewModel( newValue.saveToCollection(this@withCol) } flowOfCardsOrNotes.update { newValue } - setupColumns(newValue) } fun setTruncated(value: Boolean) { @@ -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) } } @@ -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, +)