Skip to content

Mute muted users (Chris's revision, 1 of 2) #1560

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 12 commits into
base: main
Choose a base branch
from
Open
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
8 changes: 2 additions & 6 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -1007,17 +1007,13 @@
"@noEarlierMessages": {
"description": "Text to show at the start of a message list if there are no earlier messages."
},
"mutedSender": "Muted sender",
"@mutedSender": {
"description": "Name for a muted user to display in message list."
},
"revealButtonLabel": "Reveal message for muted sender",
"revealButtonLabel": "Reveal message",
"@revealButtonLabel": {
"description": "Label for the button revealing hidden message from a muted sender in message list."
},
"mutedUser": "Muted user",
"@mutedUser": {
"description": "Name for a muted user to display all over the app."
"description": "Text to display in place of a muted user's name."
},
"scrollToBottomTooltip": "Scroll to bottom",
"@scrollToBottomTooltip": {
Expand Down
4 changes: 0 additions & 4 deletions assets/l10n/app_pl.arb
Original file line number Diff line number Diff line change
Expand Up @@ -1089,10 +1089,6 @@
"@mutedSender": {
"description": "Name for a muted user to display in message list."
},
"revealButtonLabel": "Odsłoń wiadomość od wyciszonego użytkownika",
"@revealButtonLabel": {
"description": "Label for the button revealing hidden message from a muted sender in message list."
},
"mutedUser": "Wyciszony użytkownik",
"@mutedUser": {
"description": "Name for a muted user to display all over the app."
Expand Down
4 changes: 0 additions & 4 deletions assets/l10n/app_ru.arb
Original file line number Diff line number Diff line change
Expand Up @@ -1081,10 +1081,6 @@
"@mutedSender": {
"description": "Name for a muted user to display in message list."
},
"revealButtonLabel": "Показать сообщение отключенного отправителя",
"@revealButtonLabel": {
"description": "Label for the button revealing hidden message from a muted sender in message list."
},
"mutedUser": "Отключенный пользователь",
"@mutedUser": {
"description": "Name for a muted user to display all over the app."
Expand Down
10 changes: 2 additions & 8 deletions lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1499,19 +1499,13 @@ abstract class ZulipLocalizations {
/// **'No earlier messages'**
String get noEarlierMessages;

/// Name for a muted user to display in message list.
///
/// In en, this message translates to:
/// **'Muted sender'**
String get mutedSender;

/// Label for the button revealing hidden message from a muted sender in message list.
///
/// In en, this message translates to:
/// **'Reveal message for muted sender'**
/// **'Reveal message'**
String get revealButtonLabel;

/// Name for a muted user to display all over the app.
/// Text to display in place of a muted user's name.
///
/// In en, this message translates to:
/// **'Muted user'**
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -820,10 +820,7 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
String get noEarlierMessages => 'No earlier messages';

@override
String get mutedSender => 'Muted sender';

@override
String get revealButtonLabel => 'Reveal message for muted sender';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Muted user';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_de.dart
Original file line number Diff line number Diff line change
Expand Up @@ -820,10 +820,7 @@ class ZulipLocalizationsDe extends ZulipLocalizations {
String get noEarlierMessages => 'No earlier messages';

@override
String get mutedSender => 'Muted sender';

@override
String get revealButtonLabel => 'Reveal message for muted sender';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Muted user';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_en.dart
Original file line number Diff line number Diff line change
Expand Up @@ -820,10 +820,7 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
String get noEarlierMessages => 'No earlier messages';

@override
String get mutedSender => 'Muted sender';

@override
String get revealButtonLabel => 'Reveal message for muted sender';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Muted user';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_ja.dart
Original file line number Diff line number Diff line change
Expand Up @@ -820,10 +820,7 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
String get noEarlierMessages => 'No earlier messages';

@override
String get mutedSender => 'Muted sender';

@override
String get revealButtonLabel => 'Reveal message for muted sender';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Muted user';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_nb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -820,10 +820,7 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
String get noEarlierMessages => 'No earlier messages';

@override
String get mutedSender => 'Muted sender';

@override
String get revealButtonLabel => 'Reveal message for muted sender';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Muted user';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_pl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -833,10 +833,7 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
String get noEarlierMessages => 'Brak historii';

@override
String get mutedSender => 'Wyciszony nadawca';

@override
String get revealButtonLabel => 'Odsłoń wiadomość od wyciszonego użytkownika';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Wyciszony użytkownik';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
Expand Up @@ -835,10 +835,7 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
String get noEarlierMessages => 'Предшествующих сообщений нет';

@override
String get mutedSender => 'Отключенный отправитель';

@override
String get revealButtonLabel => 'Показать сообщение отключенного отправителя';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Отключенный пользователь';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_sk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -822,10 +822,7 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
String get noEarlierMessages => 'No earlier messages';

@override
String get mutedSender => 'Muted sender';

@override
String get revealButtonLabel => 'Reveal message for muted sender';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Muted user';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_uk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -834,10 +834,7 @@ class ZulipLocalizationsUk extends ZulipLocalizations {
String get noEarlierMessages => 'Немає попередніх повідомлень';

@override
String get mutedSender => 'Muted sender';

@override
String get revealButtonLabel => 'Reveal message for muted sender';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Muted user';
Expand Down
5 changes: 1 addition & 4 deletions lib/generated/l10n/zulip_localizations_zh.dart
Original file line number Diff line number Diff line change
Expand Up @@ -820,10 +820,7 @@ class ZulipLocalizationsZh extends ZulipLocalizations {
String get noEarlierMessages => 'No earlier messages';

@override
String get mutedSender => 'Muted sender';

@override
String get revealButtonLabel => 'Reveal message for muted sender';
String get revealButtonLabel => 'Reveal message';

@override
String get mutedUser => 'Muted user';
Expand Down
43 changes: 30 additions & 13 deletions lib/model/compose.dart
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,33 @@ String wrapWithBacktickFence({required String content, String? infoString}) {
/// To omit the user ID part ("|13313") whenever the name part is unambiguous,
/// pass the full UserStore. This means accepting a linear scan
/// through all users; avoid it in performance-sensitive codepaths.
///
/// See also [userMentionFromMessage].
String userMention(User user, {bool silent = false, UserStore? users}) {
bool includeUserId = users == null
|| users.allUsers.where((u) => u.fullName == user.fullName)
.take(2).length == 2;

return '@${silent ? '_' : ''}**${user.fullName}${includeUserId ? '|${user.userId}' : ''}**';
return _userMentionImpl(
silent: silent,
fullName: user.fullName,
userId: includeUserId ? user.userId : null);
}

/// An @-mention of an individual user, like @**Chris Bobbe|13313**,
/// from sender data in a [Message].
///
/// The user ID part ("|13313") is always included.
///
/// See also [userMention].
String userMentionFromMessage(Message message, {bool silent = false, required UserStore users}) =>
_userMentionImpl(
silent: silent,
fullName: users.senderDisplayName(message, replaceIfMuted: false),
userId: message.senderId);

String _userMentionImpl({required bool silent, required String fullName, int? userId}) =>
'@${silent ? '_' : ''}**$fullName${userId != null ? '|$userId' : ''}**';

/// An @-mention of all the users in a conversation, like @**channel**.
String wildcardMention(WildcardMentionOption wildcardOption, {
required PerAccountStore store,
Expand Down Expand Up @@ -190,13 +209,11 @@ String quoteAndReplyPlaceholder(
PerAccountStore store, {
required Message message,
}) {
final sender = store.getUser(message.senderId);
assert(sender != null); // TODO(#716): should use `store.senderDisplayName`
final url = narrowLink(store,
SendableNarrow.ofMessage(message, selfUserId: store.selfUserId),
nearMessageId: message.id);
// See note in [quoteAndReply] about asking `mention` to omit the |<id> part.
return '${userMention(sender!, silent: true)} ${inlineLink('said', url)}: ' // TODO(#1285)
return '${userMentionFromMessage(message, silent: true, users: store)} '
'${inlineLink('said', url)}: ' // TODO(#1285)
'*${zulipLocalizations.composeBoxLoadingMessage(message.id)}*\n';
}

Expand All @@ -212,14 +229,14 @@ String quoteAndReply(PerAccountStore store, {
required Message message,
required String rawContent,
}) {
final sender = store.getUser(message.senderId);
assert(sender != null); // TODO(#716): should use `store.senderDisplayName`
final url = narrowLink(store,
SendableNarrow.ofMessage(message, selfUserId: store.selfUserId),
nearMessageId: message.id);
// Could ask `mention` to omit the |<id> part unless the mention is ambiguous…
// but that would mean a linear scan through all users, and the extra noise
// won't much matter with the already probably-long message link in there too.
Comment on lines -220 to -222
Copy link
Member

Choose a reason for hiding this comment

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

I think this comment is still relevant for explaining a choice we made about how this logic works.

(The name mention needs updating, though — already did, to say userMention.)

return '${userMention(sender!, silent: true)} ${inlineLink('said', url)}:\n' // TODO(#1285)
'${wrapWithBacktickFence(content: rawContent, infoString: 'quote')}';
// Could ask userMentionFromMessage to omit the |<id> part unless the mention
// is ambiguous… but that would mean a linear scan through all users,
// and the extra noise won't much matter with the already probably-long
// message link in there too.
return '${userMentionFromMessage(message, silent: true, users: store)} '
'${inlineLink('said', url)}:\n' // TODO(#1285)
'${wrapWithBacktickFence(content: rawContent, infoString: 'quote')}';
}
9 changes: 6 additions & 3 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -675,10 +675,13 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
return byDate.difference(dateJoined).inDays >= realmWaitingPeriodThreshold;
}

/// The given user's real email address, if known, for displaying in the UI.
/// The user's real email address, if known, for displaying in the UI.
///
/// Returns null if self-user isn't able to see [user]'s real email address.
String? userDisplayEmail(User user) {
/// Returns null if self-user isn't able to see the user's real email address,
/// or if the user isn't actually a user we know about.
String? userDisplayEmail(int userId) {
final user = getUser(userId);
if (user == null) return null;
if (zulipFeatureLevel >= 163) { // TODO(server-7)
// A non-null value means self-user has access to [user]'s real email,
// while a null value means it doesn't have access to the email.
Expand Down
27 changes: 20 additions & 7 deletions lib/model/user.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,27 +44,40 @@ mixin UserStore on PerAccountStoreBase {

/// The name to show the given user as in the UI, even for unknown users.
///
/// This is the user's [User.fullName] if the user is known,
/// and otherwise a translation of "(unknown user)".
/// If the user is muted and [replaceIfMuted] is true (the default),
/// this is [ZulipLocalizations.mutedUser].
///
/// Otherwise this is the user's [User.fullName] if the user is known,
/// or (if unknown) [ZulipLocalizations.unknownUserName].
///
/// When a [Message] is available which the user sent,
/// use [senderDisplayName] instead for a better-informed fallback.
String userDisplayName(int userId) {
String userDisplayName(int userId, {bool replaceIfMuted = true}) {
if (replaceIfMuted && isUserMuted(userId)) {
return GlobalLocalizations.zulipLocalizations.mutedUser;
}
Comment on lines -52 to +58
Copy link
Member

Choose a reason for hiding this comment

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

muted-users: Say "Muted user" to replace a user's name, where applicable

(Done by adding an is-muted condition in store.userDisplayName and
store.senderDisplayName, with an opt-out param.)

Cool, I like this structure.

The inventory of where we say "Muted user" and where we don't is also helpful:

If Chris is muted, we'll now show "Muted user" where before we would
show "Chris Bobbe", in the following places:

- Message-list page:
  - DM-narrow app bar.
  - DM recipient headers.
  - The sender row on messages. This and message content will get
    more treatment in a separate commit.
  - Emoji-reaction chips on messages.
  - Typing indicators ("Muted user is typing…"), but we'll be
    excluding muted users, coming up in a separate commit.
  - Voter names in poll messages.
- DM items in the Inbox page. (Messages from muted users are
  automatically marked as read, but they can end up in the inbox if
  you un-mark them as read.)
- @-mention autocomplete, but we'll be excluding muted users, coming
  up in a separate commit.
- Items in the Direct messages ("recent DMs") page. We'll be
  excluding DMs where everyone is muted, coming up in a separate
  commit.
- User items in custom profile fields.

We *don't* do this replacement in the following places, i.e., we'll
still show "Chris Bobbe" if Chris is muted:

- Sender name in the header of the lightbox page. (This follows
  web.)
- The "hint text" for the compose box in a DM narrow: it will still
  say "Message @Chris Bobbe", not "Message @Muted user". (This
  follows web.)
- The user's name at the top of the Profile page.
- We won't generate malformed @-mention syntax like
  @_**Muted user|13313**.

return getUser(userId)?.fullName
?? GlobalLocalizations.zulipLocalizations.unknownUserName;
}

/// The name to show for the given message's sender in the UI.
///
/// If the user is known (see [getUser]), this is their current [User.fullName].
/// If the sender is muted and [replaceIfMuted] is true (the default),
/// this is [ZulipLocalizations.mutedUser].
///
/// Otherwise, if the user is known (see [getUser]),
/// this is their current [User.fullName].
/// If unknown, this uses the fallback value conveniently provided on the
/// [Message] object itself, namely [Message.senderFullName].
///
/// For a user who isn't the sender of some known message,
/// see [userDisplayName].
String senderDisplayName(Message message) {
return getUser(message.senderId)?.fullName
?? message.senderFullName;
String senderDisplayName(Message message, {bool replaceIfMuted = true}) {
final senderId = message.senderId;
if (replaceIfMuted && isUserMuted(senderId)) {
return GlobalLocalizations.zulipLocalizations.mutedUser;
}
return getUser(senderId)?.fullName ?? message.senderFullName;
}

/// Whether the user with [userId] is muted by the self-user.
Expand Down
25 changes: 25 additions & 0 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,8 @@ void showMessageActionSheet({required BuildContext context, required Message mes
final markAsUnreadSupported = store.zulipFeatureLevel >= 155; // TODO(server-6)
final showMarkAsUnreadButton = markAsUnreadSupported && isMessageRead;

final isSenderMuted = store.isUserMuted(message.senderId);

final optionButtons = [
if (popularEmojiLoaded)
ReactionButtons(message: message, pageContext: pageContext),
Expand All @@ -597,6 +599,8 @@ void showMessageActionSheet({required BuildContext context, required Message mes
QuoteAndReplyButton(message: message, pageContext: pageContext),
if (showMarkAsUnreadButton)
MarkAsUnreadButton(message: message, pageContext: pageContext),
if (isSenderMuted)
UnrevealMutedMessageButton(message: message, pageContext: pageContext),
CopyMessageTextButton(message: message, pageContext: pageContext),
CopyMessageLinkButton(message: message, pageContext: pageContext),
ShareButton(message: message, pageContext: pageContext),
Expand Down Expand Up @@ -902,6 +906,27 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton {
}
}

class UnrevealMutedMessageButton extends MessageActionSheetMenuItemButton {
UnrevealMutedMessageButton({
super.key,
required super.message,
required super.pageContext,
});

@override
IconData get icon => ZulipIcons.eye_off;

@override
String label(ZulipLocalizations zulipLocalizations) {
return zulipLocalizations.actionSheetOptionHideMutedMessage;
}

@override
void onPressed() {
findMessageListPage().unrevealMutedMessage(message.id);
}
}

class CopyMessageTextButton extends MessageActionSheetMenuItemButton {
CopyMessageTextButton({super.key, required super.message, required super.pageContext});

Expand Down
5 changes: 2 additions & 3 deletions lib/widgets/autocomplete.dart
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,9 @@ class _MentionAutocompleteItem extends StatelessWidget {
String? sublabel;
switch (option) {
case UserMentionAutocompleteResult(:var userId):
final user = store.getUser(userId)!; // must exist because UserMentionAutocompleteResult
avatar = Avatar(userId: userId, size: 36, borderRadius: 4);
label = user.fullName;
sublabel = store.userDisplayEmail(user);
label = store.userDisplayName(userId);
sublabel = store.userDisplayEmail(userId);
case WildcardMentionAutocompleteResult(:var wildcardOption):
avatar = SizedBox.square(dimension: 36,
child: const Icon(ZulipIcons.three_person, size: 24));
Expand Down
Loading