From 1daa02af5252d0e280e087ec139a6dc9dfb42941 Mon Sep 17 00:00:00 2001 From: Koitharu Date: Sat, 4 Feb 2023 08:48:40 +0200 Subject: [PATCH 1/2] Handle errors properly in scrobbler selector --- .gitignore | 1 + .idea/kotlinc.xml | 9 -- app/build.gradle | 2 +- .../selector/ScrobblingSelectorBottomSheet.kt | 12 ++- .../selector/ScrobblingSelectorViewModel.kt | 89 +++++++++++++------ .../ui/selector/adapter/ScrobblerHintAD.kt | 37 ++++++++ .../adapter/ShikimoriSelectorAdapter.kt | 5 +- .../ui/selector/model/ScrobblerHint.kt | 38 ++++++++ app/src/main/res/layout/item_empty_hint.xml | 55 ++++++++++++ build.gradle | 6 +- 10 files changed, 212 insertions(+), 42 deletions(-) delete mode 100644 .idea/kotlinc.xml create mode 100644 app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/adapter/ScrobblerHintAD.kt create mode 100644 app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/model/ScrobblerHint.kt create mode 100644 app/src/main/res/layout/item_empty_hint.xml diff --git a/.gitignore b/.gitignore index 56cee6345..621f3e800 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ /.idea/navEditor.xml /.idea/assetWizardSettings.xml /.idea/kotlinScripting.xml +/.idea/kotlinc.xml /.idea/deploymentTargetDropDown.xml /.idea/androidTestResultsUserPreferences.xml /.idea/render.experimental.xml diff --git a/.idea/kotlinc.xml b/.idea/kotlinc.xml deleted file mode 100644 index 22dcb880f..000000000 --- a/.idea/kotlinc.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - - - - \ No newline at end of file diff --git a/app/build.gradle b/app/build.gradle index ad55f77f0..1b8ed2bc9 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -90,7 +90,7 @@ dependencies { exclude group: 'org.json', module: 'json' } - implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.8.0' + implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.8.10' implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4' implementation "androidx.appcompat:appcompat:1.6.0" diff --git a/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/ScrobblingSelectorBottomSheet.kt b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/ScrobblingSelectorBottomSheet.kt index a8eeb49e0..52cd3e1dd 100644 --- a/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/ScrobblingSelectorBottomSheet.kt +++ b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/ScrobblingSelectorBottomSheet.kt @@ -91,6 +91,12 @@ class ScrobblingSelectorBottomSheet : viewModel.onClose.observe(viewLifecycleOwner) { dismiss() } + viewModel.selectedScrobblerIndex.observe(viewLifecycleOwner) { index -> + val tab = binding.tabs.getTabAt(index) + if (tab != null && !tab.isSelected) { + tab.select() + } + } viewModel.searchQuery.observe(viewLifecycleOwner) { binding.headerBar.subtitle = it } @@ -106,14 +112,16 @@ class ScrobblingSelectorBottomSheet : viewModel.selectedItemId.value = item.id } - override fun onRetryClick(error: Throwable) = Unit + override fun onRetryClick(error: Throwable) { + viewModel.retry() + } override fun onEmptyActionClick() { openSearch() } override fun onScrolledToEnd() { - viewModel.loadList(append = true) + viewModel.loadNextPage() } override fun onMenuItemActionExpand(item: MenuItem): Boolean { diff --git a/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/ScrobblingSelectorViewModel.kt b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/ScrobblingSelectorViewModel.kt index cb926248f..6c8a6ada1 100644 --- a/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/ScrobblingSelectorViewModel.kt +++ b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/ScrobblingSelectorViewModel.kt @@ -11,18 +11,19 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.filterNotNull import org.koitharu.kotatsu.R import org.koitharu.kotatsu.base.ui.BaseViewModel -import org.koitharu.kotatsu.list.ui.model.EmptyHint import org.koitharu.kotatsu.list.ui.model.ListModel import org.koitharu.kotatsu.list.ui.model.LoadingFooter import org.koitharu.kotatsu.list.ui.model.LoadingState import org.koitharu.kotatsu.parsers.model.Manga +import org.koitharu.kotatsu.parsers.util.runCatchingCancellable import org.koitharu.kotatsu.scrobbling.domain.Scrobbler import org.koitharu.kotatsu.scrobbling.domain.model.ScrobblerManga +import org.koitharu.kotatsu.scrobbling.ui.selector.model.ScrobblerHint import org.koitharu.kotatsu.utils.SingleLiveEvent import org.koitharu.kotatsu.utils.ext.asLiveDataDistinct +import org.koitharu.kotatsu.utils.ext.printStackTraceDebug import org.koitharu.kotatsu.utils.ext.requireValue class ScrobblingSelectorViewModel @AssistedInject constructor( @@ -34,8 +35,9 @@ class ScrobblingSelectorViewModel @AssistedInject constructor( val selectedScrobblerIndex = MutableLiveData(0) - private val scrobblerMangaList = MutableStateFlow?>(null) - private val hasNextPage = MutableStateFlow(false) + private val scrobblerMangaList = MutableStateFlow>(emptyList()) + private val hasNextPage = MutableStateFlow(true) + private val listError = MutableStateFlow(null) private var loadingJob: Job? = null private var doneJob: Job? = null private var initJob: Job? = null @@ -44,13 +46,24 @@ class ScrobblingSelectorViewModel @AssistedInject constructor( get() = availableScrobblers[selectedScrobblerIndex.requireValue()] val content: LiveData> = combine( - scrobblerMangaList.filterNotNull(), + scrobblerMangaList, + listError, hasNextPage, - ) { list, isHasNextPage -> - when { - list.isEmpty() -> listOf(emptyResultsHint()) - isHasNextPage -> list + LoadingFooter - else -> list + ) { list, error, isHasNextPage -> + if (list.isNotEmpty()) { + if (isHasNextPage) { + list + LoadingFooter + } else { + list + } + } else { + listOf( + when { + error != null -> errorHint(error) + isHasNextPage -> LoadingFooter + else -> emptyResultsHint() + }, + ) } }.asLiveDataDistinct(viewModelScope.coroutineContext + Dispatchers.Default, listOf(LoadingState)) @@ -59,7 +72,7 @@ class ScrobblingSelectorViewModel @AssistedInject constructor( val onClose = SingleLiveEvent() val isEmpty: Boolean - get() = scrobblerMangaList.value.isNullOrEmpty() + get() = scrobblerMangaList.value.isEmpty() init { initialize() @@ -71,22 +84,39 @@ class ScrobblingSelectorViewModel @AssistedInject constructor( loadList(append = false) } - fun loadList(append: Boolean) { + fun loadNextPage() { + if (scrobblerMangaList.value.isNotEmpty() && hasNextPage.value) { + loadList(append = true) + } + } + + fun retry() { + loadingJob?.cancel() + hasNextPage.value = true + scrobblerMangaList.value = emptyList() + loadList(append = false) + } + + private fun loadList(append: Boolean) { if (loadingJob?.isActive == true) { return } - if (append && !hasNextPage.value) { - return - } loadingJob = launchLoadingJob(Dispatchers.Default) { - val offset = if (append) scrobblerMangaList.value?.size ?: 0 else 0 - val list = currentScrobbler.findManga(checkNotNull(searchQuery.value), offset) - if (!append) { - scrobblerMangaList.value = list - } else if (list.isNotEmpty()) { - scrobblerMangaList.value = scrobblerMangaList.value?.plus(list) ?: list + listError.value = null + val offset = if (append) scrobblerMangaList.value.size else 0 + runCatchingCancellable { + currentScrobbler.findManga(checkNotNull(searchQuery.value), offset) + }.onSuccess { list -> + if (!append) { + scrobblerMangaList.value = list + } else if (list.isNotEmpty()) { + scrobblerMangaList.value = scrobblerMangaList.value + list + } + hasNextPage.value = list.isNotEmpty() + }.onFailure { error -> + error.printStackTraceDebug() + listError.value = error } - hasNextPage.value = list.isNotEmpty() } } @@ -113,8 +143,8 @@ class ScrobblingSelectorViewModel @AssistedInject constructor( private fun initialize() { initJob?.cancel() loadingJob?.cancel() - hasNextPage.value = false - scrobblerMangaList.value = null + hasNextPage.value = true + scrobblerMangaList.value = emptyList() initJob = launchJob(Dispatchers.Default) { try { val info = currentScrobbler.getScrobblingInfoOrNull(manga.id) @@ -127,13 +157,22 @@ class ScrobblingSelectorViewModel @AssistedInject constructor( } } - private fun emptyResultsHint() = EmptyHint( + private fun emptyResultsHint() = ScrobblerHint( icon = R.drawable.ic_empty_history, textPrimary = R.string.nothing_found, textSecondary = R.string.text_search_holder_secondary, + error = null, actionStringRes = R.string.search, ) + private fun errorHint(e: Throwable) = ScrobblerHint( + icon = R.drawable.ic_error_large, + textPrimary = R.string.error_occurred, + error = e, + textSecondary = 0, + actionStringRes = R.string.try_again, + ) + @AssistedFactory interface Factory { diff --git a/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/adapter/ScrobblerHintAD.kt b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/adapter/ScrobblerHintAD.kt new file mode 100644 index 000000000..311615826 --- /dev/null +++ b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/adapter/ScrobblerHintAD.kt @@ -0,0 +1,37 @@ +package org.koitharu.kotatsu.scrobbling.ui.selector.adapter + +import com.hannesdorfmann.adapterdelegates4.dsl.adapterDelegateViewBinding +import org.koitharu.kotatsu.databinding.ItemEmptyHintBinding +import org.koitharu.kotatsu.list.ui.adapter.ListStateHolderListener +import org.koitharu.kotatsu.list.ui.model.ListModel +import org.koitharu.kotatsu.scrobbling.ui.selector.model.ScrobblerHint +import org.koitharu.kotatsu.utils.ext.getDisplayMessage +import org.koitharu.kotatsu.utils.ext.setTextAndVisible +import org.koitharu.kotatsu.utils.ext.textAndVisible + +fun scrobblerHintAD( + listener: ListStateHolderListener, +) = adapterDelegateViewBinding( + { inflater, parent -> ItemEmptyHintBinding.inflate(inflater, parent, false) }, +) { + + binding.buttonRetry.setOnClickListener { + val e = item.error + if (e != null) { + listener.onRetryClick(e) + } else { + listener.onEmptyActionClick() + } + } + + bind { + binding.icon.setImageResource(item.icon) + binding.textPrimary.setText(item.textPrimary) + if (item.error != null) { + binding.textSecondary.textAndVisible = item.error?.getDisplayMessage(context.resources) + } else { + binding.textSecondary.setTextAndVisible(item.textSecondary) + } + binding.buttonRetry.setTextAndVisible(item.actionStringRes) + } +} diff --git a/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/adapter/ShikimoriSelectorAdapter.kt b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/adapter/ShikimoriSelectorAdapter.kt index 656ae82de..9a792588c 100644 --- a/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/adapter/ShikimoriSelectorAdapter.kt +++ b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/adapter/ShikimoriSelectorAdapter.kt @@ -6,11 +6,11 @@ import coil.ImageLoader import com.hannesdorfmann.adapterdelegates4.AsyncListDifferDelegationAdapter import org.koitharu.kotatsu.base.ui.list.OnListItemClickListener import org.koitharu.kotatsu.list.ui.adapter.ListStateHolderListener -import org.koitharu.kotatsu.list.ui.adapter.emptyHintAD import org.koitharu.kotatsu.list.ui.adapter.loadingFooterAD import org.koitharu.kotatsu.list.ui.adapter.loadingStateAD import org.koitharu.kotatsu.list.ui.model.ListModel import org.koitharu.kotatsu.scrobbling.domain.model.ScrobblerManga +import org.koitharu.kotatsu.scrobbling.ui.selector.model.ScrobblerHint import kotlin.jvm.internal.Intrinsics class ShikimoriSelectorAdapter( @@ -24,7 +24,7 @@ class ShikimoriSelectorAdapter( delegatesManager.addDelegate(loadingStateAD()) .addDelegate(scrobblingMangaAD(lifecycleOwner, coil, clickListener)) .addDelegate(loadingFooterAD()) - .addDelegate(emptyHintAD(stateHolderListener)) + .addDelegate(scrobblerHintAD(stateHolderListener)) } private class DiffCallback : DiffUtil.ItemCallback() { @@ -33,6 +33,7 @@ class ShikimoriSelectorAdapter( return when { oldItem === newItem -> true oldItem is ScrobblerManga && newItem is ScrobblerManga -> oldItem.id == newItem.id + oldItem is ScrobblerHint && newItem is ScrobblerHint -> oldItem.textPrimary == newItem.textPrimary else -> false } } diff --git a/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/model/ScrobblerHint.kt b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/model/ScrobblerHint.kt new file mode 100644 index 000000000..e30614da2 --- /dev/null +++ b/app/src/main/java/org/koitharu/kotatsu/scrobbling/ui/selector/model/ScrobblerHint.kt @@ -0,0 +1,38 @@ +package org.koitharu.kotatsu.scrobbling.ui.selector.model + +import androidx.annotation.DrawableRes +import androidx.annotation.StringRes +import org.koitharu.kotatsu.list.ui.model.ListModel + +class ScrobblerHint( + @DrawableRes val icon: Int, + @StringRes val textPrimary: Int, + @StringRes val textSecondary: Int, + val error: Throwable?, + @StringRes val actionStringRes: Int, +) : ListModel { + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + + other as ScrobblerHint + + if (icon != other.icon) return false + if (textPrimary != other.textPrimary) return false + if (textSecondary != other.textSecondary) return false + if (error != other.error) return false + if (actionStringRes != other.actionStringRes) return false + + return true + } + + override fun hashCode(): Int { + var result = icon + result = 31 * result + textPrimary + result = 31 * result + textSecondary + result = 31 * result + (error?.hashCode() ?: 0) + result = 31 * result + actionStringRes + return result + } +} diff --git a/app/src/main/res/layout/item_empty_hint.xml b/app/src/main/res/layout/item_empty_hint.xml new file mode 100644 index 000000000..eb3fe5f7d --- /dev/null +++ b/app/src/main/res/layout/item_empty_hint.xml @@ -0,0 +1,55 @@ + + + + + + + + + +