Skip to content

general chat #2: support sending to empty topic from channel narrow, hide resolve/unresolve conditionally #1364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
78 changes: 67 additions & 11 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand All @@ -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));
}
}

Expand Down Expand Up @@ -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;
Expand Down
29 changes: 22 additions & 7 deletions test/widgets/action_sheet_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -353,24 +353,27 @@ void main() {

Future<void> showFromAppBar(WidgetTester tester, {
ZulipStream? channel,
String topic = someTopic,
TopicName? topic,
List<StreamMessage>? 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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<Finder> expectedButtonFinders) {
Expand Down Expand Up @@ -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'));
Expand All @@ -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'));
Expand Down
Loading