From a1b372667b2ca08f581caafbb3b49e520ae40daf Mon Sep 17 00:00:00 2001 From: Damontecres Date: Wed, 3 Jun 2026 20:46:57 -0400 Subject: [PATCH] Fix highlighting the wrong 'more' nav drawer item (#1515) ## Description Fixes an issue where the wrong nav drawer item would be highlighted in the "More" section Previously, displaying the discover nav item was partially handled in the view model state and partially within the UI code which caused this bug. This PR moves it all into the view model state. ### Related issues N/A ### Testing Emulator ## Screenshots N/A ## AI or LLM usage None --- .../wholphin/services/NavDrawerService.kt | 70 ++++---- .../damontecres/wholphin/ui/nav/NavDrawer.kt | 152 ++++++++---------- .../ui/preferences/NavDrawerPreference.kt | 13 +- 3 files changed, 107 insertions(+), 128 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/NavDrawerService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/NavDrawerService.kt index cbf2e955..21253e1e 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/NavDrawerService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/NavDrawerService.kt @@ -6,6 +6,7 @@ import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.data.model.JellyfinUser import com.github.damontecres.wholphin.data.model.NavPinType import com.github.damontecres.wholphin.services.hilt.DefaultCoroutineScope +import com.github.damontecres.wholphin.ui.collectLatestIn import com.github.damontecres.wholphin.ui.launchDefault import com.github.damontecres.wholphin.ui.main.settings.Library import com.github.damontecres.wholphin.ui.nav.Destination @@ -14,16 +15,14 @@ import com.github.damontecres.wholphin.ui.nav.ServerNavDrawerItem import com.github.damontecres.wholphin.ui.showToast import com.github.damontecres.wholphin.util.supportedCollectionTypes import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.update import kotlinx.coroutines.withContext import org.jellyfin.sdk.api.client.ApiClient @@ -52,35 +51,36 @@ class NavDrawerService private val seerrServerRepository: SeerrServerRepository, private val musicService: MusicService, ) { - private val _state = MutableStateFlow(NavDrawerItemState.EMPTY) + private val _state = MutableStateFlow(NavDrawerItemState()) val state: StateFlow = _state init { // Handle updating the nav drawer when the user changes - serverRepository.currentUserFlow - .combine(serverRepository.currentUserDtoFlow) { user, userDto -> - Pair(user, userDto) - }.onEach { (user, userDto) -> - Timber.d("User updated: user=%s, userDto=%s", user?.id, userDto?.id) - _state.update { - it.copy( - items = emptyList(), - moreItems = emptyList(), - ) - } + combine( + serverRepository.currentUserFlow, + serverRepository.currentUserDtoFlow, + seerrServerRepository.active, + ) { user, userDto, discoverActive -> + Triple(user, userDto, discoverActive) + }.collectLatestIn(coroutineScope) { (user, userDto, discoverActive) -> + Timber.d( + "User updated: user=%s, userDto=%s, discoverActive=%s", + user?.id, + userDto?.id, + discoverActive, + ) + _state.update { NavDrawerItemState() } + try { if (user != null && userDto != null && user.id == userDto.id) { - updateNavDrawer(user, userDto) + updateNavDrawer(user, userDto, discoverActive) } - }.catch { ex -> + } catch (ex: CancellationException) { + throw ex + } catch (ex: Exception) { Timber.e(ex, "Error updating nav drawer") showToast(context, "Error fetching user's views") - }.launchIn(coroutineScope) - - // Handle when the user has logged into a Seerr server - seerrServerRepository.active - .onEach { discoverActive -> - _state.update { it.copy(discoverEnabled = discoverActive) } - }.launchIn(coroutineScope) + } + } // Handle when music is actively playing or not coroutineScope.launchDefault { @@ -178,8 +178,13 @@ class NavDrawerService suspend fun updateNavDrawer( user: JellyfinUser, userDto: UserDto, + discoverActive: Boolean, ) { - val builtins = listOf(NavDrawerItem.Favorites, NavDrawerItem.Discover) + val builtins = + buildList { + add(NavDrawerItem.Favorites) + if (discoverActive) add(NavDrawerItem.Discover) + } val allLibraries = getAllUserLibraries(user.id, userDto.tvAccess) val libraries = allLibraries @@ -235,15 +240,10 @@ class NavDrawerService } data class NavDrawerItemState( - val items: List, - val moreItems: List, - val discoverEnabled: Boolean, - val nowPlayingEnabled: Boolean, - val nowPlayingTitle: String?, -) { - companion object { - val EMPTY = NavDrawerItemState(emptyList(), emptyList(), false, false, null) - } -} + val items: List = emptyList(), + val moreItems: List = emptyList(), + val nowPlayingEnabled: Boolean = false, + val nowPlayingTitle: String? = null, +) val UserDto.tvAccess: Boolean get() = policy?.enableLiveTvAccess == true diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/nav/NavDrawer.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/nav/NavDrawer.kt index af4ec63f..0d66fd8f 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/nav/NavDrawer.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/nav/NavDrawer.kt @@ -67,7 +67,6 @@ import androidx.tv.material3.NavigationDrawerItemDefaults import androidx.tv.material3.NavigationDrawerScope import androidx.tv.material3.ProvideTextStyle import androidx.tv.material3.Text -import androidx.tv.material3.surfaceColorAtElevation import com.github.damontecres.wholphin.R import com.github.damontecres.wholphin.data.model.JellyfinServer import com.github.damontecres.wholphin.data.model.JellyfinUser @@ -120,7 +119,6 @@ class NavDrawerViewModel index: Int, item: NavDrawerItem, ) { - if (item !is NavDrawerItem.More) setShowMore(false) when (item) { NavDrawerItem.Favorites -> { setIndex(index) @@ -163,48 +161,40 @@ class NavDrawerViewModel fun updateSelectedIndex() { viewModelScope.launchDefault { val asDestinations = - ( - serviceState.value.items + - listOf( - NavDrawerItem.More, - NavDrawerItem.Discover, - ) + serviceState.value.moreItems - ).map { - if (it is ServerNavDrawerItem) { - it.destination - } else if (it is NavDrawerItem.Favorites) { - Destination.Favorites - } else if (it is NavDrawerItem.Discover) { - Destination.Discover - } else { - null + buildList { + addAll(serviceState.value.items) + addAll(serviceState.value.moreItems) + }.map { + when (it) { + is ServerNavDrawerItem -> it.destination + is NavDrawerItem.Favorites -> Destination.Favorites + is NavDrawerItem.Discover -> Destination.Discover + else -> null } } val backstack = navigationManager.backStack.toList().reversed() for (i in 0..= 0) { + idx } else { - val idx = asDestinations.indexOf(key) - if (idx >= 0) { - idx - } else { - null - } + null } - Timber.v("Found $index => $key") - if (index != null) { - _state.update { it.copy(selectedIndex = index) } - break } + Timber.v("Found $index => $key") + if (index != null) { + _state.update { it.copy(selectedIndex = index) } + break } } } @@ -455,25 +445,23 @@ fun NavDrawer( ) } itemsIndexed(serviceState.items) { index, it -> - if (it !is NavDrawerItem.Discover || serviceState.discoverEnabled) { - val interactionSource = remember { MutableInteractionSource() } - NavItem( - library = it, - selected = selectedIndex == index, - moreExpanded = moreExpanded, - drawerOpen = isOpen, - interactionSource = interactionSource, - onClick = { - viewModel.onClickDrawerItem(index, it) - }, - modifier = - Modifier - .ifElse( - selectedIndex == index, - Modifier.focusRequester(focusRequester), - ), - ) - } + val interactionSource = remember { MutableInteractionSource() } + NavItem( + library = it, + selected = selectedIndex == index, + moreExpanded = moreExpanded, + drawerOpen = isOpen, + interactionSource = interactionSource, + onClick = { + viewModel.onClickDrawerItem(index, it) + }, + modifier = + Modifier + .ifElse( + selectedIndex == index, + Modifier.focusRequester(focusRequester), + ), + ) } if (serviceState.moreItems.isNotEmpty()) { item { @@ -481,7 +469,7 @@ fun NavDrawer( val interactionSource = remember { MutableInteractionSource() } NavItem( library = NavDrawerItem.More, - selected = selectedIndex == index, + selected = false, moreExpanded = moreExpanded, drawerOpen = isOpen, interactionSource = interactionSource, @@ -500,35 +488,33 @@ fun NavDrawer( if (moreExpanded) { itemsIndexed(serviceState.moreItems) { index, it -> val adjustedIndex = - remember(serviceState) { (index + serviceState.items.size + 1) } - if (it !is NavDrawerItem.Discover || serviceState.discoverEnabled) { - val interactionSource = remember { MutableInteractionSource() } - NavItem( - library = it, - selected = selectedIndex == adjustedIndex, - moreExpanded = moreExpanded, - drawerOpen = isOpen, - onClick = { - viewModel.onClickDrawerItem( - adjustedIndex, - it, - ) + remember(serviceState) { (index + serviceState.items.size) } + val interactionSource = remember { MutableInteractionSource() } + NavItem( + library = it, + selected = selectedIndex == adjustedIndex, + moreExpanded = moreExpanded, + drawerOpen = isOpen, + onClick = { + viewModel.onClickDrawerItem( + adjustedIndex, + it, + ) + }, + containerColor = + if (isOpen) { + MaterialTheme.colorScheme.surface.copy(alpha = .5f) + } else { + Color.Unspecified }, - containerColor = - if (isOpen) { - MaterialTheme.colorScheme.surfaceColorAtElevation(3.dp) - } else { - Color.Unspecified - }, - interactionSource = interactionSource, - modifier = - Modifier - .ifElse( - selectedIndex == adjustedIndex, - Modifier.focusRequester(focusRequester), - ), - ) - } + interactionSource = interactionSource, + modifier = + Modifier + .ifElse( + selectedIndex == adjustedIndex, + Modifier.focusRequester(focusRequester), + ), + ) } } item { diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/preferences/NavDrawerPreference.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/preferences/NavDrawerPreference.kt index e8c768a4..4f2fb968 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/preferences/NavDrawerPreference.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/preferences/NavDrawerPreference.kt @@ -31,7 +31,6 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp -import androidx.datastore.core.DataStore import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope @@ -44,9 +43,7 @@ import com.github.damontecres.wholphin.data.ServerPreferencesDao import com.github.damontecres.wholphin.data.ServerRepository import com.github.damontecres.wholphin.data.model.NavDrawerPinnedItem import com.github.damontecres.wholphin.data.model.NavPinType -import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.services.BackdropService -import com.github.damontecres.wholphin.services.NavDrawerItemState import com.github.damontecres.wholphin.services.NavDrawerService import com.github.damontecres.wholphin.services.NavigationManager import com.github.damontecres.wholphin.services.SeerrServerRepository @@ -60,7 +57,6 @@ import com.github.damontecres.wholphin.ui.main.settings.MoveDirection import com.github.damontecres.wholphin.ui.nav.NavDrawerItem import com.github.damontecres.wholphin.ui.theme.WholphinTheme import com.github.damontecres.wholphin.util.ExceptionHandler -import com.github.damontecres.wholphin.util.RememberTabManager import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.Dispatchers @@ -69,7 +65,6 @@ import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import org.jellyfin.sdk.api.client.ApiClient import javax.inject.Inject data class NavDrawerPin( @@ -304,11 +299,8 @@ class NavDrawerPreferencesViewModel @Inject constructor( @param:ApplicationContext private val context: Context, - private val api: ApiClient, - val preferenceDataStore: DataStore, val navigationManager: NavigationManager, val backdropService: BackdropService, - private val rememberTabManager: RememberTabManager, private val serverRepository: ServerRepository, private val navDrawerService: NavDrawerService, private val serverPreferencesDao: ServerPreferencesDao, @@ -321,7 +313,7 @@ class NavDrawerPreferencesViewModel val state = navDrawerService.state.value val user = serverRepository.currentUser val seerr = seerrServerRepository.active.firstOrNull() - if (state == NavDrawerItemState.EMPTY || user == null || seerr == null) { + if (user == null || seerr == null) { return@launchDefault } val navDrawerPins = @@ -372,7 +364,8 @@ class NavDrawerPreferencesViewModel ) } serverPreferencesDao.saveNavDrawerPinnedItems(*toSave.toTypedArray()) - navDrawerService.updateNavDrawer(user, userDto) + val discoverActive = seerrServerRepository.active.firstOrNull() == true + navDrawerService.updateNavDrawer(user, userDto, discoverActive) } else { throw IllegalStateException("User IDs do not match") }