compose: Support sending to empty topic

PIG208 · PIG208 · commit dcaf1659a527 · 2025-03-19T20:51:39.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,12 +163,31 @@ 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;
+  }
 
   @override
   List<TopicValidationError> _computeValidationErrors() {
@@ -561,10 +580,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
@@ -574,21 +600,30 @@ 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();
   }
 
   /// The hint text for [_ContentInput] computed from the topic input.
   ///
-  /// This is a string referring to the send destination as something like
-  /// "#stream name" when topics are [ComposeTopicController.mandatory] but the
-  /// topic input is empty.
+  /// If topics are [ComposeTopicController.mandatory], the topic name will
+  /// only be included if the topic input is non-empty, in the form of
+  /// "#stream name > topic name".
+  ///
+  /// If topics are not mandatory, the topic name will only be included when the
+  /// user intentionally leaves the topic input empty.
   ///
-  /// Otherwise, the destination is formatted like "#stream name > topic name".
+  /// Otherwise, this will be 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:
@@ -606,14 +641,25 @@ class _StreamContentInputState extends State<_StreamContentInput> {
     }
 
     assert(widget.controller.topic._isTopicVacuous);
-    // Sending to a vacuous topic (see [ComposeTopicController._isTopicVacuous])
-    // is not possible if topics are mandatory.
-    if (widget.controller.topic.mandatory) {
+    if (
+      // Sending to a vacuous topic (see [ComposeTopicController._isTopicVacuous])
+      // is not possible if topics are mandatory.
+      widget.controller.topic.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.
+      || !widget.controller.contentFocusNode.hasFocus
+    ) {
       return zulipLocalizations.composeBoxChannelContentHint('#$streamName');
     }
 
+    final textNormalized = widget.controller.topic.textNormalized;
+    final vacuousTopicDisplayName =
+      // TODO(server-10): simplify away conditional
+      textNormalized.isEmpty ? store.realmEmptyTopicDisplayName : textNormalized;
     return zulipLocalizations.composeBoxChannelContentHint(
-      '#$streamName > $kNoTopicTopic');
+      '#$streamName > $vacuousTopicDisplayName');
   }
 
   @override
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)',
