Skip to content

general chat #3: support alternative styling for topic input on edits/focus/blur #1365

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
7 changes: 7 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,13 @@
"@composeBoxTopicHintText": {
"description": "Hint text for topic input widget in compose box."
},
"composeBoxEnterTopicOrSkipHintText": "Enter a topic (skip for “{defaultTopicName}”)",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compose: Change topic input hint text

This is similar to web's behavior.  When topics are not mandatory:

- an alternative hint text "Enter a topic (skip for general chat)"
  is shown when the topic input has focus;

- an opaque placeholder text (e.g.: "general chat") is shown if
  the user skipped to content input;

Because the topic input is always shown in a message list page channel
narrow (assuming permission to send messages), this also adds an intial
state:

- a short hint text, "Topic", is shown if the user hasn't
  interacted with topic or content inputs at all.

This only changes the topic input's hint text.

See CZO discussion for design details:
  https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/general.20chat.20design.20.23F1297/near/2106736

Commit-message nits:

  • 'Enter a topic (skip for “general chat”)'
  • spelling of 'initial'

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compose: Change topic input hint text

When you rebase, this commit will need a flutter run to update the new Ukrainian translation file that exists in main.

"@composeBoxEnterTopicOrSkipHintText": {
"description": "Hint text for topic input widget in compose box when topics are optional.",
"placeholders": {
"defaultTopicName": {"type": "String", "example": "general chat"}
}
},
"composeBoxUploadingFilename": "Uploading {filename}…",
"@composeBoxUploadingFilename": {
"description": "Placeholder in compose box showing the specified file is currently uploading.",
Expand Down
6 changes: 1 addition & 5 deletions lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,7 @@ extension type const TopicName(String _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;
String? get displayName => _value.isEmpty ? null : _value;

/// The key to use for "same topic as" comparisons.
String canonicalize() => apiName.toLowerCase();
Expand Down
6 changes: 5 additions & 1 deletion lib/api/route/channels.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@ part 'channels.g.dart';
/// https://zulip.com/api/get-stream-topics
Future<GetStreamTopicsResult> getStreamTopics(ApiConnection connection, {
required int streamId,
bool? allowEmptyTopicName,
}) {
return connection.get('getStreamTopics', GetStreamTopicsResult.fromJson, 'users/me/$streamId/topics', {});
assert(allowEmptyTopicName != false, '`allowEmptyTopicName` should only be true or null');
return connection.get('getStreamTopics', GetStreamTopicsResult.fromJson, 'users/me/$streamId/topics', {
if (allowEmptyTopicName != null) 'allow_empty_topic_name': allowEmptyTopicName,
});
}

@JsonSerializable(fieldRename: FieldRename.snake)
Expand Down
1 change: 1 addition & 0 deletions lib/api/route/events.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Future<InitialSnapshot> registerQueue(ApiConnection connection) {
'user_avatar_url_field_optional': false, // TODO(#254): turn on
'stream_typing_notifications': true,
'user_settings_object': true,
'empty_topic_name': true,
},
});
}
Expand Down
9 changes: 9 additions & 0 deletions lib/api/route/messages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ part 'messages.g.dart';
Future<Message?> getMessageCompat(ApiConnection connection, {
required int messageId,
bool? applyMarkdown,
bool? allowEmptyTopicName,
}) async {
final useLegacyApi = connection.zulipFeatureLevel! < 120;
if (useLegacyApi) {
assert(allowEmptyTopicName == null);
final response = await getMessages(connection,
narrow: [ApiNarrowMessageId(messageId)],
anchor: NumericAnchor(messageId),
Expand All @@ -37,6 +39,7 @@ Future<Message?> getMessageCompat(ApiConnection connection, {
final response = await getMessage(connection,
messageId: messageId,
applyMarkdown: applyMarkdown,
allowEmptyTopicName: allowEmptyTopicName,
);
return response.message;
} on ZulipApiException catch (e) {
Expand All @@ -57,10 +60,13 @@ Future<Message?> getMessageCompat(ApiConnection connection, {
Future<GetMessageResult> getMessage(ApiConnection connection, {
required int messageId,
bool? applyMarkdown,
bool? allowEmptyTopicName,
}) {
assert(allowEmptyTopicName != false, '`allowEmptyTopicName` should only be true or null');
assert(connection.zulipFeatureLevel! >= 120);
return connection.get('getMessage', GetMessageResult.fromJson, 'messages/$messageId', {
if (applyMarkdown != null) 'apply_markdown': applyMarkdown,
if (allowEmptyTopicName != null) 'allow_empty_topic_name': allowEmptyTopicName,
});
}

Expand Down Expand Up @@ -89,8 +95,10 @@ Future<GetMessagesResult> getMessages(ApiConnection connection, {
required int numAfter,
bool? clientGravatar,
bool? applyMarkdown,
bool? allowEmptyTopicName,
// bool? useFirstUnreadAnchor // omitted because deprecated
}) {
assert(allowEmptyTopicName != false, '`allowEmptyTopicName` should only be true or null');
return connection.get('getMessages', GetMessagesResult.fromJson, 'messages', {
'narrow': resolveApiNarrowForServer(narrow, connection.zulipFeatureLevel!),
'anchor': RawParameter(anchor.toJson()),
Expand All @@ -99,6 +107,7 @@ Future<GetMessagesResult> getMessages(ApiConnection connection, {
'num_after': numAfter,
if (clientGravatar != null) 'client_gravatar': clientGravatar,
if (applyMarkdown != null) 'apply_markdown': applyMarkdown,
if (allowEmptyTopicName != null) 'allow_empty_topic_name': allowEmptyTopicName,
});
}

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,12 @@ abstract class ZulipLocalizations {
/// **'Topic'**
String get composeBoxTopicHintText;

/// Hint text for topic input widget in compose box when topics are optional.
///
/// In en, this message translates to:
/// **'Enter a topic (skip for “{defaultTopicName}”)'**
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName);

/// Placeholder in compose box showing the specified file is currently uploading.
///
/// In en, this message translates to:
Expand Down
5 changes: 5 additions & 0 deletions lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,11 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
@override
String get composeBoxTopicHintText => 'Topic';

@override
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName) {
return 'Enter a topic (skip for “$defaultTopicName”)';
}

@override
String composeBoxUploadingFilename(String filename) {
return 'Uploading $filename…';
Expand Down
5 changes: 5 additions & 0 deletions lib/generated/l10n/zulip_localizations_en.dart
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,11 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
@override
String get composeBoxTopicHintText => 'Topic';

@override
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName) {
return 'Enter a topic (skip for “$defaultTopicName”)';
}

@override
String composeBoxUploadingFilename(String filename) {
return 'Uploading $filename…';
Expand Down
5 changes: 5 additions & 0 deletions lib/generated/l10n/zulip_localizations_ja.dart
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,11 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
@override
String get composeBoxTopicHintText => 'Topic';

@override
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName) {
return 'Enter a topic (skip for “$defaultTopicName”)';
}

@override
String composeBoxUploadingFilename(String filename) {
return 'Uploading $filename…';
Expand Down
5 changes: 5 additions & 0 deletions lib/generated/l10n/zulip_localizations_nb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,11 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
@override
String get composeBoxTopicHintText => 'Topic';

@override
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName) {
return 'Enter a topic (skip for “$defaultTopicName”)';
}

@override
String composeBoxUploadingFilename(String filename) {
return 'Uploading $filename…';
Expand Down
5 changes: 5 additions & 0 deletions lib/generated/l10n/zulip_localizations_pl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,11 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
@override
String get composeBoxTopicHintText => 'Wątek';

@override
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName) {
return 'Enter a topic (skip for “$defaultTopicName”)';
}

@override
String composeBoxUploadingFilename(String filename) {
return 'Przekazywanie $filename…';
Expand Down
5 changes: 5 additions & 0 deletions lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,11 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
@override
String get composeBoxTopicHintText => 'Тема';

@override
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName) {
return 'Enter a topic (skip for “$defaultTopicName”)';
}

@override
String composeBoxUploadingFilename(String filename) {
return 'Загрузка $filename…';
Expand Down
5 changes: 5 additions & 0 deletions lib/generated/l10n/zulip_localizations_sk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,11 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
@override
String get composeBoxTopicHintText => 'Topic';

@override
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName) {
return 'Enter a topic (skip for “$defaultTopicName”)';
}

@override
String composeBoxUploadingFilename(String filename) {
return 'Uploading $filename…';
Expand Down
5 changes: 5 additions & 0 deletions lib/generated/l10n/zulip_localizations_uk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,11 @@ class ZulipLocalizationsUk extends ZulipLocalizations {
@override
String get composeBoxTopicHintText => 'Тема';

@override
String composeBoxEnterTopicOrSkipHintText(String defaultTopicName) {
return 'Enter a topic (skip for “$defaultTopicName”)';
}

@override
String composeBoxUploadingFilename(String filename) {
return 'Завантаження $filename…';
Expand Down
8 changes: 5 additions & 3 deletions lib/model/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,11 @@ class TopicAutocompleteView extends AutocompleteView<TopicAutocompleteQuery, Top
Future<void> _fetch() async {
assert(!_isFetching);
_isFetching = true;
final result = await getStreamTopics(store.connection, streamId: streamId);
final result = await getStreamTopics(store.connection, streamId: streamId,
allowEmptyTopicName:
// TODO(server-10): simplify this condition away
store.zulipFeatureLevel >= 334 ? true : null,
);
_topics = result.topics.map((e) => e.name);
_isFetching = false;
return _startSearch();
Expand Down Expand Up @@ -942,13 +946,11 @@ class TopicAutocompleteQuery extends AutocompleteQuery {
bool testTopic(TopicName topic, PerAccountStore store) {
// 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
6 changes: 6 additions & 0 deletions lib/model/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,9 @@ class MessageListView with ChangeNotifier, _MessageSequence {
anchor: AnchorCode.newest,
numBefore: kMessageListFetchBatchSize,
numAfter: 0,
allowEmptyTopicName:
// TODO(server-10): simplify this condition away
store.zulipFeatureLevel >= 334 ? true : null,
);
if (this.generation > generation) return;
_adjustNarrowForTopicPermalink(result.messages.firstOrNull);
Expand Down Expand Up @@ -582,6 +585,9 @@ class MessageListView with ChangeNotifier, _MessageSequence {
includeAnchor: false,
numBefore: kMessageListFetchBatchSize,
numAfter: 0,
allowEmptyTopicName:
// TODO(server-10): simplify this condition away
store.zulipFeatureLevel >= 334 ? true : null,
);
} catch (e) {
hasFetchError = true;
Expand Down
1 change: 1 addition & 0 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
/// be empty otherwise.
// TODO(server-10) simplify this
String get realmEmptyTopicDisplayName {
assert(zulipFeatureLevel >= 334);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be better in a separate commit after this one.

assert(_realmEmptyTopicDisplayName != null); // TODO(log)
return _realmEmptyTopicDisplayName ?? 'general chat';
}
Expand Down
4 changes: 1 addition & 3 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,7 @@ void showTopicActionSheet(BuildContext context, {

// 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) {
if (someMessageIdInTopic != null && topic.displayName != null) {
optionButtons.add(ResolveUnresolveButton(pageContext: pageContext,
topic: topic,
someMessageIdInTopic: someMessageIdInTopic));
Expand Down
3 changes: 3 additions & 0 deletions lib/widgets/actions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,12 @@ abstract final class ZulipAction {
// On final failure or success, auto-dismiss the snackbar.
final zulipLocalizations = ZulipLocalizations.of(context);
try {
final store = PerAccountStoreWidget.of(context);
fetchedMessage = await getMessageCompat(PerAccountStoreWidget.of(context).connection,
messageId: messageId,
applyMarkdown: false,
// TODO(server-10): simplify this condition away
allowEmptyTopicName: store.zulipFeatureLevel >= 334 ? true : null,
);
if (fetchedMessage == null) {
errorMessage = zulipLocalizations.errorMessageDoesNotSeemToExist;
Expand Down
2 changes: 0 additions & 2 deletions lib/widgets/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,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
Loading