Skip to content

Commit 4ef0a0a

Browse files
committed
follow-topic: Show "follow" icon in recipient headers
Together with the preceding few commits which add this icon in some other places in the UI, this completes zulip#5770. Fixes: zulip#5770
1 parent 95e887c commit 4ef0a0a

File tree

7 files changed

+244
-88
lines changed

7 files changed

+244
-88
lines changed

src/webview/__tests__/__snapshots__/generateInboundEventEditSequence-test.js.snap.android

Lines changed: 78 additions & 42 deletions
Large diffs are not rendered by default.

src/webview/__tests__/__snapshots__/generateInboundEventEditSequence-test.js.snap.ios

Lines changed: 78 additions & 42 deletions
Large diffs are not rendered by default.

src/webview/__tests__/generateInboundEventEditSequence-test.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import getMessageListElements from '../../message/getMessageListElements';
2929
import { getGlobalSettings } from '../../selectors';
3030
import { getBackgroundData } from '../backgroundData';
3131
import { randString } from '../../utils/misc';
32+
import { makeMuteState } from '../../mute/__tests__/mute-testlib';
33+
import { UserTopicVisibilityPolicy } from '../../api/modelTypes';
3234

3335
// Tell ESLint to recognize `check` as a helper function that runs
3436
// assertions.
@@ -443,6 +445,23 @@ describe('messages -> piece descriptors -> content HTML is stable/sensible', ()
443445
});
444446
});
445447

448+
test('message in followed topic', () => {
449+
check({
450+
narrow: HOME_NARROW,
451+
messages: [baseSingleMessage],
452+
state: eg.reduxStatePlus({
453+
streams: [...eg.plusReduxState.streams, stream1],
454+
subscriptions: [
455+
...eg.plusReduxState.subscriptions,
456+
eg.makeSubscription({ stream: stream1 }),
457+
],
458+
mute: makeMuteState([
459+
[stream1, baseSingleMessage.subject, UserTopicVisibilityPolicy.Followed],
460+
]),
461+
}),
462+
});
463+
});
464+
446465
describe('message with reactions', () => {
447466
describe('displayEmojiReactionUsers: false', () => {
448467
const state = eg.reduxStatePlus({
@@ -991,6 +1010,42 @@ describe('getEditSequence correct for interesting changes', () => {
9911010
);
9921011
});
9931012

1013+
test('follow a topic', () => {
1014+
const message = eg.streamMessage();
1015+
check(
1016+
{
1017+
messages: [message],
1018+
state: eg.reduxStatePlus({
1019+
mute: makeMuteState([]),
1020+
}),
1021+
},
1022+
{
1023+
messages: [message],
1024+
state: eg.reduxStatePlus({
1025+
mute: makeMuteState([[eg.stream, message.subject, UserTopicVisibilityPolicy.Followed]]),
1026+
}),
1027+
},
1028+
);
1029+
});
1030+
1031+
test('unfollow a topic', () => {
1032+
const message = eg.streamMessage();
1033+
check(
1034+
{
1035+
messages: [message],
1036+
state: eg.reduxStatePlus({
1037+
mute: makeMuteState([[eg.stream, message.subject, UserTopicVisibilityPolicy.Followed]]),
1038+
}),
1039+
},
1040+
{
1041+
messages: [message],
1042+
state: eg.reduxStatePlus({
1043+
mute: makeMuteState([]),
1044+
}),
1045+
},
1046+
);
1047+
});
1048+
9941049
// TODO(#5208): We haven't settled how we want to track name/avatar
9951050
test.todo("sender's name/avatar changed");
9961051

src/webview/generateInboundEventEditSequence.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { ensureUnreachable } from '../generics';
88
import type { BackgroundData } from './backgroundData';
99
import messageListElementHtml from './html/messageListElementHtml';
1010
import { getUserStatusFromModel } from '../user-statuses/userStatusesCore';
11+
import { isTopicFollowed } from '../mute/muteModel';
1112

1213
const NODE_ENV = process.env.NODE_ENV;
1314

@@ -72,7 +73,19 @@ function doElementsDifferInterestingly(
7273
return !isEqual(oldElement, newElement);
7374
case 'header':
7475
// TODO(?): False positives on `.subsequentMessage.content` changes
75-
return !isEqual(oldElement, newElement);
76+
if (!isEqual(oldElement, newElement)) {
77+
return true;
78+
}
79+
if (newElement.subsequentMessage?.type === 'stream') {
80+
const message = newElement.subsequentMessage;
81+
if (
82+
isTopicFollowed(message.stream_id, message.subject, oldBackgroundData.mute)
83+
!== isTopicFollowed(message.stream_id, message.subject, newBackgroundData.mute)
84+
) {
85+
return true;
86+
}
87+
}
88+
return false;
7689
case 'message': {
7790
invariant(newElement.type === 'message', 'oldElement.type equals newElement.type');
7891

src/webview/html/__tests__/header-test.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
import * as eg from '../../../__tests__/lib/exampleData';
44
import header from '../header';
55
import type { BackgroundData } from '../../backgroundData';
6+
import { makeMuteState } from '../../../mute/__tests__/mute-testlib';
67

78
const backgroundData: BackgroundData = ({
9+
mute: makeMuteState([]),
810
ownEmail: eg.selfUser.email,
911
subscriptions: [eg.stream],
1012
streams: new Map([[eg.stream.stream_id, eg.stream]]),

src/webview/html/header.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
streamNameOfStreamMessage,
2020
} from '../../utils/recipient';
2121
import { base64Utf8Encode } from '../../utils/encoding';
22+
import { isTopicFollowed } from '../../mute/muteModel';
2223

2324
const renderTopic = message =>
2425
// TODO: pin down if '' happens, and what its proper semantics are.
@@ -32,7 +33,7 @@ const renderTopic = message =>
3233
* This is a private helper of messageListElementHtml.
3334
*/
3435
export default (
35-
{ ownUser, subscriptions }: BackgroundData,
36+
{ mute, ownUser, subscriptions }: BackgroundData,
3637
element: HeaderMessageListElement,
3738
): string => {
3839
const { subsequentMessage: message, style: headerStyle } = element;
@@ -41,6 +42,7 @@ export default (
4142
const streamName = streamNameOfStreamMessage(message);
4243
const topicNarrowStr = keyFromNarrow(topicNarrow(message.stream_id, message.subject));
4344
const topicHtml = renderTopic(message);
45+
const isFollowed = isTopicFollowed(message.stream_id, message.subject, mute);
4446

4547
if (headerStyle === 'topic+date') {
4648
return template`\
@@ -49,7 +51,7 @@ export default (
4951
data-narrow="${base64Utf8Encode(topicNarrowStr)}"
5052
data-msg-id="${message.id}"
5153
>
52-
<div class="topic-text">$!${topicHtml}</div>
54+
<div class="topic-text" data-followed="${isFollowed}">$!${topicHtml}</div>
5355
<div class="topic-date">${humanDate(new Date(message.timestamp * 1000))}</div>
5456
</div>`;
5557
} else if (headerStyle === 'full') {
@@ -70,7 +72,7 @@ export default (
7072
data-narrow="${base64Utf8Encode(streamNarrowStr)}">
7173
# ${streamName}
7274
</div>
73-
<div class="topic-text">$!${topicHtml}</div>
75+
<div class="topic-text" data-followed="${isFollowed}">$!${topicHtml}</div>
7476
<div class="topic-date">${humanDate(new Date(message.timestamp * 1000))}</div>
7577
</div>`;
7678
} else {

src/webview/static/base.css

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,22 @@ body {
116116
.topic-text {
117117
flex: 1;
118118
padding: 0 8px;
119+
display: flex;
120+
align-items: center;
119121
overflow: hidden;
120122
text-overflow: ellipsis;
121123
pointer-events: none;
122124
}
125+
.topic-text[data-followed="true"]::after {
126+
content: "";
127+
background-image: url("images/follow.svg");
128+
margin-left: 12px;
129+
width: 17px;
130+
height: 17px;
131+
/* opacity: 0.2 on web, but that's with the pastel stream colors;
132+
* 0.3 stands up better to our gray. */
133+
opacity: 0.3;
134+
}
123135
.topic-date {
124136
opacity: 0.5;
125137
padding: 0 8px;

0 commit comments

Comments
 (0)