compose: Support sending to empty topic

PIG208 · gnprice · commit 769cc7df4055 · 2025-03-24T12:59:05.000-07: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 "Message #stream > general chat" appears appropriately when we make TopicName.displayName nullable. --- 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
@@ -157,15 +157,33 @@ 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 [mandatory]).
   ///
   /// The term "Vacuous" draws distinction from [String.isEmpty], in the sense
   /// that certain strings are not empty but also indicate the absence of a topic.
-  bool get isTopicVacuous => textNormalized == kNoTopicTopic;
+  ///
+  /// See also: https://zulip.com/api/send-message#parameter-topic
+  bool get isTopicVacuous {
+    if (textNormalized.isEmpty) return true;
+
+    if (textNormalized == kNoTopicTopic) return true;
+
+    // TODO(server-10): simplify
+    if (store.zulipFeatureLevel >= 334) {
+      return textNormalized == store.realmEmptyTopicDisplayName;
+    }
+
+    return false;
+  }
 
   @override
   List<TopicValidationError> _computeValidationErrors() {
@@ -558,10 +576,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
@@ -571,11 +596,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();
   }
 
@@ -586,6 +616,13 @@ class _StreamContentInputState extends State<_StreamContentInput> {
         // The chosen topic can't be sent to, so don't show it.
         return null;
       }
+      if (!widget.controller.contentFocusNode.hasFocus) {
+        // 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.
+        return null;
+      }
     }
 
     return TopicName(widget.controller.topic.textNormalized);
@@ -604,7 +641,9 @@ class _StreamContentInputState extends State<_StreamContentInput> {
       // 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
-      ? '#$streamName' : '#$streamName > ${hintTopic.displayName}';
+      ? '#$streamName'
+      // ignore: dead_null_aware_expression // null topic names soon to be enabled
+      : '#$streamName > ${hintTopic.displayName ?? store.realmEmptyTopicDisplayName}';
 
     return _ContentInput(
       narrow: widget.narrow,
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,49 @@ 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('legacy: with empty topic, topic input has focus', (tester) async {
+        await prepare(tester, narrow: narrow, mandatoryTopics: false,
+          zulipFeatureLevel: 333); // TODO(server-10)
+        await enterTopic(tester, narrow: narrow, topic: '');
+        await tester.pump();
+        checkComposeBoxHintTexts(tester,
+          topicHintText: 'Topic',
+          contentHintText: 'Message #${channel.name}');
+      });
+
+      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: '(no topic)');
+        await enterTopic(tester, narrow: narrow,
+          topic: eg.defaultRealmEmptyTopicDisplayName);
+        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 enterContent(tester, '');
+        await tester.pump();
+        checkComposeBoxHintTexts(tester,
+          topicHintText: 'Topic',
+          contentHintText: 'Message #${channel.name} > '
+                           '${eg.defaultRealmEmptyTopicDisplayName}');
+      }, skip: true); // null topic names soon to be enabled
+
+      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',
@@ -392,15 +430,36 @@ void main() {
           contentHintText: 'Message #${channel.name}');
       });
 
-      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 tester.pump();
+      testWidgets('legacy: with empty topic', (tester) async {
+        await prepare(tester, narrow: narrow, mandatoryTopics: true,
+          zulipFeatureLevel: 333); // TODO(server-10)
         checkComposeBoxHintTexts(tester,
           topicHintText: 'Topic',
           contentHintText: 'Message #${channel.name}');
       });
 
+      group('with non-empty but vacuous topics', () {
+        testWidgets('realm_empty_topic_display_name', (tester) async {
+          await prepare(tester, narrow: narrow, mandatoryTopics: true);
+          await enterTopic(tester, narrow: narrow,
+            topic: eg.defaultRealmEmptyTopicDisplayName);
+          await tester.pump();
+          checkComposeBoxHintTexts(tester,
+            topicHintText: 'Topic',
+            contentHintText: 'Message #${channel.name}');
+        });
+
+        testWidgets('"(no topic)"', (tester) async {
+          await prepare(tester, narrow: narrow, mandatoryTopics: true);
+          await enterTopic(tester, narrow: narrow,
+            topic: '(no topic)');
+          await tester.pump();
+          checkComposeBoxHintTexts(tester,
+            topicHintText: 'Topic',
+            contentHintText: 'Message #${channel.name}');
+        });
+      });
+
       testWidgets('with non-empty topic', (tester) async {
         await prepare(tester, narrow: narrow, mandatoryTopics: true);
         await enterTopic(tester, narrow: narrow, topic: 'new topic');
@@ -703,6 +762,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 +771,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 +787,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 +815,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)',
