Skip to content

Commit

Permalink
api [nfc]: Introduce TopicName.displayName, and use where needed
Browse files Browse the repository at this point in the history
Each of these call sites will need to be updated for zulip#1250.  Those
sites can now be found by listing the references to this getter.

As a bonus the type-checker will point out most of the places that
need updates, because this getter will become nullable.  (The
exceptions are where it's used in a string interpolation, because
`null.toString()` is valid and returns "null".)
  • Loading branch information
gnprice authored and Gaurav-Kushwaha-1225 committed Jan 21, 2025
1 parent 9d2c0d0 commit b56b10c
Show file tree
Hide file tree
Showing 10 changed files with 27 additions and 12 deletions.
12 changes: 12 additions & 0 deletions lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -659,8 +659,20 @@ enum MessageFlag {
// TODO(#1250): Migrate all implicit uses as String; remove "implements String".
extension type const TopicName(String _value) implements String {
/// The string this topic is identified by in the Zulip API.
///
/// This should be used in constructing HTTP requests to the server,
/// but rarely for other purposes. See [displayName] and [canonicalize].
String get apiName => _value;

/// The string this topic is displayed as to the user in our UI.
///
/// At the moment this always equals [apiName].
/// In the future this will become null for the "general chat" topic (#1250),
/// so that UI code can identify when it needs to represent the topic
/// specially in the way prescribed for "general chat".
// TODO(#1250) carry out that plan
String get displayName => _value;

/// The key to use for "same topic as" comparisons.
String canonicalize() => apiName.toLowerCase();

Expand Down
3 changes: 2 additions & 1 deletion lib/model/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,8 @@ class TopicAutocompleteQuery extends AutocompleteQuery {

bool testTopic(TopicName topic) {
// TODO(#881): Sort by match relevance, like web does.
return topic != raw && topic.toLowerCase().contains(raw.toLowerCase());
return topic.displayName != raw
&& topic.displayName.toLowerCase().contains(raw.toLowerCase());
}

@override
Expand Down
2 changes: 1 addition & 1 deletion lib/model/narrow.dart
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class TopicNarrow extends Narrow implements SendableNarrow {
StreamDestination get destination => StreamDestination(streamId, topic);

@override
String toString() => 'TopicNarrow($streamId, $topic)';
String toString() => 'TopicNarrow($streamId, ${topic.displayName})';

@override
bool operator ==(Object other) {
Expand Down
4 changes: 2 additions & 2 deletions lib/notifications/display.dart
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,9 @@ class NotificationDisplayManager {
// the first.
messagingStyle.conversationTitle = switch (data.recipient) {
FcmMessageChannelRecipient(:var streamName?, :var topic) =>
'#$streamName > $topic',
'#$streamName > ${topic.displayName}',
FcmMessageChannelRecipient(:var topic) =>
'#(unknown channel) > $topic', // TODO get stream name from data
'#(unknown channel) > ${topic.displayName}', // TODO get stream name from data
FcmMessageDmRecipient(:var allRecipientIds) when allRecipientIds.length > 2 =>
zulipLocalizations.notifGroupDmConversationLabel(
data.senderFullName, allRecipientIds.length - 2), // TODO use others' names, from data
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,6 @@ class TopicAutocomplete extends AutocompleteField<TopicAutocompleteQuery, TopicA
},
child: Padding(
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
child: Text(option.topic)));
child: Text(option.topic.displayName)));
}
}
5 changes: 3 additions & 2 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
}

void setTopic(TopicName newTopic) {
value = TextEditingValue(text: newTopic);
value = TextEditingValue(text: newTopic.displayName);
}
}

Expand Down Expand Up @@ -550,7 +550,8 @@ class _FixedDestinationContentInput extends StatelessWidget {
final store = PerAccountStoreWidget.of(context);
final streamName = store.streams[streamId]?.name
?? zulipLocalizations.composeBoxUnknownChannelName;
return zulipLocalizations.composeBoxChannelContentHint(streamName, topic);
return zulipLocalizations.composeBoxChannelContentHint(
streamName, topic.displayName);

case DmNarrow(otherRecipientIds: []): // The self-1:1 thread.
return zulipLocalizations.composeBoxSelfDmContentHint;
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/inbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ class _TopicItem extends StatelessWidget {
),
maxLines: 2,
overflow: TextOverflow.ellipsis,
topic))),
topic.displayName))),
const SizedBox(width: 12),
if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign),
// TODO(design) copies the "@" marker color; is there a better color?
Expand Down
4 changes: 2 additions & 2 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ class MessageListAppBarTitle extends StatelessWidget {
return Row(
mainAxisSize: MainAxisSize.min,
children: [
Flexible(child: Text(topic, style: const TextStyle(
Flexible(child: Text(topic.displayName, style: const TextStyle(
fontSize: 13,
).merge(weightVariableTextStyle(context)))),
if (icon != null)
Expand Down Expand Up @@ -1091,7 +1091,7 @@ class StreamMessageRecipientHeader extends StatelessWidget {
child: Row(
children: [
Flexible(
child: Text(topic,
child: Text(topic.displayName,
// TODO: Give a way to see the whole topic (maybe a
// long-press interaction?)
overflow: TextOverflow.ellipsis,
Expand Down
1 change: 1 addition & 0 deletions test/api/model/model_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ extension MessageChecks on Subject<Message> {

extension TopicNameChecks on Subject<TopicName> {
Subject<String> get apiName => has((x) => x.apiName, 'apiName');
Subject<String> get displayName => has((x) => x.displayName, 'displayName');
}

extension StreamMessageChecks on Subject<StreamMessage> {
Expand Down
4 changes: 2 additions & 2 deletions test/widgets/autocomplete_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ void main() {

group('TopicAutocomplete', () {
void checkTopicShown(GetStreamTopicsEntry topic, PerAccountStore store, {required bool expected}) {
check(find.text(topic.name).evaluate().length).equals(expected ? 1 : 0);
check(find.text(topic.name.displayName).evaluate().length).equals(expected ? 1 : 0);
}

testWidgets('options appear, disappear, and change correctly', (WidgetTester tester) async {
Expand All @@ -302,7 +302,7 @@ void main() {
await tester.tap(find.text('Topic three'));
await tester.pumpAndSettle();
check(tester.widget<TextField>(topicInputFinder).controller!.text)
.equals(topic3.name);
.equals(topic3.name.displayName);
checkTopicShown(topic1, store, expected: false);
checkTopicShown(topic2, store, expected: false);
checkTopicShown(topic3, store, expected: true); // shown in `_TopicInput` once
Expand Down

0 comments on commit b56b10c

Please sign in to comment.