copilot/komikku
1
0
Fork 0
Code Issues Pull requests Projects Releases 5 Packages Wiki Activity Actions

chore: Refactor selection state management and improve performance across various screens (#1607)

Browse source 
* chore: Refactor selection state management and improve performance across various screens

*   **MigrateMangaScreen**: Refactor state management to use a `Set<Long>` of IDs for selection instead of a list of wrapper items.
*   **LibraryUpdateErrorScreen**: Simplify `onErrorSelected` signature by removing the unused `userSelected` parameter and clean up related UI components.
*   **Performance**: Use `remember(state.selection)` in `MigrateMangaScreen` for efficient selection status lookups.
*   **Code Quality**: Fix minor typos (e.g., `inbetweenItem` to `inBetweenItem`) and optimize imports.

* Refactor selection logic to prevent redundant state updates

* Move early exit checks for item selection status outside or to the beginning of state update blocks to avoid unnecessary recompositions.
* Improve safety and correctness of item selection by returning early from state updates when an item index is not found.
* Standardize selection checks across `HistoryScreenModel`, `LibraryUpdateErrorScreenModel`, `MigrateMangaScreenModel`, `UpdatesScreenModel`, and `MangaScreenModel`.

* update selection when migration list got updated

* Update state management to reset selection and loading status on fetch failure
This commit is contained in:
Cuong-Tran 2026-05-08 12:41:46 +07:00 committed by GitHub
parent bdf81596e9
commit 4c21f019c0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 141 additions and 140 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/src/main/java/eu/kanade/presentation/libraryUpdateError/LibraryUpdateErrorScreen.kt
View file

@ -67,7 +67,7 @@ fun LibraryUpdateErrorScreen(
onInvertSelection: () -> Unit,
onErrorsDelete: () -> Unit,
onErrorDelete: (Long) -> Unit,
onErrorSelected: (LibraryUpdateErrorItem, Boolean, Boolean, Boolean) -> Unit,
onErrorSelected: (LibraryUpdateErrorItem, Boolean, Boolean) -> Unit,
navigateUp: () -> Unit,
) {
BackHandler(enabled = state.selectionMode, onBack = { onSelectAll(false) })

4
app/src/main/java/eu/kanade/presentation/libraryUpdateError/components/LibraryUpdateErrorUiItem.kt
View file

@ -44,7 +44,7 @@ import tachiyomi.presentation.core.util.selectedBackground
internal fun LazyListScope.libraryUpdateErrorUiItems(
uiModels: List<LibraryUpdateErrorUiModel>,
selectionMode: Boolean,
onErrorSelected: (LibraryUpdateErrorItem, Boolean, Boolean, Boolean) -> Unit,
onErrorSelected: (LibraryUpdateErrorItem, Boolean, Boolean) -> Unit,
onClick: (LibraryUpdateErrorItem) -> Unit,
onClickCover: (LibraryUpdateErrorItem) -> Unit,
onDelete: (Long) -> Unit,
@ -81,7 +81,6 @@ internal fun LazyListScope.libraryUpdateErrorUiItems(
selectionMode -> onErrorSelected(
libraryUpdateErrorItem,
!libraryUpdateErrorItem.selected,
true,
false,
)
@ -93,7 +92,6 @@ internal fun LazyListScope.libraryUpdateErrorUiItems(
libraryUpdateErrorItem,
!libraryUpdateErrorItem.selected,
true,
true,
)
},
onClickCover = { onClickCover(libraryUpdateErrorItem) }.takeIf { !selectionMode },

25
app/src/main/java/eu/kanade/tachiyomi/ui/browse/migration/manga/MigrateMangaScreen.kt
View file

@ -128,11 +128,9 @@ data class MigrateMangaScreen(
// KMK -->
bottomBar = {
MigrateMangaBottomBar(
selected = state.selection,
selectionMode = state.selectionMode,
onMultiMigrateClicked = {
val selection = state.selection
.map { it.manga.id }
navigator.push(MigrationConfigScreen(selection))
navigator.push(MigrationConfigScreen(state.selection))
},
enableScrollToTop = enableScrollToTop,
enableScrollToBottom = enableScrollToBottom,
@ -187,7 +185,7 @@ data class MigrateMangaScreen(
contentPadding: PaddingValues,
state: MigrateMangaScreenModel.State,
// KMK -->
onMangaSelected: (MigrateMangaItem, Boolean, Boolean, Boolean) -> Unit,
onMangaSelected: (Manga, Boolean, Boolean) -> Unit,
// KMK <--
onClickItem: (Manga) -> Unit,
onClickCover: (Manga) -> Unit,
@ -197,20 +195,23 @@ data class MigrateMangaScreen(
contentPadding = contentPadding,
) {
items(items = state.titles) { manga ->
// KMK -->
val isSelected = manga.id in state.selection
// KMK <--
MigrateMangaItem(
manga = manga.manga,
isSelected = manga.selected,
manga = manga,
isSelected = isSelected,
onClickItem = {
// KMK -->
when {
state.selectionMode -> onMangaSelected(manga, !manga.selected, true, false)
state.selectionMode -> onMangaSelected(manga, !isSelected, false)
// KMK <--
else -> onClickItem(it)
}
},
onClickCover = onClickCover,
// KMK -->
onLongClick = { onMangaSelected(manga, !manga.selected, true, true) },
onLongClick = { onMangaSelected(manga, !isSelected, true) },
modifier = Modifier.animateItemFastScroll(),
// KMK <--
)
@ -293,7 +294,7 @@ data class MigrateMangaScreen(
@Composable
private fun MigrateMangaBottomBar(
modifier: Modifier = Modifier,
selected: List<MigrateMangaItem>,
selectionMode: Boolean,
onMultiMigrateClicked: () -> Unit,
enableScrollToTop: Boolean,
enableScrollToBottom: Boolean,
@ -302,7 +303,7 @@ data class MigrateMangaScreen(
) {
val scope = rememberCoroutineScope()
val animatedElevation by animateDpAsState(
targetValue = if (selected.isNotEmpty()) 3.dp else 0.dp,
targetValue = if (selectionMode) 3.dp else 0.dp,
label = "elevation",
)
Surface(
@ -350,7 +351,7 @@ data class MigrateMangaScreen(
toConfirm = confirm[1],
onLongClick = { onLongClickItem(1) },
onClick = onMultiMigrateClicked,
enabled = selected.isNotEmpty(),
enabled = selectionMode,
)
Button(
title = stringResource(KMR.strings.action_scroll_to_bottom),

172
app/src/main/java/eu/kanade/tachiyomi/ui/browse/migration/manga/MigrateMangaScreenModel.kt
View file

@ -3,7 +3,6 @@ package eu.kanade.tachiyomi.ui.browse.migration.manga
import androidx.compose.runtime.Immutable
import cafe.adriel.voyager.core.model.StateScreenModel
import cafe.adriel.voyager.core.model.screenModelScope
import eu.kanade.core.util.addOrRemove
import eu.kanade.tachiyomi.source.Source
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
@ -17,6 +16,7 @@ import kotlinx.coroutines.flow.receiveAsFlow
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import logcat.LogPriority
import mihon.core.common.utils.mutate
import tachiyomi.core.common.util.system.logcat
import tachiyomi.domain.manga.interactor.GetFavorites
import tachiyomi.domain.manga.model.Manga
@ -36,7 +36,6 @@ class MigrateMangaScreenModel(
// KMK -->
// First and last selected index in list
private val selectedPositions: Array<Int> = arrayOf(-1, -1)
private val selectedMangaIds: HashSet<Long> = HashSet()
// KMK <--
init {
@ -50,120 +49,133 @@ class MigrateMangaScreenModel(
logcat(LogPriority.ERROR, it)
_events.send(MigrationMangaEvent.FailedFetchingFavorites)
mutableState.update { state ->
state.copy(titleList = persistentListOf())
state.copy(
titleList = persistentListOf(),
// KMK -->
selection = emptySet(),
// KMK <--
)
}
}
// KMK -->
.map { manga ->
toMigrationMangaScreenItems(manga)
}
// KMK <--
.map { manga ->
manga
.sortedWith(compareBy(String.CASE_INSENSITIVE_ORDER) { it.manga.title })
.sortedWith(compareBy(String.CASE_INSENSITIVE_ORDER) { it.title })
.toImmutableList()
}
.collectLatest { list ->
mutableState.update { it.copy(titleList = list) }
// KMK -->
mutableState.update { state ->
val titleIds = list.map { it.id }.toSet()
val selection = state.selection.intersect(titleIds).toMutableSet()
updateSelectedPositions(list, selection)
state.copy(
titleList = list,
selection = selection,
)
}
// KMK <--
}
}
}
// KMK -->
private fun toMigrationMangaScreenItems(mangas: List<Manga>): List<MigrateMangaItem> {
return mangas.map { manga ->
MigrateMangaItem(
manga = manga,
selected = manga.id in selectedMangaIds,
)
}
}
fun toggleSelection(
item: MigrateMangaItem,
item: Manga,
// KMK -->
selected: Boolean,
userSelected: Boolean = false,
fromLongPress: Boolean = false,
) {
mutableState.update { state ->
val newItems = state.titles.toMutableList().apply {
val selectedIndex = indexOfFirst { it.manga.id == item.manga.id }
if (selectedIndex < 0) return@apply
if (item.id in state.selection == selected) return@update state
val selectedIndex = state.titles.indexOfFirst { it.id == item.id }
if (selectedIndex < 0) return@update state
val selectedItem = get(selectedIndex)
if (selectedItem.selected == selected) return@apply
val selection = state.selection.mutate { list ->
state.titles.run {
val firstSelection = list.isEmpty()
if (selected) list.add(item.id) else list.remove(item.id)
val firstSelection = none { it.selected }
set(selectedIndex, selectedItem.copy(selected = selected))
selectedMangaIds.addOrRemove(item.manga.id, selected)
if (selected && userSelected && fromLongPress) {
if (firstSelection) {
selectedPositions[0] = selectedIndex
selectedPositions[1] = selectedIndex
} else {
// Try to select the items in-between when possible
val range: IntRange
if (selectedIndex < selectedPositions[0]) {
range = selectedIndex + 1 until selectedPositions[0]
if (selected && fromLongPress) {
if (firstSelection) {
selectedPositions[0] = selectedIndex
} else if (selectedIndex > selectedPositions[1]) {
range = (selectedPositions[1] + 1) until selectedIndex
selectedPositions[1] = selectedIndex
} else {
// Just select itself
range = IntRange.EMPTY
}
// Try to select the items in-between when possible
val range: IntRange
if (selectedIndex < selectedPositions[0]) {
range = selectedIndex + 1 until selectedPositions[0]
selectedPositions[0] = selectedIndex
} else if (selectedIndex > selectedPositions[1]) {
range = (selectedPositions[1] + 1) until selectedIndex
selectedPositions[1] = selectedIndex
} else {
// Just select itself
range = IntRange.EMPTY
}
range.forEach {
val inBetweenItem = get(it)
if (!inBetweenItem.selected) {
selectedMangaIds.add(inBetweenItem.manga.id)
set(it, inBetweenItem.copy(selected = true))
range.forEach {
val inBetweenItem = get(it)
if (inBetweenItem.id !in list) {
list.add(inBetweenItem.id)
}
}
}
}
} else if (userSelected && !fromLongPress) {
if (!selected) {
if (selectedIndex == selectedPositions[0]) {
selectedPositions[0] = indexOfFirst { it.selected }
} else if (selectedIndex == selectedPositions[1]) {
selectedPositions[1] = indexOfLast { it.selected }
}
} else {
if (selectedIndex < selectedPositions[0]) {
selectedPositions[0] = selectedIndex
} else if (selectedIndex > selectedPositions[1]) {
selectedPositions[1] = selectedIndex
} else if (!fromLongPress) {
if (!selected) {
if (selectedIndex == selectedPositions[0]) {
selectedPositions[0] = indexOfFirst { it.id in list }
} else if (selectedIndex == selectedPositions[1]) {
selectedPositions[1] = indexOfLast { it.id in list }
}
} else {
if (selectedIndex < selectedPositions[0]) {
selectedPositions[0] = selectedIndex
} else if (selectedIndex > selectedPositions[1]) {
selectedPositions[1] = selectedIndex
}
}
}
}
}
state.copy(titleList = newItems.toImmutableList())
// KMK <--
state.copy(selection = selection)
}
}
// KMK -->
private fun updateSelectedPositions(titles: List<Manga>, selection: Set<Long>) {
if (selection.isEmpty()) {
selectedPositions[0] = -1
selectedPositions[1] = -1
return
}
selectedPositions[0] = titles.indexOfFirst { it.id in selection }
selectedPositions[1] = titles.indexOfLast { it.id in selection }
}
fun toggleAllSelection(selected: Boolean = true) {
mutableState.update { state ->
val newItems = state.titles.map {
selectedMangaIds.addOrRemove(it.manga.id, selected)
it.copy(selected = selected)
val selection = if (selected) {
state.titles.mapTo(mutableSetOf()) { it.id }
} else {
emptySet()
}
selectedPositions[0] = -1
selectedPositions[1] = -1
state.copy(titleList = newItems.toImmutableList())
state.copy(selection = selection)
}
}
fun invertSelection() {
mutableState.update { state ->
val newItems = state.titles.map {
selectedMangaIds.addOrRemove(it.manga.id, !it.selected)
it.copy(selected = !it.selected)
val selection = state.selection.mutate { list ->
state.titles.forEach { item ->
if (!list.remove(item.id)) list.add(item.id)
}
}
selectedPositions[0] = -1
selectedPositions[1] = -1
state.copy(titleList = newItems.toImmutableList())
state.copy(selection = selection)
}
}
// KMK <--
@ -177,13 +189,11 @@ class MigrateMangaScreenModel(
@Immutable
data class State(
val source: Source? = null,
private val titleList: ImmutableList<MigrateMangaItem>? = null,
val selection: Set<Long> = emptySet(),
private val titleList: ImmutableList<Manga>? = null,
) {
// KMK -->
val selection = titles.filter { it.selected }
// KMK <--
val titles: ImmutableList<MigrateMangaItem>
val titles: ImmutableList<Manga>
get() = titleList ?: persistentListOf()
val isLoading: Boolean
@ -199,11 +209,3 @@ class MigrateMangaScreenModel(
sealed interface MigrationMangaEvent {
data object FailedFetchingFavorites : MigrationMangaEvent
}
// KMK -->
@Immutable
data class MigrateMangaItem(
val manga: Manga,
val selected: Boolean,
)
// KMK <--

8
app/src/main/java/eu/kanade/tachiyomi/ui/history/HistoryScreenModel.kt
View file

@ -298,14 +298,14 @@ class HistoryScreenModel(
selectionOptions: HistorySelectionOptions,
) {
val (selected, fromLongPress) = selectionOptions
if (item.chapterId in state.value.selection == selected) return
mutableState.update { state ->
if (item.chapterId in state.selection == selected) return@update state
val selectedIndex = state.list.indexOfFirst { it.chapterId == item.chapterId }
if (selectedIndex < 0) return@update state
val selection = state.selection.mutate { list ->
state.list.run {
val selectedIndex = indexOfFirst { it.chapterId == item.chapterId }
if (selectedIndex < 0) return@run
val firstSelection = list.isEmpty()
if (selected) list.add(item.chapterId) else list.remove(item.chapterId)

24
app/src/main/java/eu/kanade/tachiyomi/ui/libraryUpdateError/LibraryUpdateErrorScreenModel.kt
View file

@ -62,22 +62,20 @@ class LibraryUpdateErrorScreenModel(
fun toggleSelection(
item: LibraryUpdateErrorItem,
selected: Boolean,
userSelected: Boolean = false,
fromLongPress: Boolean = false,
) {
mutableState.update { state ->
val selectedIndex = state.items.indexOfFirst { it.error.errorId == item.error.errorId }
if (selectedIndex < 0) return@update state
val selectedItem = state.items[selectedIndex]
if (selectedItem.selected == selected) return@update state
val newItems = state.items.toMutableList().apply {
val selectedIndex = indexOfFirst { it.error.errorId == item.error.errorId }
if (selectedIndex < 0) return@apply
val selectedItem = get(selectedIndex)
if (selectedItem.selected == selected) return@apply
val firstSelection = none { it.selected }
set(selectedIndex, selectedItem.copy(selected = selected))
selectedErrorIds.addOrRemove(item.error.errorId, selected)
if (selected && userSelected && fromLongPress) {
if (selected && fromLongPress) {
if (firstSelection) {
selectedPositions[0] = selectedIndex
selectedPositions[1] = selectedIndex
@ -96,14 +94,14 @@ class LibraryUpdateErrorScreenModel(
}
range.forEach {
val inbetweenItem = get(it)
if (!inbetweenItem.selected) {
selectedErrorIds.add(inbetweenItem.error.errorId)
set(it, inbetweenItem.copy(selected = true))
val inBetweenItem = get(it)
if (!inBetweenItem.selected) {
selectedErrorIds.add(inBetweenItem.error.errorId)
set(it, inBetweenItem.copy(selected = true))
}
}
}
} else if (userSelected && !fromLongPress) {
} else if (!fromLongPress) {
if (!selected) {
if (selectedIndex == selectedPositions[0]) {
selectedPositions[0] = indexOfFirst { it.selected }

25
app/src/main/java/eu/kanade/tachiyomi/ui/manga/MangaScreenModel.kt
View file

@ -555,8 +555,8 @@ class MangaScreenModel(
}
val vibrantColor = it.getBestColor() ?: return@launchIO
mangaCover.vibrantCoverColor = vibrantColor
updateSuccessState {
it.copy(seedColor = Color(vibrantColor))
updateSuccessState { state ->
state.copy(seedColor = Color(vibrantColor))
}
}
}
@ -1729,13 +1729,14 @@ class MangaScreenModel(
fromLongPress: Boolean = false,
) {
updateSuccessState { successState ->
// KMK -->
val selectedIndex = successState.processedChapters.indexOfFirst { it.id == item.chapter.id }
if (selectedIndex < 0) return@updateSuccessState successState
val selectedItem = successState.processedChapters[selectedIndex]
if (selectedItem.selected == selected) return@updateSuccessState successState
// KMK <--
val newChapters = successState.processedChapters.toMutableList().apply {
val selectedIndex = successState.processedChapters.indexOfFirst { it.id == item.chapter.id }
if (selectedIndex < 0) return@apply
val selectedItem = get(selectedIndex)
if ((selectedItem.selected && selected) || (!selectedItem.selected && !selected)) return@apply
val firstSelection = none { it.selected }
set(selectedIndex, selectedItem.copy(selected = selected))
selectedChapterIds.addOrRemove(item.id, selected)
@ -1759,10 +1760,10 @@ class MangaScreenModel(
}
range.forEach {
val inbetweenItem = get(it)
if (!inbetweenItem.selected) {
selectedChapterIds.add(inbetweenItem.id)
set(it, inbetweenItem.copy(selected = true))
val inBetweenItem = get(it)
if (!inBetweenItem.selected) {
selectedChapterIds.add(inBetweenItem.id)
set(it, inBetweenItem.copy(selected = true))
}
}
}

21
app/src/main/java/eu/kanade/tachiyomi/ui/updates/UpdatesScreenModel.kt
View file

@ -379,13 +379,14 @@ class UpdatesScreenModel(
val (selected, userSelected, fromLongPress, isGroup, isExpanded) = selectionOptions
// KMK <--
mutableState.update { state ->
// KMK -->
val selectedIndex = state.items.indexOfFirst { it.update.chapterId == item.update.chapterId }
if (selectedIndex < 0) return@update state
val selectedItem = state.items[selectedIndex]
if (selectedItem.selected == selected) return@update state
// KMK <--
val newItems = state.items.toMutableList().apply {
val selectedIndex = indexOfFirst { it.update.chapterId == item.update.chapterId }
if (selectedIndex < 0) return@apply
val selectedItem = get(selectedIndex)
if (selectedItem.selected == selected) return@apply
val firstSelection = none { it.selected }
set(selectedIndex, selectedItem.copy(selected = selected))
selectedChapterIds.addOrRemove(item.update.chapterId, selected)
@ -428,10 +429,10 @@ class UpdatesScreenModel(
}
range.forEach {
val inbetweenItem = get(it)
if (!inbetweenItem.selected) {
selectedChapterIds.add(inbetweenItem.update.chapterId)
set(it, inbetweenItem.copy(selected = true))
val inBetweenItem = get(it)
if (!inBetweenItem.selected) {
selectedChapterIds.add(inBetweenItem.update.chapterId)
set(it, inBetweenItem.copy(selected = true))
}
}
}