Merge pull request #4037 from davotoula/fix/4024-portrait-sidebar-tier

fix: don't dock the sidebar on portrait tablets (#4024)

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

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),
         )
     }
 }

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<Route.Notification>() == 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<Route.Notification>() != 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,
+    )
 }
 
 /**

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))
+    }
+}