Skip to content

Commit 4214005

Browse files
committed
compose_box: Wait for send message request to complete.
This supports the new UX of disabling the topic input, content input and the buttons on the compose box when sending a message, and only re-enable them and clearing the content field when the message has been successfully sent. Signed-off-by: Zixuan James Li <[email protected]>
1 parent eb1c818 commit 4214005

File tree

3 files changed

+111
-21
lines changed

3 files changed

+111
-21
lines changed

lib/widgets/compose_box.dart

+80-19
Original file line numberDiff line numberDiff line change
@@ -275,12 +275,14 @@ class _ContentInput extends StatelessWidget {
275275
required this.controller,
276276
required this.focusNode,
277277
required this.hintText,
278+
required this.readOnly,
278279
});
279280

280281
final Narrow narrow;
281282
final ComposeContentController controller;
282283
final FocusNode focusNode;
283284
final String hintText;
285+
final bool readOnly;
284286

285287
@override
286288
Widget build(BuildContext context) {
@@ -303,10 +305,11 @@ class _ContentInput extends StatelessWidget {
303305
return TextField(
304306
controller: controller,
305307
focusNode: focusNode,
306-
style: TextStyle(color: colorScheme.onSurface),
308+
style: TextStyle(color: readOnly ? colorScheme.onSurface.withOpacity(0.5) : colorScheme.onSurface),
307309
decoration: InputDecoration.collapsed(hintText: hintText),
308310
maxLines: null,
309311
textCapitalization: TextCapitalization.sentences,
312+
readOnly: readOnly,
310313
);
311314
}),
312315
));
@@ -320,12 +323,14 @@ class _StreamContentInput extends StatefulWidget {
320323
required this.controller,
321324
required this.topicController,
322325
required this.focusNode,
326+
required this.readOnly,
323327
});
324328

325329
final ChannelNarrow narrow;
326330
final ComposeContentController controller;
327331
final ComposeTopicController topicController;
328332
final FocusNode focusNode;
333+
final bool readOnly;
329334

330335
@override
331336
State<_StreamContentInput> createState() => _StreamContentInputState();
@@ -372,7 +377,8 @@ class _StreamContentInputState extends State<_StreamContentInput> {
372377
narrow: widget.narrow,
373378
controller: widget.controller,
374379
focusNode: widget.focusNode,
375-
hintText: zulipLocalizations.composeBoxChannelContentHint(streamName, _topicTextNormalized));
380+
hintText: zulipLocalizations.composeBoxChannelContentHint(streamName, _topicTextNormalized),
381+
readOnly: widget.readOnly);
376382
}
377383
}
378384

@@ -381,12 +387,15 @@ class _TopicInput extends StatelessWidget {
381387
required this.streamId,
382388
required this.controller,
383389
required this.focusNode,
384-
required this.contentFocusNode});
390+
required this.contentFocusNode,
391+
required this.readOnly,
392+
});
385393

386394
final int streamId;
387395
final ComposeTopicController controller;
388396
final FocusNode focusNode;
389397
final FocusNode contentFocusNode;
398+
final bool readOnly;
390399

391400
@override
392401
Widget build(BuildContext context) {
@@ -404,6 +413,7 @@ class _TopicInput extends StatelessWidget {
404413
textInputAction: TextInputAction.next,
405414
style: TextStyle(color: colorScheme.onSurface),
406415
decoration: InputDecoration(hintText: zulipLocalizations.composeBoxTopicHintText),
416+
readOnly: readOnly,
407417
));
408418
}
409419
}
@@ -413,11 +423,13 @@ class _FixedDestinationContentInput extends StatelessWidget {
413423
required this.narrow,
414424
required this.controller,
415425
required this.focusNode,
426+
required this.readOnly,
416427
});
417428

418429
final SendableNarrow narrow;
419430
final ComposeContentController controller;
420431
final FocusNode focusNode;
432+
final bool readOnly;
421433

422434
String _hintText(BuildContext context) {
423435
final zulipLocalizations = ZulipLocalizations.of(context);
@@ -448,7 +460,8 @@ class _FixedDestinationContentInput extends StatelessWidget {
448460
narrow: narrow,
449461
controller: controller,
450462
focusNode: focusNode,
451-
hintText: _hintText(context));
463+
hintText: _hintText(context),
464+
readOnly: readOnly);
452465
}
453466
}
454467

@@ -538,10 +551,15 @@ Future<void> _uploadFiles({
538551
}
539552

540553
abstract class _AttachUploadsButton extends StatelessWidget {
541-
const _AttachUploadsButton({required this.contentController, required this.contentFocusNode});
554+
const _AttachUploadsButton({
555+
required this.contentController,
556+
required this.contentFocusNode,
557+
required this.disabled,
558+
});
542559

543560
final ComposeContentController contentController;
544561
final FocusNode contentFocusNode;
562+
final bool disabled;
545563

546564
IconData get icon;
547565
String tooltip(ZulipLocalizations zulipLocalizations);
@@ -580,7 +598,7 @@ abstract class _AttachUploadsButton extends StatelessWidget {
580598
return IconButton(
581599
icon: Icon(icon),
582600
tooltip: tooltip(zulipLocalizations),
583-
onPressed: () => _handlePress(context));
601+
onPressed: disabled ? null : () => _handlePress(context));
584602
}
585603
}
586604

@@ -639,7 +657,11 @@ Future<Iterable<_File>> _getFilePickerFiles(BuildContext context, FileType type)
639657
}
640658

641659
class _AttachFileButton extends _AttachUploadsButton {
642-
const _AttachFileButton({required super.contentController, required super.contentFocusNode});
660+
const _AttachFileButton({
661+
required super.contentController,
662+
required super.contentFocusNode,
663+
required super.disabled,
664+
});
643665

644666
@override
645667
IconData get icon => Icons.attach_file;
@@ -655,7 +677,11 @@ class _AttachFileButton extends _AttachUploadsButton {
655677
}
656678

657679
class _AttachMediaButton extends _AttachUploadsButton {
658-
const _AttachMediaButton({required super.contentController, required super.contentFocusNode});
680+
const _AttachMediaButton({
681+
required super.contentController,
682+
required super.contentFocusNode,
683+
required super.disabled,
684+
});
659685

660686
@override
661687
IconData get icon => Icons.image;
@@ -672,7 +698,11 @@ class _AttachMediaButton extends _AttachUploadsButton {
672698
}
673699

674700
class _AttachFromCameraButton extends _AttachUploadsButton {
675-
const _AttachFromCameraButton({required super.contentController, required super.contentFocusNode});
701+
const _AttachFromCameraButton({
702+
required super.contentController,
703+
required super.contentFocusNode,
704+
required super.disabled,
705+
});
676706

677707
@override
678708
IconData get icon => Icons.camera_alt;
@@ -748,11 +778,15 @@ class _SendButton extends StatefulWidget {
748778
required this.topicController,
749779
required this.contentController,
750780
required this.getDestination,
781+
required this.isSending,
782+
required this.setIsSending,
751783
});
752784

753785
final ComposeTopicController? topicController;
754786
final ComposeContentController contentController;
755787
final MessageDestination Function() getDestination;
788+
final bool isSending;
789+
final void Function(bool value) setIsSending;
756790

757791
@override
758792
State<_SendButton> createState() => _SendButtonState();
@@ -817,12 +851,8 @@ class _SendButtonState extends State<_SendButton> {
817851
final store = PerAccountStoreWidget.of(context);
818852
final content = widget.contentController.textNormalized;
819853

820-
widget.contentController.clear();
821-
854+
widget.setIsSending(true);
822855
try {
823-
// TODO(#720) clear content input only on success response;
824-
// while waiting, put input(s) and send button into a disabled
825-
// "working on it" state (letting input text be selected for copying).
826856
await store.sendMessage(destination: widget.getDestination(), content: content);
827857
} on ApiRequestException catch (e) {
828858
if (!mounted) return;
@@ -835,7 +865,10 @@ class _SendButtonState extends State<_SendButton> {
835865
title: zulipLocalizations.errorMessageNotSent,
836866
message: message);
837867
return;
868+
} finally {
869+
widget.setIsSending(false);
838870
}
871+
widget.contentController.clear();
839872
}
840873

841874
@override
@@ -872,7 +905,7 @@ class _SendButtonState extends State<_SendButton> {
872905
),
873906
color: foregroundColor,
874907
icon: const Icon(Icons.send),
875-
onPressed: _send));
908+
onPressed: widget.isSending ? null : _send));
876909
}
877910
}
878911

@@ -905,13 +938,15 @@ class _ComposeBoxLayout extends StatelessWidget {
905938
required this.sendButton,
906939
required this.contentController,
907940
required this.contentFocusNode,
941+
required this.isSending,
908942
});
909943

910944
final Widget? topicInput;
911945
final Widget contentInput;
912946
final Widget sendButton;
913947
final ComposeContentController contentController;
914948
final FocusNode contentFocusNode;
949+
final bool isSending;
915950

916951
@override
917952
Widget build(BuildContext context) {
@@ -933,6 +968,7 @@ class _ComposeBoxLayout extends StatelessWidget {
933968
);
934969

935970
return _ComposeBoxContainer(
971+
isSending: isSending,
936972
child: Column(children: [
937973
Row(crossAxisAlignment: CrossAxisAlignment.end, children: [
938974
Expanded(
@@ -950,9 +986,9 @@ class _ComposeBoxLayout extends StatelessWidget {
950986
data: themeData.copyWith(
951987
iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)),
952988
child: Row(children: [
953-
_AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode),
954-
_AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode),
955-
_AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode),
989+
_AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode, disabled: isSending),
990+
_AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode, disabled: isSending),
991+
_AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode, disabled: isSending),
956992
])),
957993
]));
958994
}
@@ -991,6 +1027,14 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
9911027
FocusNode get topicFocusNode => _topicFocusNode;
9921028
final _topicFocusNode = FocusNode();
9931029

1030+
bool get isSending => _isSending;
1031+
bool _isSending = false;
1032+
void setIsSending(bool value) {
1033+
setState(() {
1034+
_isSending = value;
1035+
});
1036+
}
1037+
9941038
@override
9951039
void dispose() {
9961040
_topicController.dispose();
@@ -1004,23 +1048,28 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
10041048
return _ComposeBoxLayout(
10051049
contentController: _contentController,
10061050
contentFocusNode: _contentFocusNode,
1051+
isSending: _isSending,
10071052
topicInput: _TopicInput(
10081053
streamId: widget.narrow.streamId,
10091054
controller: _topicController,
10101055
focusNode: topicFocusNode,
10111056
contentFocusNode: _contentFocusNode,
1057+
readOnly: _isSending,
10121058
),
10131059
contentInput: _StreamContentInput(
10141060
narrow: widget.narrow,
10151061
topicController: _topicController,
10161062
controller: _contentController,
10171063
focusNode: _contentFocusNode,
1064+
readOnly: _isSending,
10181065
),
10191066
sendButton: _SendButton(
10201067
topicController: _topicController,
10211068
contentController: _contentController,
10221069
getDestination: () => StreamDestination(
10231070
widget.narrow.streamId, _topicController.textNormalized),
1071+
isSending: _isSending,
1072+
setIsSending: setIsSending,
10241073
));
10251074
}
10261075
}
@@ -1064,6 +1113,14 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
10641113
@override FocusNode get contentFocusNode => _contentFocusNode;
10651114
final _contentFocusNode = FocusNode();
10661115

1116+
bool get isSending => _isSending;
1117+
bool _isSending = false;
1118+
void setIsSending(bool value) {
1119+
setState(() {
1120+
_isSending = value;
1121+
});
1122+
}
1123+
10671124
@override
10681125
void dispose() {
10691126
_contentController.dispose();
@@ -1088,22 +1145,26 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
10881145
Widget build(BuildContext context) {
10891146
final errorBanner = _errorBanner(context);
10901147
if (errorBanner != null) {
1091-
return _ComposeBoxContainer(child: errorBanner);
1148+
return _ComposeBoxContainer(isSending: false, child: errorBanner);
10921149
}
10931150

10941151
return _ComposeBoxLayout(
10951152
contentController: _contentController,
10961153
contentFocusNode: _contentFocusNode,
10971154
topicInput: null,
1155+
isSending: _isSending,
10981156
contentInput: _FixedDestinationContentInput(
10991157
narrow: widget.narrow,
11001158
controller: _contentController,
11011159
focusNode: _contentFocusNode,
1160+
readOnly: _isSending,
11021161
),
11031162
sendButton: _SendButton(
11041163
topicController: null,
11051164
contentController: _contentController,
11061165
getDestination: () => widget.narrow.destination,
1166+
isSending: _isSending,
1167+
setIsSending: setIsSending,
11071168
));
11081169
}
11091170
}

test/flutter_checks.dart

+5
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ extension IconChecks on Subject<Icon> {
3838
// TODO others
3939
}
4040

41+
extension IconButtonChecks on Subject<IconButton> {
42+
Subject<void Function()?> get onPressed => has((i) => i.onPressed, 'onPressed');
43+
}
44+
4145
extension RouteChecks<T> on Subject<Route<T>> {
4246
Subject<RouteSettings> get settings => has((r) => r.settings, 'settings');
4347
}
@@ -68,6 +72,7 @@ extension TextFieldChecks on Subject<TextField> {
6872
Subject<TextCapitalization?> get textCapitalization => has((t) => t.textCapitalization, 'textCapitalization');
6973
Subject<InputDecoration?> get decoration => has((t) => t.decoration, 'decoration');
7074
Subject<TextEditingController?> get controller => has((t) => t.controller, 'controller');
75+
Subject<bool?> get readOnly => has((t) => t.readOnly, 'readOnly');
7176
}
7277

7378
extension TextStyleChecks on Subject<TextStyle> {

0 commit comments

Comments
 (0)