From bf500934dccd4e6d2708c733a0ba8efff2fbf479 Mon Sep 17 00:00:00 2001 From: Altay Date: Sun, 25 Oct 2026 18:21:35 +0300 Subject: [PATCH 1/3] fix(mobile): keep the player across layout changes; phones in landscape keep one pane --- DESIGN.md | 11 +++-- .../android/MobileAdaptiveNavigation.kt | 18 +++++-- .../kotlin/io/putdotio/android/MobileShell.kt | 9 ++-- .../android/files/MobileFilesListDetail.kt | 18 +++++-- .../android/MobileAdaptiveNavigationTest.kt | 47 +++++++++++++++++++ 5 files changed, 87 insertions(+), 16 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index a114f376..d0430bce 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -306,13 +306,16 @@ America, where mono would fit fewer characters on the line. Five destinations: **Files, History, Transfers, Downloads, Account**, in that order. `NavigationSuiteScaffold`'s layout and suite with an explicit layout type: -a navigation bar below 600dp width and the collapsed wide navigation rail from -600dp, following the window. When five destinations don't -fit, in short windows or at large font scales, a scrollable drawer takes over. +a navigation bar below 600dp width or 480dp height, and the collapsed wide +navigation rail otherwise, following the window. A phone in landscape is compact +in height, so it keeps the bar, as Material's navigation suite does. When five +destinations don't fit, in short windows or at large font scales, a scrollable +drawer takes over. Changing layout never rebuilds the content under it. The selected item uses the stock indicator with a filled Phosphor glyph in `on-secondary-container`; unselected items use the regular glyph. -From 840dp, Files is list-detail: the folder's bar and list on the left (520dp, +From 840dp wide and 480dp tall, Files is list-detail, so a phone in landscape +keeps one pane: the folder's bar and list on the left (520dp, or half the space beside the rail when that is less), the selected file's detail on the right with its preview, primary actions and facts (size, added, length, subtitles, folder). Here a tap on a file selects it into diff --git a/mobile/src/main/kotlin/io/putdotio/android/MobileAdaptiveNavigation.kt b/mobile/src/main/kotlin/io/putdotio/android/MobileAdaptiveNavigation.kt index dd483b63..14b7b0b2 100644 --- a/mobile/src/main/kotlin/io/putdotio/android/MobileAdaptiveNavigation.kt +++ b/mobile/src/main/kotlin/io/putdotio/android/MobileAdaptiveNavigation.kt @@ -22,6 +22,10 @@ import androidx.compose.material3.TextButton import androidx.compose.material3.rememberDrawerState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.movableContentOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalDensity @@ -40,6 +44,9 @@ internal const val MOBILE_NAV_DRAWER_TAG = "mobile-navigation-drawer" internal enum class MobileNavigationLayout { Bar, Rail, Modal } +/** Material's compact window height, a phone in landscape: below 480dp. */ +internal fun isCompactHeight(height: Dp): Boolean = height < 480.dp + @Composable internal fun mobileNavigationLayout(width: Dp, height: Dp): MobileNavigationLayout { val density = LocalDensity.current @@ -58,8 +65,8 @@ internal fun mobileNavigationLayout(width: Dp, height: Dp): MobileNavigationLayo val tallest = with(density) { labels.maxOf { it.height }.toDp() } // Reserve the stock components' icon, label spacing and padding. A bar item grows to an equal // share of the bar and its label may fill it; a label on the 96dp rail takes its width less the - // item padding. - return if (width >= 600.dp) { + // item padding. A compact-height window, such as a phone in landscape, keeps the phone's bar. + return if (width >= 600.dp && !isCompactHeight(height)) { val railHeight = (24.dp + 12.dp + tallest + 24.dp) * MobileDestination.entries.size + 16.dp if (widest <= 80.dp && availableHeight >= railHeight) { MobileNavigationLayout.Rail @@ -85,8 +92,11 @@ internal fun MobileNavigationContainer( enabled: Boolean = true, content: @Composable ((() -> Unit)?) -> Unit, ) { + // Moving between layouts must keep the content, the open player included, rather than rebuild it. + val currentContent by rememberUpdatedState(content) + val slot = remember { movableContentOf<(() -> Unit)?> { currentContent(it) } } if (layout != MobileNavigationLayout.Modal) { - content(null) + slot(null) return } val drawer = rememberDrawerState(DrawerValue.Closed) @@ -129,7 +139,7 @@ internal fun MobileNavigationContainer( } }, ) { - content { scope.launch(start = CoroutineStart.UNDISPATCHED) { drawer.open() } } + slot { scope.launch(start = CoroutineStart.UNDISPATCHED) { drawer.open() } } } // Register with the overlay so NavHost callbacks added after route changes remain underneath it. if (visible) BackHandler(onBack = close) diff --git a/mobile/src/main/kotlin/io/putdotio/android/MobileShell.kt b/mobile/src/main/kotlin/io/putdotio/android/MobileShell.kt index 8bb8ae24..e868bcbe 100644 --- a/mobile/src/main/kotlin/io/putdotio/android/MobileShell.kt +++ b/mobile/src/main/kotlin/io/putdotio/android/MobileShell.kt @@ -77,7 +77,7 @@ import io.putdotio.android.files.FilesItemId import io.putdotio.android.files.FilesOpenOrigin import io.putdotio.android.files.FilesRepository import io.putdotio.android.files.FilesSubtitleLanguages -import io.putdotio.android.files.isFilesListDetailWidth +import io.putdotio.android.files.isFilesListDetail import io.putdotio.android.files.showsFilesListDetail import io.putdotio.android.files.MobileContinueWatching import io.putdotio.android.files.MobileFilesActionsController @@ -353,9 +353,10 @@ internal fun MobileShell( BoxWithConstraints(modifier = Modifier.fillMaxSize()) { val navigationLayout = mobileNavigationLayout(maxWidth, maxHeight) // Files keeps its panes while Search covers it, so nothing under Search is rebuilt. - val filesListDetail = isFilesListDetailWidth(maxWidth) + val filesListDetail = isFilesListDetail(maxWidth, maxHeight) val topBarInFilesPane = showsFilesListDetail( width = maxWidth, + height = maxHeight, onFiles = selectedDestination == MobileDestination.Files, searchOpen = searchOpen, isPlayback = isPlayback, @@ -901,8 +902,8 @@ private fun MobileNavigationAlerts( /** * The navigation suite, top bar and now-playing bar around one content slot; hiding them never - * moves it. Below 600dp the suite is a navigation bar, from 600dp a rail, and in the drawer layout - * none, since the modal drawer outside this slot takes its place. + * moves it. Below 600dp wide or 480dp tall the suite is a navigation bar, otherwise a rail, and in + * the drawer layout none, since the modal drawer outside this slot takes its place. */ @Composable private fun MobileChrome( diff --git a/mobile/src/main/kotlin/io/putdotio/android/files/MobileFilesListDetail.kt b/mobile/src/main/kotlin/io/putdotio/android/files/MobileFilesListDetail.kt index e978c404..052a37cb 100644 --- a/mobile/src/main/kotlin/io/putdotio/android/files/MobileFilesListDetail.kt +++ b/mobile/src/main/kotlin/io/putdotio/android/files/MobileFilesListDetail.kt @@ -20,15 +20,25 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.platform.testTag import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +import io.putdotio.android.isCompactHeight internal const val MOBILE_FILES_LIST_PANE_TAG = "mobile-files-list-pane" -/** Files shows list and detail side by side in a window 840dp or wider. */ -internal fun isFilesListDetailWidth(width: Dp): Boolean = width >= FILES_LIST_DETAIL_MIN_WIDTH +/** + * Files shows list and detail side by side in a window 840dp or wider that is not compact in height: + * a phone in landscape keeps one pane. + */ +internal fun isFilesListDetail(width: Dp, height: Dp): Boolean = + width >= FILES_LIST_DETAIL_MIN_WIDTH && !isCompactHeight(height) /** Whether Files' bar sits over its list pane: the panes are on screen, not under Search or the player. */ -internal fun showsFilesListDetail(width: Dp, onFiles: Boolean, searchOpen: Boolean, isPlayback: Boolean): Boolean = - isFilesListDetailWidth(width) && onFiles && !searchOpen && !isPlayback +internal fun showsFilesListDetail( + width: Dp, + height: Dp, + onFiles: Boolean, + searchOpen: Boolean, + isPlayback: Boolean, +): Boolean = isFilesListDetail(width, height) && onFiles && !searchOpen && !isPlayback private val FILES_LIST_DETAIL_MIN_WIDTH: Dp = 840.dp diff --git a/mobile/src/test/kotlin/io/putdotio/android/MobileAdaptiveNavigationTest.kt b/mobile/src/test/kotlin/io/putdotio/android/MobileAdaptiveNavigationTest.kt index 29a4c88e..3cba8fa6 100644 --- a/mobile/src/test/kotlin/io/putdotio/android/MobileAdaptiveNavigationTest.kt +++ b/mobile/src/test/kotlin/io/putdotio/android/MobileAdaptiveNavigationTest.kt @@ -6,6 +6,10 @@ import androidx.activity.OnBackPressedDispatcherOwner import androidx.activity.compose.LocalOnBackPressedDispatcherOwner import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.ime +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalView import androidx.compose.ui.semantics.SemanticsActions @@ -43,6 +47,7 @@ import io.putdotio.android.files.FilesFolder import io.putdotio.android.files.FilesItem import io.putdotio.android.files.FilesItemId import io.putdotio.android.files.FilesPage +import io.putdotio.android.files.MOBILE_FILES_LIST_PANE_TAG import io.putdotio.android.files.MOBILE_FILES_LIST_TAG import io.putdotio.android.playback.ActiveAudio import io.putdotio.android.playback.MobilePlayerFactory @@ -201,6 +206,17 @@ class MobileOrdinaryNavigationTest : NavigationShellFixture(1f) { assertNavigationLabels(MOBILE_NAV_RAIL_TAG) } + @Test + @Config(qualifiers = "en-rUS-w914dp-h411dp-land") + fun phoneLandscapeKeepsItsBarAndOneFilesPane() { + mount() + compose.onNodeWithTag(MOBILE_NAV_MENU_TAG).assertDoesNotExist() + compose.onNodeWithTag(MOBILE_NAV_RAIL_TAG).assertDoesNotExist() + compose.onNodeWithTag(MOBILE_NAV_BAR_TAG).assertIsDisplayed() + compose.onNodeWithTag(MOBILE_FILES_LIST_PANE_TAG).assertDoesNotExist() + compose.onNodeWithTag(MOBILE_FILES_LIST_TAG).assertIsDisplayed() + } + @Test fun openingAndClosingTheKeyboardPreservesPhoneNavigationAndTheFocusedTransferDraft() { assertKeyboardKeepsTransferDraft(MOBILE_NAV_BAR_TAG) @@ -240,6 +256,37 @@ class MobileOrdinaryNavigationTest : NavigationShellFixture(1f) { } } +@RunWith(AndroidJUnit4::class) +@Config(sdk = [35]) +class MobileNavigationContainerTest { + @get:Rule val compose = createComposeRule() + + @Test + fun changingLayoutKeepsTheContentItHolds() { + var layout by mutableStateOf(MobileNavigationLayout.Bar) + var created = 0 + var disposed = 0 + compose.setContent { + PutioTheme { + MobileNavigationContainer(layout, MobileDestination.Files, onDestination = {}) { + DisposableEffect(Unit) { + created++ + onDispose { disposed++ } + } + } + } + } + for (next in listOf(MobileNavigationLayout.Modal, MobileNavigationLayout.Rail, MobileNavigationLayout.Bar)) { + compose.runOnIdle { layout = next } + compose.waitForIdle() + } + compose.runOnIdle { + assertEquals("A layout change must not rebuild the content, an open player included", 1, created) + assertEquals(0, disposed) + } + } +} + abstract class NavigationShellFixture(fontScale: Float) { protected val compose = createComposeRule() protected lateinit var backOwner: OnBackPressedDispatcherOwner From 48549b293d5cfec9f2780ab6dd40ef5b2c3b90d2 Mon Sep 17 00:00:00 2001 From: Altay Date: Sun, 25 Oct 2026 18:45:48 +0300 Subject: [PATCH 2/3] test(mobile): tablets are 480dp tall; phone landscape keeps the bar --- .../putdotio/android/MobileKeyboardNavigationProofTest.kt | 3 ++- .../src/test/kotlin/io/putdotio/android/MobileShellTest.kt | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/mobile/src/androidTest/kotlin/io/putdotio/android/MobileKeyboardNavigationProofTest.kt b/mobile/src/androidTest/kotlin/io/putdotio/android/MobileKeyboardNavigationProofTest.kt index 29c5f08a..a8cd115a 100644 --- a/mobile/src/androidTest/kotlin/io/putdotio/android/MobileKeyboardNavigationProofTest.kt +++ b/mobile/src/androidTest/kotlin/io/putdotio/android/MobileKeyboardNavigationProofTest.kt @@ -70,7 +70,8 @@ class MobileKeyboardNavigationProofTest { fun keyboardKeepsNavigationAndTheFocusedSearchEditor() { val orientation = InstrumentationRegistry.getArguments().getString("putio.accessibility.orientation") require(orientation == "portrait" || orientation == "landscape") - val navigationTag = if (orientation == "portrait") MOBILE_NAV_BAR_TAG else MOBILE_NAV_RAIL_TAG + // A phone in landscape is compact in height, so it keeps the bar. + val navigationTag = MOBILE_NAV_BAR_TAG var query by mutableStateOf("") val mounted = mutableStateOf(true) val playerFactory = ShellProofPlayerFactory(null) diff --git a/mobile/src/test/kotlin/io/putdotio/android/MobileShellTest.kt b/mobile/src/test/kotlin/io/putdotio/android/MobileShellTest.kt index d4b14f42..4ec6b41e 100644 --- a/mobile/src/test/kotlin/io/putdotio/android/MobileShellTest.kt +++ b/mobile/src/test/kotlin/io/putdotio/android/MobileShellTest.kt @@ -1582,9 +1582,9 @@ class MobileShellPlaybackTest { @Test fun tabletDestinationStateSurvivesPlayback() = - assertDestinationStateSurvivesPlayback(width = 700.dp, navigationTag = MOBILE_NAV_RAIL_TAG) + assertDestinationStateSurvivesPlayback(width = 700.dp, navigationTag = MOBILE_NAV_RAIL_TAG, height = 480.dp) - private fun assertDestinationStateSurvivesPlayback(width: Dp, navigationTag: String) { + private fun assertDestinationStateSurvivesPlayback(width: Dp, navigationTag: String, height: Dp = 460.dp) { val pending = MutableStateFlow(false) val requests = NowPlayingRequests(pending) { pending.value = false } val factory = object : MobilePlayerFactory by NoAudioSessionFactory { @@ -1593,7 +1593,7 @@ class MobileShellPlaybackTest { } compose.setContent { PutioTheme { - Box(modifier = Modifier.requiredSize(width = width, height = 460.dp)) { + Box(modifier = Modifier.requiredSize(width = width, height = height)) { MobileShell( nowPlayingRequests = requests, playbackPlayerFactory = factory, From 6bc961e88f687362d8bac80c02f4a0c8ad0c5ff0 Mon Sep 17 00:00:00 2001 From: Altay Date: Sun, 25 Oct 2026 19:01:12 +0300 Subject: [PATCH 3/3] test(mobile): an open video keeps its player across navigation layouts --- .../io/putdotio/android/MobileShellTest.kt | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/mobile/src/test/kotlin/io/putdotio/android/MobileShellTest.kt b/mobile/src/test/kotlin/io/putdotio/android/MobileShellTest.kt index 4ec6b41e..75d1865d 100644 --- a/mobile/src/test/kotlin/io/putdotio/android/MobileShellTest.kt +++ b/mobile/src/test/kotlin/io/putdotio/android/MobileShellTest.kt @@ -40,6 +40,7 @@ import androidx.compose.ui.test.performScrollTo import androidx.compose.ui.test.performScrollToNode import androidx.compose.ui.test.performTouchInput import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.DpSize import androidx.compose.ui.unit.dp import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner @@ -1576,6 +1577,46 @@ class MobileShellPlaybackTest { compose.onNodeWithTag(MOBILE_NAV_BAR_TAG).assertIsDisplayed() } + @Test + fun anOpenVideoKeepsItsPlayerAcrossNavigationLayouts() { + var created = 0 + val factory = object : MobilePlayerFactory by NoAudioSessionFactory { + override fun create(context: Context, mediaType: PlaybackMediaType): Media3Player { + created += 1 + return NoAudioSessionFactory.create(context, mediaType) + } + } + var size by mutableStateOf(DpSize(411.dp, 914.dp)) + compose.setContent { + PutioTheme { + Box(modifier = Modifier.requiredSize(size)) { + MobileShell( + filesState = videoFilesState(), + accountSettingsState = readyAccountSettingsState(), + appConfigState = readyAndroidAppConfigState(), + account = Account, + playbackRepository = EndingPlaybackRepository, + playbackPlayerFactory = factory, + sessionId = Session, + onFilesEvent = { true }, + onAccountSettingsEvent = {}, + onPlaybackAuthenticationRequired = {}, + onSignOut = {}, + ) + } + } + } + compose.onNodeWithText("episode.mkv").performClick() + compose.runOnIdle { assertEquals(1, created) } + // Bar, then the drawer in a window past the 840dp list-detail width, then the rail beside + // list-detail, then a phone in landscape: the player behind each must be the same one. + for (next in listOf(DpSize(914.dp, 260.dp), DpSize(1280.dp, 800.dp), DpSize(914.dp, 411.dp))) { + compose.runOnIdle { size = next } + compose.waitForIdle() + } + compose.runOnIdle { assertEquals("A layout change must not rebuild the open player", 1, created) } + } + @Test fun phoneDestinationStateSurvivesPlayback() = assertDestinationStateSurvivesPlayback(width = 360.dp, navigationTag = MOBILE_NAV_BAR_TAG)