From 1d62abee612627e803cc817a7b67499191e380ad Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Thu, 28 Sep 2023 13:24:53 +0000 Subject: [PATCH] popovers: Simplify hide_all. Since we only have tippy popovers now, functions calling not_hide_tippy_instances have been removed. Also, all of emoji_picker, stream_popover, user_card etc popovers are tippy popovers so calling hideAll is enough to hide them all. --- web/src/activity.js | 2 +- web/src/click_handlers.js | 15 --------------- web/src/giphy.js | 2 -- web/src/popover_menus.js | 11 +++++------ web/src/popovers.js | 26 ++++---------------------- web/src/stream_popover.js | 2 +- web/src/topic_zoom.js | 4 ++-- web/src/user_card_popover.js | 7 +++---- web/src/user_group_popover.js | 2 +- web/src/user_search.js | 5 ++--- web/tests/activity.test.js | 1 - web/tests/user_search.test.js | 7 ++----- 12 files changed, 21 insertions(+), 63 deletions(-) diff --git a/web/src/activity.js b/web/src/activity.js index 7158f7ce4e..4649e757fc 100644 --- a/web/src/activity.js +++ b/web/src/activity.js @@ -127,7 +127,7 @@ export function build_user_sidebar() { function do_update_users_for_search() { // Hide all the popovers but not userlist sidebar // when the user is searching. - popovers.hide_all_except_sidebars(); + popovers.hide_all(); build_user_sidebar(); user_cursor.reset(); } diff --git a/web/src/click_handlers.js b/web/src/click_handlers.js index fdfd6be10c..fc4e354f86 100644 --- a/web/src/click_handlers.js +++ b/web/src/click_handlers.js @@ -873,21 +873,6 @@ export function initialize() { return; } - // Dismiss popovers if the user has clicked outside them - if ( - $( - '.popover-inner, #user-profile-modal, .emoji-picker-popover, .app-main [class^="column-"].expanded', - ).has(e.target).length === 0 - ) { - // Since tippy instance can handle outside clicks on their own, - // we don't need to trigger them from here. - // This fixes the bug of `hideAll` being called - // after a tippy popover has been triggered which hides - // the popover without being displayed. - const not_hide_tippy_instances = true; - popovers.hide_all(not_hide_tippy_instances); - } - if (compose_state.composing() && !$(e.target).parents("#compose").length) { if ( $(e.target).closest("a").length > 0 || diff --git a/web/src/giphy.js b/web/src/giphy.js index 1e4fbb38cf..6aa8d94385 100644 --- a/web/src/giphy.js +++ b/web/src/giphy.js @@ -8,7 +8,6 @@ import * as compose_ui from "./compose_ui"; import {media_breakpoints_num} from "./css_variables"; import {page_params} from "./page_params"; import * as popover_menus from "./popover_menus"; -import * as popovers from "./popovers"; import * as rows from "./rows"; import * as ui_util from "./ui_util"; @@ -195,7 +194,6 @@ function toggle_giphy_popover(target) { giphy_popover_instance = instance; const $popper = $(giphy_popover_instance.popper).trigger("focus"); gifs_grid = await renderGIPHYGrid($popper.find(".giphy-content")[0]); - popovers.hide_all(true); const $click_target = $(instance.reference); if ($click_target.parents(".message_edit_form").length === 1) { diff --git a/web/src/popover_menus.js b/web/src/popover_menus.js index b80d1dca5a..bf0391361b 100644 --- a/web/src/popover_menus.js +++ b/web/src/popover_menus.js @@ -242,7 +242,7 @@ function on_show_prep(instance) { e.stopPropagation(); instance.hide(); }); - popovers.hide_all_except_sidebars(); + popovers.hide_all(); } function get_props_for_popover_centering(popover_props) { @@ -584,7 +584,7 @@ export function initialize() { ), ); popover_instances.compose_control_buttons = instance; - popovers.hide_all_except_sidebars(); + popovers.hide_all(); }, onHidden(instance) { instance.destroy(); @@ -1025,7 +1025,7 @@ export function initialize() { }); }, onShow(instance) { - popovers.hide_all_except_sidebars(); + popovers.hide_all(); const show_unstar_all_button = starred_messages.get_count() > 0; instance.setContent( @@ -1057,7 +1057,7 @@ export function initialize() { }); }, onShow(instance) { - popovers.hide_all_except_sidebars(); + popovers.hide_all(); instance.setContent(parse_html(render_drafts_sidebar_actions({}))); }, @@ -1081,7 +1081,7 @@ export function initialize() { }); }, onShow(instance) { - popovers.hide_all_except_sidebars(); + popovers.hide_all(); instance.setContent(parse_html(render_all_messages_sidebar_actions())); }, onHidden(instance) { @@ -1098,7 +1098,6 @@ export function initialize() { $("#compose-textarea").trigger("focus"); }, onShow(instance) { - popovers.hide_all_except_sidebars(instance); const formatted_send_later_time = get_formatted_selected_send_later_time(); instance.setContent( parse_html( diff --git a/web/src/popovers.js b/web/src/popovers.js index 6831341633..5ae8c7ebf2 100644 --- a/web/src/popovers.js +++ b/web/src/popovers.js @@ -1,4 +1,4 @@ -import {hideAll} from "tippy.js"; +import * as tippy from "tippy.js"; import * as emoji_picker from "./emoji_picker"; import * as playground_links_popover from "./playground_links_popover"; @@ -22,25 +22,7 @@ export function any_active() { ); } -// This function will hide all true popovers (the streamlist and -// userlist sidebars use the popover infrastructure, but doesn't work -// like a popover structurally). -export function hide_all_except_sidebars(opts) { - if (!opts || !opts.not_hide_tippy_instances) { - // hideAll hides all tippy instances (tooltips and popovers). - hideAll(); - } - emoji_picker.hide_emoji_popover(); - stream_popover.hide_stream_popover(); - user_group_popover.hide(); - user_card_popover.hide_all_user_card_popovers(); - playground_links_popover.hide(); -} - -// This function will hide all the popovers, including the mobile web -// or narrow window sidebars. -export function hide_all(not_hide_tippy_instances) { - hide_all_except_sidebars({ - not_hide_tippy_instances, - }); +export function hide_all() { + // Hides all tippy instances (tooltips and popovers). + tippy.hideAll(); } diff --git a/web/src/stream_popover.js b/web/src/stream_popover.js index 5ffde4d287..df76723747 100644 --- a/web/src/stream_popover.js +++ b/web/src/stream_popover.js @@ -104,7 +104,7 @@ function build_stream_popover(opts) { return; } - popovers.hide_all_except_sidebars(); + popovers.hide_all(); const content = render_stream_sidebar_actions({ stream: sub_store.get(stream_id), }); diff --git a/web/src/topic_zoom.js b/web/src/topic_zoom.js index 7f46d0f642..248c2032bf 100644 --- a/web/src/topic_zoom.js +++ b/web/src/topic_zoom.js @@ -15,7 +15,7 @@ export function is_zoomed_in() { function zoom_in() { const stream_id = topic_list.active_stream_id(); - popovers.hide_all_except_sidebars(); + popovers.hide_all(); pm_list.close(); topic_list.zoom_in(); stream_list.zoom_in_topics({ @@ -35,7 +35,7 @@ export function zoom_out() { } const $stream_li = topic_list.get_stream_li(); - popovers.hide_all_except_sidebars(); + popovers.hide_all(); topic_list.zoom_out(); stream_list.zoom_out_topics(); diff --git a/web/src/user_card_popover.js b/web/src/user_card_popover.js index c2a9f48eda..02e290ab48 100644 --- a/web/src/user_card_popover.js +++ b/web/src/user_card_popover.js @@ -26,7 +26,7 @@ import * as overlays from "./overlays"; import {page_params} from "./page_params"; import * as people from "./people"; import * as popover_menus from "./popover_menus"; -import {hide_all, hide_all_except_sidebars} from "./popovers"; +import {hide_all} from "./popovers"; import * as rows from "./rows"; import * as settings_config from "./settings_config"; import * as sidebar_ui from "./sidebar_ui"; @@ -597,9 +597,8 @@ function toggle_sidebar_user_card_popover($target) { // Hiding popovers may mutate current_user_sidebar_user_id. const previous_user_sidebar_id = current_user_sidebar_user_id; - // Hide popovers, but we don't want to hide the sidebars on - // smaller browser windows. - hide_all_except_sidebars(); + // Hide popovers + hide_all(); if (previous_user_sidebar_id === user_id) { // If the popover is already shown, clicking again should toggle it. diff --git a/web/src/user_group_popover.js b/web/src/user_group_popover.js index 2bf1b0d880..5fc87a1983 100644 --- a/web/src/user_group_popover.js +++ b/web/src/user_group_popover.js @@ -82,7 +82,7 @@ export function toggle_user_group_info_popover(element, message_id) { ], }, onCreate(instance) { - popovers.hide_all_except_sidebars(); + popovers.hide_all(); if (message_id) { message_lists.current.select_id(message_id); } diff --git a/web/src/user_search.js b/web/src/user_search.js index 16b2a21c37..7627073e00 100644 --- a/web/src/user_search.js +++ b/web/src/user_search.js @@ -73,9 +73,8 @@ export class UserSearch { } show_widget() { - // Hide all the popovers but not userlist sidebar - // when the user wants to search. - popovers.hide_all_except_sidebars(); + // Hide all the popovers. + popovers.hide_all(); this.$widget.removeClass("notdisplayed"); resize.resize_sidebars(); } diff --git a/web/tests/activity.test.js b/web/tests/activity.test.js index f9d2f9b2d5..398dc4d5c6 100644 --- a/web/tests/activity.test.js +++ b/web/tests/activity.test.js @@ -280,7 +280,6 @@ test("handlers", ({override, mock_template}) => { override(scroll_util, "scroll_element_into_container", () => {}); override(padded_widget, "update_padding", () => {}); override(popovers, "hide_all", () => {}); - override(popovers, "hide_all_except_sidebars", () => {}); override(sidebar_ui, "show_userlist_sidebar", () => {}); override(resize, "resize_sidebars", () => {}); diff --git a/web/tests/user_search.test.js b/web/tests/user_search.test.js index fb44625043..20c465aee4 100644 --- a/web/tests/user_search.test.js +++ b/web/tests/user_search.test.js @@ -84,7 +84,7 @@ function set_input_val(val) { test("clear_search", ({override}) => { override(presence, "get_status", () => "active"); override(presence, "get_user_ids", () => all_user_ids); - override(popovers, "hide_all_except_sidebars", () => {}); + override(popovers, "hide_all", () => {}); override(resize, "resize_sidebars", () => {}); // Empty because no users match this search string. @@ -108,7 +108,7 @@ test("escape_search", ({override}) => { page_params.realm_presence_disabled = true; override(resize, "resize_sidebars", () => {}); - override(popovers, "hide_all_except_sidebars", () => {}); + override(popovers, "hide_all", () => {}); set_input_val("somevalue"); activity.escape_search(); @@ -123,7 +123,6 @@ test("escape_search", ({override}) => { test("blur search right", ({override}) => { override(sidebar_ui, "show_userlist_sidebar", () => {}); override(popovers, "hide_all", () => {}); - override(popovers, "hide_all_except_sidebars", () => {}); override(resize, "resize_sidebars", () => {}); $(".user-list-filter").closest = (selector) => { @@ -140,7 +139,6 @@ test("blur search right", ({override}) => { test("blur search left", ({override}) => { override(sidebar_ui, "show_streamlist_sidebar", () => {}); override(popovers, "hide_all", () => {}); - override(popovers, "hide_all_except_sidebars", () => {}); override(resize, "resize_sidebars", () => {}); $(".user-list-filter").closest = (selector) => { @@ -212,7 +210,6 @@ test("click on user header to toggle display", ({override}) => { const $user_filter = $(".user-list-filter"); override(popovers, "hide_all", () => {}); - override(popovers, "hide_all_except_sidebars", () => {}); override(sidebar_ui, "show_userlist_sidebar", () => {}); override(resize, "resize_sidebars", () => {});