Skip to content

Commit

Permalink
follow-topic: Show "follow" icon in recipient headers
Browse files Browse the repository at this point in the history
Together with the preceding few commits which add this icon in
some other places in the UI, this completes zulip#5770.

Fixes: zulip#5770
  • Loading branch information
gnprice committed Nov 17, 2023
1 parent 12e8e7d commit 84ee5c5
Show file tree
Hide file tree
Showing 7 changed files with 244 additions and 88 deletions.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

55 changes: 55 additions & 0 deletions src/webview/__tests__/generateInboundEventEditSequence-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import getMessageListElements from '../../message/getMessageListElements';
import { getGlobalSettings } from '../../selectors';
import { getBackgroundData } from '../backgroundData';
import { randString } from '../../utils/misc';
import { makeMuteState } from '../../mute/__tests__/mute-testlib';
import { UserTopicVisibilityPolicy } from '../../api/modelTypes';

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

test('message in followed topic', () => {
check({
narrow: HOME_NARROW,
messages: [baseSingleMessage],
state: eg.reduxStatePlus({
streams: [...eg.plusReduxState.streams, stream1],
subscriptions: [
...eg.plusReduxState.subscriptions,
eg.makeSubscription({ stream: stream1 }),
],
mute: makeMuteState([
[stream1, baseSingleMessage.subject, UserTopicVisibilityPolicy.Followed],
]),
}),
});
});

describe('message with reactions', () => {
describe('displayEmojiReactionUsers: false', () => {
const state = eg.reduxStatePlus({
Expand Down Expand Up @@ -991,6 +1010,42 @@ describe('getEditSequence correct for interesting changes', () => {
);
});

test('follow a topic', () => {
const message = eg.streamMessage();
check(
{
messages: [message],
state: eg.reduxStatePlus({
mute: makeMuteState([]),
}),
},
{
messages: [message],
state: eg.reduxStatePlus({
mute: makeMuteState([[eg.stream, message.subject, UserTopicVisibilityPolicy.Followed]]),
}),
},
);
});

test('unfollow a topic', () => {
const message = eg.streamMessage();
check(
{
messages: [message],
state: eg.reduxStatePlus({
mute: makeMuteState([[eg.stream, message.subject, UserTopicVisibilityPolicy.Followed]]),
}),
},
{
messages: [message],
state: eg.reduxStatePlus({
mute: makeMuteState([]),
}),
},
);
});

// TODO(#5208): We haven't settled how we want to track name/avatar
test.todo("sender's name/avatar changed");

Expand Down
15 changes: 14 additions & 1 deletion src/webview/generateInboundEventEditSequence.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ensureUnreachable } from '../generics';
import type { BackgroundData } from './backgroundData';
import messageListElementHtml from './html/messageListElementHtml';
import { getUserStatusFromModel } from '../user-statuses/userStatusesCore';
import { isTopicFollowed } from '../mute/muteModel';

const NODE_ENV = process.env.NODE_ENV;

Expand Down Expand Up @@ -72,7 +73,19 @@ function doElementsDifferInterestingly(
return !isEqual(oldElement, newElement);
case 'header':
// TODO(?): False positives on `.subsequentMessage.content` changes
return !isEqual(oldElement, newElement);
if (!isEqual(oldElement, newElement)) {
return true;
}
if (newElement.subsequentMessage?.type === 'stream') {
const message = newElement.subsequentMessage;
if (
isTopicFollowed(message.stream_id, message.subject, oldBackgroundData.mute)
!== isTopicFollowed(message.stream_id, message.subject, newBackgroundData.mute)
) {
return true;
}
}
return false;
case 'message': {
invariant(newElement.type === 'message', 'oldElement.type equals newElement.type');

Expand Down
2 changes: 2 additions & 0 deletions src/webview/html/__tests__/header-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import * as eg from '../../../__tests__/lib/exampleData';
import header from '../header';
import type { BackgroundData } from '../../backgroundData';
import { makeMuteState } from '../../../mute/__tests__/mute-testlib';

const backgroundData: BackgroundData = ({
mute: makeMuteState([]),
ownEmail: eg.selfUser.email,
subscriptions: [eg.stream],
streams: new Map([[eg.stream.stream_id, eg.stream]]),
Expand Down
8 changes: 5 additions & 3 deletions src/webview/html/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
streamNameOfStreamMessage,
} from '../../utils/recipient';
import { base64Utf8Encode } from '../../utils/encoding';
import { isTopicFollowed } from '../../mute/muteModel';

const renderTopic = message =>
// TODO: pin down if '' happens, and what its proper semantics are.
Expand All @@ -32,7 +33,7 @@ const renderTopic = message =>
* This is a private helper of messageListElementHtml.
*/
export default (
{ ownUser, subscriptions }: BackgroundData,
{ mute, ownUser, subscriptions }: BackgroundData,
element: HeaderMessageListElement,
): string => {
const { subsequentMessage: message, style: headerStyle } = element;
Expand All @@ -41,6 +42,7 @@ export default (
const streamName = streamNameOfStreamMessage(message);
const topicNarrowStr = keyFromNarrow(topicNarrow(message.stream_id, message.subject));
const topicHtml = renderTopic(message);
const isFollowed = isTopicFollowed(message.stream_id, message.subject, mute);

if (headerStyle === 'topic+date') {
return template`\
Expand All @@ -49,7 +51,7 @@ export default (
data-narrow="${base64Utf8Encode(topicNarrowStr)}"
data-msg-id="${message.id}"
>
<div class="topic-text">$!${topicHtml}</div>
<div class="topic-text" data-followed="${isFollowed}">$!${topicHtml}</div>
<div class="topic-date">${humanDate(new Date(message.timestamp * 1000))}</div>
</div>`;
} else if (headerStyle === 'full') {
Expand All @@ -70,7 +72,7 @@ export default (
data-narrow="${base64Utf8Encode(streamNarrowStr)}">
# ${streamName}
</div>
<div class="topic-text">$!${topicHtml}</div>
<div class="topic-text" data-followed="${isFollowed}">$!${topicHtml}</div>
<div class="topic-date">${humanDate(new Date(message.timestamp * 1000))}</div>
</div>`;
} else {
Expand Down
12 changes: 12 additions & 0 deletions src/webview/static/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,22 @@ body {
.topic-text {
flex: 1;
padding: 0 8px;
display: flex;
align-items: center;
overflow: hidden;
text-overflow: ellipsis;
pointer-events: none;
}
.topic-text[data-followed="true"]::after {
content: "";
background-image: url("images/follow.svg");
margin-left: 12px;
width: 17px;
height: 17px;
/* opacity: 0.2 on web, but that's with the pastel stream colors;
* 0.3 stands up better to our gray. */
opacity: 0.3;
}
.topic-date {
opacity: 0.5;
padding: 0 8px;
Expand Down

0 comments on commit 84ee5c5

Please sign in to comment.