From e091e1903130b28bd783ac5ff7a6396108819467 Mon Sep 17 00:00:00 2001 From: Sayam Samal Date: Sat, 31 May 2025 17:26:17 +0530 Subject: [PATCH] message_header: Update topic visibility button to use icon button. This commit is part of a series of commits aimed at updating the message header buttons to use the new icon button component which has consistent styling across the Web UI and offers a larger clickable area for the users. Due to deviation from the structure at "web/templates/components/icon_button.hbs", this commit applies the icon button classes directly on the template rather than using the component partial directly in code. Fixes #34477. --- web/src/message_list_tooltips.ts | 9 ++++-- web/src/tippyjs.ts | 24 +++++++++------ web/src/user_topic_popover.ts | 51 +++++++++++++++++++++++++------- web/styles/message_header.css | 8 ----- web/templates/recipient_row.hbs | 30 +++++++++---------- 5 files changed, 76 insertions(+), 46 deletions(-) diff --git a/web/src/message_list_tooltips.ts b/web/src/message_list_tooltips.ts index 2eab1cd48b..766c5c8536 100644 --- a/web/src/message_list_tooltips.ts +++ b/web/src/message_list_tooltips.ts @@ -310,9 +310,12 @@ export function initialize(): void { }, ); - message_list_tooltip("#message_feed_container .change_visibility_policy > i", { - ...topic_visibility_policy_tooltip_props, - }); + message_list_tooltip( + "#message_feed_container .change_visibility_policy > .recipient-bar-control-icon", + { + ...topic_visibility_policy_tooltip_props, + }, + ); message_list_tooltip("#message_feed_container .recipient-bar-control-icon:not(.toggle_resolve_topic_spinner)", { delay: LONG_HOVER_DELAY, diff --git a/web/src/tippyjs.ts b/web/src/tippyjs.ts index 78fbf87405..55abfce818 100644 --- a/web/src/tippyjs.ts +++ b/web/src/tippyjs.ts @@ -87,21 +87,27 @@ export const topic_visibility_policy_tooltip_props = { delay: LONG_HOVER_DELAY, appendTo: () => document.body, onShow(instance: tippy.Instance) { - const $elem = $(instance.reference); let should_render_privacy_icon; - let current_visibility_policy_str; - if ($elem.hasClass("zulip-icon-inherit")) { - should_render_privacy_icon = true; - } else { - should_render_privacy_icon = false; - current_visibility_policy_str = $elem.attr("data-tippy-content"); - } let current_stream_obj; - if (should_render_privacy_icon) { + const $elem = $(instance.reference); + if ($elem.hasClass("recipient-bar-control-icon")) { + // The topic visibility policy button located in the recipient bar + // uses the icon button component, and extracts the stream id from + // the message header instead of the button itself. This results in + // the need for a different logic to extract the required data. + should_render_privacy_icon = $elem + .children(".zulip-icon") + .hasClass("zulip-icon-inherit"); + current_stream_obj = stream_data.get_sub_by_id( + Number($elem.closest(".message_header").attr("data-stream-id")), + ); + } else { + should_render_privacy_icon = $elem.hasClass("zulip-icon-inherit"); current_stream_obj = stream_data.get_sub_by_id( Number($elem.parent().attr("data-stream-id")), ); } + const current_visibility_policy_str = $elem.attr("data-tippy-content"); const tooltip_context = { ...current_stream_obj, current_visibility_policy_str, diff --git a/web/src/user_topic_popover.ts b/web/src/user_topic_popover.ts index 4bfce23f20..a25446f1ba 100644 --- a/web/src/user_topic_popover.ts +++ b/web/src/user_topic_popover.ts @@ -9,6 +9,21 @@ import {parse_html} from "./ui_util.ts"; import * as user_topics from "./user_topics.ts"; import * as util from "./util.ts"; +const extract_visibility_policy_popover_context = ( + $element: JQuery, +): { + stream_id: number; + topic_name: string; +} => { + const stream_id_str = $element.attr("data-stream-id"); + assert(stream_id_str !== undefined); + const stream_id = Number.parseInt(stream_id_str, 10); + const topic_name = $element.attr("data-topic-name")!; + assert(stream_id !== undefined); + assert(topic_name !== undefined); + return {stream_id, topic_name}; +}; + export function initialize(): void { popover_menus.register_popover_menu(".change_visibility_policy", { theme: "popover-menu", @@ -28,14 +43,24 @@ export function initialize(): void { onShow(instance) { popover_menus.popover_instances.change_visibility_policy = instance; popover_menus.on_show_prep(instance); - const $elt = $(instance.reference).closest(".change_visibility_policy").expectOne(); - const stream_id_str = $elt.attr("data-stream-id"); - $elt.addClass("visibility-policy-popover-visible"); - assert(stream_id_str !== undefined); + const $reference = $(instance.reference); + const $change_visibility_policy_button = $reference + .closest(".change_visibility_policy") + .expectOne(); - const stream_id = Number.parseInt(stream_id_str, 10); - const topic_name = $elt.attr("data-topic-name")!; + // The topic visibility policy popover logic is shared between + // the recipient bar and other parts of the app. However, the + // relevant data attributes are located in different elements — + // specifically, within the message header when triggered from + // the recipient bar, instead of the button itself. Hence, we + // need to conditionally extract the data attributes below. + const $data_element = $reference.hasClass("recipient-bar-control") + ? $reference.closest(".message_header").expectOne() + : $change_visibility_policy_button; + const {stream_id, topic_name} = + extract_visibility_policy_popover_context($data_element); + $change_visibility_policy_button.addClass("visibility-policy-popover-visible"); instance.setContent( parse_html( render_change_visibility_policy_popover( @@ -49,12 +74,16 @@ export function initialize(): void { }, onMount(instance) { const $popper = $(instance.popper); - const $elt = $(instance.reference).closest(".change_visibility_policy").expectOne(); - const stream_id_str = $elt.attr("data-stream-id"); - assert(stream_id_str !== undefined); + const $reference = $(instance.reference); + const $change_visibility_policy_button = $reference + .closest(".change_visibility_policy") + .expectOne(); - const stream_id = Number.parseInt(stream_id_str, 10); - const topic_name = $elt.attr("data-topic-name")!; + const $data_element = $reference.hasClass("recipient-bar-control") + ? $reference.closest(".message_header").expectOne() + : $change_visibility_policy_button; + const {stream_id, topic_name} = + extract_visibility_policy_popover_context($data_element); if (!stream_id) { popover_menus.hide_current_popover_if_visible(instance); diff --git a/web/styles/message_header.css b/web/styles/message_header.css index 711985eca2..7c0343f512 100644 --- a/web/styles/message_header.css +++ b/web/styles/message_header.css @@ -257,14 +257,6 @@ display: flex; flex-grow: 1; align-items: center; - - .change_visibility_policy { - cursor: pointer; - - i { - display: block; - } - } } .on_hover_topic_read { diff --git a/web/templates/recipient_row.hbs b/web/templates/recipient_row.hbs index b4c188b0c1..c08d8a4bf4 100644 --- a/web/templates/recipient_row.hbs +++ b/web/templates/recipient_row.hbs @@ -63,22 +63,22 @@ {{/if}} + {{! visibility policy menu }} {{#if (and is_subscribed (not is_archived))}} - - {{#if (eq visibility_policy all_visibility_policies.FOLLOWED)}} - - {{else if (eq visibility_policy all_visibility_policies.UNMUTED)}} - - {{else if (eq visibility_policy all_visibility_policies.MUTED)}} - - {{else}} - - {{/if}} - + {{!-- We define the change_visibility_policy class in a wrapper span + since the icon button component already has a tippy tooltip attached + to it and Tippy does not support multiple tooltips on a single element. --}} + + {{#if (eq visibility_policy all_visibility_policies.FOLLOWED)}} + {{> components/icon_button icon="follow" intent="neutral" custom_classes="recipient-bar-control-icon" data-tippy-content=(t "You follow this topic.") aria-label=(t "You follow this topic.") }} + {{else if (eq visibility_policy all_visibility_policies.UNMUTED)}} + {{> components/icon_button icon="unmute" intent="neutral" custom_classes="recipient-bar-control recipient-bar-control-icon" data-tippy-content=(t "You have unmuted this topic.") aria-label=(t "You have unmuted this topic.") }} + {{else if (eq visibility_policy all_visibility_policies.MUTED)}} + {{> components/icon_button icon="mute" intent="neutral" custom_classes="recipient-bar-control-icon" data-tippy-content=(t "You have muted this topic.") aria-label=(t "You have muted this topic.") }} + {{else}} + {{> components/icon_button icon="inherit" intent="neutral" custom_classes="recipient-bar-control-icon" data-tippy-content=(t "Notifications are based on your configuration for this channel.") aria-label=(t "Notifications are based on your configuration for this channel.") }} + {{/if}} + {{/if}} {{! Topic menu }}