From 7de70436442cc9218ac90b7e1b0f35330efdb186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Sun, 16 Aug 2026 17:28:53 +0300 Subject: [PATCH 1/2] Let the theme turn a page header inline, so its two halves cannot drift `MSPageHeader.inlineActions` does two things at once: it swaps `containerClassName` for `containerInlineClassName`, and it gives the title row `flex-1 min-w-0` instead of only `sm:flex-1`. Those are two halves of one decision, and until now a consumer could set the first and not the second, because the container class is a theme string while the flex behaviour is a widget argument. **That combination silently overflows.** An app that themes the container into a row at every width, so a phone header keeps its action beside the title rather than dropping it under the subtitle, leaves the title row without `flex-1` below `sm`. The title column is `flex-initial`, a loose fit, so the text takes its intrinsic width and runs past the actions. Measured in depools at 40 logical pixels on a 390px viewport with a two-word title and three icon buttons, and reproduced here at 79. `line-clamp-2` on the title cannot save it, for the same reason `truncate` cannot without a constrained box. `MSPageScaffold` does not forward `inlineActions` at all, so a scaffold consumer had no way to reach the second half even knowing it existed. Rather than threading an argument through the scaffold and asking every screen to pass it, the flag moves to `MagicStarterPageHeaderTheme`, beside the container class that requires it: one place, stated once, and the two cannot drift apart. `inlineActions` becomes `bool?` and falls back to the theme, so an explicit argument still beats it and every existing caller is unchanged. The overflow itself is a test. Reverting the title-row half turns it red with `A RenderFlex overflowed by 79 pixels on the right`, which is the defect rather than a proxy for it. --- .claude/rules/widgets.md | 1 + .../configuration/magic_starter_theme.dart | 30 +++++ .../components/page_header/page_header.dart | 21 +++- .../page_header/page_header_test.dart | 107 ++++++++++++++++++ 4 files changed, 154 insertions(+), 5 deletions(-) diff --git a/.claude/rules/widgets.md b/.claude/rules/widgets.md index 3aa16f0..e044720 100644 --- a/.claude/rules/widgets.md +++ b/.claude/rules/widgets.md @@ -15,6 +15,7 @@ path: "lib/src/ui/widgets/**/*.dart" - Two-factor modal: multi-step wizard (enable → QR code → OTP confirm → recovery codes) - Social divider: `WDiv` + `WText('or')` centered — used between form and social login buttons - Page header: `WDiv` with `flex-col sm:flex-row` responsive layout, `border-b` separator, required `title`, optional `subtitle` (`String?`), optional `leading` widget (e.g. back button), optional `actions` (`List?`) — rendered in a trailing `flex flex-row gap-2` row only when non-empty +- Page header inline mode: `inlineActions` is `bool?` and falls back to `MagicStarterPageHeaderTheme.inlineActions`. It does TWO things and both are required together: it swaps `containerClassName` for `containerInlineClassName`, and it gives the title row `flex-1 min-w-0` instead of `sm:flex-1`. An app that themes the container into a row at every width and does not set the flag leaves the title row without `flex-1` below `sm`; the title column is `flex-initial` (a loose fit), so the text takes its intrinsic width and overflows. `MSPageScaffold` does not expose the argument, which is why the theme field exists - User profile dropdown: `PopupMenuButton` with avatar, name, role — navigates to profile/logout - User profile dropdown avatar: uses `MagicStarter.navigationTheme.dropdownAvatarClassName` for the trigger avatar background — override via `MagicStarter.useNavigationTheme()` - Confirm dialog: `MagicStarterConfirmDialog` with `static Future show(BuildContext context, {required String title, String? description, String? confirmLabel, String? cancelLabel, ConfirmDialogVariant variant, Future Function()? onConfirm})`; variant enum `ConfirmDialogVariant.primary` (default), `.danger`, `.warning` — controls confirm button styling diff --git a/lib/src/configuration/magic_starter_theme.dart b/lib/src/configuration/magic_starter_theme.dart index d50c5e7..c6a1d77 100644 --- a/lib/src/configuration/magic_starter_theme.dart +++ b/lib/src/configuration/magic_starter_theme.dart @@ -428,6 +428,35 @@ class MagicStarterPageHeaderTheme { /// `'flex items-center justify-center size-9 -ml-1 text-2xl text-fg-muted hover:text-fg'`. final String backControlClassName; + /// Whether every [MSPageHeader] lays its title and actions out on ONE row. + /// + /// Defaults to `false`, which keeps the responsive behaviour: stacked below + /// `sm`, a row above it. + /// + /// ### Why this is a theme field and not only a widget parameter + /// + /// [MSPageHeader.inlineActions] already existed, and it does two things at + /// once: it swaps [containerClassName] for [containerInlineClassName], and it + /// gives the title row `flex-1 min-w-0` unconditionally instead of only under + /// `sm:`. Those are two halves of ONE decision, and until now a consumer could + /// set the first half and not the second, because the container class is a + /// theme string while the flex behaviour is a widget argument. + /// + /// **That combination silently overflows.** An app that overrides + /// [containerClassName] to `flex-row` at every width, so a phone header keeps + /// its action beside the title, leaves the title row without `flex-1` below + /// `sm`. The title column is `flex-initial`, a loose fit, so the text takes + /// its intrinsic width, and a title long enough to need the space runs past + /// the edge: measured at 40 logical pixels on a 390px viewport with a + /// two-word title and three icon buttons. `line-clamp-2` on the title cannot + /// save it, for the same reason `truncate` cannot without a constrained box. + /// + /// Putting the flag here means the app states the layout intent once, beside + /// the container class that requires it, instead of remembering to pass an + /// argument on every screen. [MSPageScaffold] does not forward the widget + /// parameter at all, so a consumer using the scaffold had no way to reach it. + final bool inlineActions; + const MagicStarterPageHeaderTheme({ this.containerClassName = 'w-full flex flex-col sm:flex-row items-start sm:items-center sm:justify-between gap-4 p-2 lg:p-4 border-b border-gray-200 dark:border-gray-700', @@ -440,6 +469,7 @@ class MagicStarterPageHeaderTheme { this.actionContainerClassName = 'flex flex-row items-center gap-2', this.backControlClassName = 'flex items-center justify-center size-9 -ml-1 text-2xl text-fg-muted hover:text-fg', + this.inlineActions = false, }); } diff --git a/lib/src/ui/components/page_header/page_header.dart b/lib/src/ui/components/page_header/page_header.dart index 136009e..40cf62a 100644 --- a/lib/src/ui/components/page_header/page_header.dart +++ b/lib/src/ui/components/page_header/page_header.dart @@ -57,8 +57,19 @@ class MSPageHeader extends StatelessWidget { final Widget? titleSuffix; /// When `true`, the outer container uses `flex-row` instead of the default - /// responsive `flex-col sm:flex-row` stacked layout. - final bool inlineActions; + /// responsive `flex-col sm:flex-row` stacked layout, and the title row claims + /// the remaining width with `flex-1 min-w-0` so a long title SHRINKS instead + /// of pushing past the actions. + /// + /// Null (the default) reads + /// `MagicStarter.pageHeaderTheme.inlineActions`, so an app that has themed + /// the container into a row at every width states that once rather than on + /// every screen. [MSPageScaffold] does not expose this argument at all, which + /// is why the theme is the only reachable switch for a scaffold consumer. + final bool? inlineActions; + + /// Whether this header lays out on one row, resolving the theme default. + bool get isInline => inlineActions ?? MagicStarter.pageHeaderTheme.inlineActions; /// Back-affordance label (e.g. `'Settings'`). /// @@ -82,7 +93,7 @@ class MSPageHeader extends StatelessWidget { this.leading, this.actions, this.titleSuffix, - this.inlineActions = false, + this.inlineActions, this.backLabel, this.backFallback, }); @@ -119,13 +130,13 @@ class MSPageHeader extends StatelessWidget { leading ?? (backLabel != null ? _buildBackControl(context) : null); return WDiv( - className: inlineActions + className: isInline ? MagicStarter.pageHeaderTheme.containerInlineClassName : MagicStarter.pageHeaderTheme.containerClassName, children: [ // 1. Title row: optional leading + title column + optional titleSuffix. WDiv( - className: inlineActions + className: isInline ? 'flex flex-row items-center gap-3 flex-1 min-w-0' : 'flex flex-row items-center gap-3 sm:flex-1 min-w-0', children: [ diff --git a/test/ui/components/page_header/page_header_test.dart b/test/ui/components/page_header/page_header_test.dart index 0055af8..132173b 100644 --- a/test/ui/components/page_header/page_header_test.dart +++ b/test/ui/components/page_header/page_header_test.dart @@ -147,6 +147,113 @@ void main() { expect(outerDiv.className, contains('sm:flex-row')); }); + testWidgets('the theme can turn every header inline without an argument', + (tester) async { + // An app that themes the container into a row at every width has to be able + // to say so once. `MSPageScaffold` does not expose `inlineActions`, so + // before this the theme was the only half of the decision a scaffold + // consumer could set, and setting it alone is what overflows. + MagicStarter.usePageHeaderTheme( + const MagicStarterPageHeaderTheme(inlineActions: true), + ); + + await tester.pumpWidget( + wrap( + MSPageHeader( + title: 'Themed inline', + actions: [ + ElevatedButton(onPressed: () {}, child: const Text('Go')), + ], + ), + ), + ); + + final outerDiv = tester.widget(find.byType(WDiv).first); + expect(outerDiv.className, contains('flex-row')); + expect(outerDiv.className, isNot(contains('flex-col'))); + }); + + testWidgets('inline mode gives the title row flex-1 so a long title shrinks', + (tester) async { + // **The half that was missing, and the one that actually overflows.** + // `inlineActions` swaps the container class AND claims the remaining width + // for the title row. The title column is `flex-initial`, a loose fit, so + // without `flex-1` on the row the text takes its intrinsic width and runs + // past the actions: measured at 40 logical pixels on a 390px viewport. + MagicStarter.usePageHeaderTheme( + const MagicStarterPageHeaderTheme(inlineActions: true), + ); + + await tester.pumpWidget( + wrap( + MSPageHeader( + title: 'A title long enough to need the whole row to itself', + actions: [ + ElevatedButton(onPressed: () {}, child: const Text('Go')), + ], + ), + ), + ); + + final titleRow = tester.widgetList(find.byType(WDiv)).elementAt(1); + + expect(titleRow.className, contains('flex-1')); + expect(titleRow.className, isNot(contains('sm:flex-1'))); + expect(titleRow.className, contains('min-w-0')); + }); + + testWidgets('an explicit argument still beats the theme', (tester) async { + // The theme is a default rather than a lock: a single screen that wants the + // stacked layout can still ask for it. + MagicStarter.usePageHeaderTheme( + const MagicStarterPageHeaderTheme(inlineActions: true), + ); + + await tester.pumpWidget( + wrap( + MSPageHeader( + title: 'Explicitly stacked', + inlineActions: false, + actions: [ + ElevatedButton(onPressed: () {}, child: const Text('Go')), + ], + ), + ), + ); + + final outerDiv = tester.widget(find.byType(WDiv).first); + expect(outerDiv.className, contains('flex-col')); + }); + + testWidgets('a header does not overflow at phone width when themed inline', + (tester) async { + // **The defect itself, as a test.** Depools themed the container into a row + // at every width so a phone header keeps its action beside the title, and + // the product screen then reported `A RenderFlex overflowed by 40 pixels` + // at 390. Nothing in the app's own code was in that row. + tester.view.physicalSize = const Size(390, 844); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + + MagicStarter.usePageHeaderTheme( + const MagicStarterPageHeaderTheme(inlineActions: true), + ); + + await tester.pumpWidget( + wrap( + MSPageHeader( + title: 'Dishwasher Tablets', + actions: [ + for (int i = 0; i < 3; i++) + const SizedBox(width: 44, height: 44, child: Icon(Icons.more_horiz)), + ], + ), + ), + ); + + expect(tester.takeException(), isNull); + }); + // --------------------------------------------------------------------------- // Theme consumption // --------------------------------------------------------------------------- From d03658eb6423241e2523317a4856aadedaebd4aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Sun, 16 Aug 2026 17:31:38 +0300 Subject: [PATCH 2/2] Format to dart format, which this repo gates on --- lib/src/ui/components/page_header/page_header.dart | 3 ++- test/ui/components/page_header/page_header_test.dart | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/src/ui/components/page_header/page_header.dart b/lib/src/ui/components/page_header/page_header.dart index 40cf62a..e3eb2b5 100644 --- a/lib/src/ui/components/page_header/page_header.dart +++ b/lib/src/ui/components/page_header/page_header.dart @@ -69,7 +69,8 @@ class MSPageHeader extends StatelessWidget { final bool? inlineActions; /// Whether this header lays out on one row, resolving the theme default. - bool get isInline => inlineActions ?? MagicStarter.pageHeaderTheme.inlineActions; + bool get isInline => + inlineActions ?? MagicStarter.pageHeaderTheme.inlineActions; /// Back-affordance label (e.g. `'Settings'`). /// diff --git a/test/ui/components/page_header/page_header_test.dart b/test/ui/components/page_header/page_header_test.dart index 132173b..d832462 100644 --- a/test/ui/components/page_header/page_header_test.dart +++ b/test/ui/components/page_header/page_header_test.dart @@ -245,7 +245,8 @@ void main() { title: 'Dishwasher Tablets', actions: [ for (int i = 0; i < 3; i++) - const SizedBox(width: 44, height: 44, child: Icon(Icons.more_horiz)), + const SizedBox( + width: 44, height: 44, child: Icon(Icons.more_horiz)), ], ), ),