diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/WindowUtils.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/WindowUtils.kt index e9e27deb53..4bda918632 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/WindowUtils.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/WindowUtils.kt @@ -45,9 +45,6 @@ private tailrec fun Context.getActivityWindow(): Window? = else -> null } -@Composable -fun getActivity(): Activity = LocalContext.current.getActivity() - tailrec fun Context.getActivity(): ComponentActivity = when (this) { is ComponentActivity -> this diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/layouts/ScreenLayout.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/layouts/ScreenLayout.kt index 57b66bc8b5..51ceb43c1e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/layouts/ScreenLayout.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/layouts/ScreenLayout.kt @@ -26,8 +26,8 @@ import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.widthIn import androidx.compose.material3.MaterialTheme import androidx.compose.material3.windowsizeclass.ExperimentalMaterial3WindowSizeClassApi +import androidx.compose.material3.windowsizeclass.WindowSizeClass import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass -import androidx.compose.material3.windowsizeclass.calculateWindowSizeClass import androidx.compose.runtime.Composable import androidx.compose.runtime.Immutable import androidx.compose.runtime.compositionLocalOf @@ -35,8 +35,8 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalConfiguration +import androidx.compose.ui.unit.DpSize import androidx.compose.ui.unit.dp -import com.vitorpamplona.amethyst.ui.components.getActivity /** How the app shell presents its top-level navigation for the current window size. */ enum class NavigationStyle { @@ -44,12 +44,13 @@ enum class NavigationStyle { BOTTOM_BAR, /** - * Medium windows (portrait tablets, unfolded foldables): a left navigation rail - * replaces the bottom bar; the drawer stays modal behind the rail's avatar button. + * Every non-Compact window that does not dock — portrait tablets and unfolded foldables at + * any width, plus short landscape windows: a left navigation rail replaces the bottom bar + * and the drawer stays modal behind the rail's avatar button. */ NAV_RAIL, - /** Expanded windows (landscape tablets, desktop windows): the drawer docks permanently on the left. */ + /** Wide, landscape, tall windows (landscape tablets, desktop): the drawer docks permanently on the left. */ PERMANENT_DRAWER, } @@ -60,7 +61,7 @@ enum class NavigationStyle { @Immutable data class ScreenLayoutSpec( val navigationStyle: NavigationStyle, - val showsNotificationPanel: Boolean, + val hasRoomForNotificationPanel: Boolean, ) { /** * True on the rail and permanent-drawer tiers. Large screens hide the bottom bar and pin @@ -69,16 +70,18 @@ data class ScreenLayoutSpec( val isLargeScreen: Boolean get() = navigationStyle != NavigationStyle.BOTTOM_BAR companion object { - val Phone = ScreenLayoutSpec(NavigationStyle.BOTTOM_BAR, showsNotificationPanel = false) + val Phone = ScreenLayoutSpec(NavigationStyle.BOTTOM_BAR, hasRoomForNotificationPanel = false) } } val LocalScreenLayout = compositionLocalOf { ScreenLayoutSpec.Phone } /** - * Minimum window width for the docked notification panel: the permanent drawer - * ([PermanentDrawerWidth]) + a readable center pane + the panel ([NotificationPanelWidth]) - * only coexist comfortably from a landscape-tablet-sized window up. + * Minimum window width for the docked notification panel: a leading navigation pane, a + * readable center pane and the panel ([NotificationPanelWidth]) only coexist comfortably from + * a landscape-tablet-sized window up. Sized against the widest leading pane, the permanent + * drawer ([PermanentDrawerWidth]); the rail is narrower, so a railed window that clears this + * gets a roomier center pane rather than a tighter one. */ private const val NOTIFICATION_PANEL_MIN_WINDOW_DP = 1200 @@ -95,6 +98,53 @@ val NotificationPanelWidth = 360.dp */ val FeedContentMaxWidth = 600.dp +/** + * Minimum window height for the docked drawer. Higher than Material's 480dp Compact/Medium + * height boundary on purpose: the permanent drawer's own header — banner, avatar, status + * editor, follower counts — fills most of a ~540dp column before the first navigation row, so + * below this the rail shows more of the menu than the dock does. + */ +private const val DOCK_MIN_WINDOW_HEIGHT_DP = 600 + +/** + * The navigation tier for a window of this shape. + * + * The dock is not a width decision. A tablet is past the Expanded breakpoint in both + * orientations, so keying on width alone pins 300dp of menu open in portrait with no closed + * state to fall back on (issue #4024). It docks only when the window is wide, landscape, and + * tall enough for the drawer's own content to be usable; everything else that is not Compact + * falls through to the rail, which pairs with the existing swipe-in modal drawer. + * + * A square window counts as landscape and docks; `Configuration.ORIENTATION_LANDSCAPE` + * breaks that tie the other way, so the two disagree at exactly width == height. + */ +@OptIn(ExperimentalMaterial3WindowSizeClassApi::class) +internal fun decideNavigationStyle( + windowWidthDp: Int, + windowHeightDp: Int, +): NavigationStyle { + val widthSizeClass = + WindowSizeClass + .calculateFromSize(DpSize(windowWidthDp.dp, windowHeightDp.dp)) + .widthSizeClass + + return when { + widthSizeClass == WindowWidthSizeClass.Expanded && + windowWidthDp >= windowHeightDp && + windowHeightDp >= DOCK_MIN_WINDOW_HEIGHT_DP -> NavigationStyle.PERMANENT_DRAWER + widthSizeClass != WindowWidthSizeClass.Compact -> NavigationStyle.NAV_RAIL + else -> NavigationStyle.BOTTOM_BAR + } +} + +/** + * Whether the window is wide enough to dock the notification feed beside the content. + * + * Deliberately not keyed on [NavigationStyle]: a wide portrait window now gets the rail, and + * gating on the dock would strip a panel it has today. + */ +internal fun hasRoomForNotificationPanel(windowWidthDp: Int): Boolean = windowWidthDp >= NOTIFICATION_PANEL_MIN_WINDOW_DP + /** * Centers a destination's content at [FeedContentMaxWidth]. The outer box paints the theme * background so the gutters match the screens' own surfaces; on Compact windows the cap is @@ -119,23 +169,15 @@ fun CappedScreenContent(content: @Composable () -> Unit) { } } -@OptIn(ExperimentalMaterial3WindowSizeClassApi::class) @Composable fun rememberScreenLayoutSpec(): ScreenLayoutSpec { - val widthSizeClass = calculateWindowSizeClass(getActivity()).widthSizeClass - val windowWidthDp = LocalConfiguration.current.screenWidthDp - return remember(widthSizeClass, windowWidthDp) { - val style = - when (widthSizeClass) { - WindowWidthSizeClass.Expanded -> NavigationStyle.PERMANENT_DRAWER - WindowWidthSizeClass.Medium -> NavigationStyle.NAV_RAIL - else -> NavigationStyle.BOTTOM_BAR - } + val configuration = LocalConfiguration.current + val windowWidthDp = configuration.screenWidthDp + val windowHeightDp = configuration.screenHeightDp + return remember(windowWidthDp, windowHeightDp) { ScreenLayoutSpec( - navigationStyle = style, - showsNotificationPanel = - style == NavigationStyle.PERMANENT_DRAWER && - windowWidthDp >= NOTIFICATION_PANEL_MIN_WINDOW_DP, + navigationStyle = decideNavigationStyle(windowWidthDp, windowHeightDp), + hasRoomForNotificationPanel = hasRoomForNotificationPanel(windowWidthDp), ) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountSwitcherAndLeftDrawerLayout.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountSwitcherAndLeftDrawerLayout.kt index dbeddda760..1008e74dcb 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountSwitcherAndLeftDrawerLayout.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountSwitcherAndLeftDrawerLayout.kt @@ -138,8 +138,10 @@ fun AccountSwitcherAndLeftDrawerLayout( } /** - * Compact and Medium windows: the drawer slides in as a modal sheet. On Medium an - * [AppNavigationRail] sits at the left edge in place of the phone bottom bar. + * Every window that does not dock the drawer: the drawer slides in as a modal sheet. On the + * rail tier an [AppNavigationRail] sits at the left edge in place of the phone bottom bar and + * the shell becomes multi-pane — a wide portrait window rails and is still wide enough for + * the notification panel. */ @Composable private fun ModalDrawerShell( @@ -183,11 +185,12 @@ private fun ModalDrawerShell( }, content = { if (showRail) { - Row(Modifier.fillMaxSize()) { - AppNavigationRail(nav, accountViewModel) - VerticalDivider(thickness = DividerThickness) - CenterPane(Modifier.weight(1f), content) - } + MultiPaneShell( + accountViewModel = accountViewModel, + nav = nav, + leading = { AppNavigationRail(nav, accountViewModel) }, + content = content, + ) } else { content() } @@ -196,8 +199,62 @@ private fun ModalDrawerShell( } /** - * Expanded windows: the drawer is permanently docked on the left, the bottom bar disappears, - * and — when the window is wide enough — the notification feed docks on the right. + * The wide-window arrangement both shells render: a [leading] navigation pane, the centre + * content, and the notification feed when the window has room. Shared because a wide portrait + * window now rails rather than docks and is still wide enough for the panel — the two shells + * differ only in which navigation pane leads. + */ +@Composable +private fun MultiPaneShell( + accountViewModel: AccountViewModel, + nav: Nav, + leading: @Composable () -> Unit, + content: @Composable () -> Unit, +) { + Row(Modifier.fillMaxSize()) { + leading() + + VerticalDivider(thickness = DividerThickness) + + CenterPane(Modifier.weight(1f), content) + + NotificationSidePanelSlot(accountViewModel, nav) + } +} + +/** + * The docked notification feed, when there is room for it. The panel duplicates the + * Notifications screen, so it steps aside while the user is there. + * + * The back-stack entry is collected here rather than in the shells so windows too narrow for + * the panel never observe it, and so navigation churn recomposes this slot instead of the + * whole shell. The trade is that a rail-tier window wide enough for the panel ends up with a + * second collector beside [ModalDrawerShell]'s own — one extra subscriber on a shared flow, + * against a shell restart per navigation on every window that cannot show the panel. + * + * [Route] matching goes through `remember` because `hasRoute` resolves a serializer + * reflectively on every call. + */ +@Composable +private fun NotificationSidePanelSlot( + accountViewModel: AccountViewModel, + nav: Nav, +) { + if (!LocalScreenLayout.current.hasRoomForNotificationPanel) return + + val navBackStackEntry by nav.controller.currentBackStackEntryAsState() + val destination = navBackStackEntry?.destination + val onNotifications = remember(destination) { destination?.hasRoute() == true } + + if (!onNotifications) { + VerticalDivider(thickness = DividerThickness) + NotificationSidePanel(accountViewModel, nav) + } +} + +/** + * Wide, landscape, tall windows: the drawer is permanently docked on the left, the bottom bar + * disappears, and — when the window is wide enough — the notification feed docks on the right. */ @Composable private fun PermanentDrawerShell( @@ -206,24 +263,12 @@ private fun PermanentDrawerShell( openSheet: () -> Unit, content: @Composable () -> Unit, ) { - val navBackStackEntry by nav.controller.currentBackStackEntryAsState() - // The panel duplicates the Notifications screen, so it steps aside while the user is there. - val showPanel = - LocalScreenLayout.current.showsNotificationPanel && - navBackStackEntry?.destination?.hasRoute() != true - - Row(Modifier.fillMaxSize()) { - PermanentDrawerContent(nav, openSheet, accountViewModel) - - VerticalDivider(thickness = DividerThickness) - - CenterPane(Modifier.weight(1f), content) - - if (showPanel) { - VerticalDivider(thickness = DividerThickness) - NotificationSidePanel(accountViewModel, nav) - } - } + MultiPaneShell( + accountViewModel = accountViewModel, + nav = nav, + leading = { PermanentDrawerContent(nav, openSheet, accountViewModel) }, + content = content, + ) } /** diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/layouts/ScreenLayoutTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/layouts/ScreenLayoutTest.kt new file mode 100644 index 0000000000..ac5c69857b --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/layouts/ScreenLayoutTest.kt @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.layouts + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The tier rule from amethyst/plans/2026-08-31-portrait-sidebar-tier-rule.md: + * + * dock <=> widthClass == Expanded && width >= height && height >= 600dp + * + * Sizes are window dp, width first. Every row of the spec's Behaviour table appears here. + */ +class ScreenLayoutTest { + private fun assertStyle( + expected: NavigationStyle, + widthDp: Int, + heightDp: Int, + ) = assertEquals("${widthDp}x${heightDp}dp", expected, decideNavigationStyle(widthDp, heightDp)) + + // ---- Behaviour table ---- + + @Test + fun phonePortraitKeepsTheBottomBar() = assertStyle(NavigationStyle.BOTTOM_BAR, 411, 923) + + @Test + fun phoneLandscapeNoLongerDocks() = assertStyle(NavigationStyle.NAV_RAIL, 923, 411) + + @Test + fun reporterTabletPortraitNoLongerDocks() = assertStyle(NavigationStyle.NAV_RAIL, 889, 1422) + + @Test + fun wideTabletPortraitNoLongerDocks() = assertStyle(NavigationStyle.NAV_RAIL, 1201, 1920) + + @Test + fun mediumTabletPortraitStillRails() = assertStyle(NavigationStyle.NAV_RAIL, 800, 1280) + + @Test + fun tabletLandscapeStillDocks() = assertStyle(NavigationStyle.PERMANENT_DRAWER, 1422, 889) + + @Test + fun mediumTabletLandscapeStillDocks() = assertStyle(NavigationStyle.PERMANENT_DRAWER, 1280, 800) + + @Test + fun wideButShortLandscapeNoLongerDocks() = assertStyle(NavigationStyle.NAV_RAIL, 1200, 540) + + // ---- Boundaries ---- + + @Test + fun squareWindowAtTheWidthBreakpointDocks() = assertStyle(NavigationStyle.PERMANENT_DRAWER, 840, 840) + + @Test + fun portraitByOneDpDoesNotDock() = assertStyle(NavigationStyle.NAV_RAIL, 840, 841) + + @Test + fun exactlyAtTheHeightFloorDocks() = assertStyle(NavigationStyle.PERMANENT_DRAWER, 840, 600) + + @Test + fun oneDpBelowTheHeightFloorDoesNotDock() = assertStyle(NavigationStyle.NAV_RAIL, 840, 599) + + @Test + fun oneDpBelowTheWidthBreakpointRails() = assertStyle(NavigationStyle.NAV_RAIL, 839, 600) + + @Test + fun compactWidthKeepsTheBottomBar() = assertStyle(NavigationStyle.BOTTOM_BAR, 599, 900) + + // ---- Notification panel ---- + + @Test + fun panelHiddenJustBelowTheThreshold() = assertFalse(hasRoomForNotificationPanel(1199)) + + @Test + fun panelShownExactlyAtTheThreshold() = assertTrue(hasRoomForNotificationPanel(1200)) + + /** + * A landscape-short window: wide enough for the panel, too short for the dock. Rail plus + * panel is a combination that has never shipped, so assert both halves together. + */ + @Test + fun shortLandscapeYieldsRailPlusPanel() { + assertStyle(NavigationStyle.NAV_RAIL, 1200, 599) + assertTrue(hasRoomForNotificationPanel(1200)) + } +}