From 8de397b37f7a3b0c8d7cd209a65c54e293336f0c Mon Sep 17 00:00:00 2001 From: Daniil Fadeev Date: Thu, 21 Sep 2023 00:09:44 +0600 Subject: [PATCH] user_card_popover: Migrate all user card popovers to Tippy. --- web/e2e-tests/user-status.test.ts | 2 +- web/src/hotkey.js | 14 +- web/src/popovers.js | 6 +- web/src/user_card_popover.js | 278 ++++++++--------- web/styles/popovers.css | 45 +-- web/templates/no_arrow_popover.hbs | 8 - web/templates/user_card_popover_content.hbs | 313 ++++++++++---------- web/tests/hotkey.test.js | 12 +- web/third/bootstrap-tooltip/tooltip.js | 3 - 9 files changed, 310 insertions(+), 371 deletions(-) delete mode 100644 web/templates/no_arrow_popover.hbs diff --git a/web/e2e-tests/user-status.test.ts b/web/e2e-tests/user-status.test.ts index ea9c2a1da6..8f2251176e 100644 --- a/web/e2e-tests/user-status.test.ts +++ b/web/e2e-tests/user-status.test.ts @@ -18,7 +18,7 @@ async function open_set_user_status_modal(page: Page): Promise { {}, menu_icon_selector, ); - await page.waitForSelector(".user_popover", {visible: true}); + await page.waitForSelector(".user_popover_email", {visible: true}); // We are using evaluate to click because it is very hard to detect if the user info popover has opened. await page.evaluate(() => document.querySelector(".update_status_text")!.click(), diff --git a/web/src/hotkey.js b/web/src/hotkey.js index af6cfca841..ca8bcf0791 100644 --- a/web/src/hotkey.js +++ b/web/src/hotkey.js @@ -376,18 +376,18 @@ function handle_popover_events(event_name) { return true; } - if (user_card_popover.is_message_user_card_open()) { - user_card_popover.user_card_popover_for_message_handle_keyboard(event_name); + if (user_card_popover.message_user_card.is_open()) { + user_card_popover.message_user_card.handle_keyboard(event_name); return true; } - if (user_card_popover.is_user_card_open()) { - user_card_popover.user_card_popover_handle_keyboard(event_name); + if (user_card_popover.user_card.is_open()) { + user_card_popover.user_card.handle_keyboard(event_name); return true; } - if (user_card_popover.user_sidebar_popped()) { - user_card_popover.user_sidebar_popover_handle_keyboard(event_name); + if (user_card_popover.user_sidebar.is_open()) { + user_card_popover.user_sidebar.handle_keyboard(event_name); return true; } @@ -741,7 +741,7 @@ export function process_hotkey(e, hotkey) { return false; } - if (overlays.settings_open() && !user_card_popover.is_user_card_open()) { + if (overlays.settings_open() && !user_card_popover.user_card.is_open()) { return false; } diff --git a/web/src/popovers.js b/web/src/popovers.js index 32da287f0f..9a16f9961f 100644 --- a/web/src/popovers.js +++ b/web/src/popovers.js @@ -162,9 +162,9 @@ export function any_active() { popover_menus.any_active() || stream_popover.is_open() || user_group_popover.is_open() || - user_card_popover.user_sidebar_popped() || - user_card_popover.is_message_user_card_open() || - user_card_popover.is_user_card_open() || + user_card_popover.user_sidebar.is_open() || + user_card_popover.message_user_card.is_open() || + user_card_popover.user_card.is_open() || emoji_picker.is_open() || playground_links_popover.is_open() || $("[class^='column-'].expanded").length diff --git a/web/src/user_card_popover.js b/web/src/user_card_popover.js index 8caae6937d..d2d7781db4 100644 --- a/web/src/user_card_popover.js +++ b/web/src/user_card_popover.js @@ -3,7 +3,6 @@ import {parseISO} from "date-fns"; import $ from "jquery"; import tippy from "tippy.js"; -import render_no_arrow_popover from "../templates/no_arrow_popover.hbs"; import render_user_card_popover_content from "../templates/user_card_popover_content.hbs"; import render_user_card_popover_manage_menu from "../templates/user_card_popover_manage_menu.hbs"; import render_user_card_popover_title from "../templates/user_card_popover_title.hbs"; @@ -43,9 +42,6 @@ import {user_settings} from "./user_settings"; import * as user_status from "./user_status"; import * as user_status_ui from "./user_status_ui"; -let $current_message_user_card_popover_elem; -let $current_user_card_popover_elem; -let current_user_sidebar_popover; let current_user_sidebar_user_id; class PopoverMenu { @@ -82,9 +78,30 @@ class PopoverMenu { } export const manage_menu = new PopoverMenu(); +export const user_sidebar = new PopoverMenu(); +export const message_user_card = new PopoverMenu(); +export const user_card = new PopoverMenu(); + +function get_popover_classname(popover) { + const popovers = { + user_sidebar: "user-sidebar-popover-root", + message_user_card: "message-user-card-popover-root", + user_card: "user-card-popover-root", + }; + + return popovers[popover]; +} + +user_sidebar.hide = function () { + PopoverMenu.prototype.hide.call(this); + current_user_sidebar_user_id = undefined; +}; const user_card_popovers = { manage_menu, + user_sidebar, + message_user_card, + user_card, }; export function any_active() { @@ -101,14 +118,11 @@ export function hide_all_instances() { export function hide_all_user_card_popovers() { hide_all_instances(); - hide_message_user_card_popover(); - hide_user_sidebar_popover(); - hide_user_card_popover(); } export function clear_for_testing() { - $current_message_user_card_popover_elem = undefined; - $current_user_card_popover_elem = undefined; + message_user_card.instance = undefined; + user_card.instance = undefined; manage_menu.instance = undefined; } @@ -125,53 +139,15 @@ function clipboard_enable(arg) { // Functions related to user card popover. export function toggle_user_card_popover(element, user) { - const $last_popover_elem = $current_user_card_popover_elem; - hide_all(); - if ($last_popover_elem !== undefined && $last_popover_elem.get()[0] === element) { - return; - } - const $elt = $(element); render_user_card_popover( user, - $elt, + $(element), false, false, "compose_private_message", - "user-card-popover", + "user_card", "right", ); - $current_user_card_popover_elem = $elt; -} - -export function hide_user_card_popover() { - if (is_user_card_open()) { - $current_user_card_popover_elem.popover("destroy"); - $current_user_card_popover_elem = undefined; - } -} - -export function is_user_card_open() { - return $current_user_card_popover_elem !== undefined; -} - -export function user_card_popover_handle_keyboard(key) { - const $items = get_user_card_popover_items(); - popover_items_handle_keyboard(key, $items); -} - -function get_user_card_popover_items() { - const $popover_elt = $("div.user-card-popover"); - if (!$current_user_card_popover_elem || !$popover_elt.length) { - blueslip.error("Trying to get menu items when action popover is closed."); - return undefined; - } - - if ($popover_elt.length >= 2) { - blueslip.error("More than one user info popovers cannot be opened at same time."); - return undefined; - } - - return $("li:not(.divider):visible a", $popover_elt); } function get_user_card_popover_data( @@ -265,6 +241,8 @@ function render_user_card_popover( private_msg_class, template_class, popover_placement, + show_as_overlay, + on_mount, ) { const args = get_user_card_popover_data( user, @@ -273,46 +251,64 @@ function render_user_card_popover( private_msg_class, ); - const $popover_content = $(render_user_card_popover_content(args)); - $popover_element.popover({ - content: $popover_content.get(0), - fixed: true, - placement: popover_placement, - template: render_no_arrow_popover({class: template_class}), - title: render_user_card_popover_title({ - // See the load_medium_avatar comment for important background. - user_avatar: people.small_avatar_url_for_person(user), - user_is_guest: user.is_guest, - }), - html: true, - trigger: "manual", - top_offset: $("#userlist-title").get_offset_to_window().top + 15, - fix_positions: true, - }); - $popover_element.popover("show"); + popover_menus.toggle_popover_menu( + $popover_element[0], + { + placement: popover_placement, + arrow: false, + onCreate(instance) { + user_card_popovers[template_class].instance = instance; + instance.setContent(ui_util.parse_html(render_user_card_popover_content(args))); - init_email_clipboard(); - init_email_tooltip(user); - const $user_name_element = $popover_content.find(".user_full_name"); - const $bot_owner_element = $popover_content.find(".bot_owner"); - if ($user_name_element.prop("clientWidth") < $user_name_element.prop("scrollWidth")) { - $user_name_element.addClass("tippy-zulip-tooltip"); - } - if ( - args.bot_owner && - $bot_owner_element.prop("clientWidth") < $bot_owner_element.prop("scrollWidth") - ) { - $bot_owner_element.addClass("tippy-zulip-tooltip"); - } + const $popover = $(instance.popper); + const $popover_title = $popover.find(".popover-title"); - // Note: We pass the normal-size avatar in initial rendering, and - // then query the server to replace it with the medium-size - // avatar. The purpose of this double-fetch approach is to take - // advantage of the fact that the browser should already have the - // low-resolution image cached and thus display a low-resolution - // avatar rather than a blank area during the network delay for - // fetching the medium-size one. - load_medium_avatar(user, $(".popover-avatar")); + $popover.addClass(get_popover_classname(template_class)); + $popover_title.append( + render_user_card_popover_title({ + // See the load_medium_avatar comment for important background. + user_avatar: people.small_avatar_url_for_person(user), + user_is_guest: user.is_guest, + }), + ); + }, + onHidden() { + user_card_popovers[template_class].hide(); + }, + onMount(instance) { + if (on_mount) { + on_mount(instance); + } + // Note: We pass the normal-size avatar in initial rendering, and + // then query the server to replace it with the medium-size + // avatar. The purpose of this double-fetch approach is to take + // advantage of the fact that the browser should already have the + // low-resolution image cached and thus display a low-resolution + // avatar rather than a blank area during the network delay for + // fetching the medium-size one. + load_medium_avatar(user, $(".popover-avatar")); + init_email_clipboard(); + init_email_tooltip(user); + + const $popover = $(instance.popper); + const $user_name_element = $popover.find(".user_full_name"); + const $bot_owner_element = $popover.find(".bot_owner"); + + if ( + $user_name_element.prop("clientWidth") < $user_name_element.prop("scrollWidth") + ) { + $user_name_element.addClass("tippy-zulip-tooltip"); + } + if ( + args.bot_owner && + $bot_owner_element.prop("clientWidth") < $bot_owner_element.prop("scrollWidth") + ) { + $bot_owner_element.addClass("tippy-zulip-tooltip"); + } + }, + }, + {show_as_overlay}, + ); } function copy_email_handler(e) { @@ -438,17 +434,10 @@ export function get_user_card_popover_manage_menu_items() { // element is the target element to pop off of // user is the user whose profile to show // message is the message containing it, which should be selected -function toggle_user_card_popover_for_message(element, user, message) { - const $last_popover_elem = $current_message_user_card_popover_elem; - hide_all(); - if ($last_popover_elem !== undefined && $last_popover_elem.get()[0] === element) { - // We want it to be the case that a user can dismiss a popover - // by clicking on the same element that caused the popover. - return; - } +function toggle_user_card_popover_for_message(element, user, message, on_mount) { message_lists.current.select_id(message.id); const $elt = $(element); - if ($elt.data("popover") === undefined) { + if (!message_user_card.is_open()) { if (user === undefined) { // This is never supposed to happen, not even for deactivated // users, so we'll need to debug this error if it occurs. @@ -466,17 +455,25 @@ function toggle_user_card_popover_for_message(element, user, message) { is_sender_popover, true, "respond_personal_button", - "message-user-card-popover", + "message_user_card", "right", + undefined, + on_mount, ); - - $current_message_user_card_popover_elem = $elt; } } // This function serves as the entry point for toggling // the user card popover via keyboard shortcut. export function toggle_sender_info() { + if (message_user_card.is_open()) { + // We need to call the hide method here because + // the event wasn't triggered by the mouse. + // The Tippy unTrigger event wasn't called, + // so we have to manually hide the popover. + message_user_card.hide(); + return; + } const $message = $(".selected_message"); let $sender = $message.find(".message-avatar"); if ($sender.length === 0) { @@ -487,10 +484,11 @@ export function toggle_sender_info() { const message = message_lists.current.get(rows.id($message)); const user = people.get_by_user_id(message.sender_id); - toggle_user_card_popover_for_message($sender[0], user, message); - if ($current_message_user_card_popover_elem && !page_params.is_spectator) { - focus_user_card_popover_item(); - } + toggle_user_card_popover_for_message($sender[0], user, message, () => { + if (!page_params.is_spectator) { + focus_user_card_popover_item(); + } + }); } function focus_user_card_popover_item() { @@ -505,35 +503,19 @@ function focus_user_card_popover_item() { } } -export function is_message_user_card_open() { - return $current_message_user_card_popover_elem !== undefined; -} - -export function hide_message_user_card_popover() { - if (is_message_user_card_open()) { - $current_message_user_card_popover_elem.popover("destroy"); - $current_message_user_card_popover_elem = undefined; - } -} - -export function user_card_popover_for_message_handle_keyboard(key) { - const $items = get_user_card_popover_for_message_items(); - popover_items_handle_keyboard(key, $items); -} - function get_user_card_popover_for_message_items() { - if (!$current_message_user_card_popover_elem) { + if (!message_user_card.is_open()) { blueslip.error("Trying to get menu items when message user card popover is closed."); return undefined; } - const popover_data = $current_message_user_card_popover_elem.data("popover"); - if (!popover_data) { + const $popover = $(message_user_card.instance.popper); + if (!$popover) { blueslip.error("Cannot find popover data for message user card menu."); return undefined; } - return $("li:not(.divider):visible a", popover_data.$tip); + return $("li:not(.divider):visible a", $popover); } // Functions related to the user card popover in the user sidebar. @@ -560,48 +542,20 @@ function toggle_sidebar_user_card_popover($target) { false, false, "compose_private_message", - "user_popover", + "user_sidebar", "left", + undefined, + (instance) => { + /* See comment in get_props_for_popover_centering for explanation of this. */ + $(instance.popper).find(".tippy-box").addClass("show-when-reference-hidden"); + }, ); current_user_sidebar_user_id = user.user_id; - current_user_sidebar_popover = $target.data("popover"); -} - -export function user_sidebar_popped() { - return current_user_sidebar_popover !== undefined; -} - -export function hide_user_sidebar_popover() { - if (user_sidebar_popped()) { - // this hide_* method looks different from all the others since - // the presence list may be redrawn. Due to funkiness with jQuery's .data() - // this would confuse $.popover("destroy"), which looks at the .data() attached - // to a certain element. We thus save off the .data("popover") in the - // toggle_user_sidebar_popover and inject it here before calling destroy. - $("#user_presences").data("popover", current_user_sidebar_popover); - $("#user_presences").popover("destroy"); - current_user_sidebar_user_id = undefined; - current_user_sidebar_popover = undefined; - } -} - -function get_user_sidebar_popover_items() { - if (!current_user_sidebar_popover) { - blueslip.error("Trying to get menu items when user sidebar popover is closed."); - return undefined; - } - - return $("li:not(.divider):visible a", current_user_sidebar_popover.$tip); -} - -export function user_sidebar_popover_handle_keyboard(key) { - const $items = get_user_sidebar_popover_items(); - popover_items_handle_keyboard(key, $items); } function register_click_handlers() { - $("#main_div").on("click", ".sender_name, .message-avatar", function (e) { + $("#main_div").on("click", ".sender_name, .inline_profile_picture", function (e) { const $row = $(this).closest(".message_row"); e.stopPropagation(); const message = message_lists.current.get(rows.id($row)); @@ -696,7 +650,7 @@ function register_click_handlers() { e.stopPropagation(); e.preventDefault(); }); - $("body").on("click", ".user_popover .mention_user", (e) => { + $("body").on("click", ".user-card-popover-root .mention_user", (e) => { if (!compose_state.composing()) { compose_actions.start("stream", {trigger: "sidebar user actions"}); } @@ -704,13 +658,13 @@ function register_click_handlers() { const name = people.get_by_user_id(user_id).full_name; const mention = people.get_mention_syntax(name, user_id); compose_ui.insert_syntax_and_focus(mention); - hide_user_sidebar_popover(); + user_sidebar.hide(); popovers.hide_userlist_sidebar(); e.stopPropagation(); e.preventDefault(); }); - $("body").on("click", ".message-user-card-popover .mention_user", (e) => { + $("body").on("click", ".message-user-card-popover-root .mention_user", (e) => { if (!compose_state.composing()) { compose_actions.respond_to_message({trigger: "user sidebar popover"}); } @@ -718,7 +672,7 @@ function register_click_handlers() { const name = people.get_by_user_id(user_id).full_name; const mention = people.get_mention_syntax(name, user_id); compose_ui.insert_syntax_and_focus(mention); - hide_message_user_card_popover(); + message_user_card.hide(); e.stopPropagation(); e.preventDefault(); }); diff --git a/web/styles/popovers.css b/web/styles/popovers.css index 1cf89b9cd1..eef433125a 100644 --- a/web/styles/popovers.css +++ b/web/styles/popovers.css @@ -283,11 +283,11 @@ ul { } } -/* Important note: The user info popover user_popover class is applied - to user info popovers ONLY when they are opened from the right - sidebar; otherwise, it will have the user-card-popover class - instead. */ -.user_popover { +/* Important note: The user info popover user-sidebar-popover-root + class is applied to user info popovers ONLY when they are opened + from the right sidebar; otherwise, it will have the + user-card-popover-root class instead. */ +.user-sidebar-popover-root { width: 240px; margin: -14px; @@ -304,8 +304,8 @@ ul { } .group-info-popover, -.message-user-card-popover, -.user-card-popover { +.message-user-card-popover-root, +.user-card-popover-root { width: 240px; padding: 0; @@ -670,27 +670,6 @@ ul { } @media (width < $md_min) { - .user-card-popover { - display: flex !important; - justify-content: center; - align-items: center; - - /* these are to override JS embedded inline styles. */ - top: 0 !important; - left: 0 !important; - margin: 0 !important; - width: 100%; - height: 100%; - - background-color: hsl(0deg 0% 0% / 70%) !important; - border-radius: 0; - border: none; - - .popover-inner { - background-color: hsl(0deg 0% 100%); - } - } - .colorpicker-popover { display: flex !important; justify-content: center; @@ -788,7 +767,10 @@ ul { .giphy-popover, .emoji-popover-root, -.user-group-popover-root { +.user-group-popover-root, +.user-sidebar-popover-root, +.message-user-card-popover-root, +.user-card-popover-root { .tippy-content { /* We remove the default padding from this container as it is not necessary for the Giphy popover. */ @@ -798,6 +780,11 @@ ul { we can ignore the colors applied from `tippy-box`. */ color: var(--color-text-default); } + + & .popover-content ul.nav { + /* TODO: Clean this logic up after drop Bootstrap styling */ + margin: 0; + } } .user-group-popover-root { diff --git a/web/templates/no_arrow_popover.hbs b/web/templates/no_arrow_popover.hbs deleted file mode 100644 index 09837d7643..0000000000 --- a/web/templates/no_arrow_popover.hbs +++ /dev/null @@ -1,8 +0,0 @@ -
-
-
-
-
-
-
-
diff --git a/web/templates/user_card_popover_content.hbs b/web/templates/user_card_popover_content.hbs index b25d46c6a8..d59a9174cd 100644 --- a/web/templates/user_card_popover_content.hbs +++ b/web/templates/user_card_popover_content.hbs @@ -1,172 +1,175 @@ {{! Contents of the user card popover }} - + diff --git a/web/tests/hotkey.test.js b/web/tests/hotkey.test.js index 2807170d32..a35f3b437b 100644 --- a/web/tests/hotkey.test.js +++ b/web/tests/hotkey.test.js @@ -66,12 +66,18 @@ const overlays = mock_esm("../src/overlays", { is_overlay_or_modal_open: () => overlays.is_modal_open() || overlays.is_active(), }); const popovers = mock_esm("../src/user_card_popover", { - is_message_user_card_open: () => false, - user_sidebar_popped: () => false, - is_user_card_open: () => false, manage_menu: { is_open: () => false, }, + user_sidebar: { + is_open: () => false, + }, + message_user_card: { + is_open: () => false, + }, + user_card: { + is_open: () => false, + }, }); const popover_menus = mock_esm("../src/popover_menus", { get_visible_instance: () => undefined, diff --git a/web/third/bootstrap-tooltip/tooltip.js b/web/third/bootstrap-tooltip/tooltip.js index 43c23214e1..9705f1aaae 100644 --- a/web/third/bootstrap-tooltip/tooltip.js +++ b/web/third/bootstrap-tooltip/tooltip.js @@ -163,9 +163,6 @@ center--but this patch makes the popover more likely to be usable. (If the screen is super small, obviously we can't fit it completely.) - - If you use this fix_positions option, you want - to also use the "no_arrow_popover" template. */ if (top < 0) { top = 0;