From 08355bf24b9266ef83233682cfa5b04fdc8a8cc5 Mon Sep 17 00:00:00 2001 From: Ray <154766448+damontecres@users.noreply.github.com> Date: Sat, 17 Jan 2026 18:09:17 -0500 Subject: [PATCH] Fixes focusing issues on many rows (#718) ## Description Fixes focus jumping back a few cards on rows when returning to a page I think some behavior changes occurred in a recent compose release. `requestFocus` doesn't throw an exception anymore for example. Focus in compose has always been finicky. This PR basically adds some delegating parent focus down its to children. Also, most rows now maintain an internal position state to track where the focus requester is applied. ### Related issues Closes #632 --- .../damontecres/wholphin/ui/cards/ItemRow.kt | 31 ++++++++-- .../wholphin/ui/cards/PersonRow.kt | 16 ++++- .../ui/detail/CollectionFolderMovie.kt | 2 - .../wholphin/ui/detail/CollectionFolderTv.kt | 1 - .../ui/detail/series/SeriesOverviewContent.kt | 23 ++++--- .../damontecres/wholphin/ui/main/HomePage.kt | 61 +++++++------------ 6 files changed, 74 insertions(+), 60 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemRow.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemRow.kt index c03a373a..83166fa7 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemRow.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ItemRow.kt @@ -1,5 +1,6 @@ package com.github.damontecres.wholphin.ui.cards +import androidx.compose.foundation.focusGroup import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues @@ -8,15 +9,20 @@ import androidx.compose.foundation.lazy.LazyRow import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester +import androidx.compose.ui.focus.focusProperties import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.focus.focusRestorer import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.tv.material3.MaterialTheme import androidx.tv.material3.Text +import com.github.damontecres.wholphin.ui.rememberInt +import com.github.damontecres.wholphin.ui.tryRequestFocus @Composable fun ItemRow( @@ -36,9 +42,16 @@ fun ItemRow( ) { val state = rememberLazyListState() val firstFocus = remember { FocusRequester() } + val focusRequester = remember { FocusRequester() } + var position by rememberInt() Column( verticalArrangement = Arrangement.spacedBy(8.dp), - modifier = modifier, + modifier = + modifier.focusProperties { + onEnter = { + focusRequester.tryRequestFocus() + } + }, ) { Text( text = title, @@ -52,11 +65,13 @@ fun ItemRow( modifier = Modifier .fillMaxWidth() - .focusRestorer(firstFocus), + .focusGroup() + .focusRestorer(firstFocus) + .focusRequester(focusRequester), ) { itemsIndexed(items) { index, item -> val cardModifier = - if (index == 0) { + if (index == position) { Modifier.focusRequester(firstFocus) } else { Modifier @@ -65,8 +80,14 @@ fun ItemRow( index, item, cardModifier, - { if (item != null) onClickItem.invoke(index, item) }, - { if (item != null) onLongClickItem.invoke(index, item) }, + { + position = index + if (item != null) onClickItem.invoke(index, item) + }, + { + position = index + if (item != null) onLongClickItem.invoke(index, item) + }, ) } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/PersonRow.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/PersonRow.kt index f5b64ea0..2895db83 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/PersonRow.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/PersonRow.kt @@ -11,7 +11,9 @@ import androidx.compose.foundation.lazy.LazyRow import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester @@ -24,6 +26,7 @@ import com.github.damontecres.wholphin.R import com.github.damontecres.wholphin.data.model.DiscoverItem import com.github.damontecres.wholphin.data.model.Person import com.github.damontecres.wholphin.ui.ifElse +import com.github.damontecres.wholphin.ui.rememberInt @Composable fun PersonRow( @@ -34,6 +37,7 @@ fun PersonRow( onLongClick: ((Int, Person) -> Unit)? = null, ) { val firstFocus = remember { FocusRequester() } + var position by rememberInt() Column( verticalArrangement = Arrangement.spacedBy(8.dp), modifier = modifier, @@ -56,12 +60,18 @@ fun PersonRow( itemsIndexed(people) { index, person -> PersonCard( person = person, - onClick = { onClick.invoke(person) }, - onLongClick = { onLongClick?.invoke(index, person) }, + onClick = { + position = index + onClick.invoke(person) + }, + onLongClick = { + position = index + onLongClick?.invoke(index, person) + }, modifier = Modifier .width(personRowCardWidth) - .ifElse(index == 0, Modifier.focusRequester(firstFocus)) + .ifElse(index == position, Modifier.focusRequester(firstFocus)) .animateItem(), ) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CollectionFolderMovie.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CollectionFolderMovie.kt index 96e1172e..c47aeb23 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CollectionFolderMovie.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CollectionFolderMovie.kt @@ -37,7 +37,6 @@ import com.github.damontecres.wholphin.ui.data.VideoSortOptions import com.github.damontecres.wholphin.ui.logTab import com.github.damontecres.wholphin.ui.nav.Destination import com.github.damontecres.wholphin.ui.preferences.PreferencesViewModel -import com.github.damontecres.wholphin.ui.tryRequestFocus import org.jellyfin.sdk.model.api.BaseItemKind @Composable @@ -72,7 +71,6 @@ fun CollectionFolderMovie( var showHeader by rememberSaveable { mutableStateOf(true) } - LaunchedEffect(Unit) { focusRequester.tryRequestFocus() } Column( modifier = modifier, ) { diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CollectionFolderTv.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CollectionFolderTv.kt index c69fecb1..c9fd5381 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CollectionFolderTv.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/CollectionFolderTv.kt @@ -75,7 +75,6 @@ fun CollectionFolderTv( var showHeader by rememberSaveable { mutableStateOf(true) } - LaunchedEffect(Unit) { focusRequester.tryRequestFocus() } Column( modifier = modifier, ) { diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt index 3144e3f0..68563163 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/series/SeriesOverviewContent.kt @@ -59,6 +59,7 @@ import com.github.damontecres.wholphin.ui.formatDateTime import com.github.damontecres.wholphin.ui.ifElse import com.github.damontecres.wholphin.ui.logTab import com.github.damontecres.wholphin.ui.playback.isPlayKeyUp +import com.github.damontecres.wholphin.ui.rememberInt import com.github.damontecres.wholphin.ui.tryRequestFocus import com.github.damontecres.wholphin.ui.util.rememberDelayedNestedScroll import kotlinx.coroutines.launch @@ -196,7 +197,7 @@ fun SeriesOverviewContent( } } val state = rememberLazyListState(position.episodeRowIndex) - + var epPosition by rememberInt() LazyRow( state = state, horizontalArrangement = Arrangement.spacedBy(16.dp), @@ -204,7 +205,7 @@ fun SeriesOverviewContent( modifier = Modifier .focusRestorer(firstItemFocusRequester) - .focusRequester(episodeRowFocusRequester) +// .focusRequester(episodeRowFocusRequester) .onFocusChanged { cardRowHasFocus = it.hasFocus }, @@ -230,19 +231,23 @@ fun SeriesOverviewContent( playPercent = episode?.data?.userData?.playedPercentage ?: 0.0, - onClick = { if (episode != null) onClick.invoke(episode) }, + onClick = { + epPosition = episodeIndex + if (episode != null) onClick.invoke(episode) + }, onLongClick = { - if (episode != null) { - onLongClick.invoke( - episode, - ) - } + epPosition = episodeIndex + if (episode != null) onLongClick.invoke(episode) }, modifier = Modifier .ifElse( episodeIndex == position.episodeRowIndex, - Modifier.focusRequester(firstItemFocusRequester), + Modifier + .focusRequester(firstItemFocusRequester), + ).ifElse( + episodeIndex == epPosition, + Modifier.focusRequester(episodeRowFocusRequester), ).ifElse( episodeIndex != position.episodeRowIndex, Modifier diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt index 3b943f00..d71c0c1f 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/main/HomePage.kt @@ -2,6 +2,7 @@ package com.github.damontecres.wholphin.ui.main import android.widget.Toast import androidx.compose.foundation.background +import androidx.compose.foundation.focusGroup import androidx.compose.foundation.focusable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -24,7 +25,6 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.livedata.observeAsState import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment @@ -61,7 +61,6 @@ import com.github.damontecres.wholphin.ui.components.LoadingPage import com.github.damontecres.wholphin.ui.components.QuickDetails import com.github.damontecres.wholphin.ui.data.AddPlaylistViewModel import com.github.damontecres.wholphin.ui.data.RowColumn -import com.github.damontecres.wholphin.ui.data.RowColumnSaver import com.github.damontecres.wholphin.ui.detail.MoreDialogActions import com.github.damontecres.wholphin.ui.detail.PlaylistDialog import com.github.damontecres.wholphin.ui.detail.PlaylistLoadingState @@ -70,6 +69,7 @@ import com.github.damontecres.wholphin.ui.isNotNullOrBlank import com.github.damontecres.wholphin.ui.nav.Destination import com.github.damontecres.wholphin.ui.playback.isPlayKeyUp import com.github.damontecres.wholphin.ui.playback.playable +import com.github.damontecres.wholphin.ui.rememberPosition import com.github.damontecres.wholphin.ui.tryRequestFocus import com.github.damontecres.wholphin.util.HomeRowLoadingState import com.github.damontecres.wholphin.util.LoadingState @@ -209,52 +209,32 @@ fun HomePageContent( onFocusPosition: ((RowColumn) -> Unit)? = null, loadingState: LoadingState? = null, ) { - val context = LocalContext.current - val scope = rememberCoroutineScope() - val firstRow = - remember { - homeRows - .indexOfFirst { - when (it) { - is HomeRowLoadingState.Error -> false - is HomeRowLoadingState.Loading -> true - is HomeRowLoadingState.Pending -> true - is HomeRowLoadingState.Success -> it.items.isNotEmpty() - } - }.coerceAtLeast(0) - } - var position by rememberSaveable(stateSaver = RowColumnSaver) { - mutableStateOf(RowColumn(firstRow, 0)) - } + var position by rememberPosition() val focusedItem = position.let { (homeRows.getOrNull(it.row) as? HomeRowLoadingState.Success)?.items?.getOrNull(it.column) } val listState = rememberLazyListState() - val rowFocusRequesters = remember(homeRows.size) { List(homeRows.size) { FocusRequester() } } - var firstFocused by rememberSaveable { mutableStateOf(false) } + val rowFocusRequesters = remember(homeRows) { List(homeRows.size) { FocusRequester() } } + var firstFocused by remember { mutableStateOf(false) } LaunchedEffect(homeRows) { if (!firstFocused) { - // Waiting for the first home row to load, then focus on it - homeRows - .indexOfFirst { it is HomeRowLoadingState.Success && it.items.isNotEmpty() } - .takeIf { it >= 0 } - ?.let { - rowFocusRequesters[it].tryRequestFocus() - delay(50) - listState.scrollToItem(it) - firstFocused = true - } - } - } - LaunchedEffect(Unit) { - if (firstFocused) { - // After the first home row was loaded & focused, page recompositions should focus on the positioned row - val index = position.row - rowFocusRequesters.getOrNull(index)?.tryRequestFocus() - delay(50) - listState.scrollToItem(index) + if (position.row >= 0) { + rowFocusRequesters[position.row].tryRequestFocus() + firstFocused = true + } else { + // Waiting for the first home row to load, then focus on it + homeRows + .indexOfFirst { it is HomeRowLoadingState.Success && it.items.isNotEmpty() } + .takeIf { it >= 0 } + ?.let { + rowFocusRequesters[it].tryRequestFocus() + firstFocused = true + delay(50) + listState.scrollToItem(it) + } + } } } LaunchedEffect(position) { @@ -353,6 +333,7 @@ fun HomePageContent( modifier = Modifier .fillMaxWidth() + .focusGroup() .focusRequester(rowFocusRequesters[rowIndex]) .animateItem(), cardContent = { index, item, cardModifier, onClick, onLongClick ->