Skip to content

Commit e49119c

Browse files
committed
PeopleAutocomplete: Stop using fake user-like objects for wildcard mentions
As the comments point out, we've been using a hack where we make fake "user" objects for @ALL and @everyone, so we can enlist UserItemRaw to show items for these in the people autocomplete. Clear out that hack and make a new UI component for showing these wildcard mentions in the autocomplete. While we're at it, make some improvements: - Show the Font Awesome bullhorn icon, like the web app. - Re: the explanatory text, previously "(Notify everyone)": - It now says "Notify stream" when composing a stream message, and "Notify recipients" when composing a PM, following the web app. - It doesn't have parentheses anymore. The web app uses them to distinguish the explanatory text from the special string that triggers the mention, as in "all (Notify stream)". We already do that by putting the explanatory text on a new line, in a smaller font. - It's now wired up for translation. - "all" and "everyone" will now show up in a few more helpful cases: (a) if your search term matches the translation of those terms into your own language, and (b) if your search term matches the descriptive text (like "Notify stream"), in your own language. - We now use shared code (`typeahead.query_matches_source_attrs`) for match-modulo-diacritics logic, to help us decide whether to include `all` and `everyone` in the typeahead.
1 parent e4c4b26 commit e49119c

File tree

7 files changed

+246
-24
lines changed

7 files changed

+246
-24
lines changed

src/autocomplete/PeopleAutocomplete.js

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* @flow strict-local */
22

3-
import React, { useCallback } from 'react';
3+
import React, { useCallback, useContext } from 'react';
44
import type { Node } from 'react';
55
import { SectionList } from 'react-native';
66

@@ -16,6 +16,11 @@ import Popup from '../common/Popup';
1616
import { UserItemRaw } from '../users/UserItem';
1717
import UserGroupItem from '../user-groups/UserGroupItem';
1818
import { getOwnUserId } from '../users/userSelectors';
19+
import WildcardMentionItem, {
20+
getWildcardMentionForQuery,
21+
WildcardMentionType,
22+
} from './WildcardMentionItem';
23+
import { TranslationContext } from '../boot/TranslationProvider';
1924

2025
type Props = $ReadOnly<{|
2126
filter: string,
@@ -24,7 +29,10 @@ type Props = $ReadOnly<{|
2429
|}>;
2530

2631
export default function PeopleAutocomplete(props: Props): Node {
27-
const { filter, onAutocomplete } = props;
32+
const { filter, destinationNarrow, onAutocomplete } = props;
33+
34+
const _ = useContext(TranslationContext);
35+
2836
const mutedUsers = useSelector(getMutedUsers);
2937
const ownUserId = useSelector(getOwnUserId);
3038
const users = useSelector(getSortedUsers);
@@ -37,6 +45,13 @@ export default function PeopleAutocomplete(props: Props): Node {
3745
[onAutocomplete],
3846
);
3947

48+
const handleWildcardMentionAutocomplete = useCallback(
49+
(type, serverCanonicalString) => {
50+
onAutocomplete(`**${serverCanonicalString}**`);
51+
},
52+
[onAutocomplete],
53+
);
54+
4055
const handleUserItemAutocomplete = useCallback(
4156
(user: AutocompleteOption): void => {
4257
// If another user with the same full name is found, we send the
@@ -53,6 +68,7 @@ export default function PeopleAutocomplete(props: Props): Node {
5368
);
5469

5570
const filteredUserGroups = getAutocompleteUserGroupSuggestions(userGroups, filter);
71+
const wildcardMentionForQuery = getWildcardMentionForQuery(filter, destinationNarrow, _);
5672
const filteredUsers = getAutocompleteSuggestion(users, filter, ownUserId, mutedUsers);
5773

5874
if (filteredUserGroups.length + filteredUsers.length === 0) {
@@ -75,13 +91,23 @@ export default function PeopleAutocomplete(props: Props): Node {
7591
/>
7692
),
7793
}: Section<UserGroup>),
94+
({
95+
data: wildcardMentionForQuery != null ? [wildcardMentionForQuery] : [],
96+
renderItem: ({ item }) => (
97+
<WildcardMentionItem
98+
key={WildcardMentionType.getName(item)}
99+
type={item}
100+
destinationNarrow={destinationNarrow}
101+
onPress={handleWildcardMentionAutocomplete}
102+
/>
103+
),
104+
}: Section<WildcardMentionType>),
78105
({
79106
data: filteredUsers,
80107
renderItem: ({ item }) => (
81-
// "Raw" because some of our autocomplete suggestions are fake
82-
// synthetic "users" to represent @all and @everyone.
83-
// TODO display those in a UI that makes more sense for them,
84-
// and drop the fake "users" and use the normal UserItem.
108+
// "Raw" because some of these used to be fake synthetic "users" to
109+
// represent @all and @everyone. But now that we've stopped that:
110+
// TODO: Use the normal UserItem.
85111
<UserItemRaw
86112
key={item.user_id}
87113
user={item}
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
/* @flow strict-local */
2+
import React, { useCallback, useContext, useMemo } from 'react';
3+
import type { Node } from 'react';
4+
import { View } from 'react-native';
5+
import * as typeahead from '@zulip/shared/js/typeahead';
6+
7+
import type { GetText, Narrow } from '../types';
8+
import { IconWildcardMention } from '../common/Icons';
9+
import ZulipText from '../common/ZulipText';
10+
import Touchable from '../common/Touchable';
11+
import { createStyleSheet, ThemeContext } from '../styles';
12+
import { caseNarrowDefault, isStreamOrTopicNarrow } from '../utils/narrow';
13+
import { TranslationContext } from '../boot/TranslationProvider';
14+
15+
/**
16+
* A type of wildcard mention recognized by the server.
17+
*
18+
* For the string the server knows this by, see serverCanonicalStringOf.
19+
*
20+
* See user doc on wildcard mentions:
21+
* https://zulip.com/help/pm-mention-alert-notifications#wildcard-mentions
22+
*/
23+
export enum WildcardMentionType {
24+
// These values are arbitrary. For the string the server knows these by,
25+
// see serverCanonicalStringOf.
26+
All = 0,
27+
Everyone = 1,
28+
Stream = 2,
29+
}
30+
31+
/**
32+
* The canonical English string, like "everyone" or "all".
33+
*
34+
* See also serverCanonicalStringOf for the string the server knows this by.
35+
* (It might differ in a future where servers localize these.)
36+
*/
37+
// All of these should appear in messages_en.json so we can make the
38+
// wildcard mentions discoverable in the people autocomplete in the client's
39+
// own language. See getWildcardMentionForQuery.
40+
const englishCanonicalStringOf = (type: WildcardMentionType): string => {
41+
switch (type) {
42+
case WildcardMentionType.All:
43+
return 'all';
44+
case WildcardMentionType.Everyone:
45+
return 'everyone';
46+
case WildcardMentionType.Stream:
47+
return 'stream';
48+
}
49+
};
50+
51+
/**
52+
* The string recognized by the server, like "everyone" or "all".
53+
*
54+
* Currently the same as englishCanonicalStringOf, but that should change if
55+
* servers start localizing these.
56+
*/
57+
const serverCanonicalStringOf = englishCanonicalStringOf;
58+
59+
const descriptionOf = (
60+
type: WildcardMentionType,
61+
destinationNarrow: Narrow,
62+
_: GetText,
63+
): string => {
64+
switch (type) {
65+
case WildcardMentionType.All:
66+
case WildcardMentionType.Everyone:
67+
return caseNarrowDefault(
68+
destinationNarrow,
69+
{ topic: () => _('Notify stream'), pm: () => _('Notify recipients') },
70+
// A "destination narrow" should really be a conversation narrow
71+
// (i.e., stream or topic), but including this default case is easy,
72+
// and better than an error/crash in case we've missed that somehow.
73+
() => _('Notify everyone'),
74+
);
75+
case WildcardMentionType.Stream:
76+
return _('Notify stream');
77+
}
78+
};
79+
80+
/**
81+
* The enum's members, as an array in order of preference, with "stream".
82+
*
83+
* When a query matches more than one of these, choose the first one. For a
84+
* similar array that doesn't include WildcardMentionType.Stream, see
85+
* kOrderedTypesWithoutStream.
86+
*/
87+
// Greg points out:
88+
//
89+
// > The help center mentions @-all, sometimes also @-everyone, and never
90+
// > @-stream that I can see:
91+
// > https://zulip.com/help/mention-a-user-or-group
92+
// > https://zulip.com/help/pm-mention-alert-notifications
93+
// > So I think the right order of preference is [all, everyone, stream].
94+
const kOrderedTypesWithStream = [
95+
WildcardMentionType.All,
96+
WildcardMentionType.Everyone,
97+
WildcardMentionType.Stream,
98+
];
99+
100+
/**
101+
* The enum's members, as an array in order of preference, without "stream".
102+
*
103+
* When a query matches more than one of these, choose the first one. For a
104+
* similar array that includes WildcardMentionType.Stream, see
105+
* kOrderedTypesWithStream.
106+
*/
107+
// See implementation note at kOrderedTypesWithStream.
108+
const kOrderedTypesWithoutStream = [WildcardMentionType.All, WildcardMentionType.Everyone];
109+
110+
// This assumes that we'll never want to show two suggestions for one query.
111+
// That's OK, as long as all of WildcardMentionType's members are synonyms,
112+
// and it's nice not to crowd the autocomplete with multiple items that mean
113+
// the same thing. But we'll need to adapt if it turns out that all the
114+
// members aren't synonyms.
115+
export const getWildcardMentionForQuery = (
116+
query: string,
117+
destinationNarrow: Narrow,
118+
_: GetText,
119+
): WildcardMentionType | void =>
120+
(isStreamOrTopicNarrow(destinationNarrow)
121+
? kOrderedTypesWithStream
122+
: kOrderedTypesWithoutStream
123+
).find(
124+
type =>
125+
typeahead.query_matches_string(query, serverCanonicalStringOf(type), ' ')
126+
|| typeahead.query_matches_string(query, _(englishCanonicalStringOf(type)), ' '),
127+
);
128+
129+
type Props = $ReadOnly<{|
130+
type: WildcardMentionType,
131+
destinationNarrow: Narrow,
132+
onPress: (type: WildcardMentionType, serverCanonicalString: string) => void,
133+
|}>;
134+
135+
export default function WildcardMentionItem(props: Props): Node {
136+
const { type, destinationNarrow, onPress } = props;
137+
138+
const _ = useContext(TranslationContext);
139+
140+
const handlePress = useCallback(() => {
141+
onPress(type, serverCanonicalStringOf(type));
142+
}, [onPress, type]);
143+
144+
const themeContext = useContext(ThemeContext);
145+
146+
const styles = useMemo(
147+
() =>
148+
createStyleSheet({
149+
wrapper: {
150+
flexDirection: 'row',
151+
alignItems: 'center',
152+
padding: 8,
153+
154+
// Minimum touch target height:
155+
// https://material.io/design/usability/accessibility.html#layout-and-typography
156+
minHeight: 48,
157+
},
158+
textWrapper: {
159+
flex: 1,
160+
marginLeft: 8,
161+
},
162+
text: {},
163+
descriptionText: {
164+
fontSize: 10,
165+
color: 'hsl(0, 0%, 60%)',
166+
},
167+
}),
168+
[],
169+
);
170+
171+
return (
172+
<Touchable onPress={handlePress}>
173+
<View style={styles.wrapper}>
174+
<IconWildcardMention
175+
// Match the size of the avatar in UserItem, which also appears in
176+
// the people autocomplete. We're counting on this icon being a
177+
// square.
178+
size={32}
179+
color={themeContext.color}
180+
/>
181+
<View style={styles.textWrapper}>
182+
<ZulipText
183+
style={styles.text}
184+
text={serverCanonicalStringOf(type)}
185+
numberOfLines={1}
186+
ellipsizeMode="tail"
187+
/>
188+
<ZulipText
189+
style={styles.descriptionText}
190+
text={descriptionOf(type, destinationNarrow, _)}
191+
numberOfLines={1}
192+
ellipsizeMode="tail"
193+
/>
194+
</View>
195+
</View>
196+
</Touchable>
197+
);
198+
}

src/common/Icons.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,5 +109,9 @@ export const IconAttach: SpecificIconType = makeIcon(Feather, 'paperclip');
109109
export const IconAttachment: SpecificIconType = makeIcon(IoniconsIcon, 'document-attach-outline');
110110
export const IconGroup: SpecificIconType = makeIcon(FontAwesome, 'group');
111111
export const IconPlus: SpecificIconType = makeIcon(Feather, 'plus');
112+
113+
// WildcardMentionItem depends on this being square.
114+
export const IconWildcardMention: SpecificIconType = makeIcon(FontAwesome, 'bullhorn');
115+
112116
// eslint-disable-next-line react/function-component-definition
113117
export const IconWebPublic: SpecificIconType = props => <ZulipIcon name="globe" {...props} />;

src/common/UserAvatarWithPresence.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import PresenceStatusIndicator from './PresenceStatusIndicator';
99
import { AvatarURL } from '../utils/avatar';
1010
import { tryGetUserForId } from '../users/userSelectors';
1111
import { useSelector } from '../react-redux';
12+
import * as logging from '../utils/logging';
1213

1314
const styles = createStyleSheet({
1415
status: {
@@ -73,9 +74,7 @@ export function UserAvatarWithPresenceById(
7374

7475
const user = useSelector(state => tryGetUserForId(state, userId));
7576
if (!user) {
76-
// This condition really does happen, because UserItem can be passed a fake
77-
// pseudo-user by PeopleAutocomplete, to represent `@all` or `@everyone`.
78-
// TODO eliminate that, and use plain `getUserForId` here.
77+
logging.warn("UserAvatarWithPresence: couldn't find user for ID", { userId });
7978
return null;
8079
}
8180

src/users/__tests__/userHelpers-test.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,7 @@ describe('getAutocompleteSuggestion', () => {
5555
const meUser = eg.makeUser({ full_name: 'Me' });
5656
const users = deepFreeze([someGuyUser, meUser]);
5757

58-
const shouldMatch = [
59-
{ user_id: -1, full_name: 'all', email: '(Notify everyone)' },
60-
someGuyUser,
61-
];
58+
const shouldMatch = [someGuyUser];
6259
const filteredUsers = getAutocompleteSuggestion(users, '', meUser.user_id, Immutable.Map());
6360
expect(filteredUsers).toEqual(shouldMatch);
6461
});
@@ -70,7 +67,7 @@ describe('getAutocompleteSuggestion', () => {
7067

7168
const mutedUsers = Immutable.Map([[mutedUser.user_id, 0]]);
7269

73-
const shouldMatch = [{ user_id: -1, full_name: 'all', email: '(Notify everyone)' }];
70+
const shouldMatch = [];
7471

7572
const filteredUsers = getAutocompleteSuggestion(users, '', meUser.user_id, mutedUsers);
7673
expect(filteredUsers).toEqual(shouldMatch);

src/users/userHelpers.js

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import type {
1313
} from '../types';
1414
import { ensureUnreachable } from '../types';
1515
import { statusFromPresence } from '../utils/presence';
16-
import { makeUserId } from '../api/idTypes';
1716

1817
type UsersByStatus = {|
1918
active: UserOrBot[],
@@ -125,15 +124,8 @@ export const getAutocompleteSuggestion = (
125124
if (users.length === 0) {
126125
return users;
127126
}
128-
const allAutocompleteOptions = [
129-
// TODO stop using makeUserId on these fake "user IDs"; have some
130-
// more-explicit UI logic instead of these pseudo-users.
131-
{ user_id: makeUserId(-1), full_name: 'all', email: '(Notify everyone)' },
132-
{ user_id: makeUserId(-2), full_name: 'everyone', email: '(Notify everyone)' },
133-
...users,
134-
];
135-
const startWith = filterUserStartWith(allAutocompleteOptions, filter, ownUserId);
136-
const contains = filterUserThatContains(allAutocompleteOptions, filter, ownUserId);
127+
const startWith = filterUserStartWith(users, filter, ownUserId);
128+
const contains = filterUserThatContains(users, filter, ownUserId);
137129
const matchesEmail = filterUserMatchesEmail(users, filter, ownUserId);
138130
const candidates = getUniqueUsers([...startWith, ...contains, ...matchesEmail]);
139131
return candidates.filter(user => !mutedUsers.has(user.user_id));

static/translations/messages_en.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
11
{
2+
"all": "all",
3+
"everyone": "everyone",
4+
"stream": "stream",
5+
"Notify stream": "Notify stream",
6+
"Notify recipients": "Notify recipients",
7+
"Notify everyone": "Notify everyone",
28
"{num_of_people, plural,\n one {This message has been <z-link>read</z-link> by {num_of_people} person:}\n other {This message has been <z-link>read</z-link> by {num_of_people} people:}}": "{num_of_people, plural,\n one {This message has been <z-link>read</z-link> by {num_of_people} person:}\n other {This message has been <z-link>read</z-link> by {num_of_people} people:}}",
39
"View read receipts": "View read receipts",
410
"Failed to show read receipts": "Failed to show read receipts",

0 commit comments

Comments
 (0)