diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 1d3cbad495..081537cec1 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -300,7 +300,11 @@ void showTopicActionSheet(BuildContext context, { pageContext: pageContext); })); - if (someMessageIdInTopic != null) { + // TODO: check for other cases that may disallow this action (e.g.: time + // limit for editing topics). + if (someMessageIdInTopic != null + // ignore: unnecessary_null_comparison // null topic names soon to be enabled + && topic.displayName != null) { optionButtons.add(ResolveUnresolveButton(pageContext: pageContext, topic: topic, someMessageIdInTopic: someMessageIdInTopic)); diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 7f47046d11..7c0c012331 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -157,7 +157,12 @@ class ComposeTopicController extends ComposeController { @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 @@ -165,7 +170,20 @@ class ComposeTopicController extends ComposeController { /// /// 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 _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,31 +596,61 @@ 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 topic name to show in the hint text, or null to show no topic. + TopicName? _hintTopic() { + if (widget.controller.topic.isTopicVacuous) { + if (widget.controller.topic.mandatory) { + // 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); + } + @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); final zulipLocalizations = ZulipLocalizations.of(context); + final streamName = store.streams[widget.narrow.streamId]?.name ?? zulipLocalizations.unknownChannelName; - final topic = TopicName(widget.controller.topic.textNormalized); + final hintTopic = _hintTopic(); + final hintDestination = hintTopic == null + // No i18n of this use of "#" and ">" string; 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 + ? '#$streamName' + // ignore: dead_null_aware_expression // null topic names soon to be enabled + : '#$streamName > ${hintTopic.displayName ?? store.realmEmptyTopicDisplayName}'; + return _ContentInput( narrow: widget.narrow, - destination: TopicNarrow(widget.narrow.streamId, topic), + destination: TopicNarrow(widget.narrow.streamId, + TopicName(widget.controller.topic.textNormalized)), controller: widget.controller, - hintText: zulipLocalizations.composeBoxChannelContentHint( - // No i18n of this use of "#" and ">" string; 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 - '#$streamName > ${topic.displayName}')); + hintText: zulipLocalizations.composeBoxChannelContentHint(hintDestination)); } } @@ -658,7 +713,8 @@ class _FixedDestinationContentInput extends StatelessWidget { // 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 > ${topic.displayName}'); + // ignore: dead_null_aware_expression // null topic names soon to be enabled + '#$streamName > ${topic.displayName ?? store.realmEmptyTopicDisplayName}'); case DmNarrow(otherRecipientIds: []): // The self-1:1 thread. return zulipLocalizations.composeBoxSelfDmContentHint; diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index dc611d3ca9..8aeeec4eed 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -353,24 +353,27 @@ void main() { Future showFromAppBar(WidgetTester tester, { ZulipStream? channel, - String topic = someTopic, + TopicName? topic, List? messages, }) async { final effectiveChannel = channel ?? someChannel; + final effectiveTopic = topic ?? TopicName(someTopic); final effectiveMessages = messages ?? [someMessage]; - assert(effectiveMessages.every((m) => m.topic.apiName == topic)); + assert(effectiveMessages.every((m) => m.topic.apiName == effectiveTopic.apiName)); connection.prepare(json: eg.newestGetMessagesResult( foundOldest: true, messages: effectiveMessages).toJson()); await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, child: MessageListPage( - initNarrow: eg.topicNarrow(effectiveChannel.streamId, topic)))); + initNarrow: TopicNarrow(effectiveChannel.streamId, effectiveTopic)))); // global store, per-account store, and message list get loaded await tester.pumpAndSettle(); final topicRow = find.descendant( of: find.byType(ZulipAppBar), - matching: find.text(topic)); + matching: find.text( + // ignore: dead_null_aware_expression // null topic names soon to be enabled + effectiveTopic.displayName ?? eg.defaultRealmEmptyTopicDisplayName)); await tester.longPress(topicRow); // sheet appears onscreen; default duration of bottom-sheet enter animation await tester.pump(const Duration(milliseconds: 250)); @@ -446,6 +449,16 @@ void main() { check(findButtonForLabel('Mark as unresolved')).findsNothing(); }); + testWidgets('show from app bar: resolve/unresolve not offered when topic is empty', (tester) async { + await prepare(); + final message = eg.streamMessage(stream: someChannel, topic: ''); + await showFromAppBar(tester, + topic: TopicName(''), + messages: [message]); + check(findButtonForLabel('Mark as resolved')).findsNothing(); + check(findButtonForLabel('Mark as unresolved')).findsNothing(); + }, skip: true); // null topic names soon to be enabled + testWidgets('show from recipient header', (tester) async { await prepare(); await showFromRecipientHeader(tester); @@ -485,7 +498,7 @@ void main() { final message = eg.streamMessage( stream: someChannel, topic: topic, sender: eg.otherUser); await showFromAppBar(tester, - channel: someChannel, topic: topic, messages: [message]); + channel: someChannel, topic: TopicName(topic), messages: [message]); } void checkButtons(List expectedButtonFinders) { @@ -697,7 +710,8 @@ void main() { testWidgets('unresolve: happy path', (tester) async { final message = eg.streamMessage(stream: someChannel, topic: '✔ zulip'); await prepare(topic: '✔ zulip'); - await showFromAppBar(tester, topic: '✔ zulip', messages: [message]); + await showFromAppBar(tester, + topic: TopicName('✔ zulip'), messages: [message]); connection.takeRequests(); connection.prepare(json: UpdateMessageResult().toJson()); await tester.tap(findButtonForLabel('Mark as unresolved')); @@ -710,7 +724,8 @@ void main() { testWidgets('unresolve: weird prefix', (tester) async { final message = eg.streamMessage(stream: someChannel, topic: '✔ ✔ zulip'); await prepare(topic: '✔ ✔ zulip'); - await showFromAppBar(tester, topic: '✔ ✔ zulip', messages: [message]); + await showFromAppBar(tester, + topic: TopicName('✔ ✔ zulip'), messages: [message]); connection.takeRequests(); connection.prepare(json: UpdateMessageResult().toJson()); await tester.tap(findButtonForLabel('Mark as unresolved')); diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 52d2d1c851..e02fd97a15 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -47,6 +47,7 @@ void main() { List otherUsers = const [], List streams = const [], bool? mandatoryTopics, + int? zulipFeatureLevel, }) async { if (narrow case ChannelNarrow(:var streamId) || TopicNarrow(: var streamId)) { assert(streams.any((stream) => stream.streamId == streamId), @@ -54,8 +55,10 @@ void main() { } 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, )); @@ -326,11 +329,15 @@ void main() { Future prepare(WidgetTester tester, { required Narrow narrow, + bool? mandatoryTopics, + int? zulipFeatureLevel, }) async { await prepareComposeBox(tester, narrow: narrow, otherUsers: [eg.otherUser, eg.thirdUser], - streams: [channel]); + streams: [channel], + mandatoryTopics: mandatoryTopics, + zulipFeatureLevel: zulipFeatureLevel); } /// This checks the input's configured hint text without regard to whether @@ -351,17 +358,60 @@ void main() { .decoration.isNotNull().hintText.equals(contentHintText); } - group('to ChannelNarrow', () { - testWidgets('with empty topic', (tester) async { - await prepare(tester, narrow: ChannelNarrow(channel.streamId)); + group('to ChannelNarrow, topics not mandatory', () { + final narrow = ChannelNarrow(channel.streamId); + + 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}'); + }); + + 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: 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', contentHintText: 'Message #${channel.name} > (no topic)'); }); testWidgets('with non-empty topic', (tester) async { - final narrow = ChannelNarrow(channel.streamId); - await prepare(tester, narrow: narrow); + await prepare(tester, narrow: narrow, mandatoryTopics: false); await enterTopic(tester, narrow: narrow, topic: 'new topic'); await tester.pump(); checkComposeBoxHintTexts(tester, @@ -370,11 +420,70 @@ void main() { }); }); - testWidgets('to TopicNarrow', (tester) async { - await prepare(tester, - narrow: TopicNarrow(channel.streamId, TopicName('topic'))); - checkComposeBoxHintTexts(tester, - contentHintText: 'Message #${channel.name} > topic'); + group('to ChannelNarrow, mandatory topics', () { + final narrow = ChannelNarrow(channel.streamId); + + testWidgets('with empty topic', (tester) async { + await prepare(tester, narrow: narrow, mandatoryTopics: true); + checkComposeBoxHintTexts(tester, + topicHintText: 'Topic', + contentHintText: 'Message #${channel.name}'); + }); + + 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'); + await tester.pump(); + checkComposeBoxHintTexts(tester, + topicHintText: 'Topic', + contentHintText: 'Message #${channel.name} > new topic'); + }); + }); + + group('to TopicNarrow', () { + testWidgets('with non-empty topic', (tester) async { + await prepare(tester, + narrow: TopicNarrow(channel.streamId, TopicName('topic'))); + checkComposeBoxHintTexts(tester, + contentHintText: 'Message #${channel.name} > topic'); + }); + + testWidgets('with empty topic', (tester) async { + await prepare(tester, + narrow: TopicNarrow(channel.streamId, TopicName(''))); + checkComposeBoxHintTexts(tester, contentHintText: + 'Message #${channel.name} > ${eg.defaultRealmEmptyTopicDisplayName}'); + }, skip: true); // null topic names soon to be enabled }); testWidgets('to DmNarrow with self', (tester) async { @@ -653,6 +762,7 @@ void main() { Future setupAndTapSend(WidgetTester tester, { required String topicInputText, required bool mandatoryTopics, + int? zulipFeatureLevel, }) async { TypingNotifier.debugEnable = false; addTearDown(TypingNotifier.debugReset); @@ -661,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'); @@ -676,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() + ..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() ..method.equals('POST') ..url.path.equals('/api/v1/messages') @@ -693,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)',