compose: Support sending to empty topic

PIG208 · PIG208 · commit 97b48260bd1c · 2025-03-12T18:47:00.000-04:00
This does not rely on TopicName.displayName being non-nullable or "empty_topic_name" client capability, so it is not an NFC change. The key change that allows sending to empty topic is that `textNormalized` can now be actually empty, instead of being converted to "(no topic)" with `_computeTextNormalized`. --- This is accompanied with a content input hint text change, so that either "Message #stream > (no topic)" or "Message #stream > general chat" appears appropriately. Previously, "Message #stream > (no topic)" was the hint text for content input as long as the topic input is empty and topics are not mandatory. Showing the default topic does not help create incentive for the user to pick a topic first. So only show it when they intend to leave the topic empty. See discussion: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/general.20chat.20design.20.23F1297/near/2088870 --- This does not aim to implement hint text changes to the topic input, which is always "Topic". We will handle that as a follow-up. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart
@@ -163,41 +163,76 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
   @override
   String _computeTextNormalized() {
     String trimmed = text.trim();
-    return trimmed.isEmpty ? kNoTopicTopic : trimmed;
+    // TODO(server-10): simplify
+    if (store.zulipFeatureLevel < 334) {
+      return trimmed.isEmpty ? kNoTopicTopic : trimmed;
+    }
+
+    return trimmed;
   }
 
   /// Whether [textNormalized] would fail a mandatory-topics check
   /// (see [TopicValidationError.mandatoryButVacuous]).
-  bool get _isTopicVacuous => textNormalized == kNoTopicTopic;
+  bool get _isTopicVacuous {
+    bool result = textNormalized.isEmpty
+      // We keep checking for '(no topic)' regardless of the feature level
+      // because it remains equivalent to an empty topic even when FL >= 334.
+      // This can change in the future:
+      //   https://chat.zulip.org/#narrow/channel/412-api-documentation/topic/.28realm_.29mandatory_topics.20behavior/near/2062391
+      || textNormalized == kNoTopicTopic;
+
+    // TODO(server-10): simplify
+    if (store.zulipFeatureLevel >= 334) {
+      result |= textNormalized == store.realmEmptyTopicDisplayName;
+    }
+
+    return result;
+  }
 
   /// The send destination as a string.
   ///
-  /// This returns a string formatted like "#stream name" when topics are
-  /// [mandatory] but the trimmed input is empty.
+  /// If topics are [mandatory], returns a string formatted with a topic name
+  /// when the trimmed input is non-empty. E.g.: "#stream name > topic name".
+  ///
+  /// If topics are not [mandatory], returns a string formatted with a topic
+  /// name only when the user intentionally leaves the trimmed input empty.
   ///
-  /// Otherwise, returns a string formatted like "#stream name > topic name".
+  /// Otherwise, returns a string formatted without a topic name.
+  /// E.g.: "#stream name".
   // No i18n of the use of "#" and ">" strings; those are part of how
   // Zulip expresses channels and topics, not any normal English punctuation,
   // so don't make sense to translate. See:
   //   https://github.com/zulip/zulip-flutter/pull/1148#discussion_r1941990585
-  String getDestinationString({required String streamName}) {
+  String getDestinationString({
+    required String streamName,
+    required bool contentHasFocus,
+  }) {
     final textTrimmed = text.trim();
     if (textTrimmed.isNotEmpty) {
       return '#$streamName > $textTrimmed';
     }
 
     assert(_isTopicVacuous);
-    // Sending to a vacuous topic (see [isTopicVacuous]) is not possible if
-    // topics are [mandatory].
-    if (mandatory) {
+    if (
+      // Sending to a vacuous topic (see [_isTopicVacuous]) is not possible if
+      // topics are [mandatory].
+      mandatory
+      // Do not fall back to a vacuous topic unless the user explicitly chooses
+      // to do so (by skipping topic input and moving focus to content input),
+      // so that the user is not encouraged to use vacuous topic when they
+      // have not interacted with the inputs at all.
+      || !contentHasFocus
+    ) {
       return '#$streamName';
     }
 
-    return '#$streamName > $kNoTopicTopic';
+    final vacuousTopicDisplayName =
+      // TODO(server-10): simplify away conditional
+      textNormalized.isEmpty ? store.realmEmptyTopicDisplayName : textNormalized;
+    return '#$streamName > $vacuousTopicDisplayName';
   }
 
-  @override
-  List<TopicValidationError> _computeValidationErrors() {
+  @override List<TopicValidationError> _computeValidationErrors() {
     return [
       if (mandatory && _isTopicVacuous)
         TopicValidationError.mandatoryButVacuous,
@@ -587,10 +622,17 @@ class _StreamContentInputState extends State<_StreamContentInput> {
     });
   }
 
+  void _contentFocusChanged() {
+    setState(() {
+      // The relevant state lives on widget.controller.contentFocusNode itself.
+    });
+  }
+
   @override
   void initState() {
     super.initState();
     widget.controller.topic.addListener(_topicChanged);
+    widget.controller.contentFocusNode.addListener(_contentFocusChanged);
   }
 
   @override
@@ -600,11 +642,16 @@ class _StreamContentInputState extends State<_StreamContentInput> {
       oldWidget.controller.topic.removeListener(_topicChanged);
       widget.controller.topic.addListener(_topicChanged);
     }
+    if (widget.controller.contentFocusNode != oldWidget.controller.contentFocusNode) {
+      oldWidget.controller.contentFocusNode.removeListener(_contentFocusChanged);
+      widget.controller.contentFocusNode.addListener(_contentFocusChanged);
+    }
   }
 
   @override
   void dispose() {
     widget.controller.topic.removeListener(_topicChanged);
+    widget.controller.contentFocusNode.removeListener(_contentFocusChanged);
     super.dispose();
   }
 
@@ -620,7 +667,9 @@ class _StreamContentInputState extends State<_StreamContentInput> {
         TopicName(widget.controller.topic.textNormalized)),
       controller: widget.controller,
       hintText: zulipLocalizations.composeBoxChannelContentHint(
-        widget.controller.topic.getDestinationString(streamName: streamName)));
+        widget.controller.topic.getDestinationString(
+          streamName: streamName,
+          contentHasFocus: widget.controller.contentFocusNode.hasFocus)));
   }
 }
 
diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart
@@ -47,15 +47,18 @@ void main() {
     List<User> otherUsers = const [],
     List<ZulipStream> streams = const [],
     bool? mandatoryTopics,
+    int? zulipFeatureLevel,
   }) async {
     if (narrow case ChannelNarrow(:var streamId) || TopicNarrow(: var streamId)) {
       assert(streams.any((stream) => stream.streamId == streamId),
         'Add a channel with "streamId" the same as of $narrow.streamId to the store.');
     }
     addTearDown(testBinding.reset);
     selfUser ??= eg.selfUser;
-    final selfAccount = eg.account(user: selfUser);
+    zulipFeatureLevel ??= eg.futureZulipFeatureLevel;
+    final selfAccount = eg.account(user: selfUser, zulipFeatureLevel: zulipFeatureLevel);
     await testBinding.globalStore.add(selfAccount, eg.initialSnapshot(
+      zulipFeatureLevel: zulipFeatureLevel,
       realmMandatoryTopics: mandatoryTopics,
     ));
 
@@ -327,12 +330,14 @@ void main() {
     Future<void> prepare(WidgetTester tester, {
       required Narrow narrow,
       bool? mandatoryTopics,
+      int? zulipFeatureLevel,
     }) async {
       await prepareComposeBox(tester,
         narrow: narrow,
         otherUsers: [eg.otherUser, eg.thirdUser],
         streams: [channel],
-        mandatoryTopics: mandatoryTopics);
+        mandatoryTopics: mandatoryTopics,
+        zulipFeatureLevel: zulipFeatureLevel);
     }
 
     /// This checks the input's configured hint text without regard to whether
@@ -356,16 +361,50 @@ void main() {
     group('to ChannelNarrow, topics not mandatory', () {
       final narrow = ChannelNarrow(channel.streamId);
 
-      testWidgets('with empty topic', (tester) async {
+      testWidgets('with empty topic, topic input has focus', (tester) async {
         await prepare(tester, narrow: narrow, mandatoryTopics: false);
+        await enterTopic(tester, narrow: narrow, topic: '');
+        await tester.pump();
         checkComposeBoxHintTexts(tester,
           topicHintText: 'Topic',
-          contentHintText: 'Message #${channel.name} > (no topic)');
+          contentHintText: 'Message #${channel.name}');
       });
 
-      testWidgets('with non-empty but vacuous topic', (tester) async {
+      testWidgets('with non-empty but vacuous topic, topic input has focus', (tester) async {
+        await prepare(tester, narrow: narrow, mandatoryTopics: false);
+        await enterTopic(tester, narrow: narrow,
+          topic: eg.defaultRealmEmptyTopicDisplayName);
+        await tester.pump();
+        checkComposeBoxHintTexts(tester,
+          topicHintText: 'Topic',
+          contentHintText: 'Message #${channel.name} > '
+                           '${eg.defaultRealmEmptyTopicDisplayName}');
+      });
+
+      testWidgets('legacy: with empty topic, topic input has focus', (tester) async {
+        await prepare(tester, narrow: narrow, mandatoryTopics: false,
+          zulipFeatureLevel: 333);
+        await enterTopic(tester, narrow: narrow, topic: '');
+        await tester.pump();
+        checkComposeBoxHintTexts(tester,
+          topicHintText: 'Topic',
+          contentHintText: 'Message #${channel.name}');
+      });
+
+      testWidgets('with empty topic, content input has focus', (tester) async {
         await prepare(tester, narrow: narrow, mandatoryTopics: false);
-        await enterTopic(tester, narrow: narrow, topic: '(no topic)');
+        await enterContent(tester, '');
+        await tester.pump();
+        checkComposeBoxHintTexts(tester,
+          topicHintText: 'Topic',
+          contentHintText: 'Message #${channel.name} > '
+                           '${eg.defaultRealmEmptyTopicDisplayName}');
+      });
+
+      testWidgets('legacy: with empty topic, content input has focus', (tester) async {
+        await prepare(tester, narrow: narrow, mandatoryTopics: false,
+          zulipFeatureLevel: 333);
+        await enterContent(tester, '');
         await tester.pump();
         checkComposeBoxHintTexts(tester,
           topicHintText: 'Topic',
@@ -394,11 +433,21 @@ void main() {
 
       testWidgets('with non-empty but vacuous topic', (tester) async {
         await prepare(tester, narrow: narrow, mandatoryTopics: true);
-        await enterTopic(tester, narrow: narrow, topic: '(no topic)');
+        await enterTopic(tester, narrow: narrow,
+          topic: eg.defaultRealmEmptyTopicDisplayName);
         await tester.pump();
         checkComposeBoxHintTexts(tester,
           topicHintText: 'Topic',
-          contentHintText: 'Message #${channel.name} > (no topic)');
+          contentHintText: 'Message #${channel.name} > '
+                           '${eg.defaultRealmEmptyTopicDisplayName}');
+      });
+
+      testWidgets('legacy: with empty topic', (tester) async {
+        await prepare(tester, narrow: narrow, mandatoryTopics: true,
+          zulipFeatureLevel: 333);
+        checkComposeBoxHintTexts(tester,
+          topicHintText: 'Topic',
+          contentHintText: 'Message #${channel.name}');
       });
 
       testWidgets('with non-empty topic', (tester) async {
@@ -703,6 +752,7 @@ void main() {
     Future<void> setupAndTapSend(WidgetTester tester, {
       required String topicInputText,
       required bool mandatoryTopics,
+      int? zulipFeatureLevel,
     }) async {
       TypingNotifier.debugEnable = false;
       addTearDown(TypingNotifier.debugReset);
@@ -711,7 +761,8 @@ void main() {
       final narrow = ChannelNarrow(channel.streamId);
       await prepareComposeBox(tester,
         narrow: narrow, streams: [channel],
-        mandatoryTopics: mandatoryTopics);
+        mandatoryTopics: mandatoryTopics,
+        zulipFeatureLevel: zulipFeatureLevel);
 
       await enterTopic(tester, narrow: narrow, topic: topicInputText);
       await tester.enterText(contentInputFinder, 'test content');
@@ -726,10 +777,21 @@ void main() {
         expectedMessage: 'Topics are required in this organization.');
     }
 
-    testWidgets('empty topic -> "(no topic)"', (tester) async {
+    testWidgets('empty topic -> ""', (tester) async {
       await setupAndTapSend(tester,
         topicInputText: '',
         mandatoryTopics: false);
+      check(connection.lastRequest).isA<http.Request>()
+        ..method.equals('POST')
+        ..url.path.equals('/api/v1/messages')
+        ..bodyFields['topic'].equals('');
+    });
+
+    testWidgets('legacy: empty topic -> "(no topic)"', (tester) async {
+      await setupAndTapSend(tester,
+        topicInputText: '',
+        mandatoryTopics: false,
+        zulipFeatureLevel: 333);
       check(connection.lastRequest).isA<http.Request>()
         ..method.equals('POST')
         ..url.path.equals('/api/v1/messages')
@@ -743,6 +805,13 @@ void main() {
       checkMessageNotSent(tester);
     });
 
+    testWidgets('if topics are mandatory, reject `realmEmptyTopicDisplayName`', (tester) async {
+      await setupAndTapSend(tester,
+        topicInputText: eg.defaultRealmEmptyTopicDisplayName,
+        mandatoryTopics: true);
+      checkMessageNotSent(tester);
+    });
+
     testWidgets('if topics are mandatory, reject "(no topic)"', (tester) async {
       await setupAndTapSend(tester,
         topicInputText: '(no topic)',
