From 769fe01c847b4be99ba80e9d4b445b47825b94c7 Mon Sep 17 00:00:00 2001 From: ponzischeme89 Date: Wed, 19 Aug 2026 21:30:44 +1200 Subject: [PATCH] 0.2.80 --- CLAUDE.md | 8 +- app/build.gradle.kts | 2 +- .../memby/data/EmbyRepository.kt | 18 +- .../memby/data/model/GatewayModels.kt | 16 +- .../memby/data/remote/GatewayApi.kt | 14 +- .../ponzischeme89/memby/ui/HomeComponents.kt | 149 ++------ .../ponzischeme89/memby/ui/MainActivity.kt | 328 ++++++++++++------ .../memby/ui/MyShowsNotifications.kt | 30 ++ .../ponzischeme89/memby/ui/RemoteLongPress.kt | 92 +++++ .../memby/ui/UserQuickActions.kt | 290 ++++++++++++++++ .../memby/ui/genre/GenreBrowseScreen.kt | 155 ++++++--- .../memby/ui/search/SearchScreen.kt | 87 +---- .../memby/ui/search/SearchViewModel.kt | 39 +-- .../memby/data/GatewayPayloadTest.kt | 10 - .../memby/ui/ServerHomeRowsTest.kt | 36 +- .../ui/UserQuickActionsScreenshotTest.kt | 75 ++++ .../memby/ui/UserQuickActionsTest.kt | 62 ++++ server/internal/api/api.go | 4 +- server/internal/api/api_test.go | 15 - server/internal/api/home.go | 31 -- server/internal/api/my_shows.go | 66 ++++ .../api/notification_preferences_test.go | 26 ++ server/internal/store/my_shows.go | 25 ++ server/internal/store/searches.go | 53 +-- 24 files changed, 1131 insertions(+), 500 deletions(-) create mode 100644 app/src/main/java/com/ponzischeme89/memby/ui/RemoteLongPress.kt create mode 100644 app/src/main/java/com/ponzischeme89/memby/ui/UserQuickActions.kt create mode 100644 app/src/test/java/com/ponzischeme89/memby/ui/UserQuickActionsScreenshotTest.kt create mode 100644 app/src/test/java/com/ponzischeme89/memby/ui/UserQuickActionsTest.kt diff --git a/CLAUDE.md b/CLAUDE.md index f5e1217..9488e75 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -722,10 +722,10 @@ caches results per query for the session, so backspacing is instant. **Every search the tab performs is recorded, and the gateway is what records it.** It used to depend entirely on the television posting to `/v1/search/history` after a result landed, which meant a query answered from the client's own cache, one whose post failed, or one -from a build that predates the call was never written down at all — and the table feeding -the recent-searches row and future per-user ranking was a partial record of what the -household looks for. `handleSearch` now calls `recordSearchQuery` itself, before the Redis -lookup, so a cached answer counts the same as one that reached Emby. Things to preserve: +from a build that predates the call was never written down at all — and the table the +console reads back was a partial record of what the household looks for. `handleSearch` +now calls `recordSearchQuery` itself, before the Redis lookup, so a cached answer counts +the same as one that reached Emby. Things to preserve: - **The write is detached from the request context.** Instant search cancels the in-flight request on every keystroke, so a write hung off `r.Context()` would be abandoned for diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b340d4b..6424918 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -63,7 +63,7 @@ val projectNoticeText = // A release workflow can derive the app version from its Git tag without editing the // source tree. Local builds keep using the checked-in default. -val defaultVersionName = "0.2.79" +val defaultVersionName = "0.2.80" val membyVersionName: String = (project.findProperty("memby.versionName") as String?) ?.trim() diff --git a/app/src/main/java/com/ponzischeme89/memby/data/EmbyRepository.kt b/app/src/main/java/com/ponzischeme89/memby/data/EmbyRepository.kt index f231039..b7bb467 100644 --- a/app/src/main/java/com/ponzischeme89/memby/data/EmbyRepository.kt +++ b/app/src/main/java/com/ponzischeme89/memby/data/EmbyRepository.kt @@ -982,13 +982,6 @@ class EmbyRepository internal constructor( } } - /** Memby's database has no search history; this is a persisted Memby gateway feature. */ - suspend fun getRecentSearches(): List { - if (!ServerConfig.isGateway) return emptyList() - return runCatching { requireGateway().recentSearches().queries } - .getOrDefault(emptyList()) - } - /** * One month of the TV calendar, or an unavailable month when there is nobody to ask. * @@ -1179,6 +1172,17 @@ class EmbyRepository internal constructor( if (ServerConfig.isGateway) requireGateway().updateNotification(id, "dismiss") } + /** + * Clears this viewer's notifications in one request and answers how many were taken. + * + * The shortcut in the user picker calls this rather than looping [dismissNotification]: + * one request keeps the count honest and leaves the gateway one line in the log naming + * who cleared what. With no gateway there is nothing to clear — the direct path has no + * notifications at all — so it answers zero rather than pretending to have done work. + */ + suspend fun clearNotifications(): Int = + if (ServerConfig.isGateway) requireGateway().clearNotifications().cleared else 0 + fun myShowImageUrl(itemId: String, imageTag: String, maxWidth: Int = 320): String? = imageTag.takeIf(String::isNotBlank)?.let { imageUrl(itemId, "Primary", it, maxWidth) diff --git a/app/src/main/java/com/ponzischeme89/memby/data/model/GatewayModels.kt b/app/src/main/java/com/ponzischeme89/memby/data/model/GatewayModels.kt index 9c521c9..3f341db 100644 --- a/app/src/main/java/com/ponzischeme89/memby/data/model/GatewayModels.kt +++ b/app/src/main/java/com/ponzischeme89/memby/data/model/GatewayModels.kt @@ -574,11 +574,6 @@ data class GatewayMagicRequest( val availableMinutes: Int = 0, ) -@Serializable -data class GatewaySearchHistory( - val queries: List = emptyList(), -) - @Serializable data class GatewayRequestCandidate( val mediaType: String, @@ -927,6 +922,17 @@ data class NotificationsResponse( val preferences: NotificationPreferences = NotificationPreferences(), ) +/** + * How many notifications a clear-all press actually took. + * + * The count is the gateway's, not the television's: a row somebody dismissed on another set + * a moment earlier is one this must not claim, and the confirmation the viewer reads is + * built from this number. Defaults to zero so a gateway that predates the route — which + * answers 404 rather than a body — can never produce a confirmation claiming otherwise. + */ +@Serializable +data class ClearNotificationsResponse(val cleared: Int = 0) + /** One row-engagement event. See `data/analytics/RowAnalytics.kt`. */ @Serializable data class GatewayRowEvent( diff --git a/app/src/main/java/com/ponzischeme89/memby/data/remote/GatewayApi.kt b/app/src/main/java/com/ponzischeme89/memby/data/remote/GatewayApi.kt index 130d422..7db43f8 100644 --- a/app/src/main/java/com/ponzischeme89/memby/data/remote/GatewayApi.kt +++ b/app/src/main/java/com/ponzischeme89/memby/data/remote/GatewayApi.kt @@ -21,7 +21,6 @@ import com.ponzischeme89.memby.data.model.GatewayPrerollSchedule import com.ponzischeme89.memby.data.model.GatewayRowEvents import com.ponzischeme89.memby.data.model.GatewayJourneyEvents import com.ponzischeme89.memby.data.model.GatewayRows -import com.ponzischeme89.memby.data.model.GatewaySearchHistory import com.ponzischeme89.memby.data.model.GatewayRequestLookup import com.ponzischeme89.memby.data.model.GatewayServiceStatus import com.ponzischeme89.memby.data.model.GatewayUpdate @@ -111,9 +110,6 @@ interface GatewayApi { @POST("v1/search/history") suspend fun recordSearch(@Body body: Map) - @GET("v1/search/history") - suspend fun recentSearches(): GatewaySearchHistory - @GET("v1/requests/lookup") suspend fun requestLookup(@Query("q") term: String): GatewayRequestLookup @@ -177,6 +173,16 @@ interface GatewayApi { @Body body: com.ponzischeme89.memby.data.model.NotificationPreferences, ): com.ponzischeme89.memby.data.model.NotificationsResponse + /** + * Clears this viewer's list in one request and answers how many rows it took. + * + * Deliberately not a loop over the route below: the shortcut in the user picker has to + * confirm what it did, and a count of the requests a television made is a count of what + * it asked for rather than of what happened. + */ + @POST("v1/notifications/clear") + suspend fun clearNotifications(): com.ponzischeme89.memby.data.model.ClearNotificationsResponse + @POST("v1/notifications/{id}/{action}") suspend fun updateNotification( @Path("id") id: Long, diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/HomeComponents.kt b/app/src/main/java/com/ponzischeme89/memby/ui/HomeComponents.kt index 653c21e..22b1084 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/HomeComponents.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/HomeComponents.kt @@ -117,7 +117,6 @@ import com.ponzischeme89.memby.data.remoteconfig.BundledRemoteConfig import com.ponzischeme89.memby.data.remoteconfig.NavigationLabels import com.ponzischeme89.memby.data.remoteconfig.NavigationRemoteConfig import java.util.Locale -import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.launch @@ -253,6 +252,12 @@ fun TvNavigationRail( activeProfileInitials: String = "", calendarEnabled: Boolean = false, genresEnabled: Boolean = true, + /** + * A hold on the user item, which is what raises the shortcut menu over the launcher. + * Null leaves every item with the ordinary press it has always had — the rail is drawn + * beside screens that have nowhere to put a menu. + */ + onUserLongPressed: (() -> Unit)? = null, ) { var railHasFocus by remember { mutableStateOf(false) } val logoScale = remember { Animatable(0.72f) } @@ -417,6 +422,12 @@ fun TvNavigationRail( avatarInitials = activeUsername.takeIf { destination == BrowseDestination.PROFILES }?.let { profileInitials(it, activeProfileInitials) }, + // Only the user item. A hold anywhere else on the rail still means the + // press it always meant, so nothing a viewer already knows changes. + onLongClick = onUserLongPressed.takeIf { + destination == BrowseDestination.PROFILES + }, + longPressLabel = "User shortcuts", ) Spacer(Modifier.height(4.dp)) } @@ -788,6 +799,13 @@ fun ExpandableNavigationItem( modifier: Modifier = Modifier, badge: String? = null, avatarInitials: String? = null, + /** + * A hold rather than a press. Only the user item offers one today — it is where the + * shortcut that clears this viewer's notifications lives — so a rail item with nothing + * behind a hold keeps the ordinary press it always had, and this stays null. + */ + onLongClick: (() -> Unit)? = null, + longPressLabel: String = "Quick actions", ) { var focused by remember { mutableStateOf(false) } // Not `by`: both colours are read in the draw phase / at the point of use, so the @@ -820,9 +838,15 @@ fun ExpandableNavigationItem( } .clip(RoundedCornerShape(8.dp)) .drawBehind { drawRect(background.value) } - .clickable(onClick = onClick) + .remoteLongPress(onClick = onClick, onLongClick = onLongClick) .semantics { contentDescription = badge?.let { "$label, $it waiting" } ?: label + if (onLongClick != null) { + onLongClick(longPressLabel) { + onLongClick.invoke() + true + } + } } .padding(horizontal = 8.dp), verticalAlignment = Alignment.CenterVertically, @@ -1663,12 +1687,6 @@ internal fun MediaRow( } } } - row.items.isEmpty() && row.id == "favourite-shows" -> { - FavoriteShowsEmptyState( - navigationFocusRequester = navigationFocusRequester, - onContentFocused = onContentFocused, - ) - } row.items.isEmpty() -> { Text( row.emptyMessage, @@ -1756,64 +1774,6 @@ internal fun MediaRow( } } -@Composable -private fun FavoriteShowsEmptyState( - navigationFocusRequester: FocusRequester, - onContentFocused: () -> Unit, -) { - var focused by remember { mutableStateOf(false) } - Row( - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 36.dp, vertical = 10.dp) - .clip(RoundedCornerShape(MembyPanelCorner)) - .background(Color.White.copy(alpha = if (focused) 0.11f else 0.055f)) - .border( - 2.dp, - if (focused) Color.White else Color.White.copy(alpha = 0.14f), - RoundedCornerShape(MembyPanelCorner), - ) - .focusProperties { left = navigationFocusRequester } - .onFocusChanged { - focused = it.isFocused - if (it.isFocused) onContentFocused() - } - .focusable() - .padding(horizontal = 22.dp, vertical = 20.dp), - verticalAlignment = Alignment.CenterVertically, - horizontalArrangement = Arrangement.spacedBy(18.dp), - ) { - Box( - modifier = Modifier - .size(46.dp) - .clip(CircleShape) - .background(Color.White.copy(alpha = 0.09f)), - contentAlignment = Alignment.Center, - ) { - Icon( - imageVector = MembyIcon.FavouriteOutline.mark, - contentDescription = null, - tint = Color.White, - modifier = Modifier.size(25.dp), - ) - } - Column(verticalArrangement = Arrangement.spacedBy(4.dp)) { - Text( - "No favourite shows yet", - color = Color.White, - fontSize = 17.sp, - fontWeight = FontWeight.Bold, - ) - Text( - "Mark a series as a favourite and it’ll be waiting here.", - color = MutedText, - fontSize = 14.sp, - fontWeight = FontWeight.Medium, - ) - } - } -} - @Composable private fun GalleryJumpButton( forward: Boolean, @@ -2312,10 +2272,6 @@ fun FocusScaleContainer( content: @Composable BoxScope.(focused: Boolean) -> Unit, ) { var focused by remember { mutableStateOf(false) } - val pressScope = rememberCoroutineScope() - var holdJob by remember { mutableStateOf(null) } - var remoteLongPressHandled by remember { mutableStateOf(false) } - val currentOnLongClick by rememberUpdatedState(onLongClick) val scale = animateFloatAsState( targetValue = if (focused) 1.025f else 1f, animationSpec = tween(95), @@ -2333,55 +2289,12 @@ fun FocusScaleContainer( } .onFocusChanged { focused = it.isFocused - if (it.isFocused) { - onFocused() - } else { - holdJob?.cancel() - holdJob = null - remoteLongPressHandled = false - } + if (it.isFocused) onFocused() } - .then( - if (onLongClick != null) { - Modifier - .onPreviewKeyEvent { event -> - val native = event.nativeKeyEvent - val activationKey = native.keyCode == android.view.KeyEvent.KEYCODE_DPAD_CENTER || - native.keyCode == android.view.KeyEvent.KEYCODE_ENTER || - native.keyCode == android.view.KeyEvent.KEYCODE_NUMPAD_ENTER - when { - activationKey && - native.action == android.view.KeyEvent.ACTION_DOWN && - native.repeatCount == 0 && - holdJob == null && - !remoteLongPressHandled -> { - holdJob = pressScope.launch { - delay(QuickActionsHoldDurationMillis) - remoteLongPressHandled = true - holdJob = null - currentOnLongClick?.invoke() - } - true - } - activationKey && - native.action == android.view.KeyEvent.ACTION_DOWN -> true - activationKey && - native.action == android.view.KeyEvent.ACTION_UP -> { - val wasLongPress = remoteLongPressHandled - holdJob?.cancel() - holdJob = null - remoteLongPressHandled = false - if (!wasLongPress) onClick() - true - } - else -> false - } - } - .clickable(onClick = onClick) - } else { - Modifier.clickable(onClick = onClick) - }, - ) + // The hold, the swallowed repeats and the cancel-on-focus-loss are + // [Modifier.remoteLongPress]'s — shared with the rail's user item, so the length + // of press that opens a menu is the same wherever a menu can be opened. + .remoteLongPress(onClick = onClick, onLongClick = onLongClick) .semantics(mergeDescendants = true) { this.contentDescription = contentDescription if (onLongClick != null) { diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/MainActivity.kt b/app/src/main/java/com/ponzischeme89/memby/ui/MainActivity.kt index b9bad63..af20e53 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/MainActivity.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/MainActivity.kt @@ -1913,6 +1913,10 @@ private fun HomeScreen( // standing, and it is nowhere near the launcher. var addingProfile by remember { mutableStateOf(false) } var userSwitcherVisible by remember { mutableStateOf(false) } + // The shortcut menu a hold on the rail's user item raises. Kept beside the picker rather + // than inside it: the two are alternative answers to the same control, and only one of + // them may ever be on screen. + var userQuickActionsVisible by remember { mutableStateOf(false) } var switchingProfileId by remember { mutableStateOf(null) } var removingProfileId by remember { mutableStateOf(null) } var selectedDestination by rememberSaveable { mutableStateOf(BrowseDestination.HOME) } @@ -1983,8 +1987,6 @@ private fun HomeScreen( // the row list is out of scope. Saved with the rest so a recreate mid-browse does not // file the next resume under nothing. var returnRowKind by rememberSaveable { mutableStateOf(null) } - var recentSearches by remember { mutableStateOf>(emptyList()) } - var initialSearchQuery by rememberSaveable { mutableStateOf(null) } // Destination and row list states live above the conditional content branches. // Opening Search/Settings/details therefore never creates a new list at position 0. val verticalStates = rememberSaveable(saver = LazyListStateMapSaver) { @@ -2022,6 +2024,11 @@ private fun HomeScreen( // than the first, and the search obligingly skipped past the thing somebody had just // been reading about in Continue Watching. val heroRowEntryFocusRequester = remember { FocusRequester() } + // My Shows sits above the shelves on the television page, so it is the first stop + // below the hero and the thing Up out of the topmost shelf must return to. It is its + // own requester rather than the rail's entry one for the same reason the shelves' hero + // entry is: the entry requester belongs to whichever band the rail hands focus to. + val myShowsEntryFocusRequester = remember { FocusRequester() } // Feature flags can remove the node that currently owns focus. Move the route first, // wait for its replacement to compose, then establish an explicit remote target. // A viewer standing on the Requests page when an operator withdraws access is moved @@ -2145,6 +2152,7 @@ private fun HomeScreen( showSettings = false showProfiles = false userSwitcherVisible = false + userQuickActionsVisible = false showNotifications = false showRequests = false detailsItem = null @@ -2438,9 +2446,6 @@ private fun HomeScreen( ) LaunchedEffect(selectedDestination) { focusedHomeRowId = null - if (selectedDestination == BrowseDestination.FAVORITES) { - recentSearches = repo.getRecentSearches() - } } LaunchedEffect(selectedDestination, settings.forYouMinutes) { if ( @@ -2549,6 +2554,9 @@ private fun HomeScreen( BrowseDestination.PROFILES -> { railFocusDestination = BrowseDestination.PROFILES userSwitcherVisible = true + // The picker and the shortcut menu are two answers to one control; only + // one of them may ever be on screen. + userQuickActionsVisible = false navigationExpanded = false // The rail stays live beside Settings, so a destination // chosen from there has to close it or it would sit over @@ -2606,6 +2614,21 @@ private fun HomeScreen( activeProfileInitials = settings.profileInitials, calendarEnabled = tvCalendarEnabled, genresEnabled = genreBrowserEnabled, + // A hold opens the shortcut menu; the ordinary press still opens the user + // picker, so nothing a viewer already knows how to do has moved. + onUserLongPressed = { + homeViewModel.trackJourney( + category = "navigation", action = "open", screen = journeyScreen, + feature = "user_switcher_shortcuts", source = journeyScreen, + target = "user_shortcuts", + ) + userSwitcherVisible = false + navigationExpanded = false + // So closing the menu hands focus back to the item that was held, rather + // than to whichever destination the rail was last aimed at. + railFocusDestination = BrowseDestination.PROFILES + userQuickActionsVisible = true + }, ) androidx.compose.foundation.layout.BoxWithConstraints( modifier = Modifier @@ -2649,8 +2672,6 @@ private fun HomeScreen( discoveryItems = discovery, returnFocusItemId = returnItemId.takeIf { returnRowId == SEARCH_ROW_ID }, returnFocusRequester = cardReturnFocusRequester, - initialQuery = initialSearchQuery, - onInitialQueryConsumed = { initialSearchQuery = null }, onSearchStarted = { homeViewModel.trackJourney( category = "search", action = "start", screen = "search", @@ -2823,6 +2844,13 @@ private fun HomeScreen( val firstPopulatedRowId = remember(rows) { rows.firstOrNull { candidate -> candidate.items.isNotEmpty() }?.id } + // My Shows is a band of its own above the shelves, so on this page it is the + // first stop below the hero and the thing Up out of the topmost shelf has to + // return to. Both moves used to jump over it: the hero pointed Down straight + // at the first shelf, and Up out of that shelf found no shelf above it and + // went to the hero. + val myShowsStripFocusable = + selectedDestination == BrowseDestination.SHOWS && myShows.isNotEmpty() // Must agree exactly with the items placed above the rows in the LazyColumn // below, because it is what turns a row index into a scroll target. // Counting one that is not placed scrolls a row short of the destination, @@ -2830,10 +2858,7 @@ private fun HomeScreen( // the move with nothing to complete it. val leadingItemCount = (if (selectedDestination == BrowseDestination.SHOWS) 1 else 0) + - if ( - selectedDestination == BrowseDestination.FAVORITES && - recentSearches.isNotEmpty() - ) 1 else if (selectedDestination == BrowseDestination.FOR_YOU) 1 else 0 + (if (selectedDestination == BrowseDestination.FOR_YOU) 1 else 0) LaunchedEffect(rowListFocusRestoreRequest, selectedDestination) { if (rowListFocusRestoreRequest == 0) return@LaunchedEffect val rowId = returnRowId ?: run { @@ -2860,6 +2885,55 @@ private fun HomeScreen( } rowListFocusRestoreRequest = 0 } + // Leaving My Shows vertically. Neither direction can be a plain + // focusProperties target: the shelf below is a lazy child composed by the + // scroll rather than something already there to receive the press, and the + // hero is not composed at all while a shelf holds focus. + val enterFirstPopulatedRow: () -> Boolean = enter@{ + if (rowFocusMoving) return@enter true + val destinationRowIndex = rows.indexOfFirst { it.items.isNotEmpty() } + if (destinationRowIndex < 0) return@enter false + val destinationRow = rows[destinationRowIndex] + val destinationItemIndex = rowEntryItemIndex( + sourceIndex = 0, + destinationItemCount = destinationRow.items.size, + rememberedDestinationIndex = rowFocusPositions[destinationRow.id], + ) + rowFocusPositions[destinationRow.id] = destinationItemIndex + rowFocusRequestId += 1 + val request = RowFocusRequest( + rowId = destinationRow.id, + itemIndex = destinationItemIndex, + requestId = rowFocusRequestId, + ) + rowFocusMoving = true + scope.launch { + try { + verticalState.animateScrollToItem( + leadingItemCount + destinationRowIndex, + ) + } catch (cancelled: kotlinx.coroutines.CancellationException) { + // Same bargain the shelves' own moves strike: ask for the card + // anyway, and let the request expire if nothing consumes it. + pendingRowFocus = request + throw cancelled + } + pendingRowFocus = request + } + true + } + val returnToMyShows: () -> Boolean = ret@{ + if (!myShowsStripFocusable) return@ret false + // The strip sits above the shelves with the hero, so coming back up to + // it brings the hero back with it. + rowFocusedBelowHero = false + scope.launch { + verticalState.animateScrollToItem(0) + kotlinx.coroutines.delay(16L) + runCatching { myShowsEntryFocusRequester.requestFocus() } + } + true + } Column(Modifier.fillMaxSize()) { if (showHomeHero) { HomeMovieHero( @@ -2870,8 +2944,13 @@ private fun HomeScreen( returnFocusRequester = cardReturnFocusRequester, // Only when there is a card down there to attach it to: a // requester naming nothing throws the moment Down is pressed. - downFocusRequester = heroRowEntryFocusRequester.takeIf { - firstPopulatedRowId != null + // My Shows first where there is one, then the first shelf with + // cards in it. Only ever a requester with a card behind it: one + // naming nothing throws the moment Down is pressed. + downFocusRequester = if (myShowsStripFocusable) { + myShowsEntryFocusRequester + } else { + heroRowEntryFocusRequester.takeIf { firstPopulatedRowId != null } }, onItemFocused = { item -> navigationExpanded = false @@ -2949,10 +3028,40 @@ private fun HomeScreen( availableWidth = contentWidth, density = settings.homeCardDensity, navigationFocusRequester = navigationFocusRequester, - // My Shows is the first reachable content on this page. - contentFocusRequester = contentFocusRequester, + // My Shows is the first reachable content on this page — + // unless the hero is up, which owns the rail's entry + // target the way it does on every other page. Keyed on + // the hero actually being drawn rather than on there + // being one: a requester attached to two live nodes is + // ambiguous, and one attached to a hero that is not + // composed would leave the rail with nowhere to hand + // focus to. + contentFocusRequester = contentFocusRequester.takeIf { + !showHomeHero + }, + entryFocusRequester = myShowsEntryFocusRequester, returnFocusItemId = myShowReturnItemId, returnFocusRequester = myShowReturnFocusRequester, + onMoveVertical = { direction -> + when (direction) { + RowFocusDirection.DOWN -> enterFirstPopulatedRow() + RowFocusDirection.UP -> if (hasContextualHero) { + rowFocusedBelowHero = false + scope.launch { + verticalState.animateScrollToItem(0) + // Let the hero compose and attach its + // entry target before focus is handed up. + kotlinx.coroutines.delay(16L) + runCatching { + contentFocusRequester.requestFocus() + } + } + true + } else { + false + } + } + }, onShowSelected = { homeViewModel.trackJourney( category = "content", action = "open", screen = "shows", @@ -2966,26 +3075,6 @@ private fun HomeScreen( ) } } - if ( - selectedDestination == BrowseDestination.FAVORITES && - recentSearches.isNotEmpty() - ) { - item(key = "recent-searches", contentType = "recent-searches") { - RecentSearchesRow( - queries = recentSearches, - navigationFocusRequester = navigationFocusRequester, - onContentFocused = { navigationExpanded = false }, - onQuerySelected = { query -> - homeViewModel.trackJourney( - category = "search", action = "select", screen = "favourites", - feature = "recent_searches", source = "recent_searches", target = "search", - ) - initialSearchQuery = query - selectedDestination = BrowseDestination.SEARCH - }, - ) - } - } if (selectedDestination == BrowseDestination.FOR_YOU) { item(key = "for-you-time", contentType = "for-you-time") { ForYouTimeBudget( @@ -3055,6 +3144,11 @@ private fun HomeScreen( // not composed while a row holds focus, so there is // nothing above for Compose's own focus search to // find and the press would otherwise be dead. + // My Shows is the band directly above the shelves, + // so it answers this press before the hero does. + if (direction == RowFocusDirection.UP && returnToMyShows()) { + return@moveVertical true + } if (direction == RowFocusDirection.UP && hasContextualHero) { rowFocusedBelowHero = false scope.launch { @@ -3290,6 +3384,83 @@ private fun HomeScreen( }, ) } + if (userQuickActionsVisible) { + val closeUserQuickActions: () -> Unit = { + userQuickActionsVisible = false + scope.launch { + kotlinx.coroutines.delay(16L) + runCatching { navigationFocusRequester.requestFocus() } + } + } + BackHandler(onBack = closeUserQuickActions) + UserQuickActionsOverlay( + username = settings.username.orEmpty(), + alertCount = displayedNotifications.size, + busy = notificationsMutationBusy, + // Clearing from here never takes the viewer anywhere: the menu closes, the + // badge on the rail behind it empties, and a toast says what happened. The + // list is emptied optimistically for the reason the alerts page already is — + // a badge that lingered while the request was in flight is one somebody + // clears twice — and put back untouched if the gateway refuses. + onClearNotifications = { + if (!notificationsMutationBusy) { + notificationsMutationBusy = true + val previous = notificationState + val offered = displayedNotifications.size + val hadLocalUpdate = settings.updateAlertVersion != null + notificationState = notificationState.copy(notifications = emptyList()) + closeUserQuickActions() + scope.launch { + // The update notice is this television's own — there is no server + // row behind it, so it is dismissed locally and counted here. + if (hadLocalUpdate) ServiceLocator.settings.dismissUpdateAlert() + runCatching { repo.clearNotifications() } + .onSuccess { cleared -> + val total = cleared + if (hadLocalUpdate) 1 else 0 + Toast.makeText( + context, + clearedNotificationsMessage(total), + Toast.LENGTH_SHORT, + ).show() + homeViewModel.trackJourney( + category = "notifications", action = "dismiss", + screen = journeyScreen, + feature = "user_switcher_clear_notifications", + source = "user_switcher", target = "notifications", + // The one free-text field, and the count is the only + // thing that separates one use of this shortcut from + // the next. + itemName = "$total cleared", + outcome = "success", + ) + } + .onFailure { + // Put the list back and say so here rather than writing + // notificationsError: this menu never opens the page, and + // an error banner waiting on a page nobody has opened is + // one they meet later with no idea what it is about. + notificationState = previous + Toast.makeText( + context, + "Couldn’t clear notifications", + Toast.LENGTH_SHORT, + ).show() + homeViewModel.trackJourney( + category = "notifications", action = "dismiss", + screen = journeyScreen, + feature = "user_switcher_clear_notifications", + source = "user_switcher", target = "notifications", + itemName = "$offered offered", + outcome = "failure", + ) + } + notificationsMutationBusy = false + } + } + }, + onDismiss = closeUserQuickActions, + ) + } if (showSettings) { val closeSettings: () -> Unit = { showSettings = false @@ -3648,6 +3819,9 @@ private fun HomeScreen( BrowseDestination.PROFILES -> { railFocusDestination = BrowseDestination.PROFILES userSwitcherVisible = true + // The picker and the shortcut menu are two answers to one control; only + // one of them may ever be on screen. + userQuickActionsVisible = false navigationExpanded = false showSettings = false restoreRailAfterSettings = false @@ -4068,76 +4242,6 @@ private fun PlaybackLaunchOverlay(item: BaseItem, modifier: Modifier = Modifier) private const val SEARCH_ROW_ID = "search-results" private const val GENRE_BROWSER_ROW_ID = "genre-browser-results" -@Composable -private fun RecentSearchesRow( - queries: List, - navigationFocusRequester: FocusRequester, - onContentFocused: () -> Unit, - onQuerySelected: (String) -> Unit, -) { - Column(verticalArrangement = Arrangement.spacedBy(HomeRowHeaderSpacing)) { - Row( - modifier = Modifier.fillMaxWidth().padding(horizontal = 36.dp), - verticalAlignment = Alignment.CenterVertically, - ) { - HomeRowHeaderIcon(MembyIcon.Search.mark) - Spacer(Modifier.width(HomeRowHeaderIconGap)) - Text( - "Recent searches", - color = MembyOnSurface, - fontSize = 20.sp, - fontWeight = FontWeight.Bold, - ) - Spacer(Modifier.width(10.dp)) - Text( - "LAST 30 DAYS", - color = MembyQuietText, - fontSize = 10.sp, - fontWeight = FontWeight.Bold, - ) - } - LazyRow( - // Vertical padding so a focus-scaled chip has room to grow, like every other row. - contentPadding = androidx.compose.foundation.layout.PaddingValues( - horizontal = 36.dp, - vertical = 9.dp, - ), - horizontalArrangement = Arrangement.spacedBy(9.dp), - ) { - items(queries, key = { it.lowercase() }) { query -> - val first = query == queries.first() - FocusScaleContainer( - onFocused = onContentFocused, - onClick = { onQuerySelected(query) }, - contentDescription = "Search again for $query", - modifier = Modifier - .clip(RoundedCornerShape(MembyChipCorner)) - .then( - if (first) { - Modifier.focusProperties { left = navigationFocusRequester } - } else { - Modifier - }, - ), - ) { focused -> - Text( - query, - color = if (focused) MembyAccentInk else MembyOnSurface, - fontSize = 14.sp, - fontWeight = FontWeight.SemiBold, - maxLines = 1, - modifier = Modifier - .background( - if (focused) MembyAccent else MembyControlSurface, - ) - .padding(horizontal = 17.dp, vertical = 10.dp), - ) - } - } - } - } -} - @Composable private fun HomeClock( showGreeting: Boolean, @@ -4651,11 +4755,21 @@ internal fun homeRowsFor( BrowseDestination.SETTINGS -> emptyList() } val deduplicated = deduplicateBrowseRows(destinationRows) - return if (destination == BrowseDestination.HOME && state.rows.isEmpty()) { - applyHomeRowPreferences(deduplicated, settings) + // The television page carries two fixed shelves it may have nothing for — a household + // part-way through no episodes, or with no series favourited — and a heading with an + // apology under it is a row the D-pad already refuses to enter, so it is a stop on the + // way to the shelves that do have something. A row still *loading* is kept: it has + // something arriving, and withdrawing it would move every shelf below it a moment later. + val populated = if (destination == BrowseDestination.SHOWS) { + deduplicated.filter { it.items.isNotEmpty() || it.loading } } else { deduplicated } + return if (destination == BrowseDestination.HOME && state.rows.isEmpty()) { + applyHomeRowPreferences(populated, settings) + } else { + populated + } } /** diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/MyShowsNotifications.kt b/app/src/main/java/com/ponzischeme89/memby/ui/MyShowsNotifications.kt index 7876016..f94a7ad 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/MyShowsNotifications.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/MyShowsNotifications.kt @@ -32,6 +32,11 @@ import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusProperties import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.focus.onFocusChanged +import androidx.compose.ui.input.key.Key +import androidx.compose.ui.input.key.KeyEventType +import androidx.compose.ui.input.key.key +import androidx.compose.ui.input.key.onPreviewKeyEvent +import androidx.compose.ui.input.key.type import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.text.font.FontWeight @@ -64,8 +69,15 @@ internal fun MyShowsStrip( density: String, navigationFocusRequester: FocusRequester, contentFocusRequester: FocusRequester?, + // Where Down out of the hero and Up out of the shelf below land. A second requester + // rather than the entry one because while the hero is up, that one belongs to the hero. + entryFocusRequester: FocusRequester? = null, returnFocusItemId: String? = null, returnFocusRequester: FocusRequester? = null, + // Leaving the strip vertically. Handled by the caller for the reason the shelves' own + // moves are: the destination is a lazy child that has to be composed before its focus + // node can be asked for. Returning false lets the press fall through to Compose. + onMoveVertical: (RowFocusDirection) -> Boolean = { false }, onShowSelected: (MyShow) -> Unit, onContentFocused: () -> Unit, ) { @@ -155,6 +167,13 @@ internal fun MyShowsStrip( Modifier }, ) + .then( + if (show.itemId == shows.first().itemId && entryFocusRequester != null) { + Modifier.focusRequester(entryFocusRequester) + } else { + Modifier + }, + ) .then( if (show.itemId == returnFocusItemId && returnFocusRequester != null) { Modifier.focusRequester(returnFocusRequester) @@ -163,6 +182,17 @@ internal fun MyShowsStrip( }, ) .focusProperties { left = navigationFocusRequester } + .onPreviewKeyEvent { event -> + if (event.type != KeyEventType.KeyDown) { + false + } else { + when (event.key) { + Key.DirectionUp -> onMoveVertical(RowFocusDirection.UP) + Key.DirectionDown -> onMoveVertical(RowFocusDirection.DOWN) + else -> false + } + } + } .onFocusChanged { if (it.hasFocus) onContentFocused() }, ) } diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/RemoteLongPress.kt b/app/src/main/java/com/ponzischeme89/memby/ui/RemoteLongPress.kt new file mode 100644 index 0000000..67831e5 --- /dev/null +++ b/app/src/main/java/com/ponzischeme89/memby/ui/RemoteLongPress.kt @@ -0,0 +1,92 @@ +package com.ponzischeme89.memby.ui + +import androidx.compose.foundation.clickable +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.setValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.composed +import androidx.compose.ui.focus.onFocusChanged +import androidx.compose.ui.input.key.onPreviewKeyEvent +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch + +/** + * A short press and a hold, told apart on a remote that has neither. + * + * A television reports the confirm key as an ordinary key event, so a long press has to be + * measured: the DOWN is swallowed, [QuickActionsHoldDurationMillis] is waited out, and the + * UP decides which of the two happened. This is one modifier rather than a copy per surface + * because two of them are already live — a media card and the rail's user item — and the + * hold duration is the one thing that must not differ between them: a shortcut that opens + * after a different length of press on one control than another reads as an unreliable + * remote rather than as two features. + * + * Three details are load-bearing: + * + * - **Every repeat of the held key is consumed**, not acted on. Some remotes report a held + * key as a stream of DOWNs, and acting on those would open the menu again underneath the + * one already up. + * - **The short press fires on UP**, never on DOWN, because until the key is released + * there is no way to know which press this was. + * - **Losing focus cancels the hold.** Focus can move out from under a held key — the + * menu that just opened takes it — and a timer that survived that would fire the long + * press against a control the viewer had already left. + */ +@Composable +internal fun Modifier.remoteLongPress( + onClick: () -> Unit, + onLongClick: (() -> Unit)?, +): Modifier = composed { + if (onLongClick == null) return@composed clickable(onClick = onClick) + val pressScope = rememberCoroutineScope() + var holdJob by remember { mutableStateOf(null) } + var handled by remember { mutableStateOf(false) } + val currentOnLongClick by rememberUpdatedState(onLongClick) + val currentOnClick by rememberUpdatedState(onClick) + Modifier + .onFocusChanged { + if (!it.isFocused) { + holdJob?.cancel() + holdJob = null + handled = false + } + } + .onPreviewKeyEvent { event -> + val native = event.nativeKeyEvent + val activationKey = native.keyCode == android.view.KeyEvent.KEYCODE_DPAD_CENTER || + native.keyCode == android.view.KeyEvent.KEYCODE_ENTER || + native.keyCode == android.view.KeyEvent.KEYCODE_NUMPAD_ENTER + when { + activationKey && + native.action == android.view.KeyEvent.ACTION_DOWN && + native.repeatCount == 0 && + holdJob == null && + !handled -> { + holdJob = pressScope.launch { + delay(QuickActionsHoldDurationMillis) + handled = true + holdJob = null + currentOnLongClick?.invoke() + } + true + } + activationKey && native.action == android.view.KeyEvent.ACTION_DOWN -> true + activationKey && native.action == android.view.KeyEvent.ACTION_UP -> { + val wasLongPress = handled + holdJob?.cancel() + holdJob = null + handled = false + if (!wasLongPress) currentOnClick() + true + } + else -> false + } + } + .clickable(onClick = onClick) +} diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/UserQuickActions.kt b/app/src/main/java/com/ponzischeme89/memby/ui/UserQuickActions.kt new file mode 100644 index 0000000..c7ceb6e --- /dev/null +++ b/app/src/main/java/com/ponzischeme89/memby/ui/UserQuickActions.kt @@ -0,0 +1,290 @@ +package com.ponzischeme89.memby.ui + +import androidx.compose.foundation.background +import androidx.compose.foundation.border +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.draw.shadow +import androidx.compose.ui.focus.FocusRequester +import androidx.compose.ui.focus.focusRequester +import androidx.compose.ui.focus.onFocusChanged +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.vector.ImageVector +import androidx.compose.ui.input.key.Key +import androidx.compose.ui.input.key.KeyEventType +import androidx.compose.ui.input.key.key +import androidx.compose.ui.input.key.onPreviewKeyEvent +import androidx.compose.ui.input.key.type +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import androidx.compose.ui.unit.sp +import androidx.compose.ui.zIndex +import androidx.tv.material3.Icon +import androidx.tv.material3.Text +import com.ponzischeme89.memby.ui.theme.MembyChipCorner +import com.ponzischeme89.memby.ui.theme.MembyDisabledText +import com.ponzischeme89.memby.ui.theme.MembyIcon +import com.ponzischeme89.memby.ui.theme.MembyMutedText +import com.ponzischeme89.memby.ui.theme.MembyPanelCorner +import com.ponzischeme89.memby.ui.theme.MembyQuietText +import com.ponzischeme89.memby.ui.theme.MembySurface +import com.ponzischeme89.memby.ui.theme.mark +import kotlinx.coroutines.delay + +/** + * The shortcut menu behind a hold on the rail's user item. + * + * It exists so that clearing a stack of notifications is one hold and one press, rather than + * a trip into the notifications page to empty a list somebody has already read from the + * badge. Everything about *what* the menu offers is decided here, pure, because the one rule + * worth pinning is the empty case: a household with nothing waiting must be told so rather + * than offered a button that makes a request the gateway can only answer "nothing". + */ +internal enum class UserQuickActionKind { CLEAR_NOTIFICATIONS, CANCEL } + +internal data class UserQuickActionEntry( + val kind: UserQuickActionKind, + val label: String, + /** + * False draws the row quietly and refuses the press. Deliberately shown rather than + * removed: a menu whose contents change shape depending on whether there happens to be + * news is one a viewer cannot learn, and "Nothing to clear" is the answer they opened it + * for anyway. + */ + val enabled: Boolean, +) + +/** + * The menu for a viewer with [alertCount] notifications waiting. + * + * The count is named on the row because it is the whole of what the press is about, and + * because it is the number the viewer is about to see disappear from the badge — a + * confirmation that agreed with nothing on screen would read as having cleared something + * else. [AlertBadgeMax] is not applied: the badge caps at "9+" to fit its pill, where this + * row has the width to be exact and a viewer deciding whether to clear wants the real + * figure. + */ +internal fun userQuickActions(alertCount: Int, busy: Boolean = false): List { + val count = alertCount.coerceAtLeast(0) + val label = when { + count <= 0 -> "No notifications to clear" + count == 1 -> "Clear 1 notification" + else -> "Clear $count notifications" + } + return listOf( + UserQuickActionEntry( + kind = UserQuickActionKind.CLEAR_NOTIFICATIONS, + label = label, + enabled = count > 0 && !busy, + ), + UserQuickActionEntry(UserQuickActionKind.CANCEL, "Cancel", enabled = true), + ) +} + +/** + * What the television says once the gateway has answered. + * + * The figure is the server's rather than the count the menu offered: another set in the + * house may have emptied the list a moment earlier, and a confirmation claiming seven when + * the gateway took none would be the shortcut reporting its intent instead of its result. + */ +internal fun clearedNotificationsMessage(cleared: Int): String = when { + cleared <= 0 -> "Nothing left to clear" + cleared == 1 -> "1 notification cleared" + else -> "$cleared notifications cleared" +} + +/** + * The menu itself: a small panel beside the rail, not a full-screen dialog. + * + * It is the user switcher's shape deliberately — same inset, same width, same corner — so a + * hold and a press on the same rail item read as two doors on one control rather than as two + * different features. It is short enough that D-pad travel is one press, which is the whole + * point of a shortcut. + */ +@Composable +internal fun UserQuickActionsOverlay( + username: String, + alertCount: Int, + busy: Boolean, + onClearNotifications: () -> Unit, + onDismiss: () -> Unit, + modifier: Modifier = Modifier, +) { + val actions = userQuickActions(alertCount, busy) + val focusRequesters = remember(actions.size) { List(actions.size) { FocusRequester() } } + var focusedIndex by remember { mutableStateOf(0) } + // The menu appears while the confirm key is still physically held — that hold is what + // opened it. Until it is released, swallow every activation so the press that raised the + // menu cannot also run the row it landed on. + var openingPressReleased by remember { mutableStateOf(false) } + LaunchedEffect(Unit) { + delay(16) + runCatching { focusRequesters.first().requestFocus() } + } + Box( + modifier + .fillMaxSize() + .zIndex(21f) + .background(Color.Black.copy(alpha = 0.56f)), + ) { + Column( + modifier = Modifier + .align(Alignment.CenterStart) + .padding(start = 52.dp) + .width(292.dp) + .shadow(12.dp, RoundedCornerShape(MembyPanelCorner)) + .clip(RoundedCornerShape(MembyPanelCorner)) + .background(MembySurface) + .border(1.dp, Color.White.copy(alpha = 0.07f), RoundedCornerShape(MembyPanelCorner)) + .onPreviewKeyEvent { event -> + val native = event.nativeKeyEvent + val activationKey = + native.keyCode == android.view.KeyEvent.KEYCODE_DPAD_CENTER || + native.keyCode == android.view.KeyEvent.KEYCODE_ENTER || + native.keyCode == android.view.KeyEvent.KEYCODE_NUMPAD_ENTER + if (activationKey && !openingPressReleased) { + if (native.action == android.view.KeyEvent.ACTION_UP) { + openingPressReleased = true + } + return@onPreviewKeyEvent true + } + if (event.type != KeyEventType.KeyDown) return@onPreviewKeyEvent false + when (event.key) { + Key.DirectionUp, Key.DirectionDown -> { + focusedIndex = quickActionNextIndex( + currentIndex = focusedIndex, + actionCount = actions.size, + direction = if (event.key == Key.DirectionUp) { + QuickActionDirection.UP + } else { + QuickActionDirection.DOWN + }, + ) + runCatching { focusRequesters[focusedIndex].requestFocus() } + true + } + Key.DirectionLeft, Key.Back -> { + onDismiss() + true + } + // Keep focus inside the menu rather than letting spatial search find + // a dimmed card behind it. + Key.DirectionRight -> true + else -> false + } + } + .padding(10.dp), + ) { + Text( + "User shortcuts", + color = Color.White, + fontSize = 18.sp, + fontWeight = FontWeight.Bold, + modifier = Modifier.padding(start = 8.dp, top = 7.dp, end = 8.dp, bottom = 5.dp), + ) + Text( + username.takeIf(String::isNotBlank) ?: "Signed in", + color = MembyQuietText, + fontSize = 12.sp, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.padding(horizontal = 8.dp).padding(bottom = 8.dp), + ) + actions.forEachIndexed { index, action -> + if (index > 0) Spacer(Modifier.height(2.dp)) + UserQuickActionItem( + label = action.label, + icon = when (action.kind) { + UserQuickActionKind.CLEAR_NOTIFICATIONS -> MembyIcon.Notification.mark + UserQuickActionKind.CANCEL -> MembyIcon.ChevronLeft.mark + }, + enabled = action.enabled, + modifier = Modifier + .focusRequester(focusRequesters[index]) + .onFocusChanged { if (it.isFocused) focusedIndex = index }, + onClick = { + when (action.kind) { + UserQuickActionKind.CLEAR_NOTIFICATIONS -> + if (action.enabled) onClearNotifications() + UserQuickActionKind.CANCEL -> onDismiss() + } + }, + ) + } + } + } +} + +@Composable +private fun UserQuickActionItem( + label: String, + icon: ImageVector, + enabled: Boolean, + modifier: Modifier = Modifier, + onClick: () -> Unit, +) { + var focused by remember { mutableStateOf(false) } + // A row nothing can be done with is still reachable and still says what it is; it simply + // never lights up, so the focus ring is the one thing on the panel that can promise a + // press will do something. + val foreground = when { + !enabled -> MembyDisabledText + focused -> Color.White + else -> MembyMutedText + } + Row( + modifier = modifier + .fillMaxWidth() + .height(42.dp) + .onFocusChanged { focused = it.isFocused } + .clip(RoundedCornerShape(MembyChipCorner)) + .background( + if (focused && enabled) Color.White.copy(alpha = 0.11f) else Color.Transparent, + ) + .clickable(onClick = onClick) + .semantics { + contentDescription = if (enabled) label else "$label, unavailable" + } + .padding(horizontal = 12.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Icon( + icon, + contentDescription = null, + tint = if (enabled && focused) Color.White else foreground, + modifier = Modifier.size(17.dp), + ) + Spacer(Modifier.width(12.dp)) + Text( + label, + color = foreground, + fontSize = 14.sp, + fontWeight = FontWeight.SemiBold, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } +} diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/genre/GenreBrowseScreen.kt b/app/src/main/java/com/ponzischeme89/memby/ui/genre/GenreBrowseScreen.kt index 958df35..ef1346c 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/genre/GenreBrowseScreen.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/genre/GenreBrowseScreen.kt @@ -66,6 +66,7 @@ import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewmodel.compose.viewModel +import androidx.tv.material3.Icon import androidx.tv.material3.Text import com.ponzischeme89.memby.ServiceLocator import com.ponzischeme89.memby.data.model.BaseItem @@ -76,6 +77,7 @@ import com.ponzischeme89.memby.ui.theme.MembyAccentDeep import com.ponzischeme89.memby.ui.theme.MembyAccentInk import com.ponzischeme89.memby.ui.theme.MembyCardCorner import com.ponzischeme89.memby.ui.theme.MembyChipCorner +import com.ponzischeme89.memby.ui.theme.MembyIcon import com.ponzischeme89.memby.ui.theme.MembyMutedText import com.ponzischeme89.memby.ui.theme.MembyQuietText import com.ponzischeme89.memby.ui.theme.MembySurface @@ -151,11 +153,25 @@ fun GenreBrowseScreen( // The genre the remote is on, kept apart from the view model's selection so the rail's // own marker and the entry FocusRequester are never waiting on a request. The pane // follows the selection instead, because it labels the grid rather than the remote. - var activeCategoryId by remember(initialCategoryId) { - mutableStateOf(state.selectedCategoryId ?: initialCategoryId) - } + // + // It opens on the destination's own default rather than on whatever was left selected. + // The view model outlives this screen — it is scoped to the activity, so its pages + // survive a trip to Home and back, which is what makes returning to a genre free — but + // arriving at Genres is arriving at the top of a catalogue, not resuming a place in + // one, and a viewer who went Home a day ago has no reason to be put back in Crime. + var activeCategoryId by remember(initialCategoryId) { mutableStateOf(initialCategoryId) } var contentHasFocus by remember { mutableStateOf(false) } + // One requester per genre, owned here rather than by the rail, because every way back + // into the rail has to name *which* genre it is going back to. Handing the rail a + // single "the active one" requester that moved between items as the selection changed + // was what put a returning viewer on the genre they had passed through rather than on + // the one they had chosen: a requester that changes which node it is attached to has, + // for a moment, two, and the one it answers with is not the one under the marker. + val railFocusRequesters = remember(itemType) { + genreCategoryTabs(itemType).associate { it.id to FocusRequester() } + } + // One grid state per genre, so returning to a genre returns to where it was left // rather than to the top of it. Bounded by the catalogue, which is a fixed list. val gridStates = remember(itemType) { mutableMapOf() } @@ -174,24 +190,44 @@ fun GenreBrowseScreen( } val gridState = gridStates.getOrPut(shownCategoryId ?: initialCategoryId) { LazyGridState() } + // Whether the rail has been travelled *on this visit*. The debounce below is about a + // held D-pad, so it must not delay the genre the screen opens on — and the view model + // remembering a previous visit's selection is exactly what would otherwise make the + // opening genre look like a change. + var railTravelled by remember(itemType) { mutableStateOf(false) } LaunchedEffect(activeCategoryId) { // See [GENRE_SELECT_DEBOUNCE_MS]: travelling past a genre must not ask for it. // Only a *change* is a candidate for that — the genre the page opens on is the one // thing somebody is definitely waiting for, so it is asked for at once. - if (state.selectedCategoryId != null) kotlinx.coroutines.delay(GENRE_SELECT_DEBOUNCE_MS) + if (railTravelled) kotlinx.coroutines.delay(GENRE_SELECT_DEBOUNCE_MS) browseViewModel.selectCategory(activeCategoryId) + railTravelled = true } LaunchedEffect(favouriteStates) { browseViewModel.applyFavouriteStates(favouriteStates) } LaunchedEffect(playedStates) { browseViewModel.applyPlayedStates(playedStates) } + /** + * Puts the remote back on the genre the grid belongs to. + * + * Named rather than left to the shared entry requester: this is the press that has to + * remember where the viewer was, and the genre they chose is the only honest answer. + */ + fun focusRail(): Boolean = + railFocusRequesters[activeCategoryId]?.let { runCatching { it.requestFocus() }.isSuccess } + ?: runCatching { contentFocusRequester.requestFocus() }.isSuccess + LaunchedEffect(Unit) { - kotlinx.coroutines.delay(32L) - runCatching { contentFocusRequester.requestFocus() } + // The rail's items are one composition away, so the first attempt can legitimately + // find nothing attached — hence the retries rather than a single delayed request. + repeat(4) { + kotlinx.coroutines.delay(32L) + if (focusRail()) return@LaunchedEffect + } } // Back steps out of the grid before it steps off the page — one press per level, the // rule the search pane and the calendar already follow. BackHandler { if (contentHasFocus) { - runCatching { contentFocusRequester.requestFocus() } + focusRail() } else { onClose() } @@ -229,6 +265,7 @@ fun GenreBrowseScreen( activeCategoryId = activeCategoryId, navigationFocusRequester = navigationFocusRequester, activeFocusRequester = contentFocusRequester, + itemFocusRequesters = railFocusRequesters, onCategoryFocused = { id -> onContentFocused() activeCategoryId = id @@ -372,8 +409,12 @@ fun GenreBrowseScreen( .focusProperties { // Left out of the first column is the way // back to the genre this grid belongs to, - // which is where that requester is attached. - if (index % columns == 0) left = contentFocusRequester + // named directly so the press cannot land + // on a genre merely travelled through. + if (index % columns == 0) { + left = railFocusRequesters[activeCategoryId] + ?: contentFocusRequester + } }, ) } @@ -415,6 +456,11 @@ internal fun GenreRail( activeFocusRequester: FocusRequester, onCategoryFocused: (String) -> Unit, onEnterContent: () -> Boolean, + /** + * One requester per genre, owned by the screen: every way back into the rail names the + * genre it is going back to, and a requester shared between them cannot. + */ + itemFocusRequesters: Map = emptyMap(), /** * Draws one genre as though the remote were on it. * @@ -431,9 +477,14 @@ internal fun GenreRail( val index = categories.indexOfFirst { it.id == activeCategoryId } if (index > 0) runCatching { railState.scrollToItem(index) } } - val itemFocusRequesters = remember(categories) { + // Falls back to a private set only for a caller that has none of its own, which today + // is the screenshot test: the rail must be renderable without the screen around it. + val ownedFocusRequesters = remember(categories) { categories.associate { it.id to FocusRequester() } } + val requesterFor: (String) -> FocusRequester = { id -> + itemFocusRequesters[id] ?: ownedFocusRequesters.getValue(id) + } Column( Modifier .width(GenreRailWidth) @@ -468,15 +519,18 @@ internal fun GenreRail( fontSize = 11.sp, fontWeight = FontWeight.Bold, letterSpacing = 1.6.sp, - modifier = Modifier.padding(start = 22.dp, top = 46.dp, bottom = 14.dp), + modifier = Modifier.padding(start = 22.dp, top = 46.dp, bottom = 18.dp), ) LazyColumn( state = railState, - contentPadding = PaddingValues(start = 12.dp, end = 14.dp, bottom = 48.dp), + // The top inset is not decoration: the list clips at its own edge, so the first + // genre sat directly under the heading and read as cut off by it — and the + // focused item grows, which took what little clearance there was. + contentPadding = PaddingValues(start = 12.dp, end = 14.dp, top = 6.dp, bottom = 48.dp), verticalArrangement = Arrangement.spacedBy(4.dp), ) { rowItemsIndexed(categories, key = { _, category -> category.id }) { index, category -> - val requester = itemFocusRequesters.getValue(category.id) + val requester = requesterFor(category.id) GenreRailItem( category = category, active = category.id == activeCategoryId, @@ -503,12 +557,12 @@ internal fun GenreRail( up = if (index == 0) { FocusRequester.Cancel } else { - itemFocusRequesters.getValue(categories[index - 1].id) + requesterFor(categories[index - 1].id) } down = if (index == categories.lastIndex) { FocusRequester.Cancel } else { - itemFocusRequesters.getValue(categories[index + 1].id) + requesterFor(categories[index + 1].id) } }, ) @@ -521,10 +575,11 @@ internal fun GenreRail( * One genre. * * Focus and the genre in force are marked separately, the stance the player's subtitle - * menu takes: the option under the thumb is the only fill on the rail (accent), and the - * genre the grid beside it is showing wears a quiet plate with a bar in the accent. One - * causes the other while the viewer is in the rail, and they come apart the moment the - * viewer presses Right — which is the case the distinction exists for. + * menu takes: the option under the thumb is the only solid fill on the rail (white, the + * brightest thing on a near-black column), and the genre the grid beside it is showing + * wears the same white held back to a wash, with a bar in the accent. One causes the + * other while the viewer is in the rail, and they come apart the moment the viewer + * presses Right — which is the case the distinction exists for. */ @Composable private fun GenreRailItem( @@ -551,14 +606,18 @@ private fun GenreRailItem( label = "genre-rail-emphasis", ) val plate = when { - focused -> MembyAccent - // Deep green rather than a neutral: the genre in force is what the grid beside - // the rail is showing, and a grey plate read as "this row is slightly raised" - // rather than as "this is the one you are looking at". - active -> MembyAccentDeep + // White, not the accent. The rail sits on near-black beside a grid of artwork, + // and a coloured plate with dark ink on it read as a filled-in shape rather + // than as a highlight — the thing under the thumb should be the brightest + // thing on the screen, which on this surface means white. + focused -> Color.White + // The genre in force keeps the same white, held back to a wash. One vocabulary + // for both states: a viewer down in the grid is looking for the same mark they + // left behind in the rail, only quieter. + active -> Color.White.copy(alpha = 0.14f) else -> Color.Transparent } - val bar = if (focused) MembyAccentInk.copy(alpha = 0.45f) else MembyAccent + val bar = if (focused) MembyAccentDeep else MembyAccent val barVisible = focused || active Box( Modifier @@ -579,20 +638,38 @@ private fun GenreRailItem( } .padding(start = 15.dp, end = 12.dp, top = 11.dp, bottom = 11.dp), ) { - Text( - category.label, - color = when { - focused -> MembyAccentInk - active -> Color.White - else -> MembyMutedText - }, - fontSize = 15.sp, - fontWeight = if (focused || active) FontWeight.Bold else FontWeight.Medium, - // Wrapped rather than ellipsised: "Family & Animation" is a real entry and - // a rail that hid half of its own labels would be unreadable at distance. - maxLines = 2, - lineHeight = 18.sp, - ) + val ink = when { + focused -> MembyAccentInk + active -> Color.White + else -> MembyMutedText + } + Row(verticalAlignment = Alignment.CenterVertically) { + Text( + category.label, + color = ink, + modifier = Modifier.weight(1f, fill = false), + fontSize = 15.sp, + fontWeight = if (focused || active) FontWeight.Bold else FontWeight.Medium, + // Wrapped rather than ellipsised: "Family & Animation" is a real entry + // and a rail that hid half of its own labels would be unreadable at + // distance. + maxLines = 2, + lineHeight = 18.sp, + ) + // The whole-catalogue entry is the one row a viewer has no genre name to + // recognise, so it is the one that has to say it leads somewhere. It is a + // mark on the row rather than a second style of row: everything else about + // it — the plate, the marker, the ink — is what every other genre wears. + if (category.id == ALL_MEDIA_CATEGORY_ID) { + Spacer(Modifier.width(6.dp)) + Icon( + MembyIcon.ChevronRight.mark, + contentDescription = null, + tint = ink.copy(alpha = if (focused || active) 0.85f else 0.6f), + modifier = Modifier.size(16.dp), + ) + } + } } } } diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt b/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt index dd31d8e..34a0a1c 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchScreen.kt @@ -153,8 +153,6 @@ fun SearchScreen( discoveryItems: List, returnFocusItemId: String?, returnFocusRequester: FocusRequester, - initialQuery: String? = null, - onInitialQueryConsumed: () -> Unit = {}, onSearchStarted: () -> Unit = {}, onItemFocused: (BaseItem) -> Unit, onItemSelected: (BaseItem) -> Unit, @@ -169,12 +167,6 @@ fun SearchScreen( val state by viewModel.state.collectAsStateWithLifecycle() LaunchedEffect(discoveryItems) { viewModel.setDiscoveryItems(discoveryItems) } - LaunchedEffect(initialQuery) { - initialQuery?.takeIf { it.isNotBlank() }?.let { - viewModel.onQueryChanged(it) - onInitialQueryConsumed() - } - } val resultsEntry = remember { FocusRequester() } // The rail and the screen agree on one entry target: Search opens on the keyboard, @@ -199,8 +191,7 @@ fun SearchScreen( } val hasResultsTarget = when { state.errorMessage != null && state.results.isEmpty() -> true - state.isDiscovery -> discoveryItems.isNotEmpty() || - state.suggestions.any { it.kind == SearchSuggestion.Kind.GENRE } + state.isDiscovery -> discoveryItems.isNotEmpty() || state.genreSuggestions.isNotEmpty() else -> state.results.isNotEmpty() || state.requestCandidates.isNotEmpty() || (state.requestsAvailable && shouldSearch(state.query)) } @@ -310,7 +301,6 @@ fun SearchScreen( onRetry = viewModel::retry, onRequest = viewModel::request, onShowRequests = viewModel::showRequests, - onSuggestionSelected = viewModel::onQueryChanged, onGenreSelected = viewModel::onGenreSelected, onBackFromGenre = closeGenre, onLoadMore = viewModel::loadMore, @@ -814,7 +804,6 @@ private fun ResultsPane( onRetry: () -> Unit, onRequest: (GatewayRequestCandidate) -> Unit, onShowRequests: () -> Unit, - onSuggestionSelected: (String) -> Unit, onGenreSelected: (String) -> Unit, onBackFromGenre: () -> Unit, onLoadMore: () -> Unit, @@ -843,37 +832,19 @@ private fun ResultsPane( requestFocusRequester = resultsEntry, keyboardReturn = keyboardReturn, ) - val genres = state.suggestions.filter { it.kind == SearchSuggestion.Kind.GENRE } - val recent = state.suggestions.filter { it.kind == SearchSuggestion.Kind.RECENT } + val genres = state.genreSuggestions if (showingDiscovery && genres.isNotEmpty()) { Spacer(Modifier.height(8.dp)) GenreTiles( genres = genres, resultsEntry = resultsEntry, keyboardReturn = keyboardReturn, - // Not onSuggestionSelected: a genre opens the shelf of titles that are - // in it, where running its name through search matched a film called - // Drama and missed most of the drama. + // A genre opens the shelf of titles that are in it: running its name + // through search matched a film called Drama and missed most of the + // drama. onSelected = onGenreSelected, ) } - if (showingDiscovery && recent.isNotEmpty()) { - Spacer(Modifier.height(8.dp)) - Text( - "RECENT SEARCHES", - color = Muted, - fontSize = 11.sp, - fontWeight = FontWeight.Bold, - letterSpacing = 0.8.sp, - ) - Spacer(Modifier.height(4.dp)) - SuggestionChips( - suggestions = recent, - resultsEntry = null, - keyboardReturn = keyboardReturn, - onSelected = onSuggestionSelected, - ) - } Spacer(Modifier.height(12.dp)) when { @@ -1391,18 +1362,18 @@ private fun genreIcon(label: String): ImageVector = when { @Composable private fun GenreTiles( - genres: List, + genres: List, resultsEntry: FocusRequester, keyboardReturn: FocusRequester, onSelected: (String) -> Unit, ) { LazyRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - rowItemsIndexed(genres.take(6), key = { _, item -> item.label }) { index, genre -> + rowItemsIndexed(genres.take(6), key = { _, item -> item }) { index, genre -> val color = GenreColors[index % GenreColors.size] FocusScaleContainer( onFocused = {}, - onClick = { onSelected(genre.label) }, - contentDescription = "Search the ${genre.label} genre", + onClick = { onSelected(genre) }, + contentDescription = "Search the $genre genre", modifier = Modifier .width(126.dp) .height(72.dp) @@ -1416,7 +1387,7 @@ private fun GenreTiles( .background(if (focused) color.copy(alpha = 0.95f) else color), ) { Icon( - genreIcon(genre.label), + genreIcon(genre), contentDescription = null, tint = Color.White.copy(alpha = 0.9f), modifier = Modifier @@ -1425,7 +1396,7 @@ private fun GenreTiles( .size(30.dp), ) Text( - genre.label, + genre, color = Color.White, fontSize = 14.sp, fontWeight = FontWeight.Bold, @@ -1439,42 +1410,6 @@ private fun GenreTiles( } } -@Composable -private fun SuggestionChips( - suggestions: List, - resultsEntry: FocusRequester?, - keyboardReturn: FocusRequester, - onSelected: (String) -> Unit, -) { - Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - suggestions.take(6).forEachIndexed { index, suggestion -> - FocusScaleContainer( - onFocused = {}, - onClick = { onSelected(suggestion.label) }, - contentDescription = when (suggestion.kind) { - SearchSuggestion.Kind.RECENT -> "Search again for ${suggestion.label}" - SearchSuggestion.Kind.GENRE -> "Search the ${suggestion.label} genre" - }, - modifier = Modifier - .clip(RoundedCornerShape(999.dp)) - .then(if (index == 0 && resultsEntry != null) Modifier.focusRequester(resultsEntry) else Modifier) - .focusProperties { if (index == 0) left = keyboardReturn }, - ) { focused -> - Text( - suggestion.label.uppercase(), - color = if (focused) KeyLabelFocused else KeyLabel, - fontSize = 11.sp, - fontWeight = FontWeight.Bold, - maxLines = 1, - modifier = Modifier - .background(if (focused) KeyFocused else KeyIdle) - .padding(horizontal = 12.dp, vertical = 7.dp), - ) - } - } - } -} - @Composable private fun SearchEmptyMessage(state: SearchUiState, showingDiscovery: Boolean) { val message = when { diff --git a/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchViewModel.kt b/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchViewModel.kt index 394228b..5a0926c 100644 --- a/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchViewModel.kt +++ b/app/src/main/java/com/ponzischeme89/memby/ui/search/SearchViewModel.kt @@ -23,15 +23,6 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch -/** - * Something to offer when there is nothing to search for yet. Kept deliberately cheap: - * both kinds are built from data the app already has in memory, because an empty search - * box is not worth a server request. - */ -data class SearchSuggestion(val label: String, val kind: Kind) { - enum class Kind { RECENT, GENRE } -} - data class SearchUiState( val query: String = "", val results: List = emptyList(), @@ -59,7 +50,12 @@ data class SearchUiState( val genreOffset: Int = 0, /** There is more of this genre to ask for. See [hasMoreGenreItems]. */ val canLoadMore: Boolean = false, - val suggestions: List = emptyList(), + /** + * Genres to offer when there is nothing to search for yet. Kept deliberately cheap: + * they are built from data the app already has in memory, because an empty search box + * is not worth a server request. + */ + val genreSuggestions: List = emptyList(), val isLoading: Boolean = false, /** True once a query has run to completion, so "no matches" is distinguishable from "not yet". */ val hasSearched: Boolean = false, @@ -110,7 +106,6 @@ class SearchViewModel(private val repository: EmbyRepository) : ViewModel() { ): Boolean = size > CACHE_ENTRIES } - private val recentQueries = ArrayDeque() private var genreSuggestions: List = emptyList() /** @@ -132,15 +127,6 @@ class SearchViewModel(private val repository: EmbyRepository) : ViewModel() { // same terms while keeping the repository's plain suspend signature. .collectLatest { term -> runSearch(term) } } - viewModelScope.launch { - repository.getRecentSearches().forEach { term -> - if (recentQueries.none { it.equals(term, ignoreCase = true) }) { - recentQueries.addLast(term) - } - } - while (recentQueries.size > MAX_RECENT_QUERIES) recentQueries.removeLast() - refreshSuggestions() - } refreshSuggestions() } @@ -460,7 +446,6 @@ class SearchViewModel(private val repository: EmbyRepository) : ViewModel() { rankSearchResults(term, items) } cache[term] = ranked - rememberQuery(term) viewModelScope.launch { repository.recordSearch(term) } _state.update { it.copy(results = ranked, isLoading = false, hasSearched = true, errorMessage = null) @@ -502,24 +487,14 @@ class SearchViewModel(private val repository: EmbyRepository) : ViewModel() { } } - private fun rememberQuery(term: String) { - recentQueries.removeAll { it.equals(term, ignoreCase = true) } - recentQueries.addFirst(term) - while (recentQueries.size > MAX_RECENT_QUERIES) recentQueries.removeLast() - refreshSuggestions() - } - private fun refreshSuggestions() { - val suggestions = recentQueries.map { SearchSuggestion(it, SearchSuggestion.Kind.RECENT) } + - genreSuggestions.map { SearchSuggestion(it, SearchSuggestion.Kind.GENRE) } - _state.update { it.copy(suggestions = suggestions) } + _state.update { it.copy(genreSuggestions = genreSuggestions) } } companion object { const val DEBOUNCE_MS = 250L const val MIN_QUERY_LENGTH = 2 private const val CACHE_ENTRIES = 24 - private const val MAX_RECENT_QUERIES = 6 private const val MAX_GENRE_SUGGESTIONS = 6 } } diff --git a/app/src/test/java/com/ponzischeme89/memby/data/GatewayPayloadTest.kt b/app/src/test/java/com/ponzischeme89/memby/data/GatewayPayloadTest.kt index 7b3aa11..ecd26c1 100644 --- a/app/src/test/java/com/ponzischeme89/memby/data/GatewayPayloadTest.kt +++ b/app/src/test/java/com/ponzischeme89/memby/data/GatewayPayloadTest.kt @@ -7,7 +7,6 @@ import com.ponzischeme89.memby.data.model.GatewayNextEpisode import com.ponzischeme89.memby.data.model.GatewayMovieRatings import com.ponzischeme89.memby.data.model.GatewayPlayback import com.ponzischeme89.memby.data.model.GatewayPlaybackReportResponse -import com.ponzischeme89.memby.data.model.GatewaySearchHistory import com.ponzischeme89.memby.data.model.GatewayPreferences import com.ponzischeme89.memby.data.model.GatewayServiceStatus import com.ponzischeme89.memby.data.model.GatewayIntro @@ -497,15 +496,6 @@ class GatewayPayloadTest { assertTrue(home.partial) } - @Test - fun `decodes persisted recent searches`() { - val history = json.decodeFromString( - """{"queries":["severance","slow horses","arrival"]}""", - ) - - assertEquals(listOf("severance", "slow horses", "arrival"), history.queries) - } - @Test fun `decodes a playback response`() { val playback = json.decodeFromString( diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/ServerHomeRowsTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/ServerHomeRowsTest.kt index c828c6b..af69785 100644 --- a/app/src/test/java/com/ponzischeme89/memby/ui/ServerHomeRowsTest.kt +++ b/app/src/test/java/com/ponzischeme89/memby/ui/ServerHomeRowsTest.kt @@ -196,7 +196,7 @@ class ServerHomeRowsTest { } @Test - fun `shows destination includes empty favorites and ranked server shelves`() { + fun `shows destination drops empty shelves and keeps ranked server ones`() { val rankedRows = listOf( row("curated:comedy-shows", "shows", "c1"), row("curated:horror-shows", "shows", "h1"), @@ -210,17 +210,45 @@ class ServerHomeRowsTest { val rows = homeRowsFor(BrowseDestination.SHOWS, state, Settings()) + // Nothing part-way through and nothing favourited, so neither fixed shelf is a + // heading worth printing: the D-pad already refuses to enter an empty one. assertEquals( listOf( - "continue-shows", - "favourite-shows", "curated:comedy-shows", "curated:horror-shows", "curated:drama-shows", ), rows.map { it.id }, ) - assertTrue(rows.first { it.id == "favourite-shows" }.items.isEmpty()) + } + + @Test + fun `shows destination keeps its fixed shelves once they have something in them`() { + val state = HomeUiState( + rows = serverRows, + continueWatching = listOf(BaseItem(id = "ep", type = "Episode")), + favorites = listOf(BaseItem(id = "series", type = "Series")), + loading = emptySet(), + ) + + val rows = homeRowsFor(BrowseDestination.SHOWS, state, Settings()) + + assertEquals(listOf("continue-shows", "favourite-shows"), rows.map { it.id }) + } + + @Test + fun `a shows shelf that is still loading is kept`() { + val state = HomeUiState( + rows = serverRows, + favorites = emptyList(), + loading = setOf(HomeSection.FAVORITES), + ) + + val rows = homeRowsFor(BrowseDestination.SHOWS, state, Settings()) + + // Withdrawing it would move every shelf below it a moment later, when the + // favourites land. + assertTrue(rows.any { it.id == "favourite-shows" }) } @Test diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/UserQuickActionsScreenshotTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/UserQuickActionsScreenshotTest.kt new file mode 100644 index 0000000..566bbaf --- /dev/null +++ b/app/src/test/java/com/ponzischeme89/memby/ui/UserQuickActionsScreenshotTest.kt @@ -0,0 +1,75 @@ +package com.ponzischeme89.memby.ui + +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onRoot +import com.github.takahirom.roborazzi.captureRoboImage +import com.ponzischeme89.memby.ui.theme.MembyTheme +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** + * The shortcut menu a hold on the rail's user item raises, into + * `build/screenshots/user-shortcuts/`. + * + * ```powershell + * .\gradlew.bat :app:testDebugUnitTest --tests "*UserQuickActionsScreenshotTest" + * ``` + * + * The claim this feature makes is that the menu is *small and immediate* — the alternative + * to walking into a page — and that the unavailable row reads as deliberately unavailable + * rather than as one that failed to load. Neither is a thing a unit test can check, so the + * empty case is the capture worth keeping. + */ +@RunWith(RobolectricTestRunner::class) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +@Config(sdk = [34], qualifiers = "w960dp-h540dp-television-xhdpi") +class UserQuickActionsScreenshotTest { + + @get:Rule + val compose = createComposeRule() + + @Test + fun `alerts waiting`() { + capture("user-shortcuts-populated", alertCount = 7) + } + + /** One, which is the row's singular wording. */ + @Test + fun `a single alert`() { + capture("user-shortcuts-single", alertCount = 1) + } + + /** + * Nothing waiting. The row is drawn quiet and unpressable — the capture is the check + * that it reads that way beside a Cancel that is not. + */ + @Test + fun `nothing to clear`() { + capture("user-shortcuts-empty", alertCount = 0) + } + + /** A clear already in flight: both rows drawn, neither of them offering a second press. */ + @Test + fun `already clearing`() { + capture("user-shortcuts-busy", alertCount = 7, busy = true) + } + + private fun capture(name: String, alertCount: Int, busy: Boolean = false) { + compose.setContent { + MembyTheme { + UserQuickActionsOverlay( + username = "Matt", + alertCount = alertCount, + busy = busy, + onClearNotifications = {}, + onDismiss = {}, + ) + } + } + compose.onRoot().captureRoboImage("build/screenshots/user-shortcuts/$name.png") + } +} diff --git a/app/src/test/java/com/ponzischeme89/memby/ui/UserQuickActionsTest.kt b/app/src/test/java/com/ponzischeme89/memby/ui/UserQuickActionsTest.kt new file mode 100644 index 0000000..1afbbf5 --- /dev/null +++ b/app/src/test/java/com/ponzischeme89/memby/ui/UserQuickActionsTest.kt @@ -0,0 +1,62 @@ +package com.ponzischeme89.memby.ui + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The shortcut menu's wording and its one real rule: nothing waiting means nothing to press. + * + * A menu offering "Clear notifications" over an empty list would send a request the gateway + * can only answer with zero, and would tell somebody it had done something it had not. + */ +class UserQuickActionsTest { + + @Test + fun `nothing waiting disables the clear action and says so`() { + val actions = userQuickActions(alertCount = 0) + val clear = actions.first { it.kind == UserQuickActionKind.CLEAR_NOTIFICATIONS } + assertFalse(clear.enabled) + assertEquals("No notifications to clear", clear.label) + // Still offered rather than removed: a menu that changes shape with the news is one + // nobody can learn, and Cancel must always be there to leave by. + assertEquals(2, actions.size) + assertTrue(actions.last().enabled) + assertEquals(UserQuickActionKind.CANCEL, actions.last().kind) + } + + @Test + fun `the count is named exactly, past the badge's cap`() { + assertEquals("Clear 1 notification", labelFor(1)) + assertEquals("Clear 7 notifications", labelFor(7)) + // The badge says "9+" because its pill is narrow; this row has the width to be exact, + // and a viewer deciding whether to clear wants the real figure. + assertEquals("Clear 42 notifications", labelFor(42)) + } + + @Test + fun `a clear already in flight cannot be started twice`() { + val clear = userQuickActions(alertCount = 5, busy = true) + .first { it.kind == UserQuickActionKind.CLEAR_NOTIFICATIONS } + assertFalse(clear.enabled) + // The wording still names what is waiting: the row is unavailable for a moment, not + // a different row. + assertEquals("Clear 5 notifications", clear.label) + } + + /** + * The confirmation reports the gateway's count, so zero is a real answer — another set + * in the house emptying the list first — and reads as such rather than as a failure. + */ + @Test + fun `the confirmation reports what actually happened`() { + assertEquals("Nothing left to clear", clearedNotificationsMessage(0)) + assertEquals("1 notification cleared", clearedNotificationsMessage(1)) + assertEquals("6 notifications cleared", clearedNotificationsMessage(6)) + assertEquals("Nothing left to clear", clearedNotificationsMessage(-3)) + } + + private fun labelFor(count: Int) = userQuickActions(count) + .first { it.kind == UserQuickActionKind.CLEAR_NOTIFICATIONS }.label +} diff --git a/server/internal/api/api.go b/server/internal/api/api.go index 361d71e..5f705ee 100644 --- a/server/internal/api/api.go +++ b/server/internal/api/api.go @@ -272,7 +272,6 @@ func (s *Server) Routes() http.Handler { // treats it as one. Paged, because a household's Drama shelf is not a screenful. v1.Handle("GET /v1/genres/{genre}/items", s.authed(s.handleGenreItems)) v1.Handle("GET /v1/library/items", s.authed(s.handleLibraryItems)) - v1.Handle("GET /v1/search/history", s.authed(s.handleRecentSearches)) v1.Handle("POST /v1/search/history", s.authed(s.handleSearchHistory)) v1.Handle("GET /v1/requests/lookup", s.authed(s.handleRequestLookup)) v1.Handle("GET /v1/requests", s.authed(s.handleMyRequests)) @@ -291,6 +290,9 @@ func (s *Server) Routes() http.Handler { v1.Handle("DELETE /v1/my-shows/{id}", s.authed(s.handleMyShow)) v1.Handle("GET /v1/notifications", s.authed(s.handleNotifications)) v1.Handle("PUT /v1/notifications", s.authed(s.handleNotifications)) + // Ahead of the per-alert route: three path segments rather than four, so the two never + // compete, and a shortcut that clears the lot needs one request and one log line. + v1.Handle("POST /v1/notifications/clear", s.authed(s.handleClearNotifications)) v1.Handle("POST /v1/notifications/{id}/{action}", s.authed(s.handleNotificationAction)) v1.Handle("GET /v1/features", s.authed(s.handleFeatures)) // A viewer's settings follow the person, not the television. Both verbs land on one diff --git a/server/internal/api/api_test.go b/server/internal/api/api_test.go index 29b3b19..a965f92 100644 --- a/server/internal/api/api_test.go +++ b/server/internal/api/api_test.go @@ -298,21 +298,6 @@ func TestHomeResponseEncodesEmptyRowsAsArrays(t *testing.T) { } } -func TestSearchHistoryResponseEncodesEmptyQueriesAsArray(t *testing.T) { - resp := searchHistoryResponse{Queries: []string{}} - body, err := json.Marshal(resp) - if err != nil { - t.Fatalf("marshal: %v", err) - } - var decoded map[string]any - if err := json.Unmarshal(body, &decoded); err != nil { - t.Fatalf("unmarshal: %v", err) - } - if _, ok := decoded["queries"].([]any); !ok { - t.Fatalf("queries encoded as %T, want array", decoded["queries"]) - } -} - // Both routes that write search_history apply one rule, so a query /v1/search records is // exactly one /v1/search/history would have accepted. The length is counted in runes: // bytes would reject a Japanese title at a third of an English one's length. diff --git a/server/internal/api/home.go b/server/internal/api/home.go index a4ce09d..50e2467 100644 --- a/server/internal/api/home.go +++ b/server/internal/api/home.go @@ -664,37 +664,6 @@ type searchHistoryRequest struct { Query string `json:"query"` } -type searchHistoryResponse struct { - Queries []string `json:"queries"` -} - -const ( - recentSearchDays = 30 - recentSearchLimit = 10 -) - -func (s *Server) handleRecentSearches(w http.ResponseWriter, r *http.Request, sess store.Session) { - if s.store == nil { - writeError(w, http.StatusInternalServerError, "could not load recent searches") - return - } - since := time.Now().Add(-recentSearchDays * 24 * time.Hour) - queries, err := s.store.RecentSearches( - r.Context(), - sess.EmbyUserID, - since, - recentSearchLimit, - ) - if err != nil { - writeError(w, http.StatusInternalServerError, "could not load recent searches") - return - } - if queries == nil { - queries = []string{} - } - writeJSON(w, http.StatusOK, searchHistoryResponse{Queries: queries}) -} - func (s *Server) handleSearchHistory(w http.ResponseWriter, r *http.Request, sess store.Session) { var req searchHistoryRequest if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 2<<10)).Decode(&req); err != nil { diff --git a/server/internal/api/my_shows.go b/server/internal/api/my_shows.go index 36b795c..34b3770 100644 --- a/server/internal/api/my_shows.go +++ b/server/internal/api/my_shows.go @@ -229,6 +229,72 @@ func (s *Server) syncReturnNotifications( } } +// clearNotificationsResponse says how many of this viewer's notifications the gateway +// actually cleared. The television prints the figure back as its confirmation, so it must be +// what happened rather than what was asked for. +type clearNotificationsResponse struct { + Cleared int `json:"cleared"` +} + +// handleClearNotifications empties one viewer's list in a single request. +// +// It exists because clearing from the user picker is a shortcut for somebody who does not +// want to go into the page at all, and a television looping the per-alert dismiss route +// could neither report a trustworthy count nor leave one line in the log an operator could +// read. The two rules worth preserving: +// +// - What it clears is what that viewer can *see*. filterStoredNotifications is what the +// list route already applies, so a summary their preferences have withdrawn is not +// quietly dismissed underneath them by a press aimed at the seven alerts on screen — +// and the count agrees with the badge that was showing. +// - Nothing to clear is a success, not an error. It answers 0 and says so, because the +// television disables the action on an empty list and a race with another set finishing +// the job first is not a failure anybody should be shown. +// +// clearableNotificationIDs is the rows a clear-all press may take: exactly the ones the +// list route would have shown this viewer, and nothing their preferences have withdrawn. +// +// Pure and separate from the handler so the one rule that matters here — a press aimed at +// what is on screen never reaches past it — is pinned by a test rather than by a database. +func clearableNotificationIDs( + notifications []store.UserNotification, prefs store.NotificationPreferences, +) []int64 { + visible := filterStoredNotifications(notifications, prefs) + ids := make([]int64, 0, len(visible)) + for _, notification := range visible { + ids = append(ids, notification.ID) + } + return ids +} + +func (s *Server) handleClearNotifications( + w http.ResponseWriter, r *http.Request, sess store.Session, +) { + log := s.loggerFor(r.Context()) + prefs, err := s.notificationPreferencesFor(r.Context(), sess.EmbyUserID) + if err != nil { + log.Warn("notifications not cleared", "reason", "preferences unavailable", "error", err) + writeError(w, http.StatusInternalServerError, "could not load notification preferences") + return + } + notifications, err := s.store.UserNotifications(r.Context(), sess.EmbyUserID) + if err != nil { + log.Warn("notifications not cleared", "reason", "list unavailable", "error", err) + writeError(w, http.StatusInternalServerError, "could not load notifications") + return + } + ids := clearableNotificationIDs(notifications, prefs) + cleared, err := s.store.DismissNotifications(r.Context(), sess.EmbyUserID, ids) + if err != nil { + log.Warn("notifications not cleared", "reason", "write failed", + "requested", len(ids), "error", err) + writeError(w, http.StatusInternalServerError, "could not clear notifications") + return + } + log.Info("notifications cleared", "cleared", cleared, "source", "user-switcher") + writeJSON(w, http.StatusOK, clearNotificationsResponse{Cleared: cleared}) +} + func (s *Server) handleNotificationAction( w http.ResponseWriter, r *http.Request, sess store.Session, ) { diff --git a/server/internal/api/notification_preferences_test.go b/server/internal/api/notification_preferences_test.go index c1a65ae..935a90c 100644 --- a/server/internal/api/notification_preferences_test.go +++ b/server/internal/api/notification_preferences_test.go @@ -55,3 +55,29 @@ func TestUpdatePreferenceNeverSuppressesMandatoryUpdate(t *testing.T) { t.Fatalf("optional update was not suppressed: %#v", got) } } + +// A clear-all press aimed at the alerts on screen must never reach past them. The one case +// that can differ is a kind the viewer's own preferences have withdrawn: it is still a row +// in the table, it is not in their list, and clearing it would be this shortcut deciding +// something the viewer never saw. +func TestClearableNotificationIDsHonourPreferences(t *testing.T) { + notifications := []store.UserNotification{ + {ID: 1, Kind: "show-return"}, + {ID: 2, Kind: watchTimeWeeklyKind}, + {ID: 3, Kind: "library-added"}, + } + prefs := store.DefaultNotificationPreferences() + prefs.WatchTimeDigest = false + ids := clearableNotificationIDs(notifications, prefs) + if len(ids) != 2 || ids[0] != 1 || ids[1] != 3 { + t.Fatalf("expected the two visible rows, got %v", ids) + } + + prefs.Enabled = false + if ids := clearableNotificationIDs(notifications, prefs); len(ids) != 0 { + t.Fatalf("notifications switched off should clear nothing, got %v", ids) + } + if ids := clearableNotificationIDs(nil, store.DefaultNotificationPreferences()); len(ids) != 0 { + t.Fatalf("an empty list should clear nothing, got %v", ids) + } +} diff --git a/server/internal/store/my_shows.go b/server/internal/store/my_shows.go index fdafbb1..d8d736a 100644 --- a/server/internal/store/my_shows.go +++ b/server/internal/store/my_shows.go @@ -354,3 +354,28 @@ func (s *Store) DismissNotification(ctx context.Context, userID string, id int64 WHERE id = $1 AND emby_user_id = $2`, id, userID) return err } + +// DismissNotifications clears several of one viewer's notifications at once and reports how +// many rows it actually took. +// +// The count is the whole reason this is a route rather than the client's loop: "cleared 7" +// is what the log line and the activity record are worth reading for, and a television +// counting the requests it made would be counting what it asked for rather than what +// happened — a row somebody dismissed on another set in the meantime is one this must not +// claim. Already-dismissed rows are excluded rather than re-stamped, so a repeated press +// honestly reports nothing left to clear. +func (s *Store) DismissNotifications( + ctx context.Context, userID string, ids []int64, +) (int, error) { + if len(ids) == 0 { + return 0, nil + } + tag, err := s.pool.Exec(ctx, ` + UPDATE user_notifications SET dismissed_at = now() + WHERE emby_user_id = $1 AND id = ANY($2::bigint[]) AND dismissed_at IS NULL`, + userID, ids) + if err != nil { + return 0, fmt.Errorf("store: dismiss notifications: %w", err) + } + return int(tag.RowsAffected()), nil +} diff --git a/server/internal/store/searches.go b/server/internal/store/searches.go index c13111c..1ae02d5 100644 --- a/server/internal/store/searches.go +++ b/server/internal/store/searches.go @@ -6,10 +6,9 @@ import ( "time" ) -// Search history is the record of what a household looks for, and it has two readers with -// quite different appetites: a television asking for one viewer's last few queries, and -// the console asking what the house as a whole has been searching. Both read the one -// table, which is why the writer's rules live here beside them. +// Search history is the record of what a household looks for, read by the console as the +// summary of what the house searches for and as the uncollapsed log of what happened just +// now. The writer's rules live here beside those readers. // SearchDedupeWindow is how long an identical query counts as the same search. // @@ -27,8 +26,8 @@ const SearchRetention = 30 * 24 * time.Hour // RecordSearch stores a normalized query for future per-user ranking analysis. // -// Case-insensitive within the dedupe window, matching RecentSearches, which collapses -// case-only duplicates when it reads them back. +// Case-insensitive within the dedupe window, so a query retyped with a different +// capitalisation is not a second search. func (s *Store) RecordSearch(ctx context.Context, userID, query string) error { _, err := s.pool.Exec(ctx, `WITH inserted AS ( @@ -49,44 +48,6 @@ func (s *Store) RecordSearch(ctx context.Context, userID, query string) error { return err } -// RecentSearches returns a user's distinct queries in most-recently-used order. -// Case-only duplicates collapse to the spelling used most recently. -func (s *Store) RecentSearches( - ctx context.Context, - userID string, - since time.Time, - limit int, -) ([]string, error) { - rows, err := s.pool.Query(ctx, ` - SELECT query - FROM ( - SELECT DISTINCT ON (lower(query)) query, occurred_at - FROM search_history - WHERE emby_user_id = $1 AND occurred_at >= $2 - ORDER BY lower(query), occurred_at DESC - ) AS latest - ORDER BY occurred_at DESC - LIMIT $3`, - userID, since, limit) - if err != nil { - return nil, fmt.Errorf("store: recent searches: %w", err) - } - defer rows.Close() - - queries := make([]string, 0, limit) - for rows.Next() { - var query string - if err := rows.Scan(&query); err != nil { - return nil, fmt.Errorf("store: scan recent search: %w", err) - } - queries = append(queries, query) - } - if err := rows.Err(); err != nil { - return nil, fmt.Errorf("store: read recent searches: %w", err) - } - return queries, nil -} - // SearchTerm is one query the household searched for, aggregated across everyone. type SearchTerm struct { Query string `json:"query"` @@ -114,8 +75,8 @@ type SearchTotals struct { // SearchTerms aggregates the household's queries since a point in time, most-searched // first. Grouped case-insensitively and labelled with the spelling used most recently, -// the same rule RecentSearches applies, so one query cannot appear as two rows because -// somebody's on-screen keyboard capitalised it. +// the same rule the writer's dedupe window applies, so one query cannot appear as two +// rows because somebody's on-screen keyboard capitalised it. func (s *Store) SearchTerms(ctx context.Context, since time.Time, limit int) ([]SearchTerm, error) { rows, err := s.pool.Query(ctx, ` SELECT (array_agg(query ORDER BY occurred_at DESC))[1] AS query,