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
This commit is contained in:
Damontecres 2026-06-03 20:46:57 -04:00 • committed by GitHub
parent 471dbd62db
commit a1b372667b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 107 additions and 128 deletions

View file

@ -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.JellyfinUser
import com.github.damontecres.wholphin.data.model.NavPinType import com.github.damontecres.wholphin.data.model.NavPinType
import com.github.damontecres.wholphin.services.hilt.DefaultCoroutineScope 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.launchDefault
import com.github.damontecres.wholphin.ui.main.settings.Library import com.github.damontecres.wholphin.ui.main.settings.Library
import com.github.damontecres.wholphin.ui.nav.Destination 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.ui.showToast
import com.github.damontecres.wholphin.util.supportedCollectionTypes import com.github.damontecres.wholphin.util.supportedCollectionTypes
import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.update import kotlinx.coroutines.flow.update
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import org.jellyfin.sdk.api.client.ApiClient import org.jellyfin.sdk.api.client.ApiClient
@ -52,35 +51,36 @@ class NavDrawerService
private val seerrServerRepository: SeerrServerRepository, private val seerrServerRepository: SeerrServerRepository,
private val musicService: MusicService, private val musicService: MusicService,
) { ) {
private val _state = MutableStateFlow(NavDrawerItemState.EMPTY) private val _state = MutableStateFlow(NavDrawerItemState())
val state: StateFlow<NavDrawerItemState> = _state val state: StateFlow<NavDrawerItemState> = _state
init { init {
// Handle updating the nav drawer when the user changes // Handle updating the nav drawer when the user changes
serverRepository.currentUserFlow combine(
.combine(serverRepository.currentUserDtoFlow) { user, userDto -> serverRepository.currentUserFlow,
Pair(user, userDto) serverRepository.currentUserDtoFlow,
}.onEach { (user, userDto) -> seerrServerRepository.active,
Timber.d("User updated: user=%s, userDto=%s", user?.id, userDto?.id) ) { user, userDto, discoverActive ->
_state.update { Triple(user, userDto, discoverActive)
it.copy( }.collectLatestIn(coroutineScope) { (user, userDto, discoverActive) ->
items = emptyList(), Timber.d(
moreItems = emptyList(), "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) { 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") Timber.e(ex, "Error updating nav drawer")
showToast(context, "Error fetching user's views") 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 // Handle when music is actively playing or not
coroutineScope.launchDefault { coroutineScope.launchDefault {
@ -178,8 +178,13 @@ class NavDrawerService
suspend fun updateNavDrawer( suspend fun updateNavDrawer(
user: JellyfinUser, user: JellyfinUser,
userDto: UserDto, 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 allLibraries = getAllUserLibraries(user.id, userDto.tvAccess)
val libraries = val libraries =
allLibraries allLibraries
@ -235,15 +240,10 @@ class NavDrawerService
} }
data class NavDrawerItemState( data class NavDrawerItemState(
val items: List<NavDrawerItem>, val items: List<NavDrawerItem> = emptyList(),
val moreItems: List<NavDrawerItem>, val moreItems: List<NavDrawerItem> = emptyList(),
val discoverEnabled: Boolean, val nowPlayingEnabled: Boolean = false,
val nowPlayingEnabled: Boolean, val nowPlayingTitle: String? = null,
val nowPlayingTitle: String?, )
) {
companion object {
val EMPTY = NavDrawerItemState(emptyList(), emptyList(), false, false, null)
}
}
val UserDto.tvAccess: Boolean get() = policy?.enableLiveTvAccess == true val UserDto.tvAccess: Boolean get() = policy?.enableLiveTvAccess == true

View file

@ -67,7 +67,6 @@ import androidx.tv.material3.NavigationDrawerItemDefaults
import androidx.tv.material3.NavigationDrawerScope import androidx.tv.material3.NavigationDrawerScope
import androidx.tv.material3.ProvideTextStyle import androidx.tv.material3.ProvideTextStyle
import androidx.tv.material3.Text import androidx.tv.material3.Text
import androidx.tv.material3.surfaceColorAtElevation
import com.github.damontecres.wholphin.R import com.github.damontecres.wholphin.R
import com.github.damontecres.wholphin.data.model.JellyfinServer import com.github.damontecres.wholphin.data.model.JellyfinServer
import com.github.damontecres.wholphin.data.model.JellyfinUser import com.github.damontecres.wholphin.data.model.JellyfinUser
@ -120,7 +119,6 @@ class NavDrawerViewModel
index: Int, index: Int,
item: NavDrawerItem, item: NavDrawerItem,
) { ) {
if (item !is NavDrawerItem.More) setShowMore(false)
when (item) { when (item) {
NavDrawerItem.Favorites -> { NavDrawerItem.Favorites -> {
setIndex(index) setIndex(index)
@ -163,48 +161,40 @@ class NavDrawerViewModel
fun updateSelectedIndex() { fun updateSelectedIndex() {
viewModelScope.launchDefault { viewModelScope.launchDefault {
val asDestinations = val asDestinations =
( buildList {
serviceState.value.items + addAll(serviceState.value.items)
listOf( addAll(serviceState.value.moreItems)
NavDrawerItem.More, }.map {
NavDrawerItem.Discover, when (it) {
) + serviceState.value.moreItems is ServerNavDrawerItem -> it.destination
).map { is NavDrawerItem.Favorites -> Destination.Favorites
if (it is ServerNavDrawerItem) { is NavDrawerItem.Discover -> Destination.Discover
it.destination else -> null
} else if (it is NavDrawerItem.Favorites) {
Destination.Favorites
} else if (it is NavDrawerItem.Discover) {
Destination.Discover
} else {
null
} }
} }
val backstack = navigationManager.backStack.toList().reversed() val backstack = navigationManager.backStack.toList().reversed()
for (i in 0..<backstack.size) { for (i in 0..<backstack.size) {
val key = backstack[i] val key = backstack[i]
if (key is Destination) { val index =
val index = if (key is Destination.Home) {
if (key is Destination.Home) { HOME_INDEX
HOME_INDEX } else if (key is Destination.Search) {
} else if (key is Destination.Search) { SEARCH_INDEX
SEARCH_INDEX } else if (key is Destination.NowPlaying) {
} else if (key is Destination.NowPlaying) { NOW_PLAYING_INDEX
NOW_PLAYING_INDEX } else {
val idx = asDestinations.indexOf(key)
if (idx >= 0) {
idx
} else { } else {
val idx = asDestinations.indexOf(key) null
if (idx >= 0) {
idx
} else {
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 -> itemsIndexed(serviceState.items) { index, it ->
if (it !is NavDrawerItem.Discover || serviceState.discoverEnabled) { val interactionSource = remember { MutableInteractionSource() }
val interactionSource = remember { MutableInteractionSource() } NavItem(
NavItem( library = it,
library = it, selected = selectedIndex == index,
selected = selectedIndex == index, moreExpanded = moreExpanded,
moreExpanded = moreExpanded, drawerOpen = isOpen,
drawerOpen = isOpen, interactionSource = interactionSource,
interactionSource = interactionSource, onClick = {
onClick = { viewModel.onClickDrawerItem(index, it)
viewModel.onClickDrawerItem(index, it) },
}, modifier =
modifier = Modifier
Modifier .ifElse(
.ifElse( selectedIndex == index,
selectedIndex == index, Modifier.focusRequester(focusRequester),
Modifier.focusRequester(focusRequester), ),
), )
)
}
} }
if (serviceState.moreItems.isNotEmpty()) { if (serviceState.moreItems.isNotEmpty()) {
item { item {
@ -481,7 +469,7 @@ fun NavDrawer(
val interactionSource = remember { MutableInteractionSource() } val interactionSource = remember { MutableInteractionSource() }
NavItem( NavItem(
library = NavDrawerItem.More, library = NavDrawerItem.More,
selected = selectedIndex == index, selected = false,
moreExpanded = moreExpanded, moreExpanded = moreExpanded,
drawerOpen = isOpen, drawerOpen = isOpen,
interactionSource = interactionSource, interactionSource = interactionSource,
@ -500,35 +488,33 @@ fun NavDrawer(
if (moreExpanded) { if (moreExpanded) {
itemsIndexed(serviceState.moreItems) { index, it -> itemsIndexed(serviceState.moreItems) { index, it ->
val adjustedIndex = val adjustedIndex =
remember(serviceState) { (index + serviceState.items.size + 1) } remember(serviceState) { (index + serviceState.items.size) }
if (it !is NavDrawerItem.Discover || serviceState.discoverEnabled) { val interactionSource = remember { MutableInteractionSource() }
val interactionSource = remember { MutableInteractionSource() } NavItem(
NavItem( library = it,
library = it, selected = selectedIndex == adjustedIndex,
selected = selectedIndex == adjustedIndex, moreExpanded = moreExpanded,
moreExpanded = moreExpanded, drawerOpen = isOpen,
drawerOpen = isOpen, onClick = {
onClick = { viewModel.onClickDrawerItem(
viewModel.onClickDrawerItem( adjustedIndex,
adjustedIndex, it,
it, )
) },
containerColor =
if (isOpen) {
MaterialTheme.colorScheme.surface.copy(alpha = .5f)
} else {
Color.Unspecified
}, },
containerColor = interactionSource = interactionSource,
if (isOpen) { modifier =
MaterialTheme.colorScheme.surfaceColorAtElevation(3.dp) Modifier
} else { .ifElse(
Color.Unspecified selectedIndex == adjustedIndex,
}, Modifier.focusRequester(focusRequester),
interactionSource = interactionSource, ),
modifier = )
Modifier
.ifElse(
selectedIndex == adjustedIndex,
Modifier.focusRequester(focusRequester),
),
)
}
} }
} }
item { item {

View file

@ -31,7 +31,6 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp import androidx.compose.ui.unit.sp
import androidx.datastore.core.DataStore
import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel
import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope 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.ServerRepository
import com.github.damontecres.wholphin.data.model.NavDrawerPinnedItem import com.github.damontecres.wholphin.data.model.NavDrawerPinnedItem
import com.github.damontecres.wholphin.data.model.NavPinType 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.BackdropService
import com.github.damontecres.wholphin.services.NavDrawerItemState
import com.github.damontecres.wholphin.services.NavDrawerService import com.github.damontecres.wholphin.services.NavDrawerService
import com.github.damontecres.wholphin.services.NavigationManager import com.github.damontecres.wholphin.services.NavigationManager
import com.github.damontecres.wholphin.services.SeerrServerRepository 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.nav.NavDrawerItem
import com.github.damontecres.wholphin.ui.theme.WholphinTheme import com.github.damontecres.wholphin.ui.theme.WholphinTheme
import com.github.damontecres.wholphin.util.ExceptionHandler import com.github.damontecres.wholphin.util.ExceptionHandler
import com.github.damontecres.wholphin.util.RememberTabManager
import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.lifecycle.HiltViewModel
import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
@ -69,7 +65,6 @@ import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.update import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import org.jellyfin.sdk.api.client.ApiClient
import javax.inject.Inject import javax.inject.Inject
data class NavDrawerPin( data class NavDrawerPin(
@ -304,11 +299,8 @@ class NavDrawerPreferencesViewModel
@Inject @Inject
constructor( constructor(
@param:ApplicationContext private val context: Context, @param:ApplicationContext private val context: Context,
private val api: ApiClient,
val preferenceDataStore: DataStore<AppPreferences>,
val navigationManager: NavigationManager, val navigationManager: NavigationManager,
val backdropService: BackdropService, val backdropService: BackdropService,
private val rememberTabManager: RememberTabManager,
private val serverRepository: ServerRepository, private val serverRepository: ServerRepository,
private val navDrawerService: NavDrawerService, private val navDrawerService: NavDrawerService,
private val serverPreferencesDao: ServerPreferencesDao, private val serverPreferencesDao: ServerPreferencesDao,
@ -321,7 +313,7 @@ class NavDrawerPreferencesViewModel
val state = navDrawerService.state.value val state = navDrawerService.state.value
val user = serverRepository.currentUser val user = serverRepository.currentUser
val seerr = seerrServerRepository.active.firstOrNull() val seerr = seerrServerRepository.active.firstOrNull()
if (state == NavDrawerItemState.EMPTY || user == null || seerr == null) { if (user == null || seerr == null) {
return@launchDefault return@launchDefault
} }
val navDrawerPins = val navDrawerPins =
@ -372,7 +364,8 @@ class NavDrawerPreferencesViewModel
) )
} }
serverPreferencesDao.saveNavDrawerPinnedItems(*toSave.toTypedArray()) serverPreferencesDao.saveNavDrawerPinnedItems(*toSave.toTypedArray())
navDrawerService.updateNavDrawer(user, userDto) val discoverActive = seerrServerRepository.active.firstOrNull() == true
navDrawerService.updateNavDrawer(user, userDto, discoverActive)
} else { } else {
throw IllegalStateException("User IDs do not match") throw IllegalStateException("User IDs do not match")
} }