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 343995e..6ca82bb 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,28 +190,64 @@ private fun InfoColumn( } val descFocus = remember { FocusRequester() } - // 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"). + 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. LaunchedEffect(Unit) { kotlinx.coroutines.delay(150) - runCatching { descFocus.requestFocus() } + runCatching { + if (description.isNotBlank()) descFocus.requestFocus() + else firstLinkFocus.requestFocus() + } } Column(Modifier.fillMaxSize()) { - if (description.isBlank()) { - SidebarFallback("No description.", onDismiss) - } else { + if (description.isNotBlank()) { DescriptionScroll( annotated = annotated, focusRequester = descFocus, onDismiss = onDismiss, - // 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) } }, + onLeaveBottom = { + if (links.isNotEmpty()) { + runCatching { firstLinkFocus.requestFocus() }.isSuccess + } else false + }, 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, + ) + } + } + } + } + } } } @@ -227,7 +263,7 @@ private fun DescriptionScroll( annotated: androidx.compose.ui.text.AnnotatedString, focusRequester: FocusRequester, onDismiss: () -> Unit, - onActivate: () -> Unit, + onLeaveBottom: () -> Boolean, modifier: Modifier = Modifier, ) { val scroll = androidx.compose.foundation.rememberScrollState() @@ -245,23 +281,19 @@ private fun DescriptionScroll( if (ke.type != KeyEventType.KeyDown) return@onPreviewKeyEvent false val step = (viewportPx * 0.85f).coerceAtLeast(240f) when (ke.nativeKeyEvent.keyCode) { - // 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_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 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 57d4fa3..131a64d 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,17 +127,11 @@ 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 { @@ -166,7 +160,6 @@ class ChannelsViewModel(private val store: TokenStore) : ViewModel() { error = e.message ?: "Could not load channels" } finally { loading = false - inFlight = false } } } @@ -317,8 +310,6 @@ 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() } @@ -326,18 +317,21 @@ fun ChannelsScreen( when { viewModel.loading && viewModel.userRows.isEmpty() -> - // One row header stub + a single loading card, not a fake grid (#31). + // Placeholder rows while the channels load (issue #19): header stub + // + a row of shimmer card-shaped placeholders, twice. androidx.compose.foundation.layout.Column { - 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)), - ) + 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() } - ShimmerRow(size = 1) } viewModel.error != null && viewModel.userRows.isEmpty() -> Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { @@ -367,22 +361,13 @@ 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 && ( - if (focusHandle != null) isTargetRow - else group == viewModel.userRows.firstOrNull() - ), - focusIndex = if (isTargetRow) target else 0, + focused = !focusHandedOut.value && + group == viewModel.userRows.firstOrNull(), onFocused = { focusHandedOut.value = true }, stats = { viewModel.stats[it.name + "@" + it.host] }, platformName = { viewModel.platformNames[it.host ?: ""] }, @@ -437,8 +422,6 @@ 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?, @@ -465,30 +448,17 @@ private fun ChannelRow( } } if (focused) { - LaunchedEffect(channels.isNotEmpty(), focusIndex) { + LaunchedEffect(channels.isNotEmpty()) { if (channels.isNotEmpty()) { - 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 - } - } + if (runCatching { focusRequesters[0]?.requestFocus() }.isSuccess) onFocused() } } } LazyRow( state = rowState, - // 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(), + // Land on the last-focused card when moving between rows (issue #29). + modifier = Modifier.focusRestorer { focusRequesters[focusedIndex.coerceAtLeast(0)] ?: FocusRequester.Default }, 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 f8a14a9..7ebda20 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,10 +457,8 @@ 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. No custom fallback — - // handing back a requester whose card has scrolled out of composition - // crashed with "FocusRequester is not initialized". - modifier = Modifier.focusRestorer(), + // left/right as you move up and down between them. + modifier = Modifier.focusRestorer { focusRequesters[focusedIndex.coerceAtLeast(0)] ?: FocusRequester.Default }, 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 1e28d40..5f8726b 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,16 +87,6 @@ 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). @@ -230,16 +220,10 @@ fun HomeScreen(viewModel: AppViewModel = androidx.lifecycle.viewmodel.compose.vi drawerPulse = drawerPulse, ) Destination.Channels -> ChannelsScreen( - onOpenChannel = { handle -> - channelDetails = handle - lastOpenedChannel = handle - }, + onOpenChannel = { handle -> channelDetails = 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 310584c..d11778a 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,10 +438,7 @@ private fun PlayerSurface( } override fun onPlaybackStateChanged(playbackState: Int) { - // 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 + bufferingPending = playbackState != Player.STATE_READY } override fun onRenderedFirstFrame() { @@ -646,9 +643,9 @@ private fun PlayerSurface( closeSidebar() }, onOpenVideo = onOpenVideo, - // 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), + modifier = Modifier + .align(Alignment.CenterEnd) + .padding(top = 48.dp), ) } } 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 4057eab..cedb561 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,6 +1,5 @@ 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 @@ -109,15 +108,6 @@ 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 @@ -172,27 +162,29 @@ 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.fillMaxSize().padding(vertical = 8.dp), + modifier = Modifier.fillMaxWidth().weight(1f).padding(vertical = 8.dp), ) { - // 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) { + // 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) { item(key = "page-header", span = { androidx.compose.foundation.lazy.grid.GridItemSpan(maxLineSpan) }) { - when (source) { - is ListSource.Search -> SearchHeader(viewModel, onOpenDrawer) - is ListSource.Channel -> ChannelHeader( - channelHeader = viewModel.channelHeader, - videoTotal = viewModel.videoTotal, - handle = source.handle, - onLeftEdge = onOpenDrawer, - ) - else -> {} - } + SearchHeader(viewModel, onOpenDrawer) } } if (source is ListSource.Search && items.isEmpty() && !viewModel.loadingInitial) { @@ -219,12 +211,19 @@ 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) } - // A single loading card, not a full fake grid (issue #31). - ShimmerCard( - modifier = Modifier - .padding(6.dp) - .width(rowCardWidth() - 12.dp), - ) + androidx.compose.foundation.layout.Column { + repeat(2) { + androidx.compose.foundation.layout.Row { + repeat(cardsPerRow) { + ShimmerCard( + modifier = Modifier + .padding(6.dp) + .width(rowCardWidth()), + ) + } + } + } + } } } itemsIndexed(items, key = { _, item -> itemStableKey(item) }) { index, item -> @@ -261,6 +260,7 @@ fun VideoListScreen( } } } + } } @Composable @@ -406,11 +406,8 @@ 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 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. + * 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. */ @Composable private fun ChannelHeader( @@ -419,10 +416,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 accent = remember(handle) { channelAccent(handle) } + val banner = channelHeader.banners.maxByOrNull { it.width } + val description = channelHeader.description?.takeIf { it.isNotBlank() } androidx.compose.foundation.layout.Column( Modifier .fillMaxWidth() @@ -431,12 +428,13 @@ private fun ChannelHeader( .background(Color.Black), ) { // Banner area: a fixed height with the identity + stats anchored to its - // bottom; the description sits in the black strip below it. + // bottom. A long description flows into the black strip BELOW it rather than + // being pushed up over the banner or clipped (user request). Box( Modifier .fillMaxWidth() .height(178.dp) - .background(androidx.compose.ui.graphics.Brush.linearGradient(accent)), + .background(Color(0xFF1C1C1C)), ) { if (banner != null) { ShimmerAsyncImage( @@ -460,9 +458,7 @@ private fun ChannelHeader( ), ) // Identity + stats, bottom-left. The avatar sits beside the title (as it - // did before the redesign — user request). Only once the channel loaded; - // the box above keeps the height reserved in the meantime. - if (channelHeader != null) { + // did before the redesign — user request). Column( modifier = Modifier .align(Alignment.BottomStart) @@ -480,22 +476,6 @@ 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( @@ -550,42 +530,22 @@ private fun ChannelHeader( .align(Alignment.TopEnd) .padding(16.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, - ) - } + // 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), + ) } } } -/** - * 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