Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reader IA] Fix top navigation bar disappearing when system has low-memory #20114

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,12 @@ class ReaderFragment : Fragment(R.layout.reader_fragment_layout), MenuProvider,

observeJetpackOverlayEvent(savedInstanceState)

viewModel.start()
viewModel.start(savedInstanceState)
}

override fun onSaveInstanceState(outState: Bundle) {
super.onSaveInstanceState(outState)
viewModel.onSaveInstanceState(outState)
}

private fun updateUiState(uiState: ReaderViewModel.ReaderUiState) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
package org.wordpress.android.ui.reader.viewmodels

import android.os.Bundle
import android.os.Parcelable
import androidx.annotation.DrawableRes
import androidx.annotation.StringRes
import androidx.core.os.BundleCompat
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import kotlinx.parcelize.Parcelize
import org.greenrobot.eventbus.EventBus
import org.greenrobot.eventbus.Subscribe
import org.greenrobot.eventbus.ThreadMode.MAIN
Expand Down Expand Up @@ -52,6 +55,7 @@ import org.wordpress.android.viewmodel.Event
import org.wordpress.android.viewmodel.ScopedViewModel
import javax.inject.Inject
import javax.inject.Named
import kotlin.coroutines.CoroutineContext

const val UPDATE_TAGS_THRESHOLD = 1000 * 60 * 60 // 1 hr
const val TRACK_TAB_CHANGED_THROTTLE = 100L
Expand Down Expand Up @@ -114,24 +118,31 @@ class ReaderViewModel @Inject constructor(
EventBus.getDefault().register(this)
}

fun start() {
fun start(savedInstanceState: Bundle? = null) {
if (tagsRequireUpdate()) _updateTags.value = Event(Unit)
if (initialized) return
loadTabs()
loadTabs(savedInstanceState)
if (jetpackBrandingUtils.shouldShowJetpackPoweredBottomSheet()) showJetpackPoweredBottomSheet()
}

fun onSaveInstanceState(out: Bundle) {
_topBarUiState.value?.let {
out.putString(KEY_TOP_BAR_UI_STATE_SELECTED_ITEM_ID, it.selectedItem.id)
out.putParcelable(KEY_TOP_BAR_UI_STATE_FILTER_UI_STATE, it.filterUiState)
}
}

private fun showJetpackPoweredBottomSheet() {
// _showJetpackPoweredBottomSheet.value = Event(true)
}

private fun loadTabs() {
private fun loadTabs(savedInstanceState: Bundle? = null) {
launch {
val currentContentUiState = _uiState.value as? ContentUiState
val tagList = loadReaderTabsUseCase.loadTabs()
if (tagList.isNotEmpty() && readerTagsList != tagList) {
updateReaderTagsList(tagList)
updateTopBarUiState()
updateTopBarUiState(savedInstanceState)
_uiState.value = ContentUiState(
tabUiStates = tagList.map { TabUiState(label = UiStringText(it.label)) },
selectedReaderTag = selectedReaderTag(),
Expand Down Expand Up @@ -352,20 +363,34 @@ class ReaderViewModel @Inject constructor(
readerTagsList[readerTopBarMenuHelper.getReaderTagIndexFromMenuItem(it.selectedItem)]
}

private suspend fun updateTopBarUiState() {
private suspend fun updateTopBarUiState(savedInstanceState: Bundle? = null) {
withContext(bgDispatcher) {
val menuItems = readerTopBarMenuHelper.createMenu(readerTagsList)

// if menu is exactly the same as before, don't update
if (_topBarUiState.value?.menuItems == menuItems) return@withContext

// if there's already a selected item, use it, otherwise use the first item

// if there's already a selected item, use it, otherwise use the first item, also try to use the saved state
val savedStateSelectedId = savedInstanceState?.getString(KEY_TOP_BAR_UI_STATE_SELECTED_ITEM_ID)
val selectedItem = _topBarUiState.value?.selectedItem
?: menuItems.first { it is MenuElementData.Item.Single } as MenuElementData.Item.Single
?: menuItems.filterSingleItems()
.let { singleItems ->
singleItems.firstOrNull { it.id == savedStateSelectedId } ?: singleItems.first()
}

// if there's a selected item and filter state, also use the filter state
// if there's a selected item and filter state, also use the filter state, also try to use the saved state
val filterUiState = _topBarUiState.value?.filterUiState
?.takeIf { _topBarUiState.value?.selectedItem != null }
?: savedInstanceState
?.let {
BundleCompat.getParcelable(
it,
KEY_TOP_BAR_UI_STATE_FILTER_UI_STATE,
TopBarUiState.FilterUiState::class.java
)
}
?.takeIf { selectedItem.id == savedStateSelectedId }

_topBarUiState.postValue(
TopBarUiState(
Expand Down Expand Up @@ -428,55 +453,70 @@ class ReaderViewModel @Inject constructor(
}
}

private fun clearTopBarFilter() {
val filterUiState = _topBarUiState.value?.filterUiState
?.copy(selectedItem = null)
private fun tryWaitNonNullTopBarUiStateThenRun(
initialDelay: Long = 0L,
retryTime: Long = 50L,
maxRetries: Int = 10,
runContext: CoroutineContext = mainDispatcher,
block: suspend CoroutineScope.(topBarUiState: TopBarUiState) -> Unit
) {
launch(bgDispatcher) {
if (initialDelay > 0L) delay(initialDelay)

viewModelScope.launch(mainDispatcher) {
delay(FILTER_UPDATE_DELAY) // small delay to achieve a fluid animation since other UI updates are happening
_topBarUiState.postValue(
_topBarUiState.value
?.copy(filterUiState = filterUiState)
)
var remainingTries = maxRetries
while (_topBarUiState.value == null && remainingTries > 0) {
delay(retryTime)
remainingTries--
}

// only run the block if the topBarUiState is not null, otherwise do nothing
_topBarUiState.value?.let { topBarUiState ->
withContext(runContext) {
block(topBarUiState)
}
}
}
}

private fun updateTopBarFilter(itemName: String, type: ReaderFilterType) {
val filterUiState = _topBarUiState.value?.filterUiState
?.copy(selectedItem = ReaderFilterSelectedItem(UiStringText(itemName), type))
private fun clearTopBarFilter() {
// small delay to achieve a fluid animation since other UI updates are happening
tryWaitNonNullTopBarUiStateThenRun(initialDelay = FILTER_UPDATE_DELAY) { topBarUiState ->
val filterUiState = topBarUiState.filterUiState?.copy(selectedItem = null)
_topBarUiState.postValue(topBarUiState.copy(filterUiState = filterUiState))
}
}

viewModelScope.launch(mainDispatcher) {
delay(FILTER_UPDATE_DELAY) // small delay to achieve a fluid animation since other UI updates are happening
_topBarUiState.postValue(
_topBarUiState.value
?.copy(filterUiState = filterUiState)
)
private fun updateTopBarFilter(itemName: String, type: ReaderFilterType) {
// small delay to achieve a fluid animation since other UI updates are happening
tryWaitNonNullTopBarUiStateThenRun(initialDelay = FILTER_UPDATE_DELAY) { topBarUiState ->
val filterUiState = topBarUiState.filterUiState
?.copy(selectedItem = ReaderFilterSelectedItem(UiStringText(itemName), type))
_topBarUiState.postValue(topBarUiState.copy(filterUiState = filterUiState))
}
}

fun hideTopBarFilterGroup(readerTab: ReaderTag) {
val selectedReaderTag = _topBarUiState.value?.selectedItem?.let {
readerTagsList[readerTopBarMenuHelper.getReaderTagIndexFromMenuItem(it)]
} ?: return
fun hideTopBarFilterGroup(readerTab: ReaderTag) = tryWaitNonNullTopBarUiStateThenRun { topBarUiState ->
val readerTagIndex = readerTopBarMenuHelper.getReaderTagIndexFromMenuItem(topBarUiState.selectedItem)
val selectedReaderTag = readerTagsList[readerTagIndex]

if (readerTab != selectedReaderTag) return
if (readerTab != selectedReaderTag) return@tryWaitNonNullTopBarUiStateThenRun

_topBarUiState.postValue(
topBarUiState.value?.copy(filterUiState = null)
)
_topBarUiState.postValue(topBarUiState.copy(filterUiState = null))
}

fun showTopBarFilterGroup(readerTab: ReaderTag, subFilterItems: List<SubfilterListItem>) {
val selectedReaderTag = _topBarUiState.value?.selectedItem?.let {
readerTagsList[readerTopBarMenuHelper.getReaderTagIndexFromMenuItem(it)]
} ?: return
fun showTopBarFilterGroup(
readerTab: ReaderTag,
subFilterItems: List<SubfilterListItem>
) = tryWaitNonNullTopBarUiStateThenRun { topBarUiState ->
val readerTagIndex = readerTopBarMenuHelper.getReaderTagIndexFromMenuItem(topBarUiState.selectedItem)
val selectedReaderTag = readerTagsList[readerTagIndex]

if (readerTab != selectedReaderTag) return
if (readerTab != selectedReaderTag) return@tryWaitNonNullTopBarUiStateThenRun

val blogsFilterCount = subFilterItems.filterIsInstance<SubfilterListItem.Site>().size
val tagsFilterCount = subFilterItems.filterIsInstance<SubfilterListItem.Tag>().size

val filterState = _topBarUiState.value?.filterUiState
val filterState = topBarUiState.filterUiState
?.copy(
blogsFilterCount = blogsFilterCount,
tagsFilterCount = tagsFilterCount,
Expand All @@ -491,7 +531,7 @@ class ReaderViewModel @Inject constructor(
)

_topBarUiState.postValue(
topBarUiState.value?.copy(filterUiState = filterState)
topBarUiState.copy(filterUiState = filterState)
)
}

Expand All @@ -510,13 +550,14 @@ class ReaderViewModel @Inject constructor(
val onDropdownMenuClick: () -> Unit,
val isSearchActionVisible: Boolean = false,
) {
@Parcelize
data class FilterUiState(
val blogsFilterCount: Int,
val tagsFilterCount: Int,
val selectedItem: ReaderFilterSelectedItem? = null,
val showBlogsFilter: Boolean = blogsFilterCount > 0,
val showTagsFilter: Boolean = tagsFilterCount > 0,
)
) : Parcelable
}

sealed class ReaderUiState(
Expand Down Expand Up @@ -558,6 +599,9 @@ class ReaderViewModel @Inject constructor(
private const val QUICK_START_DISCOVER_TAB_STEP_DELAY = 2000L
private const val QUICK_START_PROMPT_DURATION = 5000
private const val FILTER_UPDATE_DELAY = 50L

private const val KEY_TOP_BAR_UI_STATE_SELECTED_ITEM_ID = "topBarUiState_selectedItem_id"
private const val KEY_TOP_BAR_UI_STATE_FILTER_UI_STATE = "topBarUiState_filterUiState"
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.wordpress.android.ui.reader.views.compose.filter

import android.content.res.Configuration.UI_MODE_NIGHT_YES
import android.os.Parcelable
import androidx.compose.animation.AnimatedVisibility
import androidx.compose.animation.animateColorAsState
import androidx.compose.animation.animateContentSize
Expand Down Expand Up @@ -37,6 +38,7 @@ import androidx.compose.ui.semantics.Role
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import kotlinx.parcelize.Parcelize
import org.wordpress.android.R
import org.wordpress.android.ui.compose.theme.AppThemeWithoutBackground
import org.wordpress.android.ui.compose.unit.Margin
Expand Down Expand Up @@ -210,10 +212,11 @@ enum class ReaderFilterType {
TAG,
}

@Parcelize
data class ReaderFilterSelectedItem(
val text: UiString,
val type: ReaderFilterType,
)
) : Parcelable

@Preview(name = "Light Mode", showBackground = true)
@Preview(name = "Dark Mode", showBackground = true, uiMode = UI_MODE_NIGHT_YES)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
package org.wordpress.android.ui.utils

import android.os.Parcelable
import androidx.annotation.StringRes
import kotlinx.parcelize.Parcelize

/**
* [UiString] is a utility sealed class that represents a string to be used in the UI. It allows a string to be
* represented as both string resource and text.
*/
sealed class UiString {
sealed class UiString : Parcelable {
@Parcelize
data class UiStringText(val text: CharSequence) : UiString()
@Parcelize
data class UiStringRes(@StringRes val stringRes: Int) : UiString()
@Parcelize
data class UiStringResWithParams(@StringRes val stringRes: Int, val params: List<UiString>) : UiString() {
constructor(@StringRes stringRes: Int, vararg varargParams: UiString) : this(stringRes, varargParams.toList())
}

// Current localization process does not support <plurals> resource strings,
// so we need to use multiple string resources. Switch to @PluralsRes when it is supported by localization process.
@Parcelize
data class UiStringPluralRes(
@StringRes val zeroRes: Int,
@StringRes val oneRes: Int,
Expand Down
Loading