diff --git a/CHANGELOG.md b/CHANGELOG.md index d1277cc6..1cd03876 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,15 +10,21 @@ All notable changes to this project will be documented in this file. - **Overlapping `withoutNotifying` calls no longer un-suppress each other.** The suppression was a flag cleared by whichever action finished first, so a save still in flight when the reader opened a page that loads on mount notified every listener on completion. It is a count now. (`lib/src/http/controllers/magic_starter_profile_controller.dart`) -- **The invitation-accept screen scrolls itself.** It was the one starter screen in the app shell with no scroll of its own, so a host whose content box does not scroll (the setting a host with `.stacked()` routes needs) handed it a bounded height, and on a short viewport the card overflowed with nothing to drag. It now sits in its own `SingleChildScrollView(primary: false)`, as `MSPageScaffold` screens do. (`lib/src/ui/views/teams/magic_starter_team_invitation_accept_view.dart`) +- **The invitation-accept screen scrolls itself.** It was the one starter screen in the app shell with no scroll of its own, so a content box that does not scroll (the default from this release, see Changed) handed it a bounded height, and on a short viewport the card overflowed with nothing to drag. It now sits in its own `SingleChildScrollView(primary: false)`, as `MSPageScaffold` screens do. (`lib/src/ui/views/teams/magic_starter_team_invitation_accept_view.dart`) ### Added - **`resetQuietly()` on `MagicStarterProfileController` and `MagicStarterTeamController`**, the non-notifying reset those views call: errors cleared, state empty, no listener told. A host view that binds either controller and resets it in `onInit` should call it too. (`lib/src/http/controllers/`) -### Changed +### Breaking + +- **The app shell's content box no longer scrolls; each routed page scrolls itself.** `MagicStarterLayoutTheme.contentClassName` defaults to `'flex-1 min-h-0'` and `contentScrollPrimary` to `false` (they were `'flex-1 overflow-y-auto'` and `true`). In a go_router shell the route child is the nested Navigator, so the old default scrolled the Navigator and laid its Overlay out under an unbounded height: a page left under a `.stacked()` route was never laid out again, and its second rebuild while hidden failed `_debugRelayoutBoundaryAlreadyMarkedNeedsLayout` in debug and left the tree inconsistent. Closes #160. (`lib/src/configuration/magic_starter_theme.dart`) + + The starter's own screens already scroll themselves (`MSPageScaffold`, the invitation card), and two changed to keep doing so correctly: the notification screens now take the host page geometry inside their own scroll view rather than wrapped around it, where the padding would inset the viewport and clip a long list (`lib/src/routes/notification_routes.dart`), and the `starter:install` dashboard stub now scrolls itself (`assets/stubs/install/dashboard_view.stub`). + + **Migrating:** a page of your own that relied on the shell to scroll it now renders cut off at the window. Put it through `MSPageScaffold` or wrap it in `SingleChildScrollView(primary: false)`; an app installed from an earlier stub has a `DashboardView` that needs the same. Or set the old pair back with `MagicStarter.useLayoutTheme(const MagicStarterLayoutTheme(contentClassName: 'flex-1 overflow-y-auto', contentScrollPrimary: true))`, accepting that stacked routes then carry the hazard above. `doc/basics/views-and-layouts.md` covers both. -- **`MagicStarterLayoutTheme.contentClassName` documents the stacked-route hazard.** The default `'flex-1 overflow-y-auto'` scrolls the shell's nested Navigator, which breaks a page left under a `.stacked()` route on its second hidden rebuild. The default is unchanged; the doc now says to set `'flex-1 min-h-0'` in any app that stacks routes under this layout. Refs #160. (`lib/src/configuration/magic_starter_theme.dart`) + **Lost:** tapping the iOS status bar no longer scrolls a shell page to the top. The shell's `Scaffold` scrolls the primary controller above the nested Navigator, which the old scrolling content box was attached to; every page's scroll now sits inside a route that scopes its own controller. Restoring it needs a status-bar handler below the Navigator, which is a separate change. ## [0.0.36] - 2026-09-23 diff --git a/assets/stubs/install/dashboard_view.stub b/assets/stubs/install/dashboard_view.stub index 9c65a0e5..62056547 100644 --- a/assets/stubs/install/dashboard_view.stub +++ b/assets/stubs/install/dashboard_view.stub @@ -13,174 +13,179 @@ class DashboardView extends StatelessWidget { Widget build(BuildContext context) { final appName = Config.get('app.name', 'My App') ?? 'My App'; - return WDiv( - className: 'w-full max-w-[480px] md:max-w-4xl mx-auto p-4 lg:p-8', + // Its own scroll: the app shell's content box does not scroll, so a + // page taller than the window scrolls itself. + return SingleChildScrollView( + primary: false, child: WDiv( - className: ''' - rounded-2xl bg-white dark:bg-gray-800 - border border-gray-200 dark:border-gray-700 - p-6 lg:p-8 flex flex-col items-center - ''', - children: [ - // -------------------------------------------------- - // Hero Section - // -------------------------------------------------- - WDiv( - className: ''' - w-20 h-20 rounded-2xl - flex items-center justify-center - bg-gradient-to-br from-violet-500 to-purple-600 - dark:from-violet-600 dark:to-purple-700 - animate-bounce - ''', - child: WIcon( - Icons.auto_awesome, - className: 'text-4xl text-white dark:text-gray-100', - ), - ), - const WSpacer(className: 'h-6'), - WText( - appName, - className: ''' - text-2xl lg:text-3xl font-bold - text-gray-900 dark:text-white text-center - ''', - ), - const WSpacer(className: 'h-2'), - WText( - 'Built with Magic Starter', - className: 'text-base text-gray-500 dark:text-gray-400', - ), - - const WSpacer(className: 'h-8'), - - // -------------------------------------------------- - // Quick-link cards - // -------------------------------------------------- - WDiv( - className: 'w-full grid grid-cols-1 md:grid-cols-3 gap-3', - children: [ - _buildLinkCard( - icon: Icons.menu_book, - title: 'Documentation', - description: 'Read the Magic Framework docs to get started.', - url: 'https://magic.fluttersdk.com', - ), - _buildLinkCard( - icon: Icons.code, - title: 'GitHub', - description: 'Star the repo, report issues, or contribute code.', - url: 'https://github.com/fluttersdk/magic', - ), - _buildLinkCard( - icon: Icons.terminal, - title: 'CLI Commands', - description: 'Run `magic --help` to see all available commands.', - url: 'https://magic.fluttersdk.com/cli', - ), - ], - ), - - const WSpacer(className: 'h-8'), - - // -------------------------------------------------- - // Footer - // -------------------------------------------------- - WDiv( - className: 'flex flex-row items-center justify-center gap-1', - children: [ - WText( - 'Made with', - className: 'text-xs text-gray-400 dark:text-gray-500', - ), - WDiv( - className: 'animate-pulse', - child: WText( - '❤️', - className: 'text-xs text-red-500 dark:text-red-400', - ), - ), - WText( - 'by', - className: 'text-xs text-gray-400 dark:text-gray-500', - ), - WAnchor( - onTap: () => Launch.url('https://anilcancakir.com'), - child: WText( - 'Anılcan Çakır', - className: ''' - text-xs font-medium text-primary - dark:text-primary-400 - ''', - ), - ), - ], - ), - ], - ), - ); - } - - /// Builds a single quick-link card with icon, title, description, and link. - Widget _buildLinkCard({ - required IconData icon, - required String title, - required String description, - required String url, - }) { - return WDiv( - className: ''' - flex flex-col p-4 rounded-xl - bg-gray-50 dark:bg-gray-700/30 - border border-gray-100 dark:border-gray-700 - ''', - children: [ - WDiv( - className: 'flex flex-row items-center gap-3 mb-2', + className: 'w-full max-w-[480px] md:max-w-4xl mx-auto p-4 lg:p-8', + child: WDiv( + className: ''' + rounded-2xl bg-white dark:bg-gray-800 + border border-gray-200 dark:border-gray-700 + p-6 lg:p-8 flex flex-col items-center + ''', children: [ + // -------------------------------------------------- + // Hero Section + // -------------------------------------------------- WDiv( className: ''' - p-2 rounded-lg - bg-primary/10 dark:bg-primary-900/30 + w-20 h-20 rounded-2xl + flex items-center justify-center + bg-gradient-to-br from-violet-500 to-purple-600 + dark:from-violet-600 dark:to-purple-700 + animate-bounce ''', child: WIcon( - icon, - className: 'text-lg text-primary dark:text-primary-400', + Icons.auto_awesome, + className: 'text-4xl text-white dark:text-gray-100', ), ), + const WSpacer(className: 'h-6'), WText( - title, + appName, className: ''' - text-base font-semibold - text-gray-900 dark:text-white + text-2xl lg:text-3xl font-bold + text-gray-900 dark:text-white text-center ''', ), + const WSpacer(className: 'h-2'), + WText( + 'Built with Magic Starter', + className: 'text-base text-gray-500 dark:text-gray-400', + ), + + const WSpacer(className: 'h-8'), + + // -------------------------------------------------- + // Quick-link cards + // -------------------------------------------------- + WDiv( + className: 'w-full grid grid-cols-1 md:grid-cols-3 gap-3', + children: [ + _buildLinkCard( + icon: Icons.menu_book, + title: 'Documentation', + description: 'Read the Magic Framework docs to get started.', + url: 'https://magic.fluttersdk.com', + ), + _buildLinkCard( + icon: Icons.code, + title: 'GitHub', + description: 'Star the repo, report issues, or contribute code.', + url: 'https://github.com/fluttersdk/magic', + ), + _buildLinkCard( + icon: Icons.terminal, + title: 'CLI Commands', + description: 'Run `magic --help` to see all available commands.', + url: 'https://magic.fluttersdk.com/cli', + ), + ], + ), + + const WSpacer(className: 'h-8'), + + // -------------------------------------------------- + // Footer + // -------------------------------------------------- + WDiv( + className: 'flex flex-row items-center justify-center gap-1', + children: [ + WText( + 'Made with', + className: 'text-xs text-gray-400 dark:text-gray-500', + ), + WDiv( + className: 'animate-pulse', + child: WText( + '❤️', + className: 'text-xs text-red-500 dark:text-red-400', + ), + ), + WText( + 'by', + className: 'text-xs text-gray-400 dark:text-gray-500', + ), + WAnchor( + onTap: () => Launch.url('https://anilcancakir.com'), + child: WText( + 'Anılcan Çakır', + className: ''' + text-xs font-medium text-primary + dark:text-primary-400 + ''', + ), + ), + ], + ), ], ), - WText( - description, - className: 'text-sm text-gray-500 dark:text-gray-400 mb-3', - ), - WAnchor( - onTap: () => Launch.url(url), - child: WDiv( - className: 'flex flex-row items-center gap-1', + ); + } + + /// Builds a single quick-link card with icon, title, description, and link. + Widget _buildLinkCard({ + required IconData icon, + required String title, + required String description, + required String url, + }) { + return WDiv( + className: ''' + flex flex-col p-4 rounded-xl + bg-gray-50 dark:bg-gray-700/30 + border border-gray-100 dark:border-gray-700 + ''', + children: [ + WDiv( + className: 'flex flex-row items-center gap-3 mb-2', children: [ - WText( - 'Learn more', + WDiv( className: ''' - text-sm font-medium - text-primary dark:text-primary-400 + p-2 rounded-lg + bg-primary/10 dark:bg-primary-900/30 ''', + child: WIcon( + icon, + className: 'text-lg text-primary dark:text-primary-400', + ), ), - WIcon( - Icons.arrow_forward_outlined, - className: 'text-sm text-primary dark:text-primary-400', + WText( + title, + className: ''' + text-base font-semibold + text-gray-900 dark:text-white + ''', ), ], ), - ), - ], + WText( + description, + className: 'text-sm text-gray-500 dark:text-gray-400 mb-3', + ), + WAnchor( + onTap: () => Launch.url(url), + child: WDiv( + className: 'flex flex-row items-center gap-1', + children: [ + WText( + 'Learn more', + className: ''' + text-sm font-medium + text-primary dark:text-primary-400 + ''', + ), + WIcon( + Icons.arrow_forward_outlined, + className: 'text-sm text-primary dark:text-primary-400', + ), + ], + ), + ), + ], + ), ); } } diff --git a/doc/basics/views-and-layouts.md b/doc/basics/views-and-layouts.md index 6ebde0ce..2cabcb2d 100644 --- a/doc/basics/views-and-layouts.md +++ b/doc/basics/views-and-layouts.md @@ -326,24 +326,25 @@ The shell wraps the route child in one `WDiv`, and two fields on `MagicStarterLa | Field | Default | What it decides | |-------|---------|-----------------| -| `contentClassName` | `'flex-1 overflow-y-auto'` | The content area's own className | -| `contentScrollPrimary` | `true` | Whether that area attaches to the ambient `PrimaryScrollController` | +| `contentClassName` | `'flex-1 min-h-0'` | The content area's own className | +| `contentScrollPrimary` | `false` | Whether that area attaches to the ambient `PrimaryScrollController` | -The default scrolls the child, which hands it an unbounded height. That suits a page as tall as its content and is wrong for a fill-shaped screen: an `h-full` column whose body takes the slack and scrolls inside itself resolves its height against infinity, fails to lay out, and renders nothing at all. A host whose screens are all that shape, which is what a television guide or a media catalogue is, sets: +The default is a box of bounded height that does not scroll, so **each routed page scrolls itself**. Every view this package ships already does, through `MSPageScaffold` (which brings its own `SingleChildScrollView(primary: false)`) or, for the invitation card, its own scroll view. A fill-shaped screen, an `h-full` column whose body takes the slack and scrolls inside itself (a television guide, a media catalogue), lays out under it as well. + +A page of your own that is taller than the window needs its own vertical scroll: put it through `MSPageScaffold`, or wrap it: ```dart -MagicStarter.useLayoutTheme( - const MagicStarterLayoutTheme( - contentClassName: 'flex-1 min-h-0', - contentScrollPrimary: false, - ), +MagicRoute.page( + '/reports', + // `primary: false`, as MSPageScaffold does: the page owns its controller. + () => const SingleChildScrollView(primary: false, child: ReportsView()), ); ``` -Set the two together. Wind reads `scrollPrimary` only where it builds a scroll view, so leaving it true beside a non-scrolling className claims nothing and is inert; it matters the moment the className scrolls horizontally, or scrolls in a way that belongs to the page rather than to the shell. Two `primary: true` scrollables in one tree contend for the single controller. +> [!WARNING] +> The default used to be `'flex-1 overflow-y-auto'` with `contentScrollPrimary: true`, and you can still set that pair. Know what it costs before you do. In a go_router shell the route child is the nested Navigator, so a scrolling content box scrolls the Navigator and lays its Overlay out under an unbounded height. A page left under a `.stacked()` route is then never laid out again, and its second rebuild while hidden (a controller notify, a resize, a locale switch) fails `_debugRelayoutBoundaryAlreadyMarkedNeedsLayout` in debug and leaves the tree inconsistent from there. The same setting also gives a fill-shaped screen an unbounded height, which it cannot lay out. See [#160](https://github.com/fluttersdk/magic_starter/issues/160) and [flutter/flutter#193247](https://github.com/flutter/flutter/issues/193247). -> [!NOTE] -> Every view this package ships goes through `MSPageScaffold`, which brings its own `SingleChildScrollView(primary: false)`. So the default nests two scrollables on each of them. The default stays as it is, because changing it would move layout for every existing host, but a host that puts every one of its own pages through `MSPageScaffold` can set `'flex-1 min-h-0'` and lose nothing. +Set the two fields together. Wind reads `scrollPrimary` only where it builds a scroll view, so beside a non-scrolling className it claims nothing and is inert; it matters the moment the className scrolls. Two `primary: true` scrollables in one tree contend for the single controller. ### An immersive route @@ -359,7 +360,7 @@ MagicRoute.group( ); ``` -No sidebar, no drawer, no header and no bottom bar. The route also gets the window with no safe-area inset and without the shell's own scroll container, since a surface that sizes itself cannot be handed unbounded height. The shell stays in the tree: its layout state, its notification polling and its auth listeners all survive the route, which wrapping the route in a bare page would throw away. +No sidebar, no drawer, no header and no bottom bar. The route also gets the window with no safe-area inset, since a surface that sizes itself wants the whole of it. The shell stays in the tree: its layout state, its notification polling and its auth listeners all survive the route, which wrapping the route in a bare page would throw away. ## GuestLayout diff --git a/lib/src/configuration/magic_starter_theme.dart b/lib/src/configuration/magic_starter_theme.dart index 431e4db8..58ec4c67 100644 --- a/lib/src/configuration/magic_starter_theme.dart +++ b/lib/src/configuration/magic_starter_theme.dart @@ -660,47 +660,43 @@ class MagicStarterLayoutTheme { /// Content area className: the box the shell hands the route child. /// - /// Defaults to `'flex-1 overflow-y-auto'`, which scrolls the child and so - /// hands it an unbounded height. That suits a page that is as tall as its - /// content and is wrong for a fill-shaped screen: an `h-full` column with a - /// body that scrolls inside itself resolves its height against infinity and - /// fails to lay out at all, rendering nothing. Such a host sets - /// `'flex-1 min-h-0'` and owns its own scrolling. - /// - /// Worth knowing before choosing: every view this package ships scrolls - /// itself (through `MSPageScaffold`, or its own `SingleChildScrollView` for - /// the invitation card), so the default nests two scrollables on each of - /// them. The default stays because changing it would move layout for every - /// existing host, but a host whose own pages scroll themselves can set - /// `'flex-1 min-h-0'` and lose nothing. - /// - /// A host that registers any `.stacked()` route under this layout should - /// make that change. In a go_router shell the route child is the nested - /// Navigator, so the default scrolls the Navigator and lays its Overlay out - /// under an unbounded height. The page left under a stacked route is then - /// never laid out again, and its second rebuild while hidden (a controller - /// notify, a resize, a locale switch) fails - /// `_debugRelayoutBoundaryAlreadyMarkedNeedsLayout` in debug and leaves the - /// tree inconsistent. With a non-scrolling content box, each routed page - /// scrolls itself instead. See #160 and flutter/flutter#193247. + /// Defaults to `'flex-1 min-h-0'`: a box of bounded height that does not + /// scroll, so each routed page scrolls itself. Every view this package + /// ships does (through `MSPageScaffold`, or its own `SingleChildScrollView` + /// for the invitation card), and a fill-shaped screen, an `h-full` column + /// whose body scrolls inside itself, lays out under it too. + /// + /// It used to be `'flex-1 overflow-y-auto'`, and that default was the + /// defect. In a go_router shell the route child is the nested Navigator, so + /// scrolling the child scrolled the Navigator and laid its Overlay out + /// under an unbounded height. The page left under a `.stacked()` route was + /// then never laid out again, and its second rebuild while hidden (a + /// controller notify, a resize, a locale switch) failed + /// `_debugRelayoutBoundaryAlreadyMarkedNeedsLayout` in debug and left the + /// tree inconsistent from there. See #160 and flutter/flutter#193247. + /// + /// A host whose own pages leaned on the shell for scrolling either wraps + /// them in a vertical scroll (or `MSPageScaffold`), or sets the old value + /// back, accepting that stacked routes then carry the hazard above. final String contentClassName; /// Whether the content area attaches to the ambient /// `PrimaryScrollController`. /// - /// Defaults to `true`, which is what the shell has always passed. Set it - /// false alongside a [contentClassName] that scrolls horizontally, or one - /// whose scrolling belongs to the page rather than to the shell: two + /// Defaults to `false`, since the default content box does not scroll: each + /// page's own scroll belongs to its route, and every route scopes its own + /// primary controller. Set it true alongside a [contentClassName] that + /// scrolls, which is the pairing the old default shipped. Two /// `primary: true` scrollables in one tree contend for the single /// controller, which is the `dropChild` cascade `MSPageScaffold` already /// avoids by passing `primary: false`. /// /// A separate field rather than a value derived from [contentClassName], /// for two measured reasons. Wind reads this flag only inside the branch - /// that builds a scroll view (`w_div.dart`), so leaving it true beside a - /// non-scrolling className claims nothing and is inert. And deriving it - /// would mean restating Wind's own overflow branch order here, which is a - /// copy that goes silently wrong the first time Wind reorders it. + /// that builds a scroll view (`w_div.dart`), so beside a non-scrolling + /// className it claims nothing and is inert. And deriving it would mean + /// restating Wind's own overflow branch order here, which is a copy that + /// goes silently wrong the first time Wind reorders it. final bool contentScrollPrimary; const MagicStarterLayoutTheme({ @@ -726,8 +722,8 @@ class MagicStarterLayoutTheme { this.brandBarClassName = 'h-14 px-5 flex items-center justify-between border-b border-gray-100 dark:border-gray-800', this.bottomNavClassName = '', - this.contentClassName = 'flex-1 overflow-y-auto', - this.contentScrollPrimary = true, + this.contentClassName = 'flex-1 min-h-0', + this.contentScrollPrimary = false, }); } diff --git a/lib/src/routes/notification_routes.dart b/lib/src/routes/notification_routes.dart index 8c9b9514..e59743ad 100644 --- a/lib/src/routes/notification_routes.dart +++ b/lib/src/routes/notification_routes.dart @@ -5,7 +5,7 @@ import 'package:magic_notifications/magic_notifications.dart'; import '../configuration/magic_starter_config.dart'; import '../facades/magic_starter.dart'; import '../ui/components/confirm_dialog/confirm_dialog.dart'; -import '../ui/components/page_container/page_container.dart'; +import '../ui/components/page_container/page_container.recipe.dart'; import '../ui/components/page_scaffold/page_scaffold.recipe.dart'; /// Registers notification routes provided by Magic Starter plugin. @@ -89,20 +89,21 @@ void _mountNotificationViews() { // geometry, so a null here is the whole ecosystem's answer. Left unpassed, // `Notify.deleteNotification` and the backend route behind it had no // surface anywhere: a working endpoint nothing could call. - // `contentClassName: ''` because the wrap below already carries the host's - // edge margins. Left at the package default the two pad the same edge and - // this one page sits twice as far from the display as its neighbours: - // measured on a phone at 32 logical pixels against the host's 16. + // The host's page geometry travels as `contentClassName`, which the view + // applies INSIDE its own scroll view; see [_inHostPageGeometry]. () => _inHostPageGeometry( - NotificationsListView(onDelete: _confirmThenDelete, contentClassName: ''), + (String geometry) => NotificationsListView( + onDelete: _confirmThenDelete, + contentClassName: geometry, + ), ), ); _mountUnlessOverridden( 'notifications.preferences', () => _inHostPageGeometry( - NotificationPreferencesView( + (String geometry) => NotificationPreferencesView( backRoute: MagicStarterConfig.settingsHubRoute(), - contentClassName: '', + contentClassName: geometry, ), ), ); @@ -188,16 +189,26 @@ void _mountUnlessOverridden(String key, Widget Function() builder) { Notify.view.register(key, builder); } -/// Wraps [view] in the host's shared page geometry. -/// -/// The surface fill sits OUTSIDE the container, exactly as [MSPageScaffold] -/// composes it, so the page token paints the whole content viewport rather -/// than only the capped column; the shell's own content background is a grey, -/// so a surface that stopped at the cap would show gutters no other page in the -/// app has. -Widget _inHostPageGeometry(Widget view) { +/// Builds a notification screen in the host's shared page geometry. +/// +/// The geometry is handed to the screen, which applies it inside its own +/// scroll view, rather than wrapped around it. The shell's content box does +/// not scroll, so the screen's scroll view is the page's scroll: wrapped +/// around it, the container's top and bottom padding inset the viewport and a +/// long list clipped at a hard line with the padding never scrolling away. +/// Handing it over also keeps the one set of edge margins; padded twice, this +/// page sat 32 logical pixels from a phone's edge against its neighbours' 16. +/// +/// The surface fill still sits outside, exactly as [MSPageScaffold] composes +/// it, so the page token paints the whole content viewport rather than only +/// the capped column. +Widget _inHostPageGeometry(Widget Function(String geometry) view) { return WDiv( className: pageScaffoldSurfaceRecipe(), - child: MSPageContainer(child: view), + child: view( + pageContainerRecipe( + hostClassName: MagicStarter.manager.pageContainerClassName, + ), + ), ); } diff --git a/lib/src/ui/components/page_scaffold/page_scaffold.recipe.dart b/lib/src/ui/components/page_scaffold/page_scaffold.recipe.dart index 694f4e67..7ec5f260 100644 --- a/lib/src/ui/components/page_scaffold/page_scaffold.recipe.dart +++ b/lib/src/ui/components/page_scaffold/page_scaffold.recipe.dart @@ -7,10 +7,10 @@ import 'package:magic/magic.dart'; /// not just the content height — otherwise everything below the last section /// shows the layout's grey content background. /// -/// Uses `min-h-full` (not `h-full`): the scaffold sits inside the app layout's -/// vertical scroll (`overflow-y-auto`), where a hard `h-full` resolves to an -/// unbounded height and Wind rejects it. `min-h-full` fills the viewport as a -/// floor while still growing with taller content. +/// Uses `min-h-full` (not `h-full`): a host whose content box still scrolls +/// (the pre-#160 default, available as an opt-in) hands the scaffold an +/// unbounded height, where a hard `h-full` does not resolve and Wind rejects +/// it. `min-h-full` fills the viewport as a floor under either shape. /// /// Emission order: base (width + min-height + surface token). String pageScaffoldSurfaceRecipe() { diff --git a/lib/src/ui/layouts/magic_starter_app_layout.dart b/lib/src/ui/layouts/magic_starter_app_layout.dart index f3648a57..f41d96eb 100644 --- a/lib/src/ui/layouts/magic_starter_app_layout.dart +++ b/lib/src/ui/layouts/magic_starter_app_layout.dart @@ -174,9 +174,8 @@ class _MagicStarterAppLayoutState extends State { final hasBottomNav = navConfig != null && navConfig.bottomItems.isNotEmpty; // An immersive route keeps the shell mounted (polling, auth listeners, the - // route key) and gives the window to its child: no bar, no safe-area inset - // and no scroll container, since a player or a map sizes itself and a - // scroll view would hand it unbounded height. + // route key) and gives the window to its child: no bar and no safe-area + // inset, since a player or a map sizes itself to the whole window. final hideChrome = MagicStarterHideChrome.of(context); // Responsive breakpoint via MediaQuery (wScreenIs reads MediaQuery.size), @@ -259,12 +258,11 @@ class _MagicStarterAppLayoutState extends State { children: [ _buildHeader(context, isDesktop), // The content box is the host's, because only the host - // knows the shape of its screens. The shipped - // `flex-1 overflow-y-auto` scrolls the child and so - // hands it an unbounded height, which a fill-shaped - // screen (an `h-full` column with a body that scrolls - // inside itself) cannot resolve: it fails to lay out and - // renders nothing. + // knows the shape of its screens. The shipped one does + // not scroll: the child is the shell's nested Navigator, + // and scrolling it gave its Overlay an unbounded height + // that breaks a page left under a stacked route (see + // `MagicStarterLayoutTheme.contentClassName`). WDiv( className: layoutTheme.contentClassName, scrollPrimary: layoutTheme.contentScrollPrimary, diff --git a/test/routes/notification_routes_test.dart b/test/routes/notification_routes_test.dart index f3a6affc..1c30b037 100644 --- a/test/routes/notification_routes_test.dart +++ b/test/routes/notification_routes_test.dart @@ -170,8 +170,15 @@ void main() { expect(find.byType(NotificationPreferencesView), findsOneWidget); // The width cap and the edge margins come from the host, and the package - // cannot resolve them; the mount point is what restores them. - expect(find.byType(MSPageContainer), findsOneWidget); + // cannot resolve them; the mount point is what hands them over. + expect( + tester + .widget( + find.byType(NotificationPreferencesView), + ) + .contentClassName, + contains(MagicStarter.manager.pageContainerClassName), + ); final view = tester.widget( find.byType(NotificationPreferencesView), @@ -191,7 +198,12 @@ void main() { await tester.pump(const Duration(milliseconds: 100)); expect(find.byType(NotificationsListView), findsOneWidget); - expect(find.byType(MSPageContainer), findsOneWidget); + expect( + tester + .widget(find.byType(NotificationsListView)) + .contentClassName, + contains(MagicStarter.manager.pageContainerClassName), + ); }); testWidgets('the list route wires the delete affordance', (tester) async { @@ -360,26 +372,42 @@ void main() { await tester.pump(const Duration(milliseconds: 100)); // `max-w-7xl px-4 lg:px-8` from the manager's default geometry: 1280 of - // column minus 32 of horizontal padding per side. + // column minus 32 of horizontal padding per side. Measured on the + // content column, which carries the geometry inside the screen's scroll. + final Finder column = find.descendant( + of: find.byType(NotificationPreferencesView), + matching: find.byWidgetPredicate( + (Widget widget) => + widget is WDiv && + (widget.className ?? '').contains( + MagicStarter.manager.pageContainerClassName, + ), + ), + ); + // The WDiv itself is the centring wrapper and spans the viewport; the + // box it constrains is the capped one. final width = tester - .getSize(find.byType(NotificationPreferencesView)) + .getSize( + find + .descendant( + of: column.first, + matching: find.byType(ConstrainedBox), + ) + .first, + ) .width; expect(width, lessThanOrEqualTo(1280)); expect(width, greaterThan(1000)); }); - testWidgets('both mounted screens hand their padding to the container', ( + testWidgets('both mounted screens scroll their page geometry with them', ( tester, ) async { - // `MSPageContainer` already carries the host's edge margins, so a screen - // that also pads its own content column puts these two pages twice as far - // from the display as every neighbour: measured on a phone at 32 logical - // pixels against the host's 16. - // - // Asserted on the argument rather than on a measured width because the - // two are not the same claim. A width assertion passes on a page whose - // margins happen to add up, and the doubled padding is a regression that - // compiles and renders. + // The shell's content box does not scroll, so each screen's own scroll + // view is the page's scroll. The host's page geometry has to sit INSIDE + // it: wrapped around it, the top and bottom padding inset the viewport, + // and a long list clipped at a hard line above the bottom bar with the + // padding never scrolling away (#160). fakeNotificationEndpoints(); registerMagicStarterNotificationRoutes(); @@ -390,6 +418,20 @@ void main() { await tester.pumpWidget(wrap(routeFor(route)!.buildWidget(const {}))); await tester.pump(); + final Rect page = tester.getRect(find.byType(Scaffold)); + final Rect scroll = tester.getRect( + find.byType(SingleChildScrollView).first, + ); + expect(scroll.top, page.top, reason: '$route insets its scroll top'); + expect( + scroll.bottom, + page.bottom, + reason: '$route insets its scroll bottom', + ); + + // And the geometry is the host's, handed to the screen rather than + // doubled around it: measured on a phone at 32 logical pixels + // against the host's 16 when both padded the same edge. final String passed = find.byType(NotificationsListView).evaluate().isNotEmpty ? tester @@ -402,8 +444,11 @@ void main() { find.byType(NotificationPreferencesView), ) .contentClassName; - - expect(passed, '', reason: '$route pads its own content column'); + expect( + passed, + contains(MagicStarter.manager.pageContainerClassName), + reason: '$route does not carry the host page geometry', + ); } }); diff --git a/test/ui/layouts/magic_starter_app_layout_test.dart b/test/ui/layouts/magic_starter_app_layout_test.dart index b0c10c6a..d357d0eb 100644 --- a/test/ui/layouts/magic_starter_app_layout_test.dart +++ b/test/ui/layouts/magic_starter_app_layout_test.dart @@ -1092,10 +1092,18 @@ void main() { } testWidgets( - 'the default content area scrolls and owns the primary scroll', + 'a scrolling content area scrolls and owns the primary scroll', (tester) async { useViewport(tester, 400, 800); + // The previous default, still available as an opt-in. + MagicStarter.useLayoutTheme( + const MagicStarterLayoutTheme( + contentClassName: 'flex-1 overflow-y-auto', + contentScrollPrimary: true, + ), + ); + await tester.pumpWidget( createApp(child: const SizedBox(key: Key('route-content'))), ); @@ -1114,18 +1122,11 @@ void main() { }, ); - testWidgets('a non-scrolling contentClassName leaves the child unscrolled', ( + testWidgets('the default content area leaves the child unscrolled', ( tester, ) async { useViewport(tester, 400, 800); - MagicStarter.useLayoutTheme( - const MagicStarterLayoutTheme( - contentClassName: 'flex-1 min-h-0', - contentScrollPrimary: false, - ), - ); - await tester.pumpWidget( createApp(child: const SizedBox(key: Key('route-content'))), ); @@ -1142,18 +1143,11 @@ void main() { ); }); - testWidgets('a fill-shaped child lays out under a non-scrolling content', ( + testWidgets('a fill-shaped child lays out under the default content', ( tester, ) async { useViewport(tester, 1440, 900); - MagicStarter.useLayoutTheme( - const MagicStarterLayoutTheme( - contentClassName: 'flex-1 min-h-0', - contentScrollPrimary: false, - ), - ); - await tester.pumpWidget(createApp(child: fillShapedScreen())); await tester.pumpAndSettle(); @@ -1162,17 +1156,24 @@ void main() { expect(find.text('Body'), findsOneWidget); }); - testWidgets('the shipped scrolling content area cannot hold that child', ( + testWidgets('a scrolling content area cannot hold that child', ( tester, ) async { useViewport(tester, 1440, 900); + MagicStarter.useLayoutTheme( + const MagicStarterLayoutTheme( + contentClassName: 'flex-1 overflow-y-auto', + contentScrollPrimary: true, + ), + ); + await tester.pumpWidget(createApp(child: fillShapedScreen())); await tester.pump(); - // The reason the field exists, asserted rather than described: under the - // default the same screen is handed an unbounded height and fails to lay - // out. A host whose screens are all this shape renders nothing. + // One of the reasons the default stopped scrolling, asserted rather than + // described: under a scrolling content area the same screen is handed an + // unbounded height and fails to lay out, rendering nothing. // // Asserted on the cause rather than on "something threw", so the test // cannot go on passing for a different reason. Wind names it here; a @@ -1184,7 +1185,7 @@ void main() { ); }); - test('the new theme fields default to today shell behaviour', () { + test('the theme fields default to the shell behaviour', () { const layout = MagicStarterLayoutTheme(); const navigation = MagicStarterNavigationTheme(); @@ -1195,8 +1196,9 @@ void main() { // was measured overflowing by exactly 1. expect(layout.sidebarCompactWidth, equals(80)); expect(navigation.focusItemClassName, equals('')); - expect(layout.contentClassName, equals('flex-1 overflow-y-auto')); - expect(layout.contentScrollPrimary, isTrue); + // A box that does not scroll, so each routed page scrolls itself (#160). + expect(layout.contentClassName, equals('flex-1 min-h-0')); + expect(layout.contentScrollPrimary, isFalse); }); }); @@ -1548,4 +1550,72 @@ void main() { expect(navigation.compactBrandBuilder, isNull); }); }); + // --------------------------------------------------------------------------- + // A page left under a stacked route (#160) + // --------------------------------------------------------------------------- + + group('MagicStarterAppLayout with a page under a stacked route', () { + testWidgets('the hidden page takes live rebuilds without breaking layout', ( + tester, + ) async { + // In a go_router shell the route child is the nested Navigator. A + // content box that scrolls hands its Overlay an unbounded height, the + // Overlay then never lays the hidden page out again, and that page's + // second rebuild fails `_debugRelayoutBoundaryAlreadyMarkedNeedsLayout` + // and leaves the tree inconsistent (flutter/flutter#193247). Under the + // shipped content box the Navigator gets a bounded height instead. + final ValueNotifier reading = ValueNotifier(0); + addTearDown(reading.dispose); + + final GoRouter router = GoRouter( + initialLocation: '/list', + routes: [ + ShellRoute( + builder: (context, state, child) => + MagicStarterAppLayout(child: child), + routes: [ + GoRoute( + path: '/list', + pageBuilder: (context, state) => NoTransitionPage( + child: ValueListenableBuilder( + valueListenable: reading, + builder: (context, value, _) => Row( + children: [ + const Expanded(child: Text('Checkout')), + SizedBox(width: 64, child: Text('${value}ms')), + ], + ), + ), + ), + ), + GoRoute( + path: '/list/detail', + pageBuilder: (context, state) => + const NoTransitionPage(child: Text('detail')), + ), + ], + ), + ], + ); + addTearDown(router.dispose); + + await tester.pumpWidget( + WindTheme( + data: WindThemeData(), + child: MaterialApp.router(routerConfig: router), + ), + ); + await tester.pumpAndSettle(); + router.push('/list/detail'); + await tester.pumpAndSettle(); + expect(find.text('detail'), findsOneWidget); + + for (int i = 1; i <= 3; i++) { + reading.value = i * 100; + await tester.pump(const Duration(milliseconds: 300)); + } + + expect(tester.takeException(), isNull); + }); + }); }