From 9f2ef69c71188aeaecb3f76046d1fc35f7a78173 Mon Sep 17 00:00:00 2001 From: N-Shar-ma Date: Thu, 13 Apr 2023 22:10:17 +0530 Subject: [PATCH] compose: Add banner for unmuting the muted topic a message is sent to. We add a new banner informing the user if and when they send a message to a muted topic / stream. It also has a button to unmute the topic. Fixes: #24246. --- web/src/compose.js | 23 ++++++++ web/src/compose_banner.ts | 15 +++++- web/src/message_scroll.js | 2 +- web/src/notifications.js | 53 +++++++++++++++---- web/src/user_topics.js | 13 ++++- web/styles/compose.css | 6 ++- web/styles/dark_theme.css | 3 +- .../compose_banner/unmute_topic_banner.hbs | 10 ++++ web/tests/lib/compose_banner.js | 2 +- 9 files changed, 111 insertions(+), 16 deletions(-) create mode 100644 web/templates/compose_banner/unmute_topic_banner.hbs diff --git a/web/src/compose.js b/web/src/compose.js index 88f5f9b0ab..9b4d5d3b20 100644 --- a/web/src/compose.js +++ b/web/src/compose.js @@ -39,6 +39,7 @@ import * as transmit from "./transmit"; import * as ui_report from "./ui_report"; import * as upload from "./upload"; import {user_settings} from "./user_settings"; +import * as user_topics from "./user_topics"; import * as util from "./util"; import * as zcommand from "./zcommand"; @@ -515,6 +516,28 @@ export function initialize() { }, ); + $("#compose_banners").on( + "click", + `.${CSS.escape( + compose_banner.CLASSNAMES.unmute_topic_notification, + )} .compose_banner_action_button`, + (event) => { + event.preventDefault(); + + const $target = $(event.target).parents(".compose_banner"); + const stream_id = Number.parseInt($target.attr("data-stream-id"), 10); + const topic_name = $target.attr("data-topic-name"); + + user_topics.set_user_topic_visibility_policy( + stream_id, + topic_name, + user_topics.all_visibility_policies.UNMUTED, + false, + true, + ); + }, + ); + $("#compose_banners").on( "click", `.${CSS.escape( diff --git a/web/src/compose_banner.ts b/web/src/compose_banner.ts index 8ab8e643ec..48256facf2 100644 --- a/web/src/compose_banner.ts +++ b/web/src/compose_banner.ts @@ -17,9 +17,15 @@ const MESSAGE_SENT_CLASSNAMES = { narrow_to_recipient: "narrow_to_recipient", scheduled_message_banner: "scheduled_message_banner", }; +// Technically, unmute_topic_notification is a message sent banner, but +// it has distinct behavior / look - it has an associated action button, +// does not disappear on scroll - so we don't include it here, as it needs +// to be handled separately. export const CLASSNAMES = { ...MESSAGE_SENT_CLASSNAMES, + // unmute topic notifications are styled like warnings but have distinct behaviour + unmute_topic_notification: "unmute_topic_notification warning-style", // warnings topic_resolved: "topic_resolved", recipient_not_subscribed: "recipient_not_subscribed", @@ -43,10 +49,13 @@ export const CLASSNAMES = { user_not_subscribed: "user_not_subscribed", }; -export function clear_message_sent_banners(): void { +export function clear_message_sent_banners(include_unmute_banner = true): void { for (const classname of Object.values(MESSAGE_SENT_CLASSNAMES)) { $(`#compose_banners .${CSS.escape(classname)}`).remove(); } + if (include_unmute_banner) { + clear_unmute_topic_notifications(); + } scroll_to_message_banner_message_id = null; } @@ -65,6 +74,10 @@ export function clear_warnings(): void { $(`#compose_banners .${CSS.escape(WARNING)}`).remove(); } +export function clear_unmute_topic_notifications(): void { + $(`#compose_banners .${CLASSNAMES.unmute_topic_notification.replaceAll(" ", ".")}`).remove(); +} + export function clear_all(): void { $(`#compose_banners`).empty(); } diff --git a/web/src/message_scroll.js b/web/src/message_scroll.js index eb7deb4e06..0f309c47b3 100644 --- a/web/src/message_scroll.js +++ b/web/src/message_scroll.js @@ -198,7 +198,7 @@ export function scroll_finished() { compose_banner.scroll_to_message_banner_message_id, ); if ($message_row.length > 0 && !message_viewport.is_message_below_viewport($message_row)) { - compose_banner.clear_message_sent_banners(); + compose_banner.clear_message_sent_banners(false); } } diff --git a/web/src/notifications.js b/web/src/notifications.js index cfc5d01016..92593d2539 100644 --- a/web/src/notifications.js +++ b/web/src/notifications.js @@ -1,6 +1,7 @@ import $ from "jquery"; import render_message_sent_banner from "../templates/compose_banner/message_sent_banner.hbs"; +import render_unmute_topic_banner from "../templates/compose_banner/unmute_topic_banner.hbs"; import * as alert_words from "./alert_words"; import * as blueslip from "./blueslip"; @@ -166,6 +167,21 @@ export function is_window_focused() { return window_focused; } +function notify_unmute(muted_narrow, stream_id, topic_name) { + const $unmute_notification = $( + render_unmute_topic_banner({ + muted_narrow, + stream_id, + topic_name, + classname: compose_banner.CLASSNAMES.unmute_topic_notification, + banner_type: "", + button_text: $t({defaultMessage: "Unmute topic"}), + }), + ); + compose_banner.clear_unmute_topic_notifications(); + $("#compose_banners").append($unmute_notification); +} + export function notify_above_composebox( banner_text, classname, @@ -182,7 +198,9 @@ export function notify_above_composebox( link_text, }), ); - compose_banner.clear_message_sent_banners(); + // We pass in include_unmute_banner as false because we don't want to + // clear any unmute_banner associated with this same message. + compose_banner.clear_message_sent_banners(false); $("#compose_banners").append($notification); } @@ -559,6 +577,20 @@ function get_message_header(message) { ); } +export function get_muted_narrow(message) { + if ( + message.type === "stream" && + stream_data.is_muted(message.stream_id) && + !user_topics.is_topic_unmuted(message.stream_id, message.topic) + ) { + return "stream"; + } + if (message.type === "stream" && user_topics.is_topic_muted(message.stream_id, message.topic)) { + return "topic"; + } + return undefined; +} + export function get_local_notify_mix_reason(message) { const $row = message_lists.current.get_row(message.id); if ($row.length > 0) { @@ -567,14 +599,6 @@ export function get_local_notify_mix_reason(message) { return undefined; } - if (message.type === "stream" && user_topics.is_topic_muted(message.stream_id, message.topic)) { - return $t({defaultMessage: "Sent! Your message was sent to a topic you have muted."}); - } - - if (message.type === "stream" && stream_data.is_muted(message.stream_id)) { - return $t({defaultMessage: "Sent! Your message was sent to a stream you have muted."}); - } - // offscreen because it is outside narrow // we can only look for these on non-search (can_apply_locally) messages // see also: exports.notify_messages_outside_current_search @@ -619,6 +643,15 @@ export function notify_local_mixes(messages, need_user_to_scroll) { continue; } + const muted_narrow = get_muted_narrow(message); + if (muted_narrow) { + notify_unmute(muted_narrow, message.stream_id, message.topic); + // We don't `continue` after showing the unmute banner, allowing multiple + // banners (at max 2 including the unmute banner) to be shown at once, + // as it's common for the unmute case to occur simultaneously with + // another banner's case, like sending a message to another narrow. + } + let banner_text = get_local_notify_mix_reason(message); const link_msg_id = message.id; @@ -728,7 +761,7 @@ export function register_click_handlers() { const message_id = $(e.currentTarget).data("message-id"); message_lists.current.select_id(message_id); navigate.scroll_to_selected(); - compose_banner.clear_message_sent_banners(); + compose_banner.clear_message_sent_banners(false); e.stopPropagation(); e.preventDefault(); }, diff --git a/web/src/user_topics.js b/web/src/user_topics.js index d84a3859db..57c65a35bd 100644 --- a/web/src/user_topics.js +++ b/web/src/user_topics.js @@ -2,6 +2,7 @@ import render_topic_muted from "../templates/topic_muted.hbs"; import * as blueslip from "./blueslip"; import * as channel from "./channel"; +import * as compose_banner from "./compose_banner"; import * as feedback_widget from "./feedback_widget"; import {FoldDict} from "./fold_dict"; import {$t} from "./i18n"; @@ -75,7 +76,13 @@ export function get_user_topics_for_visibility_policy(visibility_policy) { return topics; } -export function set_user_topic_visibility_policy(stream_id, topic, visibility_policy, from_hotkey) { +export function set_user_topic_visibility_policy( + stream_id, + topic, + visibility_policy, + from_hotkey, + from_banner, +) { const data = { stream_id, topic, @@ -90,6 +97,10 @@ export function set_user_topic_visibility_policy(stream_id, topic, visibility_po feedback_widget.dismiss(); return; } + if (from_banner) { + compose_banner.clear_unmute_topic_notifications(); + return; + } if (!from_hotkey) { return; } diff --git a/web/styles/compose.css b/web/styles/compose.css index d0785ed7da..b57714fa9f 100644 --- a/web/styles/compose.css +++ b/web/styles/compose.css @@ -323,7 +323,11 @@ } } - &.warning { + /* warning and warning-style classes have the same CSS; this is since + the warning class has some associated javascript which we do not want + for some of the banners, for which we use the warning-style class. */ + &.warning, + &.warning-style { background-color: hsl(50deg 75% 92%); border-color: hsl(38deg 44% 27% / 40%); color: hsl(38deg 44% 27%); diff --git a/web/styles/dark_theme.css b/web/styles/dark_theme.css index cfc375039c..c9913c1611 100644 --- a/web/styles/dark_theme.css +++ b/web/styles/dark_theme.css @@ -226,7 +226,8 @@ } } - &.warning { + &.warning, + &.warning-style { background-color: hsl(53deg 100% 11%); border-color: hsl(38deg 44% 60% / 40%); color: hsl(50deg 45% 80%); diff --git a/web/templates/compose_banner/unmute_topic_banner.hbs b/web/templates/compose_banner/unmute_topic_banner.hbs new file mode 100644 index 0000000000..6794b9ba4e --- /dev/null +++ b/web/templates/compose_banner/unmute_topic_banner.hbs @@ -0,0 +1,10 @@ +{{#> compose_banner }} +

+ {{#if (eq muted_narrow "stream")}} + {{#tr}}Your message was sent to a stream you have muted.{{/tr}} + {{else if (eq muted_narrow "topic")}} + {{#tr}}Your message was sent to a topic you have muted.{{/tr}} + {{/if}} + {{#tr}}You will not receive notifications about new messages.{{/tr}} +

+{{/compose_banner}} diff --git a/web/tests/lib/compose_banner.js b/web/tests/lib/compose_banner.js index a009a86f0c..75a28fe412 100644 --- a/web/tests/lib/compose_banner.js +++ b/web/tests/lib/compose_banner.js @@ -8,7 +8,7 @@ exports.mock_banners = () => { // zjquery doesn't support `remove`, which is used when clearing the compose box. // TODO: improve how we test this so that we don't have to mock things like this. for (const classname of Object.values(compose_banner.CLASSNAMES)) { - $(`#compose_banners .${classname}`).remove = () => {}; + $(`#compose_banners .${classname.replaceAll(" ", ".")}`).remove = () => {}; } $("#compose_banners .warning").remove = () => {}; $("#compose_banners .error").remove = () => {};