compose: Support sending to empty topic

PIG208 · PIG208 · commit 2835293947c3 · 2025-02-26T17:13:18.000-05: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
@@ -157,38 +157,71 @@ 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;
+  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 [textNormalized] is vacuous (see [isTopicVacuous]).
+  /// This returns a string formatted like "#stream name > topic name"
+  /// when [textNormalized] is not vacuous (see [isTopicVacuous]), or when
+  /// the topic is not [mandatory] and the user has chosen to leave it vacuous.
   ///
-  /// 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,
+  }) {
     if (!isTopicVacuous) {
       return '#$streamName > $textNormalized';
     }
 
-    // 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),
+      // because we expect a call to action for the user to pick one first.
+      || !contentHasFocus
+    ) {
       return '#$streamName';
     }
 
-    return '#$streamName > $kNoTopicTopic';
+    final vacuousTopicDisplayName =
+      // TODO(server-10): simplify away conditional
+      textNormalized.isEmpty ? store.realmEmptyTopicDisplayName : textNormalized;
+    return '#$streamName > $vacuousTopicDisplayName';
   }
 
   @override
@@ -582,10 +615,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
@@ -595,11 +635,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();
   }
 
@@ -615,7 +660,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,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('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('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 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,13 +432,22 @@ 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}');
       });
 
+      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 {
         await prepare(tester, narrow: narrow, mandatoryTopics: true);
         await enterTopic(tester, narrow: narrow, topic: 'new topic');
@@ -705,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);
@@ -713,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');
@@ -728,10 +777,21 @@ void main() {
         expectedMessage: 'Topics are required in this organization.');
     }
 
-    testWidgets('empty topic -> "(no topic)"', (tester) async {
+    testWidgets('empty topic -> 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('');
+    }, skip: true); // null topic names soon to be enabled
+
+    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')
@@ -745,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);
+    }, skip: true); // null topic names soon to be enabled
+
     testWidgets('if topics are mandatory, reject "(no topic)"', (tester) async {
       await setupAndTapSend(tester,
         topicInputText: '(no topic)',
