From 8a574fc0d656142bed006c25a8a718b08677f85c Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 2 Aug 2024 12:42:44 -0700 Subject: [PATCH 1/3] compose_box [nfc]: Factor out _ComposeBoxContainer --- lib/widgets/compose_box.dart | 75 +++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 30 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 1294fc787f..f0a7c91e73 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -844,6 +844,25 @@ class _SendButtonState extends State<_SendButton> { } } +class _ComposeBoxContainer extends StatelessWidget { + const _ComposeBoxContainer({required this.child}); + + final Widget child; + + @override + Widget build(BuildContext context) { + ColorScheme colorScheme = Theme.of(context).colorScheme; + + return Material( + color: colorScheme.surfaceContainerHighest, + child: SafeArea( + minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8), + child: Padding( + padding: const EdgeInsets.only(top: 8.0), + child: child))); + } +} + class _ComposeBoxLayout extends StatelessWidget { const _ComposeBoxLayout({ required this.topicInput, @@ -880,36 +899,32 @@ class _ComposeBoxLayout extends StatelessWidget { ), ); - return Material( - color: colorScheme.surfaceContainerHighest, - child: SafeArea( - minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8), - child: Padding( - padding: const EdgeInsets.only(top: 8.0), - child: blockingErrorBanner != null - ? SizedBox(width: double.infinity, child: blockingErrorBanner) - : Column(children: [ - Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ - Expanded( - child: Theme( - data: inputThemeData, - child: Column(children: [ - if (topicInput != null) topicInput!, - if (topicInput != null) const SizedBox(height: 8), - contentInput, - ]))), - const SizedBox(width: 8), - sendButton, - ]), - Theme( - data: themeData.copyWith( - iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)), - child: Row(children: [ - _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), - _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), - _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), - ])), - ])))); } + return _ComposeBoxContainer( + child: blockingErrorBanner != null + ? SizedBox(width: double.infinity, child: blockingErrorBanner) + : Column(children: [ + Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ + Expanded( + child: Theme( + data: inputThemeData, + child: Column(children: [ + if (topicInput != null) topicInput!, + if (topicInput != null) const SizedBox(height: 8), + contentInput, + ]))), + const SizedBox(width: 8), + sendButton, + ]), + Theme( + data: themeData.copyWith( + iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)), + child: Row(children: [ + _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), + ])), + ])); + } } abstract class ComposeBoxController extends State { From 8c862adba9a0c167462f7549cff7803e1d304f8d Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 2 Aug 2024 12:47:35 -0700 Subject: [PATCH 2/3] compose_box [nfc]: Move blockingErrorBanner from _ComposeBoxLayout to caller This lets _ComposeBoxLayout stay focused on the complexity of how the text inputs, the editing buttons, and the send button relate to each other. In the case where an error banner is present, this also lets us skip in the build method some logic to compute things that won't end up getting used. --- lib/widgets/compose_box.dart | 58 +++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index f0a7c91e73..6728743ca4 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -870,13 +870,11 @@ class _ComposeBoxLayout extends StatelessWidget { required this.sendButton, required this.contentController, required this.contentFocusNode, - this.blockingErrorBanner, }); final Widget? topicInput; final Widget contentInput; final Widget sendButton; - final Widget? blockingErrorBanner; final ComposeContentController contentController; final FocusNode contentFocusNode; @@ -900,30 +898,28 @@ class _ComposeBoxLayout extends StatelessWidget { ); return _ComposeBoxContainer( - child: blockingErrorBanner != null - ? SizedBox(width: double.infinity, child: blockingErrorBanner) - : Column(children: [ - Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ - Expanded( - child: Theme( - data: inputThemeData, - child: Column(children: [ - if (topicInput != null) topicInput!, - if (topicInput != null) const SizedBox(height: 8), - contentInput, - ]))), - const SizedBox(width: 8), - sendButton, - ]), - Theme( - data: themeData.copyWith( - iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)), - child: Row(children: [ - _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), - _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), - _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), - ])), - ])); + child: Column(children: [ + Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ + Expanded( + child: Theme( + data: inputThemeData, + child: Column(children: [ + if (topicInput != null) topicInput!, + if (topicInput != null) const SizedBox(height: 8), + contentInput, + ]))), + const SizedBox(width: 8), + sendButton, + ]), + Theme( + data: themeData.copyWith( + iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)), + child: Row(children: [ + _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), + ])), + ])); } } @@ -1054,6 +1050,13 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox @override Widget build(BuildContext context) { + final errorBanner = _errorBanner(context); + if (errorBanner != null) { + return _ComposeBoxContainer( + child: SizedBox(width: double.infinity, + child: errorBanner)); + } + return _ComposeBoxLayout( contentController: _contentController, contentFocusNode: _contentFocusNode, @@ -1067,8 +1070,7 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox topicController: null, contentController: _contentController, getDestination: () => widget.narrow.destination, - ), - blockingErrorBanner: _errorBanner(context)); + )); } } From 96b4f0895fe7434bd87f919c8c9f43403d24a7a5 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 2 Aug 2024 13:02:12 -0700 Subject: [PATCH 3/3] compose_box [nfc]: Control width at _ComposeBoxContainer This way the caller doesn't have to do so. This change is NFC because in the non-error-banner case, where the caller is _ComposeBoxLayout, the child already fills the available horizontal space because of the Expanded inside the Row. --- lib/widgets/compose_box.dart | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 6728743ca4..1c62bc237b 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -853,13 +853,16 @@ class _ComposeBoxContainer extends StatelessWidget { Widget build(BuildContext context) { ColorScheme colorScheme = Theme.of(context).colorScheme; - return Material( - color: colorScheme.surfaceContainerHighest, - child: SafeArea( - minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8), - child: Padding( - padding: const EdgeInsets.only(top: 8.0), - child: child))); + // TODO(design): Maybe put a max width on the compose box, like we do on + // the message list itself + return SizedBox(width: double.infinity, + child: Material( + color: colorScheme.surfaceContainerHighest, + child: SafeArea( + minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8), + child: Padding( + padding: const EdgeInsets.only(top: 8.0), + child: child)))); } } @@ -1052,9 +1055,7 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox Widget build(BuildContext context) { final errorBanner = _errorBanner(context); if (errorBanner != null) { - return _ComposeBoxContainer( - child: SizedBox(width: double.infinity, - child: errorBanner)); + return _ComposeBoxContainer(child: errorBanner); } return _ComposeBoxLayout(