Improve TV and WebUI interaction flows#41
Conversation
📝 WalkthroughWalkthroughThe changes add playback track-state reporting, TV-focused search and navigation behavior, player retry and exit confirmation, manual-match dismissal guards, settings improvements, WebUI access gating, authenticated requests, and a default application version update. ChangesPlayback controls and track state
TV navigation and content discovery
WebUI access and source workflows
Release metadata
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Viewer
participant PlayerScreen
participant PlayerViewModel
participant PlaybackController
Viewer->>PlayerScreen: select track or press retry
PlayerScreen->>PlayerViewModel: update playback action
PlayerViewModel->>PlaybackController: select track or replay source
PlaybackController-->>PlayerViewModel: refreshed track state
PlayerViewModel-->>PlayerScreen: selected indices
PlayerScreen-->>Viewer: updated controls
sequenceDiagram
participant App
participant API
participant Server
App->>API: initialize access
API->>Server: request application data
Server-->>API: response or HTTP 401
API-->>App: ready state or unauthorized event
App-->>Viewer: show application or token gate
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-control/frontend/src/App.vue (1)
1941-1952: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
loadSources()should not rethrow from floating callers.
watch(activeView)andvoid Promise.all([loadCloudDriveAutomation(), loadLibrary(), loadSources()])don't observe the rejection, so a failed/api/sourcescan still become an unhandled promise rejection. Either stop rethrowing inloadSources()or addcatch/awaitat those call sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-control/frontend/src/App.vue` around lines 1941 - 1952, Update loadSources() to handle the /api/sources failure without rethrowing, since its callers watch(activeView) and the floating Promise.all invocation do not observe rejection; preserve setting sourcesLoadFailed and resetting loading.sources in the existing catch/finally flow.
🧹 Nitpick comments (1)
ui-tv/src/main/kotlin/com/miruplay/tv/ui/library/LibraryScreen.kt (1)
75-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
rememberSaveableforsearchQueryto survive process death.
searchQueryusesrememberinstead ofrememberSaveable, so the typed query is lost if the system kills the app while the user is mid-search. On a TV where text entry is slow with a remote, this could be frustrating.searchActiveandrestoreSearchFocusare transient control states and are fine withremember.♻️ Optional refactor
- var searchQuery by remember { mutableStateOf("") } + var searchQuery by rememberSaveable { mutableStateOf("") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui-tv/src/main/kotlin/com/miruplay/tv/ui/library/LibraryScreen.kt` around lines 75 - 77, Change the searchQuery state declaration in LibraryScreen from remember to rememberSaveable so the typed query is restored after process death. Leave searchActive and restoreSearchFocus using remember because they remain transient control state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@player-core/src/main/kotlin/com/miruplay/tv/player/ExoPlaybackController.kt`:
- Line 904: Update the subtitle and audio track mapping logic in
ExoPlaybackController to use each Tracks.Group’s isSelected state instead of
checking isTrackSelected(0). Ensure selectedSubtitleTrackIndex and the
corresponding audio index are set whenever the group is selected, regardless of
which track within the group ExoPlayer chose.
In `@ui-tv/src/main/kotlin/com/miruplay/tv/ui/library/LibraryScreen.kt`:
- Around line 244-250: Update the Down-key handler in LibraryHeader so it
requests contentFocusRequester and consumes the event only when contentState is
non-null, preserving the existing behavior otherwise while allowing the event to
propagate when no library content is attached.
---
Outside diff comments:
In `@web-control/frontend/src/App.vue`:
- Around line 1941-1952: Update loadSources() to handle the /api/sources failure
without rethrowing, since its callers watch(activeView) and the floating
Promise.all invocation do not observe rejection; preserve setting
sourcesLoadFailed and resetting loading.sources in the existing catch/finally
flow.
---
Nitpick comments:
In `@ui-tv/src/main/kotlin/com/miruplay/tv/ui/library/LibraryScreen.kt`:
- Around line 75-77: Change the searchQuery state declaration in LibraryScreen
from remember to rememberSaveable so the typed query is restored after process
death. Leave searchActive and restoreSearchFocus using remember because they
remain transient control state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14cfed90-bcd9-431e-a7b5-239fa2409ca0
📒 Files selected for processing (21)
app/build.gradle.ktscore/model/src/main/kotlin/com/miruplay/tv/model/PlaybackUiConventions.ktplayer-core/src/main/kotlin/com/miruplay/tv/player/ExoPlaybackController.ktplayer-core/src/main/kotlin/com/miruplay/tv/player/PlaybackController.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/components/AnimeCards.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/components/InitialFocus.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/components/TvKeyEvents.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/components/TvTextField.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/detail/AnimeDetailScreen.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/detail/DramaDetailScreen.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/library/LibraryScreen.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlayerScreen.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/player/PlayerViewModel.ktui-tv/src/main/kotlin/com/miruplay/tv/ui/settings/AddSourceScreen.ktui-tv/src/test/kotlin/com/miruplay/tv/ui/library/LibrarySearchTest.ktui-tv/src/test/kotlin/com/miruplay/tv/ui/player/PlayerExitConfirmationTest.ktui-tv/src/test/kotlin/com/miruplay/tv/ui/player/PlayerOptionsPanelFocusTest.ktui-tv/src/test/kotlin/com/miruplay/tv/ui/player/PlayerScreenPictureMenuLabelsTest.ktweb-control/frontend/src/App.vueweb-control/frontend/src/api.jsweb-control/frontend/src/styles.css
| format = SubtitleFormat.SRT, | ||
| ), | ||
| ) | ||
| if (group.isTrackSelected(0)) selectedSubtitleTrackIndex = trackIndex |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'androidx\.media3|media3-(common|exoplayer)|media3Version' \
--glob '*.gradle' --glob '*.gradle.kts' --glob 'libs.versions.toml' .
rg -n 'isTrackSelected|\.isSelected' player-core ui-tv --glob '*.kt'Repository: ModerRAS/MiruPlay
Length of output: 1589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Media3 version ==="
sed -n '1,90p' gradle/libs.versions.toml | nl -ba | sed -n '1,90p'
echo
echo "=== ExoPlaybackController selection mapping ==="
sed -n '880,930p' player-core/src/main/kotlin/com/miruplay/tv/player/ExoPlaybackController.kt | nl -ba
echo
echo "=== Any other uses of group selection APIs ==="
rg -n 'isTrackSelected\(|group\.isSelected|selectedSubtitleTrackIndex|selectedAudioTrackIndex' player-core ui-tv --glob '*.kt'Repository: ModerRAS/MiruPlay
Length of output: 218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== gradle/libs.versions.toml media3 entries ==="
sed -n '1,80p' gradle/libs.versions.toml | awk 'NR>=1 && NR<=80 {print NR ":" $0}' | grep -n 'media3'
echo
echo "=== ExoPlaybackController relevant lines ==="
sed -n '896,922p' player-core/src/main/kotlin/com/miruplay/tv/player/ExoPlaybackController.kt | awk '{print NR ":" $0}'
echo
echo "=== Local usages of group selection APIs ==="
rg -n 'isTrackSelected\(|group\.isSelected|selectedSubtitleTrackIndex|selectedAudioTrackIndex' player-core ui-tv --glob '*.kt'Repository: ModerRAS/MiruPlay
Length of output: 7590
🌐 Web query:
androidx.media3 Tracks.Group isSelected API docs version media3-exoplayer media3-common
💡 Result:
In the androidx.media3.common library, the Tracks.Group class provides information about a single group of tracks [1][2]. The isSelected API is used to determine the selection status of tracks within that group [3][4]. There are two primary methods related to selection in this class: 1. isSelected: This method returns a boolean indicating whether at least one track in the group is currently selected for playback [1][2]. 2. isTrackSelected(int trackIndex): This method returns a boolean indicating whether a specific track, identified by its index within the group, is selected for playback [1][2]. Important context regarding track selection: - Multiple tracks in a single group can be selected simultaneously [1]. This is common in adaptive streaming scenarios where the player selects multiple tracks of varying quality to switch between based on network conditions [1][3]. - A track is considered selected if it has been chosen for playback based on the current TrackSelectionParameters [3][4]. - While this class identifies which tracks are selected, it does not explicitly indicate which of the selected tracks is currently active (the one actually being played) [1]. Some player implementations, such as ExoPlayer, provide mechanisms to retrieve this, for example via ExoTrackSelection.getSelectedFormat [1].
Citations:
- 1: https://developer.android.google.cn/reference/androidx/media3/common/Tracks.Group
- 2: https://developer.android.google.cn/reference/kotlin/androidx/media3/common/Tracks.Group
- 3: https://developer.android.com/media/media3/exoplayer/track-selection
- 4: https://developer.android.google.cn/media/media3/exoplayer/track-selection
Use the group’s selected state here. Tracks.Group exposes isSelected, and checking only track 0 can leave the UI index unset when ExoPlayer selects a different track in the same group. Use group.isSelected for both subtitle and audio mapping.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@player-core/src/main/kotlin/com/miruplay/tv/player/ExoPlaybackController.kt`
at line 904, Update the subtitle and audio track mapping logic in
ExoPlaybackController to use each Tracks.Group’s isSelected state instead of
checking isTrackSelected(0). Ensure selectedSubtitleTrackIndex and the
corresponding audio index are set whenever the group is selected, regardless of
which track within the group ExoPlayer chose.
| .onPreviewKeyEvent { event -> | ||
| if (event.type == KeyEventType.KeyDown && event.key == Key.DirectionDown) { | ||
| contentFocusRequester.requestFocus() | ||
| true | ||
| } else { | ||
| false | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify FocusRequester.requestFocus() behavior when not attached to a composable.
# Check the Compose UI version declared in the project.
rg -n "compose.*ui" app/build.gradle.kts ui-tv/build.gradle.kts 2>/dev/null || fd -t f "build.gradle.kts" --exec rg -l "compose" {} \;Repository: ModerRAS/MiruPlay
Length of output: 306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant screen and focus wiring.
file="ui-tv/src/main/kotlin/com/miruplay/tv/ui/library/LibraryScreen.kt"
wc -l "$file"
sed -n '100,320p' "$file"
# Find where the content focus requester is attached.
rg -n "initialContentFocus|contentFocusRequester|requestFocus\\(" ui-tv/src/main/kotlin/com/miruplay/tv/ui/library -n
# Check the Compose version pins in the version catalog / build files.
fd -t f "libs.versions.toml" . -x sh -c 'echo "== {} =="; rg -n "compose|focus" "{}"' \;
fd -t f "build.gradle.kts" . -x sh -c 'echo "== {} =="; rg -n "compose|focus" "{}"' \;Repository: ModerRAS/MiruPlay
Length of output: 13453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the focus handle implementation and the content attachment site.
rg -n "rememberInitialFocusHandle|class InitialFocusHandle|fun requester|fun modifier|FocusRequester" ui-tv/src/main/kotlin -n
sed -n '360,460p' ui-tv/src/main/kotlin/com/miruplay/tv/ui/library/LibraryScreen.kt
# Look for any other use of requestFocus() on potentially unattached requesters.
rg -n "requestFocus\\(" ui-tv/src/main/kotlin -nRepository: ModerRAS/MiruPlay
Length of output: 13630
🌐 Web query:
Jetpack Compose FocusRequester.requestFocus unattached IllegalStateException documentation
💡 Result:
The IllegalStateException related to FocusRequester in Jetpack Compose, typically manifesting as "FocusRequester is not initialized," occurs when you call requestFocus on a FocusRequester instance before it has been successfully associated with a composable modifier in the UI tree [1][2][3]. Common Causes and Solutions: 1. Modifiers Not Yet Attached: This is the most frequent cause, often occurring when requestFocus is called too early—such as immediately upon entering a composition or before a parent layout (like Scaffold or a SubcomposeLayout) has completed its own layout phase and attached its children [1][2][4]. Because these layouts use subcomposition, the associated Modifier.focusRequester may not be attached when the initial effect runs [1][4]. Solution: Ensure the request happens after the UI has been composed and attached. Moving the request into a LaunchedEffect within the scope of the subcomposed container (e.g., inside the Scaffold content) often resolves this [1]. 2. Requesting Focus During Composition: You should never call requestFocus directly during the composition phase [1][2]. Solution: Focus requests must be triggered by an event or a side effect [1][5]. Use side-effect handlers like LaunchedEffect or event callbacks (e.g., in response to a user click or navigation completion) [1][6][5]. 3. Missing Modifier: The FocusRequester must be explicitly linked to a composable using the Modifier.focusRequester(focusRequester) modifier [1][7][4]. If this modifier is missing or has not been applied to a target component, the FocusRequester remains uninitialized [1][4]. Best Practices: - Always remember the FocusRequester instance using remember { FocusRequester } to maintain its state across recompositions [1][7][2]. - Ensure the Modifier.focusRequester is applied to the intended composable [6][8][9]. - If you are dealing with complex layouts or navigation, ensure the component is actually present in the hierarchy before triggering the request [1][3]. For testing scenarios, consider using appropriate delays or ensuring the test environment allows for full composition/layout cycles before the request is made [7][4].
Citations:
- 1: https://stackoverflow.com/questions/69962662/illegalstateexception-focusrequester-is-not-initialized-during-jetpack-compose
- 2: https://issuetracker.google.com/issues/206249038
- 3: JetStream crashes on home screem : Focus restoration on navigation popBackStack and focus restoration using state Modifiers android/tv-samples#148
- 4: FocusRequester is not initialized robolectric/robolectric#9703
- 5: https://developer.android.com/develop/ui/compose/touch-input/focus/change-focus-behavior
- 6: https://developer.android.com/reference/kotlin/androidx/compose/ui/focus/FocusRequesterModifier
- 7: https://stackoverflow.com/questions/79009729/how-to-solve-focusrequester-is-not-initialized
- 8: https://developer.android.com/reference/kotlin/androidx/compose/ui/focus/FocusRequesterModifierNode
- 9: https://developer.android.google.cn/reference/kotlin/androidx/compose/ui/focus/FocusRequester
Guard the Down handler when no library content exists. contentFocusRequester is only attached inside LibraryContent, so in Loading/NoSources/HasSources/ScanError states requestFocus() can target an unattached requester while still consuming the D-pad event. Gate this on contentState != null (or pass hasContent into LibraryHeader) before returning true.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ui-tv/src/main/kotlin/com/miruplay/tv/ui/library/LibraryScreen.kt` around
lines 244 - 250, Update the Down-key handler in LibraryHeader so it requests
contentFocusRequester and consumes the event only when contentState is non-null,
preserving the existing behavior otherwise while allowing the event to propagate
when no library content is attached.
1 participant
Summary
Validation
./gradlew.bat :player-core:testDebugUnitTest :ui-tv:testDebugUnitTest./gradlew.bat :app:assembleDebugnpm run buildinweb-control/frontendadb install -rand UIAutomator focus/navigation checksInteraction details
Summary by CodeRabbit