Skip to content

Commit

Permalink
wip; api: Make displayName nullable
Browse files Browse the repository at this point in the history
Signed-off-by: Zixuan James Li <[email protected]>
  • Loading branch information
PIG208 committed Jan 28, 2025
1 parent a2e4aa4 commit 0fe861f
Show file tree
Hide file tree
Showing 11 changed files with 14 additions and 28 deletions.
2 changes: 1 addition & 1 deletion lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ extension type const TopicName(String _value) {
/// 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;
String? get displayName => _value.isEmpty ? null : _value;

/// The key to use for "same topic as" comparisons.
String canonicalize() => apiName.toLowerCase();
Expand Down
2 changes: 0 additions & 2 deletions lib/model/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -865,13 +865,11 @@ class TopicAutocompleteQuery extends AutocompleteQuery {
bool testTopic(PerAccountStore store, TopicName topic) {
// TODO(#881): Sort by match relevance, like web does.

// ignore: unnecessary_null_comparison // null topic names soon to be enabled
if (topic.displayName == null) {
return store.realmEmptyTopicDisplayName.toLowerCase()
.contains(raw.toLowerCase());
}
return topic.displayName != raw
// ignore: unnecessary_non_null_assertion // null topic names soon to be enabled
&& topic.displayName!.toLowerCase().contains(raw.toLowerCase());
}

Expand Down
2 changes: 0 additions & 2 deletions lib/widgets/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -329,13 +329,11 @@ class TopicAutocomplete extends AutocompleteField<TopicAutocompleteQuery, TopicA
@override
Widget buildItem(BuildContext context, int index, TopicAutocompleteResult option) {
final Widget child;
// ignore: unnecessary_null_comparison // null topic names soon to be enabled
if (option.topic.displayName == null) {
final store = PerAccountStoreWidget.of(context);
child = Text(store.realmEmptyTopicDisplayName,
style: const TextStyle(fontStyle: FontStyle.italic));
} else {
// ignore: unnecessary_non_null_assertion // null topic names soon to be enabled
child = Text(option.topic.displayName!);
}

Expand Down
4 changes: 0 additions & 4 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ class ComposeTopicController extends ComposeController<TopicValidationError> {
}

void setTopic(TopicName newTopic) {
// ignore: dead_null_aware_expression // null topic names soon to be enabled
value = TextEditingValue(text: newTopic.displayName ?? '');
}
}
Expand Down Expand Up @@ -440,7 +439,6 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve
color: designVariables.textInput.withFadedAlpha(0.5));

if (widget.destination.destination
// ignore: constant_pattern_never_matches_value_type // null topic names soon to be enabled
case StreamDestination(topic: TopicName(displayName: null))) {
// TODO: This applies to the entire hint text; ideally we'd only want to
// italize the "general chat" text, but [TextField] doesn't seem to have
Expand Down Expand Up @@ -546,7 +544,6 @@ class _StreamContentInputState extends State<_StreamContentInput> {
destination: TopicNarrow(widget.narrow.streamId, topic),
controller: widget.controller,
hintText: zulipLocalizations.composeBoxChannelContentHint(
// ignore: dead_null_aware_expression // null topic names soon to be enabled
streamName, topic.displayName ?? store.realmEmptyTopicDisplayName));
}
}
Expand Down Expand Up @@ -612,7 +609,6 @@ class _FixedDestinationContentInput extends StatelessWidget {
final streamName = store.streams[streamId]?.name
?? zulipLocalizations.composeBoxUnknownChannelName;
return zulipLocalizations.composeBoxChannelContentHint(
// 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.
Expand Down
2 changes: 0 additions & 2 deletions lib/widgets/inbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -526,15 +526,13 @@ class _TopicItem extends StatelessWidget {
child: Text(
style: TextStyle(
fontSize: 17,
// ignore: unnecessary_null_comparison // null topic names soon to be enabled
fontStyle: (topic.displayName == null) ? null : FontStyle.italic,
height: (20 / 17),
// TODO(design) check if this is the right variable
color: designVariables.labelMenuButton,
),
maxLines: 2,
overflow: TextOverflow.ellipsis,
// ignore: dead_null_aware_expression // null topic names soon to be enabled
topic.displayName ?? store.realmEmptyTopicDisplayName))),
const SizedBox(width: 12),
if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign),
Expand Down
4 changes: 0 additions & 4 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -362,10 +362,8 @@ class MessageListAppBarTitle extends StatelessWidget {
return Row(
mainAxisSize: MainAxisSize.min,
children: [
// ignore: dead_null_aware_expression // null topic names soon to be enabled
Flexible(child: Text(topic.displayName ?? store.realmEmptyTopicDisplayName, style: TextStyle(
fontSize: 13,
// ignore: unnecessary_null_comparison // null topic names soon to be enabled
fontStyle: (topic.displayName == null) ? null : FontStyle.italic,
).merge(weightVariableTextStyle(context)))),
if (icon != null)
Expand Down Expand Up @@ -1098,13 +1096,11 @@ class StreamMessageRecipientHeader extends StatelessWidget {
child: Row(
children: [
Flexible(
// ignore: dead_null_aware_expression // null topic names soon to be enabled
child: Text(topic.displayName ?? store.realmEmptyTopicDisplayName,
// TODO: Give a way to see the whole topic (maybe a
// long-press interaction?)
overflow: TextOverflow.ellipsis,
style: recipientHeaderTextStyle(context).copyWith(
// ignore: unnecessary_null_comparison // null topic names soon to be enabled
fontStyle: (topic.displayName == null) ? null : FontStyle.italic,
))),
const SizedBox(width: 4),
Expand Down
2 changes: 1 addition & 1 deletion test/api/model/model_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +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');
Subject<String?> get displayName => has((x) => x.displayName, 'displayName');
}

extension StreamMessageChecks on Subject<StreamMessage> {
Expand Down
8 changes: 4 additions & 4 deletions test/widgets/autocomplete_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ void main() {
await tester.tap(find.text('Topic three'));
await tester.pumpAndSettle();
check(tester.widget<TextField>(topicInputFinder).controller!.text)
.equals(topic3.name.displayName);
.equals(topic3.name.displayName!);
check(find.text('Topic one' )).findsNothing();
check(find.text('Topic two' )).findsNothing();
check(find.text('Topic three')).findsOne(); // shown in `_TopicInput` once
Expand Down Expand Up @@ -354,7 +354,7 @@ void main() {
await tester.pumpAndSettle();

check(find.text(eg.defaultRealmEmptyTopicDisplayName)).findsOne();
}, skip: true); // null topic names soon to be enabled
});

testWidgets('match general chat in autocomplete', (tester) async {
final topic = eg.getStreamTopicsEntry(name: '');
Expand All @@ -366,7 +366,7 @@ void main() {
await tester.pumpAndSettle();

check(find.text(eg.defaultRealmEmptyTopicDisplayName)).findsOne();
}, skip: true); // null topic names soon to be enabled
});

testWidgets('autocomplete to general chat sets topic to empty string', (tester) async {
final topic = eg.getStreamTopicsEntry(name: '');
Expand All @@ -383,6 +383,6 @@ void main() {
check(controller.value).text.equals('');

await tester.pump(Duration.zero);
}, skip: true); // null topic names soon to be enabled
});
});
}
10 changes: 5 additions & 5 deletions test/widgets/compose_box_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -350,15 +350,15 @@ void main() {
checkComposeBoxHintTexts(tester,
topicHintText: eg.defaultRealmEmptyTopicDisplayName,
contentHintText: 'Message #${channel.name} > ${eg.defaultRealmEmptyTopicDisplayName}');
}, skip: true); // null topic names soon to be enabled
});

testWidgets('to ChannelNarrow without topic; mandatory topics', (tester) async {
await prepare(tester, narrow: ChannelNarrow(channel.streamId),
mandatoryTopics: true);
checkComposeBoxHintTexts(tester,
topicHintText: 'Topic',
contentHintText: 'Message #${channel.name} > ${eg.defaultRealmEmptyTopicDisplayName}');
}, skip: true); // null topic names soon to be enabled
});

testWidgets('legacy: to ChannelNarrow without topic', (tester) async {
await prepare(tester, narrow: ChannelNarrow(channel.streamId),
Expand Down Expand Up @@ -390,7 +390,7 @@ void main() {
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 {
await prepare(tester, narrow: DmNarrow.withUser(
Expand Down Expand Up @@ -710,7 +710,7 @@ void main() {
'content': 'test content',
'read_by_sender': 'true',
});
}, skip: true); // null topic names soon to be enabled
});

testWidgets('legacy: empty topic -> (no topic)', (tester) async {
await setupAndTapSend(tester,
Expand Down Expand Up @@ -740,7 +740,7 @@ void main() {
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,
Expand Down
2 changes: 1 addition & 1 deletion test/widgets/inbox_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ void main() {
unreadMessages: [eg.streamMessage(stream: channel, topic: '')]);

check(find.text(eg.defaultRealmEmptyTopicDisplayName)).findsOne();
}, skip: true); // null topic names soon to be enabled
});
});

group('topic visibility', () {
Expand Down
4 changes: 2 additions & 2 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ void main() {
await tester.pump();
check(findInMessageList('stream name')).length.equals(1);
check(findInMessageList(eg.defaultRealmEmptyTopicDisplayName)).length.equals(1);
}, skip: true); // null topic names soon to be enabled
});

testWidgets('show general chat for empty topics without channel name', (tester) async {
await setupMessageListPage(tester,
Expand All @@ -822,7 +822,7 @@ void main() {
await tester.pump();
check(findInMessageList('stream name')).length.equals(0);
check(findInMessageList(eg.defaultRealmEmptyTopicDisplayName)).length.equals(1);
}, skip: true); // null topic names soon to be enabled
});

testWidgets('show topic visibility icon when followed', (tester) async {
await setupMessageListPage(tester,
Expand Down

0 comments on commit 0fe861f

Please sign in to comment.