diff --git a/app/src/main/java/net/jeena/jtubetv/ui/player/PlayerSidebar.kt b/app/src/main/java/net/jeena/jtubetv/ui/player/PlayerSidebar.kt index 6ca82bb..343995e 100644 --- a/app/src/main/java/net/jeena/jtubetv/ui/player/PlayerSidebar.kt +++ b/app/src/main/java/net/jeena/jtubetv/ui/player/PlayerSidebar.kt @@ -190,64 +190,28 @@ private fun InfoColumn( } val descFocus = remember { FocusRequester() } - val firstLinkFocus = remember { FocusRequester() } - // Focus the description so the D-pad can scroll it right away (issue #23): - // a description with no links used to have nothing focusable, so a long - // text could never be scrolled down with the remote. Fall back to the - // first link when there is no description text. + // The description owns the focus for the whole panel: UP/DOWN scroll it and are + // always consumed, so scrolling can never strand focus somewhere it can't come + // back from (issue #23 review: "it loses focus and it's impossible to scroll up"). LaunchedEffect(Unit) { kotlinx.coroutines.delay(150) - runCatching { - if (description.isNotBlank()) descFocus.requestFocus() - else firstLinkFocus.requestFocus() - } + runCatching { descFocus.requestFocus() } } Column(Modifier.fillMaxSize()) { - if (description.isNotBlank()) { + if (description.isBlank()) { + SidebarFallback("No description.", onDismiss) + } else { DescriptionScroll( annotated = annotated, focusRequester = descFocus, onDismiss = onDismiss, - onLeaveBottom = { - if (links.isNotEmpty()) { - runCatching { firstLinkFocus.requestFocus() }.isSuccess - } else false - }, + // Links stay inline in the running text (issue #23 review) instead of + // being repeated in a list underneath; OK opens the first one. + onActivate = { links.firstOrNull()?.let { open(it) } }, modifier = Modifier.weight(1f, fill = false), ) } - if (links.isNotEmpty()) { - Text( - text = "Links", - color = TextDim, - style = MaterialTheme.typography.labelMedium, - modifier = Modifier.padding(start = 20.dp, top = 10.dp, bottom = 2.dp), - ) - LazyColumn(Modifier.heightIn(max = 220.dp)) { - links.forEachIndexed { index, url -> - item(key = "l" + index + url) { - SidebarRow( - onLeft = onDismiss, - onUp = if (index == 0 && description.isNotBlank()) { - { runCatching { descFocus.requestFocus() } } - } else null, - atTop = index == 0 && description.isBlank(), - atBottom = index == links.size - 1, - focusRequester = if (index == 0) firstLinkFocus else null, - onClick = { open(url) }, - ) { - Text( - text = linkLabel(url), - color = TextMain, - style = MaterialTheme.typography.bodySmall, - maxLines = 1, - ) - } - } - } - } - } } } @@ -263,7 +227,7 @@ private fun DescriptionScroll( annotated: androidx.compose.ui.text.AnnotatedString, focusRequester: FocusRequester, onDismiss: () -> Unit, - onLeaveBottom: () -> Boolean, + onActivate: () -> Unit, modifier: Modifier = Modifier, ) { val scroll = androidx.compose.foundation.rememberScrollState() @@ -281,19 +245,23 @@ private fun DescriptionScroll( if (ke.type != KeyEventType.KeyDown) return@onPreviewKeyEvent false val step = (viewportPx * 0.85f).coerceAtLeast(240f) when (ke.nativeKeyEvent.keyCode) { - KeyEvent.KEYCODE_DPAD_DOWN -> - if (scroll.value < scroll.maxValue) { - scope.launch { scroll.animateScrollBy(step) } - true - } else onLeaveBottom() - KeyEvent.KEYCODE_DPAD_UP -> - if (scroll.value > 0) { - scope.launch { scroll.animateScrollBy(-step) } - true - } else true // nothing above the description; stay put + // UP/DOWN are always consumed: they scroll while there is room and + // otherwise do nothing, so focus stays here and the text can always + // be scrolled back up (issue #23 review). + KeyEvent.KEYCODE_DPAD_DOWN -> { + if (scroll.value < scroll.maxValue) scope.launch { scroll.animateScrollBy(step) } + true + } + KeyEvent.KEYCODE_DPAD_UP -> { + if (scroll.value > 0) scope.launch { scroll.animateScrollBy(-step) } + true + } KeyEvent.KEYCODE_DPAD_LEFT -> { onDismiss(); true } + KeyEvent.KEYCODE_DPAD_CENTER, KeyEvent.KEYCODE_ENTER -> { + onActivate(); true + } else -> false } } diff --git a/app/src/main/java/net/jeena/jtubetv/ui/screens/ChannelsScreen.kt b/app/src/main/java/net/jeena/jtubetv/ui/screens/ChannelsScreen.kt index 131a64d..57d4fa3 100644 --- a/app/src/main/java/net/jeena/jtubetv/ui/screens/ChannelsScreen.kt +++ b/app/src/main/java/net/jeena/jtubetv/ui/screens/ChannelsScreen.kt @@ -127,11 +127,17 @@ class ChannelsViewModel(private val store: TokenStore) : ViewModel() { // its shimmer placeholder rows until the (fast-path, few-request) load lands, // instead of painting a stale snapshot that reshuffled as stats streamed in. + /** Guards against a second pass while the first is still running (issue #32: + * the load now starts at app launch, so opening the view can overlap it). */ + private var inFlight = false + fun load(force: Boolean = false) { // Re-entry (destination switch away + back) inside the freshness window: // repaint the in-memory rows and skip the network (issue #13) — rows are // never torn down on re-entry; an account switch builds a new ViewModel. + if (inFlight) return if (!force && userRows.isNotEmpty() && fresh()) return + inFlight = true loading = true error = null viewModelScope.launch { @@ -160,6 +166,7 @@ class ChannelsViewModel(private val store: TokenStore) : ViewModel() { error = e.message ?: "Could not load channels" } finally { loading = false + inFlight = false } } } @@ -310,6 +317,8 @@ fun ChannelsScreen( epoch: Int = 0, onOpenDrawer: () -> Unit = {}, drawerPulse: Int = 0, + /** Channel opened last — gets the focus back on return (issue #34). */ + focusHandle: String? = null, viewModel: ChannelsViewModel = viewModel(key = "channels$epoch", factory = ChannelsViewModel.factory), ) { LaunchedEffect(epoch) { viewModel.load() } @@ -317,21 +326,18 @@ fun ChannelsScreen( when { viewModel.loading && viewModel.userRows.isEmpty() -> - // Placeholder rows while the channels load (issue #19): header stub - // + a row of shimmer card-shaped placeholders, twice. + // One row header stub + a single loading card, not a fake grid (#31). androidx.compose.foundation.layout.Column { - repeat(2) { - Box(Modifier.padding(start = 16.dp, top = 12.dp, bottom = 4.dp)) { - Box( - Modifier - .width(180.dp) - .height(22.dp) - .clip(CircleShape) - .background(Color(0xFF232323)), - ) - } - ShimmerRow() + Box(Modifier.padding(start = 16.dp, top = 12.dp, bottom = 4.dp)) { + Box( + Modifier + .width(180.dp) + .height(22.dp) + .clip(CircleShape) + .background(Color(0xFF232323)), + ) } + ShimmerRow(size = 1) } viewModel.error != null && viewModel.userRows.isEmpty() -> Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { @@ -361,13 +367,22 @@ fun ChannelsScreen( ) } item(key = "row-" + group.ownerName) { + // Coming back from a channel page, focus the card we opened + // instead of the very first one (issue #34). + val target = group.channels.indexOfFirst { + it.name + "@" + it.host == focusHandle + } + val isTargetRow = target >= 0 ChannelRow( group.channels, onOpenChannel, onOpenDrawer, drawerPulse = drawerPulse, - focused = !focusHandedOut.value && - group == viewModel.userRows.firstOrNull(), + focused = !focusHandedOut.value && ( + if (focusHandle != null) isTargetRow + else group == viewModel.userRows.firstOrNull() + ), + focusIndex = if (isTargetRow) target else 0, onFocused = { focusHandedOut.value = true }, stats = { viewModel.stats[it.name + "@" + it.host] }, platformName = { viewModel.platformNames[it.host ?: ""] }, @@ -422,6 +437,8 @@ private fun ChannelRow( onOpenDrawer: () -> Unit = {}, drawerPulse: Int = 0, focused: Boolean, + /** Which card to hand the initial focus to (issue #34). */ + focusIndex: Int = 0, onFocused: () -> Unit = {}, stats: (VideoChannelDto) -> net.jeena.jtubetv.data.api.ChannelStats?, platformName: (VideoChannelDto) -> String?, @@ -448,17 +465,30 @@ private fun ChannelRow( } } if (focused) { - LaunchedEffect(channels.isNotEmpty()) { + LaunchedEffect(channels.isNotEmpty(), focusIndex) { if (channels.isNotEmpty()) { - if (runCatching { focusRequesters[0]?.requestFocus() }.isSuccess) onFocused() + val target = focusIndex.coerceIn(0, channels.size - 1) + // Bring it on screen first; a card further right isn't composed yet, + // so retry until its focus requester is attached (issue #34). + if (target > 0) runCatching { rowState.scrollToItem(target) } + repeat(8) { + delay(80) + val ok = runCatching { focusRequesters[target]?.requestFocus() }.getOrNull() + if (ok != null) { + onFocused() + return@LaunchedEffect + } + } } } } LazyRow( state = rowState, - // Land on the last-focused card when moving between rows (issue #29). - modifier = Modifier.focusRestorer { focusRequesters[focusedIndex.coerceAtLeast(0)] ?: FocusRequester.Default }, + // Land on the last-focused card when moving between rows (issue #29). No + // custom fallback: a requester whose card has scrolled out of composition + // crashed with "FocusRequester is not initialized". + modifier = Modifier.focusRestorer(), contentPadding = PaddingValues(horizontal = 12.dp), horizontalArrangement = Arrangement.spacedBy(4.dp), ) { diff --git a/app/src/main/java/net/jeena/jtubetv/ui/screens/HomeRows.kt b/app/src/main/java/net/jeena/jtubetv/ui/screens/HomeRows.kt index 7ebda20..f8a14a9 100644 --- a/app/src/main/java/net/jeena/jtubetv/ui/screens/HomeRows.kt +++ b/app/src/main/java/net/jeena/jtubetv/ui/screens/HomeRows.kt @@ -457,8 +457,10 @@ fun FocusScrollRow( state = rowState, // Jumping between rows lands on the card this row last had focus on, not // the geometrically-nearest one (issue #29): the rows stop shifting - // left/right as you move up and down between them. - modifier = Modifier.focusRestorer { focusRequesters[focusedIndex.coerceAtLeast(0)] ?: FocusRequester.Default }, + // left/right as you move up and down between them. No custom fallback — + // handing back a requester whose card has scrolled out of composition + // crashed with "FocusRequester is not initialized". + modifier = Modifier.focusRestorer(), contentPadding = PaddingValues(horizontal = 12.dp), horizontalArrangement = Arrangement.spacedBy(4.dp), ) { diff --git a/app/src/main/java/net/jeena/jtubetv/ui/screens/HomeScreen.kt b/app/src/main/java/net/jeena/jtubetv/ui/screens/HomeScreen.kt index 5f8726b..1e28d40 100644 --- a/app/src/main/java/net/jeena/jtubetv/ui/screens/HomeScreen.kt +++ b/app/src/main/java/net/jeena/jtubetv/ui/screens/HomeScreen.kt @@ -87,6 +87,16 @@ fun HomeScreen(viewModel: AppViewModel = androidx.lifecycle.viewmodel.compose.vi var destination by rememberSaveable { mutableStateOf(Destination.Home) } var channelDetails by rememberSaveable { mutableStateOf(null) } // handle var channelName by rememberSaveable { mutableStateOf(null) } // display name + // Last channel opened from the Channels screen — refocused on return (#34). + var lastOpenedChannel by rememberSaveable { mutableStateOf(null) } + // Channels are expensive to assemble, so start that work at app launch rather + // than when the view is first opened (issue #32); the view then usually finds + // the rows already there, and load() no-ops while fresh. + val channelsViewModel: ChannelsViewModel = androidx.lifecycle.viewmodel.compose.viewModel( + key = "channels$epoch", + factory = ChannelsViewModel.factory, + ) + LaunchedEffect(epoch) { channelsViewModel.load() } // NOT saveable: launching the app should never resume a half-watched player session. var playingVideo by rememberSaveable { mutableStateOf(null) } // shortUUID // Non-null while the playing video lives on ANOTHER instance (vidiverse results). @@ -220,10 +230,16 @@ fun HomeScreen(viewModel: AppViewModel = androidx.lifecycle.viewmodel.compose.vi drawerPulse = drawerPulse, ) Destination.Channels -> ChannelsScreen( - onOpenChannel = { handle -> channelDetails = handle }, + onOpenChannel = { handle -> + channelDetails = handle + lastOpenedChannel = handle + }, epoch = epoch, onOpenDrawer = { drawerOpen = true }, drawerPulse = drawerPulse, + // Back from a channel page re-selects it (issue #34). + focusHandle = lastOpenedChannel, + viewModel = channelsViewModel, ) Destination.Search -> VideoListScreen( diff --git a/app/src/main/java/net/jeena/jtubetv/ui/screens/PlayerScreen.kt b/app/src/main/java/net/jeena/jtubetv/ui/screens/PlayerScreen.kt index d11778a..310584c 100644 --- a/app/src/main/java/net/jeena/jtubetv/ui/screens/PlayerScreen.kt +++ b/app/src/main/java/net/jeena/jtubetv/ui/screens/PlayerScreen.kt @@ -438,7 +438,10 @@ private fun PlayerSurface( } override fun onPlaybackStateChanged(playbackState: Int) { - bufferingPending = playbackState != Player.STATE_READY + // Only real buffering shows the overlay (issue #33): STATE_ENDED used + // to count as "not ready" and brought the thumbnail + spinner back + // over the last frame. The initial load is covered by !firstFrame. + bufferingPending = playbackState == Player.STATE_BUFFERING } override fun onRenderedFirstFrame() { @@ -643,9 +646,9 @@ private fun PlayerSurface( closeSidebar() }, onOpenVideo = onOpenVideo, - modifier = Modifier - .align(Alignment.CenterEnd) - .padding(top = 48.dp), + // Full height from the very top — the old top padding left a black + // strip above the panel (issues #23/#24). + modifier = Modifier.align(Alignment.TopEnd), ) } } diff --git a/app/src/main/java/net/jeena/jtubetv/ui/screens/list/VideoListScreen.kt b/app/src/main/java/net/jeena/jtubetv/ui/screens/list/VideoListScreen.kt index cedb561..4057eab 100644 --- a/app/src/main/java/net/jeena/jtubetv/ui/screens/list/VideoListScreen.kt +++ b/app/src/main/java/net/jeena/jtubetv/ui/screens/list/VideoListScreen.kt @@ -1,5 +1,6 @@ package net.jeena.jtubetv.ui.screens.list import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.AccountCircle import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.filled.CalendarMonth import androidx.compose.material.icons.filled.Check @@ -108,6 +109,15 @@ fun VideoListScreen( if (source is ListSource.Channel) { // Fetch the banner/avatar/counts for the channel page header. LaunchedEffect(source) { viewModel.loadChannelHeader(AppGraph.repository) } + // Focusing the first card nudges the grid down far enough to clip the + // header's top (and its Subscribe button); the header and the first row + // both fit, so park the grid back at the top when the page opens. + LaunchedEffect(items.isNotEmpty(), source) { + if (items.isNotEmpty()) { + kotlinx.coroutines.delay(150) + runCatching { gridState.scrollToItem(0) } + } + } } // Row starts of the grid: LEFT from a row's first card opens the drawer @@ -162,29 +172,27 @@ fun VideoListScreen( return } - androidx.compose.foundation.layout.Column(Modifier.fillMaxSize()) { - // Channel page: the header is PINNED above the grid (issue #27) rather than - // scrolling away as the first grid item — auto-focusing the first video card - // no longer hides the banner, and the Subscribe button on it stays reachable. - if (source is ListSource.Channel) { - ChannelHeader( - channelHeader = viewModel.channelHeader, - videoTotal = viewModel.videoTotal, - handle = source.handle, - onLeftEdge = onOpenDrawer, - ) - } LazyVerticalGrid( // Same card size as the Home rows: ~4 fit per row (user request). columns = GridCells.Adaptive(rowCardWidth() - 12.dp), state = gridState, - modifier = Modifier.fillMaxWidth().weight(1f).padding(vertical = 8.dp), + modifier = Modifier.fillMaxSize().padding(vertical = 8.dp), ) { - // Search field still scrolls with its results (its own header item); - // the channel banner is pinned above the grid instead (issue #27). - if (source is ListSource.Search) { + // Page headers scroll WITH the content (issue #27 review: a pinned header + // stayed put and the videos slid behind it). The header is compact enough + // that it's still fully visible when the first card takes focus. + if (source is ListSource.Search || source is ListSource.Channel) { item(key = "page-header", span = { androidx.compose.foundation.lazy.grid.GridItemSpan(maxLineSpan) }) { - SearchHeader(viewModel, onOpenDrawer) + when (source) { + is ListSource.Search -> SearchHeader(viewModel, onOpenDrawer) + is ListSource.Channel -> ChannelHeader( + channelHeader = viewModel.channelHeader, + videoTotal = viewModel.videoTotal, + handle = source.handle, + onLeftEdge = onOpenDrawer, + ) + else -> {} + } } } if (source is ListSource.Search && items.isEmpty() && !viewModel.loadingInitial) { @@ -211,19 +219,12 @@ fun VideoListScreen( // VideoListScreen (My videos, Search, Subscriptions, History, channel pages). item(key = "initial-placeholders", span = { androidx.compose.foundation.lazy.grid.GridItemSpan(maxLineSpan) }) { LaunchedEffect(source) { viewModel.loadMore(force = true) } - androidx.compose.foundation.layout.Column { - repeat(2) { - androidx.compose.foundation.layout.Row { - repeat(cardsPerRow) { - ShimmerCard( - modifier = Modifier - .padding(6.dp) - .width(rowCardWidth()), - ) - } - } - } - } + // A single loading card, not a full fake grid (issue #31). + ShimmerCard( + modifier = Modifier + .padding(6.dp) + .width(rowCardWidth() - 12.dp), + ) } } itemsIndexed(items, key = { _, item -> itemStableKey(item) }) { index, item -> @@ -260,7 +261,6 @@ fun VideoListScreen( } } } - } } @Composable @@ -406,8 +406,11 @@ private fun ScopeChip( /** * Channel page header (issue #27): the banner is the backdrop of a fixed-height * header, darkened with a bottom-up gradient, with the channel identity + stats - * and a reachable Subscribe button laid over it. It's pinned above the grid so - * it never scrolls away, and the video cards start directly beneath it. + * and a reachable Subscribe button laid over it. It scrolls with the video grid. + * + * Its height is the same before and after the channel loads, so the cards never + * get pushed down when it arrives (issue #27 review), and a channel with no + * banner gets a colour gradient derived from its handle instead of flat black. */ @Composable private fun ChannelHeader( @@ -416,10 +419,10 @@ private fun ChannelHeader( handle: String, onLeftEdge: () -> Unit = {}, ) { - if (channelHeader == null) return val metaColor = Color(0xFFCFCFCF) - val banner = channelHeader.banners.maxByOrNull { it.width } - val description = channelHeader.description?.takeIf { it.isNotBlank() } + val banner = channelHeader?.banners?.maxByOrNull { it.width } + val description = channelHeader?.description?.takeIf { it.isNotBlank() } + val accent = remember(handle) { channelAccent(handle) } androidx.compose.foundation.layout.Column( Modifier .fillMaxWidth() @@ -428,13 +431,12 @@ private fun ChannelHeader( .background(Color.Black), ) { // Banner area: a fixed height with the identity + stats anchored to its - // bottom. A long description flows into the black strip BELOW it rather than - // being pushed up over the banner or clipped (user request). + // bottom; the description sits in the black strip below it. Box( Modifier .fillMaxWidth() .height(178.dp) - .background(Color(0xFF1C1C1C)), + .background(androidx.compose.ui.graphics.Brush.linearGradient(accent)), ) { if (banner != null) { ShimmerAsyncImage( @@ -458,7 +460,9 @@ private fun ChannelHeader( ), ) // Identity + stats, bottom-left. The avatar sits beside the title (as it - // did before the redesign — user request). + // did before the redesign — user request). Only once the channel loaded; + // the box above keeps the height reserved in the meantime. + if (channelHeader != null) { Column( modifier = Modifier .align(Alignment.BottomStart) @@ -476,6 +480,22 @@ private fun ChannelHeader( .size(56.dp) .clip(androidx.compose.foundation.shape.CircleShape), ) + } else { + // A channel with no icon gets a stand-in, not a gap (#27 review). + Box( + Modifier + .size(56.dp) + .clip(androidx.compose.foundation.shape.CircleShape) + .background(Color(0x40FFFFFF)), + contentAlignment = Alignment.Center, + ) { + androidx.compose.material3.Icon( + imageVector = Icons.Filled.AccountCircle, + contentDescription = null, + tint = Color(0xFFE8E8E8), + modifier = Modifier.size(42.dp), + ) + } } Column(modifier = Modifier.padding(start = 14.dp)) { Text( @@ -530,22 +550,42 @@ private fun ChannelHeader( .align(Alignment.TopEnd) .padding(16.dp), ) + } } - // Description in the black beneath the banner: a long one grows the header - // downward, fading out of the banner that already darkens into black. - if (description != null) { - Text( - text = description, - color = Color(0xFFCCCCCC), - style = MaterialTheme.typography.bodySmall, - maxLines = 3, - overflow = androidx.compose.ui.text.style.TextOverflow.Ellipsis, - modifier = Modifier.padding(start = 16.dp, end = 16.dp, top = 6.dp, bottom = 12.dp), - ) + // Description strip in the black beneath the banner. Fixed height so the + // header measures the same before and after the channel loads (#27 review). + Box( + Modifier + .fillMaxWidth() + .height(42.dp) + .padding(start = 16.dp, end = 16.dp, top = 6.dp), + ) { + if (description != null) { + Text( + text = description, + color = Color(0xFFCCCCCC), + style = MaterialTheme.typography.bodySmall, + maxLines = 2, + overflow = androidx.compose.ui.text.style.TextOverflow.Ellipsis, + ) + } } } } +/** + * Deterministic backdrop colours for a channel without a banner (#27 review): + * the handle's hash picks a hue, so every channel keeps the same recognisable + * gradient instead of a flat black header. + */ +private fun channelAccent(handle: String): List { + val hue = ((handle.hashCode() ushr 8) % 360 + 360) % 360 + return listOf( + Color.hsv(hue.toFloat(), 0.45f, 0.42f), + Color.hsv(((hue + 38) % 360).toFloat(), 0.55f, 0.16f), + ) +} + /** * Subscribe/unsubscribe toggle for the channel header (issue #27). Mirrors the * player's action: the server's follow-exists check can lag for federated