From 236fb1f668e6942c64b5685f5dc58a3bd2d9ca21 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Tue, 28 Jul 2026 12:30:27 -0400 Subject: [PATCH] feat(remix): add structured dialog scrolling Related to #89. --- .../lib/src/components/dialog/dialog.g.dart | 4 + .../src/components/dialog/dialog_style.dart | 2 + .../src/components/dialog/dialog_widget.dart | 66 +++++++-- .../components/dialog/dialog_style_test.dart | 2 + .../components/dialog/dialog_widget_test.dart | 139 +++++++++++++++++- 5 files changed, 199 insertions(+), 14 deletions(-) diff --git a/packages/remix/lib/src/components/dialog/dialog.g.dart b/packages/remix/lib/src/components/dialog/dialog.g.dart index e39ddabd..adcf5525 100644 --- a/packages/remix/lib/src/components/dialog/dialog.g.dart +++ b/packages/remix/lib/src/components/dialog/dialog.g.dart @@ -107,6 +107,7 @@ class FortalDialog extends StatelessWidget { this.title, this.description, this.actions, + this.scrollable = false, this.modal = true, this.semanticLabel, }); @@ -119,6 +120,8 @@ class FortalDialog extends StatelessWidget { final List? actions; + final bool scrollable; + final bool modal; final String? semanticLabel; @@ -132,6 +135,7 @@ class FortalDialog extends StatelessWidget { title: this.title, description: this.description, actions: this.actions, + scrollable: this.scrollable, modal: this.modal, semanticLabel: this.semanticLabel, ); diff --git a/packages/remix/lib/src/components/dialog/dialog_style.dart b/packages/remix/lib/src/components/dialog/dialog_style.dart index bdb1c113..913f6528 100644 --- a/packages/remix/lib/src/components/dialog/dialog_style.dart +++ b/packages/remix/lib/src/components/dialog/dialog_style.dart @@ -18,6 +18,7 @@ extension RemixDialogStylerRemixHelpers on RemixDialogStyler { String? title, String? description, List? actions, + bool scrollable = false, bool modal = true, String? semanticLabel, }) { @@ -26,6 +27,7 @@ extension RemixDialogStylerRemixHelpers on RemixDialogStyler { title: title, description: description, actions: actions, + scrollable: scrollable, modal: modal, semanticLabel: semanticLabel, style: this, diff --git a/packages/remix/lib/src/components/dialog/dialog_widget.dart b/packages/remix/lib/src/components/dialog/dialog_widget.dart index 38d0f758..8f8bd5a4 100644 --- a/packages/remix/lib/src/components/dialog/dialog_widget.dart +++ b/packages/remix/lib/src/components/dialog/dialog_widget.dart @@ -130,6 +130,7 @@ class RemixDialog extends StatelessWidget { this.title, this.description, this.actions, + this.scrollable = false, this.modal = true, this.semanticLabel, this.style = const RemixDialogStyler.create(), @@ -154,6 +155,13 @@ class RemixDialog extends StatelessWidget { /// Action buttons (typically placed at the bottom). final List? actions; + /// Whether structured body content can scroll within a bounded dialog. + /// + /// When true, [description] and [child] share a vertical scroll region while + /// [title] and [actions] remain outside it. A lone [child] remains fully + /// caller-owned and is never wrapped in the structured scroll region. + final bool scrollable; + /// Whether to block background content interaction. final bool modal; @@ -186,22 +194,54 @@ class RemixDialog extends StatelessWidget { return Box(styleSpec: spec.container, child: child!); } + final hasBody = description != null || child != null; + final titleWidget = title == null + ? null + : StyledText(title!, styleSpec: spec.title); + final bodyChildren = [ + if (description != null) + StyledText(description!, styleSpec: spec.description), + ?child, + ]; + final actionsWidget = hasActions + ? FlexBox(styleSpec: spec.actions, children: actions!) + : null; + // title → description → child → actions; never discard provided content. return Box( styleSpec: spec.container, - child: Column( - mainAxisAlignment: .start, - mainAxisSize: .min, - crossAxisAlignment: .start, - children: [ - if (title != null) StyledText(title!, styleSpec: spec.title), - if (description != null) - StyledText(description!, styleSpec: spec.description), - ?child, - if (hasActions) - FlexBox(styleSpec: spec.actions, children: actions!), - ], - ), + child: scrollable && hasBody + ? LayoutBuilder( + builder: (context, constraints) { + final body = SingleChildScrollView( + child: Column( + mainAxisSize: .min, + crossAxisAlignment: .start, + children: bodyChildren, + ), + ); + + return Column( + mainAxisAlignment: .start, + mainAxisSize: .min, + crossAxisAlignment: .start, + children: [ + ?titleWidget, + if (constraints.hasBoundedHeight) + Flexible(fit: FlexFit.loose, child: body) + else + body, + ?actionsWidget, + ], + ); + }, + ) + : Column( + mainAxisAlignment: .start, + mainAxisSize: .min, + crossAxisAlignment: .start, + children: [?titleWidget, ...bodyChildren, ?actionsWidget], + ), ); }, ); diff --git a/packages/remix/test/components/dialog/dialog_style_test.dart b/packages/remix/test/components/dialog/dialog_style_test.dart index ea576404..91a9c599 100644 --- a/packages/remix/test/components/dialog/dialog_style_test.dart +++ b/packages/remix/test/components/dialog/dialog_style_test.dart @@ -311,6 +311,7 @@ void main() { title: 'Confirm', description: 'Continue?', actions: const [Text('OK')], + scrollable: true, modal: false, semanticLabel: 'Confirmation dialog', ); @@ -320,6 +321,7 @@ void main() { expect(dialog.title, 'Confirm'); expect(dialog.description, 'Continue?'); expect(dialog.actions, hasLength(1)); + expect(dialog.scrollable, isTrue); expect(dialog.modal, isFalse); expect(dialog.semanticLabel, 'Confirmation dialog'); }); diff --git a/packages/remix/test/components/dialog/dialog_widget_test.dart b/packages/remix/test/components/dialog/dialog_widget_test.dart index eb61df48..87e6e85c 100644 --- a/packages/remix/test/components/dialog/dialog_widget_test.dart +++ b/packages/remix/test/components/dialog/dialog_widget_test.dart @@ -450,10 +450,13 @@ void main() { }); testWidgets('a lone child fills the container directly', (tester) async { - await tester.pumpRemixApp(RemixDialog(child: Text('Only child'))); + await tester.pumpRemixApp( + RemixDialog(child: Text('Only child'), scrollable: true), + ); await tester.pumpAndSettle(); expect(find.text('Only child'), findsOneWidget); + expect(find.byType(SingleChildScrollView), findsNothing); expect( find.descendant(of: find.byType(Box), matching: find.byType(Column)), findsNothing, @@ -482,9 +485,143 @@ void main() { expect(titleY, lessThan(descY)); expect(descY, lessThan(bodyY)); expect(bodyY, lessThan(actionY)); + expect(find.byType(SingleChildScrollView), findsNothing); }, ); + testWidgets('bounded large-text structured content does not overflow', ( + tester, + ) async { + final semantics = tester.ensureSemantics(); + final bodyFocus = FocusNode(debugLabel: 'final environment variable'); + final actionFocus = FocusNode(debugLabel: 'save environment'); + addTearDown(bodyFocus.dispose); + addTearDown(actionFocus.dispose); + + try { + await tester.pumpRemixApp( + MediaQuery( + data: const MediaQueryData(textScaler: TextScaler.linear(2)), + child: SizedBox( + width: 400, + height: 320, + child: FortalDialog( + title: 'Environment', + description: 'Variables available to this workspace.', + scrollable: true, + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: List.generate( + 8, + (index) => index == 7 + ? TextButton( + focusNode: bodyFocus, + onPressed: () {}, + child: const Text('Environment variable 8'), + ) + : Text('Environment variable ${index + 1}'), + ), + ), + actions: [ + IconButton( + key: const ValueKey('save-environment-action'), + focusNode: actionFocus, + tooltip: 'Save environment', + onPressed: () {}, + icon: const Icon(Icons.save), + ), + ], + ), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect( + tester.widget(find.byType(FortalDialog)).scrollable, + isTrue, + ); + expect( + tester.widget(find.byType(RemixDialog)).scrollable, + isTrue, + ); + + final scrollView = find.byType(SingleChildScrollView); + final action = find.byKey(const ValueKey('save-environment-action')); + expect(scrollView, findsOneWidget); + expect( + find.descendant(of: scrollView, matching: find.text('Environment')), + findsNothing, + ); + expect( + find.descendant(of: scrollView, matching: action), + findsNothing, + ); + + final scrollable = find.descendant( + of: scrollView, + matching: find.byType(Scrollable), + ); + final position = tester.state(scrollable).position; + final actionY = tester.getTopLeft(action).dy; + expect(position.maxScrollExtent, greaterThan(0)); + + await tester.drag(scrollView, const Offset(0, -1000)); + await tester.pumpAndSettle(); + + expect(position.pixels, greaterThan(0)); + expect(tester.getTopLeft(action).dy, closeTo(actionY, 0.01)); + expect( + find.bySemanticsLabel('Environment variable 8'), + findsOneWidget, + ); + expect( + tester.getSemantics(action), + isSemantics( + tooltip: 'Save environment', + isButton: true, + hasTapAction: true, + ), + ); + + bodyFocus.requestFocus(); + await tester.pumpAndSettle(); + expect(bodyFocus.hasFocus, isTrue); + + for ( + var attempt = 0; + attempt < 2 && !actionFocus.hasFocus; + attempt++ + ) { + await tester.sendKeyEvent(LogicalKeyboardKey.tab); + await tester.pumpAndSettle(); + } + expect(actionFocus.hasFocus, isTrue); + } finally { + semantics.dispose(); + } + }); + + testWidgets('scrollable content shrink-wraps unbounded height', ( + tester, + ) async { + await tester.pumpRemixApp( + SingleChildScrollView( + child: RemixDialog( + title: 'Environment', + description: 'Variables available to this workspace.', + scrollable: true, + ), + ), + ); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + expect(find.byType(SingleChildScrollView), findsNWidgets(2)); + expect(find.byType(Flexible), findsNothing); + }); + testWidgets('title and description are rendered together', ( tester, ) async {