diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index f03f26734f..c58e639b48 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -7,7 +7,7 @@ import '../widgets/compose_box.dart'; import 'narrow.dart'; import 'store.dart'; -extension Autocomplete on ContentTextEditingController { +extension Autocomplete on ComposeContentController { AutocompleteIntent? autocompleteIntent() { if (!selection.isValid || !selection.isNormalized) { // We don't require [isCollapsed] to be true because we've seen that diff --git a/lib/model/narrow.dart b/lib/model/narrow.dart index 52f82adb4f..1b640f6665 100644 --- a/lib/model/narrow.dart +++ b/lib/model/narrow.dart @@ -1,6 +1,7 @@ import '../api/model/model.dart'; import '../api/model/narrow.dart'; +import '../api/route/messages.dart'; /// A Zulip narrow. sealed class Narrow { @@ -15,6 +16,11 @@ sealed class Narrow { ApiNarrow apiEncode(); } +/// A non-interleaved narrow, completely specifying a place to send a message. +sealed class SendableNarrow extends Narrow { + MessageDestination get destination; +} + /// The narrow called "All messages" in the UI. /// /// This does not literally mean all messages, or even all messages @@ -65,7 +71,7 @@ class StreamNarrow extends Narrow { int get hashCode => Object.hash('StreamNarrow', streamId); } -class TopicNarrow extends Narrow { +class TopicNarrow extends Narrow implements SendableNarrow { const TopicNarrow(this.streamId, this.topic); final int streamId; @@ -80,6 +86,9 @@ class TopicNarrow extends Narrow { @override ApiNarrow apiEncode() => [ApiNarrowStream(streamId), ApiNarrowTopic(topic)]; + @override + StreamDestination get destination => StreamDestination(streamId, topic); + @override bool operator ==(Object other) { if (other is! TopicNarrow) return false; @@ -111,7 +120,7 @@ bool _isSortedWithoutDuplicates(List items) { // handling many of them, see zulip-mobile:src/utils/recipient.js . // Please add more constructors and getters here to handle any of those // as we turn out to need them. -class DmNarrow extends Narrow { +class DmNarrow extends Narrow implements SendableNarrow { DmNarrow({required this.allRecipientIds, required int selfUserId}) : assert(_isSortedWithoutDuplicates(allRecipientIds)), assert(allRecipientIds.contains(selfUserId)), @@ -166,9 +175,15 @@ class DmNarrow extends Narrow { return true; } + // Not [otherRecipientIds], because for the self-1:1 thread the server rejects + // that as of Zulip Server 7 (2023-06), with BAD_REQUEST. + @override + DmDestination get destination => DmDestination(userIds: allRecipientIds); + // Not [otherRecipientIds], because for the self-1:1 thread that triggers // a server bug as of Zulip Server 7 (2023-05): an empty list here // causes a 5xx response from the server. + // TODO(server): fix bug on empty operand to dm/pm-with narrow operator @override ApiNarrow apiEncode() => [ApiNarrowDm(allRecipientIds)]; diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 90424efb58..703947f667 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -13,6 +13,33 @@ import 'store.dart'; const double _inputVerticalPadding = 8; const double _sendButtonSize = 36; +/// A [TextEditingController] for use in the compose box. +/// +/// Subclasses must ensure that [_update] is called in all exposed constructors. +abstract class ComposeController extends TextEditingController { + String get textNormalized => _textNormalized; + late String _textNormalized; + String _computeTextNormalized(); + + List get validationErrors => _validationErrors; + late List _validationErrors; + List _computeValidationErrors(); + + ValueNotifier hasValidationErrors = ValueNotifier(false); + + void _update() { + _textNormalized = _computeTextNormalized(); + _validationErrors = _computeValidationErrors(); + hasValidationErrors.value = _validationErrors.isNotEmpty; + } + + @override + void notifyListeners() { + _update(); + super.notifyListeners(); + } +} + enum TopicValidationError { mandatoryButEmpty, tooLong; @@ -27,22 +54,27 @@ enum TopicValidationError { } } -class TopicTextEditingController extends TextEditingController { +class ComposeTopicController extends ComposeController { + ComposeTopicController() { + _update(); + } + // TODO: subscribe to this value: // https://zulip.com/help/require-topics final mandatory = true; - String textNormalized() { + @override + String _computeTextNormalized() { String trimmed = text.trim(); return trimmed.isEmpty ? kNoTopicTopic : trimmed; } - List validationErrors() { - final normalized = textNormalized(); + @override + List _computeValidationErrors() { return [ - if (mandatory && normalized == kNoTopicTopic) + if (mandatory && textNormalized == kNoTopicTopic) TopicValidationError.mandatoryButEmpty, - if (normalized.length > kMaxTopicLength) + if (textNormalized.length > kMaxTopicLength) TopicValidationError.tooLong, ]; } @@ -67,7 +99,11 @@ enum ContentValidationError { } } -class ContentTextEditingController extends TextEditingController { +class ComposeContentController extends ComposeController { + ComposeContentController() { + _update(); + } + int _nextUploadTag = 0; final Map _uploads = {}; @@ -128,20 +164,21 @@ class ContentTextEditingController extends TextEditingController { notifyListeners(); // _uploads change could affect validationErrors } - String textNormalized() { + @override + String _computeTextNormalized() { return text.trim(); } - List validationErrors() { - final normalized = textNormalized(); + @override + List _computeValidationErrors() { return [ - if (normalized.isEmpty) + if (textNormalized.isEmpty) ContentValidationError.empty, // normalized.length is the number of UTF-16 code units, while the server // API expresses the max in Unicode code points. So this comparison will // be conservative and may cut the user off shorter than necessary. - if (normalized.length > kMaxMessageLengthCodePoints) + if (textNormalized.length > kMaxMessageLengthCodePoints) ContentValidationError.tooLong, if (_uploads.isNotEmpty) @@ -150,37 +187,26 @@ class ContentTextEditingController extends TextEditingController { } } -/// The content input for StreamComposeBox. -class _StreamContentInput extends StatefulWidget { - const _StreamContentInput({ +class _ContentInput extends StatefulWidget { + const _ContentInput({ required this.narrow, - required this.streamId, required this.controller, - required this.topicController, required this.focusNode, + required this.hintText, }); final Narrow narrow; - final int streamId; - final ContentTextEditingController controller; - final TopicTextEditingController topicController; + final ComposeContentController controller; final FocusNode focusNode; + final String hintText; @override - State<_StreamContentInput> createState() => _StreamContentInputState(); + State<_ContentInput> createState() => _ContentInputState(); } -class _StreamContentInputState extends State<_StreamContentInput> { +class _ContentInputState extends State<_ContentInput> { MentionAutocompleteView? _mentionAutocompleteView; // TODO different autocomplete view types - late String _topicTextNormalized; - - _topicChanged() { - setState(() { - _topicTextNormalized = widget.topicController.textNormalized(); - }); - } - _changed() { final newAutocompleteIntent = widget.controller.autocompleteIntent(); if (newAutocompleteIntent != null) { @@ -199,23 +225,26 @@ class _StreamContentInputState extends State<_StreamContentInput> { @override void initState() { super.initState(); - _topicTextNormalized = widget.topicController.textNormalized(); - widget.topicController.addListener(_topicChanged); widget.controller.addListener(_changed); } + @override + void didUpdateWidget(covariant _ContentInput oldWidget) { + super.didUpdateWidget(oldWidget); + if (widget.controller != oldWidget.controller) { + oldWidget.controller.removeListener(_changed); + widget.controller.addListener(_changed); + } + } + @override void dispose() { - widget.topicController.removeListener(_topicChanged); widget.controller.removeListener(_changed); super.dispose(); } @override Widget build(BuildContext context) { - final store = PerAccountStoreWidget.of(context); - final streamName = store.streams[widget.streamId]?.name ?? '(unknown stream)'; - ColorScheme colorScheme = Theme.of(context).colorScheme; return InputDecorator( @@ -231,14 +260,115 @@ class _StreamContentInputState extends State<_StreamContentInput> { controller: widget.controller, focusNode: widget.focusNode, style: TextStyle(color: colorScheme.onSurface), - decoration: InputDecoration.collapsed( - hintText: "Message #$streamName > $_topicTextNormalized", - ), + decoration: InputDecoration.collapsed(hintText: widget.hintText), maxLines: null, ))); } } +/// The content input for _StreamComposeBox. +class _StreamContentInput extends StatefulWidget { + const _StreamContentInput({ + required this.narrow, + required this.controller, + required this.topicController, + required this.focusNode, + }); + + final StreamNarrow narrow; + final ComposeContentController controller; + final ComposeTopicController topicController; + final FocusNode focusNode; + + @override + State<_StreamContentInput> createState() => _StreamContentInputState(); +} + +class _StreamContentInputState extends State<_StreamContentInput> { + late String _topicTextNormalized; + + _topicChanged() { + setState(() { + _topicTextNormalized = widget.topicController.textNormalized; + }); + } + + @override + void initState() { + super.initState(); + _topicTextNormalized = widget.topicController.textNormalized; + widget.topicController.addListener(_topicChanged); + } + + @override + void didUpdateWidget(covariant _StreamContentInput oldWidget) { + super.didUpdateWidget(oldWidget); + if (widget.topicController != oldWidget.topicController) { + oldWidget.topicController.removeListener(_topicChanged); + widget.topicController.addListener(_topicChanged); + } + } + + @override + void dispose() { + widget.topicController.removeListener(_topicChanged); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final streamName = store.streams[widget.narrow.streamId]?.name ?? '(unknown stream)'; + return _ContentInput( + narrow: widget.narrow, + controller: widget.controller, + focusNode: widget.focusNode, + hintText: "Message #$streamName > $_topicTextNormalized"); + } +} + +class _FixedDestinationContentInput extends StatelessWidget { + const _FixedDestinationContentInput({ + required this.narrow, + required this.controller, + required this.focusNode, + }); + + final SendableNarrow narrow; + final ComposeContentController controller; + final FocusNode focusNode; + + String _hintText(BuildContext context) { + switch (narrow) { + case TopicNarrow(:final streamId, :final topic): + final store = PerAccountStoreWidget.of(context); + final streamName = store.streams[streamId]?.name ?? '(unknown stream)'; + return "Message #$streamName > $topic"; + + case DmNarrow(otherRecipientIds: []): // The self-1:1 thread. + return "Jot down something"; + + case DmNarrow(otherRecipientIds: [final otherUserId]): + final store = PerAccountStoreWidget.of(context); + final fullName = store.users[otherUserId]?.fullName; + if (fullName == null) return 'Type a message'; + return 'Message @$fullName'; + + case DmNarrow(): // A group DM thread. + return 'Message group'; + } + } + + @override + Widget build(BuildContext context) { + return _ContentInput( + narrow: narrow, + controller: controller, + focusNode: focusNode, + hintText: _hintText(context)); + } +} + /// Data on a file to be uploaded, from any source. /// /// A convenience class to represent data from the generic file picker, @@ -253,7 +383,7 @@ class _File { Future _uploadFiles({ required BuildContext context, - required ContentTextEditingController contentController, + required ComposeContentController contentController, required FocusNode contentFocusNode, required Iterable<_File> files, }) async { @@ -314,7 +444,7 @@ Future _uploadFiles({ abstract class _AttachUploadsButton extends StatelessWidget { const _AttachUploadsButton({required this.contentController, required this.contentFocusNode}); - final ContentTextEditingController contentController; + final ComposeContentController contentController; final FocusNode contentFocusNode; IconData get icon; @@ -469,99 +599,86 @@ class _AttachFromCameraButton extends _AttachUploadsButton { } } -/// The send button for StreamComposeBox. -class _StreamSendButton extends StatefulWidget { - const _StreamSendButton({ - required this.streamId, +class _SendButton extends StatefulWidget { + const _SendButton({ required this.topicController, required this.contentController, + required this.getDestination, }); - final int streamId; - final TopicTextEditingController topicController; - final ContentTextEditingController contentController; + final ComposeTopicController? topicController; + final ComposeContentController contentController; + final MessageDestination Function() getDestination; @override - State<_StreamSendButton> createState() => _StreamSendButtonState(); + State<_SendButton> createState() => _SendButtonState(); } -class _StreamSendButtonState extends State<_StreamSendButton> { - late List _topicValidationErrors; - late List _contentValidationErrors; - - _topicChanged() { - final oldIsEmpty = _topicValidationErrors.isEmpty; - final newErrors = widget.topicController.validationErrors(); - final newIsEmpty = newErrors.isEmpty; - _topicValidationErrors = newErrors; - if (oldIsEmpty != newIsEmpty) { - setState(() { - // Update disabled/non-disabled state - }); - } - } - - _contentChanged() { - final oldIsEmpty = _contentValidationErrors.isEmpty; - final newErrors = widget.contentController.validationErrors(); - final newIsEmpty = newErrors.isEmpty; - _contentValidationErrors = newErrors; - if (oldIsEmpty != newIsEmpty) { - setState(() { - // Update disabled/non-disabled state - }); - } +class _SendButtonState extends State<_SendButton> { + _hasErrorsChanged() { + setState(() { + // Update disabled/non-disabled state + }); } @override void initState() { super.initState(); - _topicValidationErrors = widget.topicController.validationErrors(); - _contentValidationErrors = widget.contentController.validationErrors(); - widget.topicController.addListener(_topicChanged); - widget.contentController.addListener(_contentChanged); + widget.topicController?.hasValidationErrors.addListener(_hasErrorsChanged); + widget.contentController.hasValidationErrors.addListener(_hasErrorsChanged); + } + + @override + void didUpdateWidget(covariant _SendButton oldWidget) { + super.didUpdateWidget(oldWidget); + if (widget.topicController != oldWidget.topicController) { + oldWidget.topicController?.hasValidationErrors.removeListener(_hasErrorsChanged); + widget.topicController?.hasValidationErrors.addListener(_hasErrorsChanged); + } + if (widget.contentController != oldWidget.contentController) { + oldWidget.contentController.hasValidationErrors.removeListener(_hasErrorsChanged); + widget.contentController.hasValidationErrors.addListener(_hasErrorsChanged); + } } @override void dispose() { - widget.topicController.removeListener(_topicChanged); - widget.contentController.removeListener(_contentChanged); + widget.topicController?.hasValidationErrors.removeListener(_hasErrorsChanged); + widget.contentController.hasValidationErrors.removeListener(_hasErrorsChanged); super.dispose(); } - void _showSendFailedDialog(BuildContext context) { - List validationErrorMessages = [ - for (final error in _topicValidationErrors) - error.message(), - for (final error in _contentValidationErrors) - error.message(), - ]; - - return showErrorDialog( - context: context, - title: 'Message not sent', - message: validationErrorMessages.join('\n\n')); + bool get _hasValidationErrors { + return (widget.topicController?.hasValidationErrors.value ?? false) + || widget.contentController.hasValidationErrors.value; } - void _handleSendPressed(BuildContext context) { - if (_topicValidationErrors.isNotEmpty || _contentValidationErrors.isNotEmpty) { - _showSendFailedDialog(context); + void _send() { + if (_hasValidationErrors) { + List validationErrorMessages = [ + for (final error in widget.topicController?.validationErrors ?? const []) + error.message(), + for (final error in widget.contentController.validationErrors) + error.message(), + ]; + showErrorDialog( + context: context, + title: 'Message not sent', + message: validationErrorMessages.join('\n\n')); return; } final store = PerAccountStoreWidget.of(context); - final destination = StreamDestination(widget.streamId, widget.topicController.textNormalized()); - final content = widget.contentController.textNormalized(); - store.sendMessage(destination: destination, content: content); + final content = widget.contentController.textNormalized; + store.sendMessage(destination: widget.getDestination(), content: content); widget.contentController.clear(); } @override Widget build(BuildContext context) { - ColorScheme colorScheme = Theme.of(context).colorScheme; - - bool disabled = _topicValidationErrors.isNotEmpty || _contentValidationErrors.isNotEmpty; + final disabled = _hasValidationErrors; + final colorScheme = Theme.of(context).colorScheme; // Copy FilledButton defaults (_FilledButtonDefaultsM3.backgroundColor) final backgroundColor = disabled @@ -588,35 +705,24 @@ class _StreamSendButtonState extends State<_StreamSendButton> { color: foregroundColor, icon: const Icon(Icons.send), - onPressed: () => _handleSendPressed(context))); + onPressed: _send)); } } -/// The compose box for writing a stream message. -class StreamComposeBox extends StatefulWidget { - const StreamComposeBox({super.key, required this.narrow, required this.streamId}); - - /// The narrow on view in the message list. - final Narrow narrow; - - final int streamId; - - @override - State createState() => _StreamComposeBoxState(); -} - -class _StreamComposeBoxState extends State { - final _topicController = TopicTextEditingController(); - final _contentController = ContentTextEditingController(); - final _contentFocusNode = FocusNode(); +class _ComposeBoxLayout extends StatelessWidget { + const _ComposeBoxLayout({ + required this.topicInput, + required this.contentInput, + required this.sendButton, + required this.contentController, + required this.contentFocusNode, + }); - @override - void dispose() { - _topicController.dispose(); - _contentController.dispose(); - _contentFocusNode.dispose(); - super.dispose(); - } + final Widget? topicInput; + final Widget contentInput; + final Widget sendButton; + final ComposeContentController contentController; + final FocusNode contentFocusNode; @override Widget build(BuildContext context) { @@ -637,12 +743,6 @@ class _StreamComposeBoxState extends State { ), ); - final topicInput = TextField( - controller: _topicController, - style: TextStyle(color: colorScheme.onSurface), - decoration: const InputDecoration(hintText: 'Topic'), - ); - return Material( color: colorScheme.surfaceVariant, child: SafeArea( @@ -655,31 +755,114 @@ class _StreamComposeBoxState extends State { child: Theme( data: inputThemeData, child: Column(children: [ - topicInput, - const SizedBox(height: 8), - _StreamContentInput( - narrow: widget.narrow, - streamId: widget.streamId, - topicController: _topicController, - controller: _contentController, - focusNode: _contentFocusNode), + if (topicInput != null) topicInput!, + if (topicInput != null) const SizedBox(height: 8), + contentInput, ]))), const SizedBox(width: 8), - _StreamSendButton( - streamId: widget.streamId, - topicController: _topicController, - contentController: _contentController, - ), + 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), + _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), ])), - ])))); + ])))); } +} + +/// A compose box for use in a stream narrow. +/// +/// This offers a text input for the topic to send to, +/// in addition to a text input for the message content. +class _StreamComposeBox extends StatefulWidget { + const _StreamComposeBox({required this.narrow}); + + /// The narrow on view in the message list. + final StreamNarrow narrow; + + @override + State<_StreamComposeBox> createState() => _StreamComposeBoxState(); +} + +class _StreamComposeBoxState extends State<_StreamComposeBox> { + final _topicController = ComposeTopicController(); + final _contentController = ComposeContentController(); + final _contentFocusNode = FocusNode(); + + @override + void dispose() { + _topicController.dispose(); + _contentController.dispose(); + _contentFocusNode.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + final colorScheme = Theme.of(context).colorScheme; + + return _ComposeBoxLayout( + contentController: _contentController, + contentFocusNode: _contentFocusNode, + topicInput: TextField( + controller: _topicController, + style: TextStyle(color: colorScheme.onSurface), + decoration: const InputDecoration(hintText: 'Topic'), + ), + contentInput: _StreamContentInput( + narrow: widget.narrow, + topicController: _topicController, + controller: _contentController, + focusNode: _contentFocusNode, + ), + sendButton: _SendButton( + topicController: _topicController, + contentController: _contentController, + getDestination: () => StreamDestination( + widget.narrow.streamId, _topicController.textNormalized), + )); + } +} + +class _FixedDestinationComposeBox extends StatefulWidget { + const _FixedDestinationComposeBox({required this.narrow}); + + final SendableNarrow narrow; + + @override + State<_FixedDestinationComposeBox> createState() => _FixedDestinationComposeBoxState(); +} + +class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox> { + final _contentController = ComposeContentController(); + final _contentFocusNode = FocusNode(); + + @override + void dispose() { + _contentController.dispose(); + _contentFocusNode.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return _ComposeBoxLayout( + contentController: _contentController, + contentFocusNode: _contentFocusNode, + topicInput: null, + contentInput: _FixedDestinationContentInput( + narrow: widget.narrow, + controller: _contentController, + focusNode: _contentFocusNode, + ), + sendButton: _SendButton( + topicController: null, + contentController: _contentController, + getDestination: () => widget.narrow.destination, + )); } } @@ -692,11 +875,11 @@ class ComposeBox extends StatelessWidget { Widget build(BuildContext context) { final narrow = this.narrow; if (narrow is StreamNarrow) { - return StreamComposeBox(narrow: narrow, streamId: narrow.streamId); + return _StreamComposeBox(narrow: narrow); } else if (narrow is TopicNarrow) { - return const SizedBox.shrink(); // TODO(#144): add a single-topic compose box + return _FixedDestinationComposeBox(narrow: narrow); } else if (narrow is DmNarrow) { - return const SizedBox.shrink(); // TODO(#144): add a DM compose box + return _FixedDestinationComposeBox(narrow: narrow); } else if (narrow is AllMessagesNarrow) { return const SizedBox.shrink(); } else { diff --git a/test/model/autocomplete_checks.dart b/test/model/autocomplete_checks.dart index f3317da586..4b7e3c8bad 100644 --- a/test/model/autocomplete_checks.dart +++ b/test/model/autocomplete_checks.dart @@ -2,7 +2,7 @@ import 'package:checks/checks.dart'; import 'package:zulip/model/autocomplete.dart'; import 'package:zulip/widgets/compose_box.dart'; -extension ContentTextEditingControllerChecks on Subject { +extension ComposeContentControllerChecks on Subject { Subject get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent'); } diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 9a6cd48c21..c6b7dd23f2 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -15,7 +15,7 @@ import 'test_store.dart'; import 'autocomplete_checks.dart'; void main() { - group('ContentTextEditingController.autocompleteIntent', () { + group('ComposeContentController.autocompleteIntent', () { parseMarkedText(String markedText) { final TextSelection selection; int? expectedSyntaxStart; @@ -67,7 +67,7 @@ void main() { ? 'in ${jsonEncode(markedText)}, query ${jsonEncode(expectedQuery.raw)}' : 'no query in ${jsonEncode(markedText)}'; test(description, () { - final controller = ContentTextEditingController(); + final controller = ComposeContentController(); final parsed = parseMarkedText(markedText); assert((expectedQuery == null) == (parsed.expectedSyntaxStart == null)); controller.value = parsed.value;