From e0772ed9b89cbf6501158d5a897894fe3541e5ae Mon Sep 17 00:00:00 2001 From: damontecres <154766448+damontecres@users.noreply.github.com> Date: Fri, 17 Oct 2025 18:26:03 -0400 Subject: [PATCH] Fixes a few UI issues (#27) Fixes: - Rounded corners on dialogs during playback - Follow up to #23 to show errors on Main thread - Show login errors higher up to ensure they aren't blocked by the keyboard - Faster tracks switching by not recomposing the player surface - Refocus on controller buttons after switching tracks - Show playback errors when they occur --- .../wholphin/ui/playback/PlaybackControls.kt | 34 ++++++- .../wholphin/ui/playback/PlaybackPage.kt | 96 ++++++++++--------- .../wholphin/ui/playback/PlaybackViewModel.kt | 21 +++- .../wholphin/ui/setup/SwitchUserContent.kt | 20 ++-- .../wholphin/ui/setup/SwitchUserViewModel.kt | 11 ++- 5 files changed, 118 insertions(+), 64 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackControls.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackControls.kt index ecd57950..d628d30e 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackControls.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackControls.kt @@ -23,6 +23,7 @@ import androidx.compose.foundation.layout.wrapContentWidth import androidx.compose.foundation.relocation.BringIntoViewRequester import androidx.compose.foundation.relocation.bringIntoViewRequester import androidx.compose.foundation.shape.CircleShape +import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -306,6 +307,7 @@ fun LeftPlaybackButtons( modifier: Modifier = Modifier, ) { var showMoreOptions by remember { mutableStateOf(false) } + val focusRequester = remember { FocusRequester() } Row( modifier = modifier.focusGroup(), horizontalArrangement = Arrangement.spacedBy(buttonSpacing), @@ -319,6 +321,7 @@ fun LeftPlaybackButtons( }, enabled = true, onControllerInteraction = onControllerInteraction, + modifier = Modifier.focusRequester(focusRequester), ) } if (showMoreOptions) { @@ -329,7 +332,10 @@ fun LeftPlaybackButtons( } BottomDialog( choices = options, - onDismissRequest = { showMoreOptions = false }, + onDismissRequest = { + showMoreOptions = false + focusRequester.tryRequestFocus() + }, onSelectChoice = { index, choice -> val action = moreButtonOptions.options[choice] ?: PlaybackAction.ShowDebug onPlaybackActionClick.invoke(action) @@ -359,6 +365,10 @@ fun RightPlaybackButtons( var showAudioDialog by remember { mutableStateOf(false) } var showSpeedDialog by remember { mutableStateOf(false) } var showScaleDialog by remember { mutableStateOf(false) } + + val captionFocusRequester = remember { FocusRequester() } + val settingsFocusRequester = remember { FocusRequester() } + Row( modifier = modifier.focusGroup(), horizontalArrangement = Arrangement.spacedBy(buttonSpacing), @@ -372,6 +382,7 @@ fun RightPlaybackButtons( showCaptionDialog = true }, onControllerInteraction = onControllerInteraction, + modifier = Modifier.focusRequester(captionFocusRequester), ) // Playback speed, etc PlaybackButton( @@ -382,8 +393,10 @@ fun RightPlaybackButtons( }, enabled = true, onControllerInteraction = onControllerInteraction, + modifier = Modifier.focusRequester(settingsFocusRequester), ) } + val scope = rememberCoroutineScope() if (showCaptionDialog) { val options = subtitleStreams.map { it.displayName } Timber.v("subtitleIndex=$subtitleIndex, options=$options") @@ -395,6 +408,11 @@ fun RightPlaybackButtons( onDismissRequest = { onControllerInteraction.invoke() showCaptionDialog = false + scope.launch { + // TODO this is hacky, but playback changes force refocus and this is a workaround + delay(250L) + captionFocusRequester.tryRequestFocus() + } }, onSelectChoice = { index, _ -> val send = @@ -434,6 +452,10 @@ fun RightPlaybackButtons( onDismissRequest = { onControllerInteraction.invoke() showAudioDialog = false + scope.launch { + delay(250L) + settingsFocusRequester.tryRequestFocus() + } }, onSelectChoice = { index, _ -> onPlaybackActionClick.invoke(PlaybackAction.ToggleAudio(audioStreams[index].index)) @@ -448,6 +470,10 @@ fun RightPlaybackButtons( onDismissRequest = { onControllerInteraction.invoke() showSpeedDialog = false + scope.launch { + delay(250L) + settingsFocusRequester.tryRequestFocus() + } }, onSelectChoice = { _, value -> onPlaybackActionClick.invoke(PlaybackAction.PlaybackSpeed(value.toFloat())) @@ -462,6 +488,10 @@ fun RightPlaybackButtons( onDismissRequest = { onControllerInteraction.invoke() showScaleDialog = false + scope.launch { + delay(250L) + settingsFocusRequester.tryRequestFocus() + } }, onSelectChoice = { index, _ -> onPlaybackActionClick.invoke(PlaybackAction.Scale(playbackScaleOptions.keys.toList()[index])) @@ -601,7 +631,7 @@ private fun BottomDialog( Modifier .wrapContentSize() .padding(8.dp) - .background(Color.DarkGray), + .background(Color.DarkGray, shape = RoundedCornerShape(16.dp)), ) { Column( modifier = diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt index 528e0716..be9511c4 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackPage.kt @@ -38,7 +38,6 @@ import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.RectangleShape import androidx.compose.ui.input.key.onKeyEvent import androidx.compose.ui.layout.ContentScale -import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel @@ -61,9 +60,11 @@ import com.github.damontecres.wholphin.data.model.Playlist import com.github.damontecres.wholphin.preferences.UserPreferences import com.github.damontecres.wholphin.preferences.skipBackOnResume import com.github.damontecres.wholphin.ui.OneTimeLaunchedEffect +import com.github.damontecres.wholphin.ui.components.ErrorMessage import com.github.damontecres.wholphin.ui.components.LoadingPage import com.github.damontecres.wholphin.ui.nav.Destination import com.github.damontecres.wholphin.ui.tryRequestFocus +import com.github.damontecres.wholphin.util.LoadingState import com.github.damontecres.wholphin.util.seasonEpisode import kotlinx.coroutines.delay import org.jellyfin.sdk.model.api.DeviceProfile @@ -84,60 +85,62 @@ fun PlaybackPage( modifier: Modifier = Modifier, viewModel: PlaybackViewModel = hiltViewModel(), ) { - val prefs = preferences.appPreferences.playbackPreferences - val context = LocalContext.current - val scope = rememberCoroutineScope() LaunchedEffect(destination.itemId) { viewModel.init(destination, deviceProfile, preferences) } - val player = viewModel.player - val stream by viewModel.stream.observeAsState(null) - val title by viewModel.title.observeAsState(null) - val subtitle by viewModel.subtitle.observeAsState(null) - val duration by viewModel.duration.observeAsState(null) - val audioStreams by viewModel.audioStreams.observeAsState(listOf()) - val subtitleStreams by viewModel.subtitleStreams.observeAsState(listOf()) - val trickplay by viewModel.trickplay.observeAsState(null) - val chapters by viewModel.chapters.observeAsState(listOf()) - val currentPlayback by viewModel.currentPlayback.observeAsState(null) - val currentItemPlayback by viewModel.currentItemPlayback.observeAsState( - ItemPlayback( - userId = -1, - itemId = UUID.randomUUID(), - ), - ) - val currentSegment by viewModel.currentSegment.observeAsState(null) - var segmentCancelled by remember(currentSegment?.id) { mutableStateOf(false) } + val loading by viewModel.loading.observeAsState(LoadingState.Loading) + when (val st = loading) { + is LoadingState.Error -> ErrorMessage(st, modifier) + LoadingState.Pending, + LoadingState.Loading, + -> LoadingPage(modifier) - var cues by remember { mutableStateOf>(listOf()) } - var showDebugInfo by remember { mutableStateOf(prefs.showDebugInfo) } + LoadingState.Success -> { + val prefs = preferences.appPreferences.playbackPreferences + val scope = rememberCoroutineScope() - val nextUp by viewModel.nextUp.observeAsState(null) - val playlist by viewModel.playlist.observeAsState(Playlist(listOf())) + val player = viewModel.player + val title by viewModel.title.observeAsState(null) + val subtitle by viewModel.subtitle.observeAsState(null) + val duration by viewModel.duration.observeAsState(null) + val audioStreams by viewModel.audioStreams.observeAsState(listOf()) + val subtitleStreams by viewModel.subtitleStreams.observeAsState(listOf()) + val trickplay by viewModel.trickplay.observeAsState(null) + val chapters by viewModel.chapters.observeAsState(listOf()) + val currentPlayback by viewModel.currentPlayback.observeAsState(null) + val currentItemPlayback by viewModel.currentItemPlayback.observeAsState( + ItemPlayback( + userId = -1, + itemId = UUID.randomUUID(), + ), + ) + val currentSegment by viewModel.currentSegment.observeAsState(null) + var segmentCancelled by remember(currentSegment?.id) { mutableStateOf(false) } - // TODO move to viewmodel? - val cueListener = - remember { - object : Player.Listener { - override fun onCues(cueGroup: CueGroup) { - cues = cueGroup.cues + var cues by remember { mutableStateOf>(listOf()) } + var showDebugInfo by remember { mutableStateOf(prefs.showDebugInfo) } + + val nextUp by viewModel.nextUp.observeAsState(null) + val playlist by viewModel.playlist.observeAsState(Playlist(listOf())) + + // TODO move to viewmodel? + val cueListener = + remember { + object : Player.Listener { + override fun onCues(cueGroup: CueGroup) { + cues = cueGroup.cues + } + } } + + OneTimeLaunchedEffect { + player.addListener(cueListener) } - } - - OneTimeLaunchedEffect { - player.addListener(cueListener) - } - DisposableEffect(Unit) { - onDispose { player.removeListener(cueListener) } - } - AmbientPlayerListener(player) - - if (stream == null) { - LoadingPage() - } else { - stream?.let { + DisposableEffect(Unit) { + onDispose { player.removeListener(cueListener) } + } + AmbientPlayerListener(player) var contentScale by remember { mutableStateOf(ContentScale.Fit) } var playbackSpeed by remember { mutableFloatStateOf(1.0f) } LaunchedEffect(playbackSpeed) { player.setPlaybackSpeed(playbackSpeed) } @@ -146,6 +149,7 @@ fun PlaybackPage( val scaledModifier = Modifier.resizeWithContentScale(contentScale, presentationState.videoSizeDp) val focusRequester = remember { FocusRequester() } + val controllerFocusRequester = remember { FocusRequester() } val playPauseState = rememberPlayPauseButtonState(player) val seekBarState = rememberSeekBarState(player, scope) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt index 770f299e..2138cb0f 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/PlaybackViewModel.kt @@ -10,6 +10,7 @@ import androidx.lifecycle.viewModelScope import androidx.media3.common.C import androidx.media3.common.Format import androidx.media3.common.MediaItem +import androidx.media3.common.PlaybackException import androidx.media3.common.Player import androidx.media3.common.TrackSelectionOverride import androidx.media3.common.Tracks @@ -32,6 +33,8 @@ import com.github.damontecres.wholphin.ui.showToast import com.github.damontecres.wholphin.ui.toServerString import com.github.damontecres.wholphin.util.EqualityMutableLiveData import com.github.damontecres.wholphin.util.ExceptionHandler +import com.github.damontecres.wholphin.util.LoadingExceptionHandler +import com.github.damontecres.wholphin.util.LoadingState import com.github.damontecres.wholphin.util.TrackActivityPlaybackListener import com.github.damontecres.wholphin.util.TrackSupport import com.github.damontecres.wholphin.util.checkForSupport @@ -105,7 +108,7 @@ class PlaybackViewModel playWhenReady = true } - val stream = MutableLiveData(null) + val loading = MutableLiveData(LoadingState.Loading) val title = MutableLiveData(null) val subtitle = MutableLiveData(null) @@ -152,7 +155,12 @@ class PlaybackViewModel val itemId = destination.itemId this.itemId = itemId val item = destination.item - viewModelScope.launch(ExceptionHandler() + Dispatchers.IO) { + viewModelScope.launch( + LoadingExceptionHandler( + loading, + "Error preparing for plaback on ${destination.itemId}", + ) + Dispatchers.IO, + ) { val queriedItem = item?.data ?: api.userLibraryApi.getItem(itemId).content val base = if (queriedItem.type == BaseItemKind.PLAYLIST) { @@ -493,7 +501,7 @@ class PlaybackViewModel this@PlaybackViewModel.activityListener = activityListener duration.value = source.runTimeTicks?.ticks - stream.value = decision + loading.value = LoadingState.Success currentPlayback.value = playback this@PlaybackViewModel.currentItemPlayback.value = itemPlayback player.setMediaItem( @@ -708,6 +716,13 @@ class PlaybackViewModel tracks = checkForSupport(tracks), ) } + + override fun onPlayerError(error: PlaybackException) { + Timber.e(error, "Playback error") + viewModelScope.launch(Dispatchers.Main + ExceptionHandler()) { + loading.value = LoadingState.Error("Error during playback", error) + } + } } data class CurrentPlayback( diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserContent.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserContent.kt index e0552fee..b7e656b2 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserContent.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserContent.kt @@ -178,6 +178,16 @@ fun SwitchUserContent( style = MaterialTheme.typography.titleMedium, color = MaterialTheme.colorScheme.onSurface, ) + when (val s = userState) { + is LoadingState.Error -> { + Text( + text = s.message ?: s.exception?.localizedMessage ?: "Error", + color = MaterialTheme.colorScheme.error, + ) + } + + else -> {} + } Row( verticalAlignment = Alignment.CenterVertically, modifier = Modifier.focusGroup(), @@ -236,16 +246,6 @@ fun SwitchUserContent( Text("Login") } } - when (val s = userState) { - is LoadingState.Error -> { - Text( - text = s.message ?: s.exception?.localizedMessage ?: "Error", - color = MaterialTheme.colorScheme.error, - ) - } - - else -> {} - } } } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserViewModel.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserViewModel.kt index 67694b8d..83244068 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserViewModel.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/setup/SwitchUserViewModel.kt @@ -154,7 +154,7 @@ class SwitchUserViewModel } } catch (ex: Exception) { Timber.e(ex, "Error switching user") - switchUserState.value = LoadingState.Error(exception = ex) + setError(ex) } } } @@ -178,7 +178,7 @@ class SwitchUserViewModel } } catch (ex: Exception) { Timber.e(ex, "Error logging in user") - switchUserState.value = LoadingState.Error(ex) + setError(ex) } } } @@ -230,7 +230,7 @@ class SwitchUserViewModel put(server.id, false) } } - switchUserState.value = LoadingState.Error(ex) + setError(ex) } } } @@ -328,4 +328,9 @@ class SwitchUserViewModel } } } + + private suspend fun setError(ex: Exception) = + withContext(Dispatchers.Main) { + switchUserState.value = LoadingState.Error(ex) + } }