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..e3eb2b5 100644 --- a/lib/src/ui/components/page_header/page_header.dart +++ b/lib/src/ui/components/page_header/page_header.dart @@ -57,8 +57,20 @@ 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 +94,7 @@ class MSPageHeader extends StatelessWidget { this.leading, this.actions, this.titleSuffix, - this.inlineActions = false, + this.inlineActions, this.backLabel, this.backFallback, }); @@ -119,13 +131,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..d832462 100644 --- a/test/ui/components/page_header/page_header_test.dart +++ b/test/ui/components/page_header/page_header_test.dart @@ -147,6 +147,114 @@ 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 // ---------------------------------------------------------------------------