Skip to content

Commit cca7b00

Browse files
committed
compose: Support sending to empty topic
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 <[email protected]>
1 parent 091da1c commit cca7b00

File tree

2 files changed

+119
-12
lines changed

2 files changed

+119
-12
lines changed

lib/widgets/compose_box.dart

+43-3
Original file line numberDiff line numberDiff line change
@@ -157,15 +157,34 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
157157
@override
158158
String _computeTextNormalized() {
159159
String trimmed = text.trim();
160-
return trimmed.isEmpty ? kNoTopicTopic : trimmed;
160+
// TODO(server-10): simplify
161+
if (store.zulipFeatureLevel < 334) {
162+
return trimmed.isEmpty ? kNoTopicTopic : trimmed;
163+
}
164+
165+
return trimmed;
161166
}
162167

163168
/// Whether [textNormalized] would fail a mandatory-topics check
164169
/// (see [mandatory]).
165170
///
166171
/// The term "Vacuous" draws distinction from [String.isEmpty], in the sense
167172
/// that certain strings are not empty but also indicate the absence of a topic.
168-
bool get isTopicVacuous => textNormalized == kNoTopicTopic;
173+
bool get isTopicVacuous {
174+
bool result = textNormalized.isEmpty
175+
// We keep checking for '(no topic)' regardless of the feature level
176+
// because it remains equivalent to an empty topic even when FL >= 334.
177+
// This can change in the future:
178+
// https://chat.zulip.org/#narrow/channel/412-api-documentation/topic/.28realm_.29mandatory_topics.20behavior/near/2062391
179+
|| textNormalized == kNoTopicTopic;
180+
181+
// TODO(server-10): simplify
182+
if (store.zulipFeatureLevel >= 334) {
183+
result |= textNormalized == store.realmEmptyTopicDisplayName;
184+
}
185+
186+
return result;
187+
}
169188

170189
@override
171190
List<TopicValidationError> _computeValidationErrors() {
@@ -558,10 +577,17 @@ class _StreamContentInputState extends State<_StreamContentInput> {
558577
});
559578
}
560579

580+
void _contentFocusChanged() {
581+
setState(() {
582+
// The relevant state lives on widget.controller.contentFocusNode itself.
583+
});
584+
}
585+
561586
@override
562587
void initState() {
563588
super.initState();
564589
widget.controller.topic.addListener(_topicChanged);
590+
widget.controller.contentFocusNode.addListener(_contentFocusChanged);
565591
}
566592

567593
@override
@@ -571,11 +597,16 @@ class _StreamContentInputState extends State<_StreamContentInput> {
571597
oldWidget.controller.topic.removeListener(_topicChanged);
572598
widget.controller.topic.addListener(_topicChanged);
573599
}
600+
if (widget.controller.contentFocusNode != oldWidget.controller.contentFocusNode) {
601+
oldWidget.controller.contentFocusNode.removeListener(_contentFocusChanged);
602+
widget.controller.contentFocusNode.addListener(_contentFocusChanged);
603+
}
574604
}
575605

576606
@override
577607
void dispose() {
578608
widget.controller.topic.removeListener(_topicChanged);
609+
widget.controller.contentFocusNode.removeListener(_contentFocusChanged);
579610
super.dispose();
580611
}
581612

@@ -586,6 +617,13 @@ class _StreamContentInputState extends State<_StreamContentInput> {
586617
// The chosen topic can't be sent to, so don't show it.
587618
return null;
588619
}
620+
if (!widget.controller.contentFocusNode.hasFocus) {
621+
// Do not fall back to a vacuous topic unless the user explicitly chooses
622+
// to do so (by skipping topic input and moving focus to content input),
623+
// so that the user is not encouraged to use vacuous topic when they
624+
// have not interacted with the inputs at all.
625+
return null;
626+
}
589627
}
590628

591629
return TopicName(widget.controller.topic.textNormalized);
@@ -604,7 +642,9 @@ class _StreamContentInputState extends State<_StreamContentInput> {
604642
// Zulip expresses channels and topics, not any normal English punctuation,
605643
// so don't make sense to translate. See:
606644
// https://github.com/zulip/zulip-flutter/pull/1148#discussion_r1941990585
607-
? '#$streamName' : '#$streamName > ${hintTopic.displayName}';
645+
? '#$streamName'
646+
// ignore: dead_null_aware_expression // null topic names soon to be enabled
647+
: '#$streamName > ${hintTopic.displayName ?? store.realmEmptyTopicDisplayName}';
608648

609649
return _ContentInput(
610650
narrow: widget.narrow,

test/widgets/compose_box_test.dart

+76-9
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,18 @@ void main() {
4747
List<User> otherUsers = const [],
4848
List<ZulipStream> streams = const [],
4949
bool? mandatoryTopics,
50+
int? zulipFeatureLevel,
5051
}) async {
5152
if (narrow case ChannelNarrow(:var streamId) || TopicNarrow(: var streamId)) {
5253
assert(streams.any((stream) => stream.streamId == streamId),
5354
'Add a channel with "streamId" the same as of $narrow.streamId to the store.');
5455
}
5556
addTearDown(testBinding.reset);
5657
selfUser ??= eg.selfUser;
57-
final selfAccount = eg.account(user: selfUser);
58+
zulipFeatureLevel ??= eg.futureZulipFeatureLevel;
59+
final selfAccount = eg.account(user: selfUser, zulipFeatureLevel: zulipFeatureLevel);
5860
await testBinding.globalStore.add(selfAccount, eg.initialSnapshot(
61+
zulipFeatureLevel: zulipFeatureLevel,
5962
realmMandatoryTopics: mandatoryTopics,
6063
));
6164

@@ -327,12 +330,14 @@ void main() {
327330
Future<void> prepare(WidgetTester tester, {
328331
required Narrow narrow,
329332
bool? mandatoryTopics,
333+
int? zulipFeatureLevel,
330334
}) async {
331335
await prepareComposeBox(tester,
332336
narrow: narrow,
333337
otherUsers: [eg.otherUser, eg.thirdUser],
334338
streams: [channel],
335-
mandatoryTopics: mandatoryTopics);
339+
mandatoryTopics: mandatoryTopics,
340+
zulipFeatureLevel: zulipFeatureLevel);
336341
}
337342

338343
/// This checks the input's configured hint text without regard to whether
@@ -356,16 +361,49 @@ void main() {
356361
group('to ChannelNarrow, topics not mandatory', () {
357362
final narrow = ChannelNarrow(channel.streamId);
358363

359-
testWidgets('with empty topic', (tester) async {
364+
testWidgets('with empty topic, topic input has focus', (tester) async {
360365
await prepare(tester, narrow: narrow, mandatoryTopics: false);
366+
await enterTopic(tester, narrow: narrow, topic: '');
367+
await tester.pump();
361368
checkComposeBoxHintTexts(tester,
362369
topicHintText: 'Topic',
363-
contentHintText: 'Message #${channel.name} > (no topic)');
370+
contentHintText: 'Message #${channel.name}');
364371
});
365372

366-
testWidgets('with non-empty but vacuous topic', (tester) async {
373+
testWidgets('with non-empty but vacuous topic, topic input has focus', (tester) async {
374+
await prepare(tester, narrow: narrow, mandatoryTopics: false);
375+
await enterTopic(tester, narrow: narrow,
376+
topic: eg.defaultRealmEmptyTopicDisplayName);
377+
await tester.pump();
378+
checkComposeBoxHintTexts(tester,
379+
topicHintText: 'Topic',
380+
contentHintText: 'Message #${channel.name}');
381+
});
382+
383+
testWidgets('legacy: with empty topic, topic input has focus', (tester) async {
384+
await prepare(tester, narrow: narrow, mandatoryTopics: false,
385+
zulipFeatureLevel: 333);
386+
await enterTopic(tester, narrow: narrow, topic: '');
387+
await tester.pump();
388+
checkComposeBoxHintTexts(tester,
389+
topicHintText: 'Topic',
390+
contentHintText: 'Message #${channel.name}');
391+
});
392+
393+
testWidgets('with empty topic, content input has focus', (tester) async {
367394
await prepare(tester, narrow: narrow, mandatoryTopics: false);
368-
await enterTopic(tester, narrow: narrow, topic: '(no topic)');
395+
await enterContent(tester, '');
396+
await tester.pump();
397+
checkComposeBoxHintTexts(tester,
398+
topicHintText: 'Topic',
399+
contentHintText: 'Message #${channel.name} > '
400+
'${eg.defaultRealmEmptyTopicDisplayName}');
401+
}, skip: true); // null topic names soon to be enabled
402+
403+
testWidgets('legacy: with empty topic, content input has focus', (tester) async {
404+
await prepare(tester, narrow: narrow, mandatoryTopics: false,
405+
zulipFeatureLevel: 333);
406+
await enterContent(tester, '');
369407
await tester.pump();
370408
checkComposeBoxHintTexts(tester,
371409
topicHintText: 'Topic',
@@ -394,13 +432,22 @@ void main() {
394432

395433
testWidgets('with non-empty but vacuous topic', (tester) async {
396434
await prepare(tester, narrow: narrow, mandatoryTopics: true);
397-
await enterTopic(tester, narrow: narrow, topic: '(no topic)');
435+
await enterTopic(tester, narrow: narrow,
436+
topic: eg.defaultRealmEmptyTopicDisplayName);
398437
await tester.pump();
399438
checkComposeBoxHintTexts(tester,
400439
topicHintText: 'Topic',
401440
contentHintText: 'Message #${channel.name}');
402441
});
403442

443+
testWidgets('legacy: with empty topic', (tester) async {
444+
await prepare(tester, narrow: narrow, mandatoryTopics: true,
445+
zulipFeatureLevel: 333);
446+
checkComposeBoxHintTexts(tester,
447+
topicHintText: 'Topic',
448+
contentHintText: 'Message #${channel.name}');
449+
});
450+
404451
testWidgets('with non-empty topic', (tester) async {
405452
await prepare(tester, narrow: narrow, mandatoryTopics: true);
406453
await enterTopic(tester, narrow: narrow, topic: 'new topic');
@@ -703,6 +750,7 @@ void main() {
703750
Future<void> setupAndTapSend(WidgetTester tester, {
704751
required String topicInputText,
705752
required bool mandatoryTopics,
753+
int? zulipFeatureLevel,
706754
}) async {
707755
TypingNotifier.debugEnable = false;
708756
addTearDown(TypingNotifier.debugReset);
@@ -711,7 +759,8 @@ void main() {
711759
final narrow = ChannelNarrow(channel.streamId);
712760
await prepareComposeBox(tester,
713761
narrow: narrow, streams: [channel],
714-
mandatoryTopics: mandatoryTopics);
762+
mandatoryTopics: mandatoryTopics,
763+
zulipFeatureLevel: zulipFeatureLevel);
715764

716765
await enterTopic(tester, narrow: narrow, topic: topicInputText);
717766
await tester.enterText(contentInputFinder, 'test content');
@@ -726,10 +775,21 @@ void main() {
726775
expectedMessage: 'Topics are required in this organization.');
727776
}
728777

729-
testWidgets('empty topic -> "(no topic)"', (tester) async {
778+
testWidgets('empty topic -> ""', (tester) async {
730779
await setupAndTapSend(tester,
731780
topicInputText: '',
732781
mandatoryTopics: false);
782+
check(connection.lastRequest).isA<http.Request>()
783+
..method.equals('POST')
784+
..url.path.equals('/api/v1/messages')
785+
..bodyFields['topic'].equals('');
786+
});
787+
788+
testWidgets('legacy: empty topic -> "(no topic)"', (tester) async {
789+
await setupAndTapSend(tester,
790+
topicInputText: '',
791+
mandatoryTopics: false,
792+
zulipFeatureLevel: 333);
733793
check(connection.lastRequest).isA<http.Request>()
734794
..method.equals('POST')
735795
..url.path.equals('/api/v1/messages')
@@ -743,6 +803,13 @@ void main() {
743803
checkMessageNotSent(tester);
744804
});
745805

806+
testWidgets('if topics are mandatory, reject `realmEmptyTopicDisplayName`', (tester) async {
807+
await setupAndTapSend(tester,
808+
topicInputText: eg.defaultRealmEmptyTopicDisplayName,
809+
mandatoryTopics: true);
810+
checkMessageNotSent(tester);
811+
});
812+
746813
testWidgets('if topics are mandatory, reject "(no topic)"', (tester) async {
747814
await setupAndTapSend(tester,
748815
topicInputText: '(no topic)',

0 commit comments

Comments
 (0)