diff --git a/help/user-list.md b/help/user-list.md index 5d78d52f7b..6cdc613668 100644 --- a/help/user-list.md +++ b/help/user-list.md @@ -23,11 +23,7 @@ distraction, you can hide the user list any time. class="zulip-icon zulip-icon-triple-users"> ) icon in the upper right to show it. -1. Click the **search** () icon at the top of the right sidebar to open the - search box. - -1. Type the name of the user you are looking for. +1. Type the name of the user you are looking for in the search box. !!! keyboard_tip "" diff --git a/web/e2e-tests/message-basics.test.ts b/web/e2e-tests/message-basics.test.ts index 7bcc170670..eceed790ba 100644 --- a/web/e2e-tests/message-basics.test.ts +++ b/web/e2e-tests/message-basics.test.ts @@ -429,7 +429,7 @@ async function test_users_search(page: Page): Promise { await assert_in_list(page, "aaron"); // Enter the search box and test selected suggestion navigation - await page.click("#user_filter_icon"); + await page.click(".user-list-filter"); await page.waitForSelector("#buddy-list-other-users .highlighted_user", {visible: true}); await assert_selected(page, "Desdemona"); await assert_not_selected(page, "Cordelia, Lear's daughter"); diff --git a/web/src/activity_ui.ts b/web/src/activity_ui.ts index cd49215a90..7148b8077f 100644 --- a/web/src/activity_ui.ts +++ b/web/src/activity_ui.ts @@ -212,7 +212,7 @@ export function narrow_for_user_id(opts: {user_id: number}): void { assert(narrow_by_email); narrow_by_email(email); assert(user_filter !== undefined); - user_filter.clear_and_hide_search(); + user_filter.clear_search(); } function keydown_enter_key(): void { @@ -274,9 +274,9 @@ export function initiate_search(): void { } } -export function escape_search(): void { +export function clear_search(): void { if (user_filter) { - user_filter.clear_and_hide_search(); + user_filter.clear_search(); } } diff --git a/web/src/buddy_list.ts b/web/src/buddy_list.ts index ca16f19deb..82378a3974 100644 --- a/web/src/buddy_list.ts +++ b/web/src/buddy_list.ts @@ -77,7 +77,6 @@ type BuddyListRenderData = { pm_ids_set: Set; total_human_subscribers_count: number; other_users_count: number; - total_human_users: number; hide_headers: boolean; all_participant_ids: Set; }; @@ -87,8 +86,7 @@ function get_render_data(): BuddyListRenderData { const pm_ids_set = narrow_state.pm_ids_set(); const total_human_subscribers_count = get_total_human_subscriber_count(current_sub, pm_ids_set); - const total_human_users = people.get_active_human_count(); - const other_users_count = total_human_users - total_human_subscribers_count; + const other_users_count = people.get_active_human_count() - total_human_subscribers_count; const hide_headers = should_hide_headers(current_sub, pm_ids_set); const all_participant_ids = buddy_data.get_conversation_participants(); @@ -97,7 +95,6 @@ function get_render_data(): BuddyListRenderData { pm_ids_set, total_human_subscribers_count, other_users_count, - total_human_users, hide_headers, all_participant_ids, }; @@ -247,9 +244,8 @@ export class BuddyList extends BuddyListConf { ); } } else { - const total_human_users = people.get_active_human_count(); const other_users_count = - total_human_users - total_human_subscribers_count; + people.get_active_human_count() - total_human_subscribers_count; tooltip_text = $t( { defaultMessage: @@ -429,8 +425,7 @@ export class BuddyList extends BuddyListConf { } this.current_filter = narrow_state.filter(); - const {current_sub, total_human_subscribers_count, other_users_count, total_human_users} = - this.render_data; + const {current_sub, total_human_subscribers_count, other_users_count} = this.render_data; $(".buddy-list-subsection-header").empty(); // If we're in the mode of hiding headers, that means we're only showing the "other users" @@ -446,16 +441,9 @@ export class BuddyList extends BuddyListConf { $("#buddy-list-users-matching-view-container").toggleClass("no-display", true); } - // Usually we show the user counts in the headers, but if we're hiding - // those headers then we show the total user count in the main title. - const default_userlist_title = $t({defaultMessage: "USERS"}); if (hide_headers) { - const formatted_count = get_formatted_sub_count(total_human_users); - const userlist_title = `${default_userlist_title} (${formatted_count})`; - $("#userlist-title").text(userlist_title); return; } - $("#userlist-title").text(default_userlist_title); let header_text; if (current_sub) { diff --git a/web/src/hotkey.js b/web/src/hotkey.js index 0c6fb3f2ef..735a9bbaf7 100644 --- a/web/src/hotkey.js +++ b/web/src/hotkey.js @@ -306,7 +306,7 @@ export function process_escape_key(e) { if (processing_text()) { if (activity_ui.searching()) { - activity_ui.escape_search(); + activity_ui.clear_search(); return true; } diff --git a/web/src/resize.ts b/web/src/resize.ts index 52ddfabf02..53b12e694a 100644 --- a/web/src/resize.ts +++ b/web/src/resize.ts @@ -33,8 +33,7 @@ function get_new_heights(): { const usable_height = viewport_height - Number.parseInt($("#right-sidebar").css("paddingTop"), 10) - - ($("#userlist-header").outerHeight(true) ?? 0) - - ($("#user_search_section:not(.notdisplayed)").outerHeight(true) ?? 0); + ($("#userlist-header").outerHeight(true) ?? 0); const buddy_list_wrapper_max_height = Math.max(80, usable_height); diff --git a/web/src/tippyjs.ts b/web/src/tippyjs.ts index d2c6f1d5b0..8079251829 100644 --- a/web/src/tippyjs.ts +++ b/web/src/tippyjs.ts @@ -698,6 +698,7 @@ export function initialize(): void { tippy.delegate("body", { target: "#userlist-header-search", + delay: LONG_HOVER_DELAY, placement: "top", appendTo: () => document.body, onShow(instance) { diff --git a/web/src/user_search.ts b/web/src/user_search.ts index e66ea11dad..4898942eef 100644 --- a/web/src/user_search.ts +++ b/web/src/user_search.ts @@ -3,7 +3,6 @@ import assert from "minimalistic-assert"; import * as buddy_data from "./buddy_data"; import * as popovers from "./popovers"; -import * as resize from "./resize"; import * as sidebar_ui from "./sidebar_ui"; export class UserSearch { @@ -11,7 +10,7 @@ export class UserSearch { // above the buddy list. We rely on other code to manage the // details of populating the list when we change. - $widget = $("#user_search_section").expectOne(); + $widget = $("#userlist-header-search").expectOne(); $input = $("input.user-list-filter").expectOne(); _reset_items: () => void; _update_list: () => void; @@ -25,12 +24,11 @@ export class UserSearch { $("#clear_search_people_button").on("click", () => { this.clear_search(); }); - $("#userlist-header-search").on("click", () => { - this.toggle_filter_displayed(); - }); this.$input.on("input", () => { - buddy_data.set_is_searching_users(this.$input.val() !== ""); + const input_is_empty = this.$input.val() === ""; + buddy_data.set_is_searching_users(!input_is_empty); + $("#clear_search_people_button").toggleClass("hidden", input_is_empty); opts.update_list(); }); this.$input.on("focus", (e) => { @@ -52,54 +50,17 @@ export class UserSearch { return this.$input.is(":focus"); } - empty(): boolean { - return this.text() === ""; - } - // This clears search input but doesn't close // the search widget unless it was already empty. clear_search(): void { buddy_data.set_is_searching_users(false); - - if (this.empty()) { - this.close_widget(); - return; - } + $("#clear_search_people_button").toggleClass("hidden", true); this.$input.val(""); this.$input.trigger("blur"); this._reset_items(); } - // This always clears and closes search. - clear_and_hide_search(): void { - this.clear_search(); - this._update_list(); - this.close_widget(); - } - - hide_widget(): void { - this.$widget.addClass("notdisplayed"); - resize.resize_sidebars(); - } - - show_widget(): void { - // Hide all the popovers. - popovers.hide_all(); - this.$widget.removeClass("notdisplayed"); - resize.resize_sidebars(); - } - - widget_shown(): boolean { - return this.$widget.hasClass("notdisplayed"); - } - - close_widget(): void { - this.$input.trigger("blur"); - this.hide_widget(); - this._reset_items(); - } - expand_column(): void { const $column = this.$input.closest(".app-main [class^='column-']"); if (!$column.hasClass("expanded")) { @@ -114,21 +75,12 @@ export class UserSearch { initiate_search(): void { this.expand_column(); - this.show_widget(); // Needs to be called when input is visible after fix_invite_user_button_flicker. setTimeout(() => { this.$input.trigger("focus"); }, 0); } - toggle_filter_displayed(): void { - if (this.widget_shown()) { - this.initiate_search(); - } else { - this.clear_and_hide_search(); - } - } - on_focus(e: JQuery.FocusEvent): void { this._on_focus(); e.stopPropagation(); diff --git a/web/styles/right_sidebar.css b/web/styles/right_sidebar.css index ba32252d0a..60be2c9d61 100644 --- a/web/styles/right_sidebar.css +++ b/web/styles/right_sidebar.css @@ -363,37 +363,59 @@ $user_status_emoji_width: 24px; grid-template-rows: var(--line-height-sidebar-row-prominent); grid-template-columns: minmax(0, 1fr) auto; align-items: center; + margin-bottom: 10px; #userlist-header-search { display: grid; grid-template-rows: var(--line-height-sidebar-row-prominent); - grid-template-columns: minmax(0, 1fr) 20px; + grid-template-columns: minmax(0, 1fr) 30px; align-items: center; - } - #userlist-title { - margin: 0; - } + & .user-list-filter { + grid-area: 1 / 1 / 2 / 3; + white-space: nowrap; + text-overflow: ellipsis; + overflow: hidden; + /* Prevent text from colliding with #clear_search_button */ + padding-right: 28px; + height: var(--line-height-sidebar-row-prominent); + box-sizing: border-box; + } - #user_filter_icon { - opacity: 0.5; - justify-self: center; + #clear_search_people_button { + grid-area: 1 / 2 / 2 / 3; + padding: 0; + background: none; + color: var(--color-text-clear-search-button); + display: grid; - &:hover { - opacity: 1; - cursor: pointer; + &:hover { + color: var(--color-text-clear-search-button-hover); + } + + &:focus, + &:focus-visible, + &:active { + box-shadow: none; + outline: none; + } + + .zulip-icon-close { + align-self: center; + } } } - /* hovering over the userlist-header creates the same highlight effect as hovering over the user_filter_icon */ - &:hover > #user_filter_icon { - opacity: 1; - cursor: pointer; - } - #buddy-list-menu-icon { + color: var(--color-vdots-visible); justify-content: center; display: grid; + width: 25px; + margin-left: 5px; + + &:hover { + color: var(--color-vdots-hover); + } } } @@ -407,29 +429,3 @@ $user_status_emoji_width: 24px; from the legacy value. */ margin-top: calc(25px - (var(--legacy-body-line-height-unitless) * 1em)); } - -#user_search_section { - display: grid; - grid-template-columns: minmax(0, 1fr) 28px; - grid-template-rows: var(--line-height-sidebar-row-prominent); - white-space: nowrap; - margin-bottom: 10px; - - & .user-list-filter { - grid-area: 1 / 1 / 2 / 3; - white-space: nowrap; - text-overflow: ellipsis; - overflow: hidden; - /* Prevent text from colliding with .clear_search_button */ - padding-right: 28px; - /* Push back against inherited styles; let CSS Grid be in - charge of the height. */ - height: auto; - } - - .clear_search_button { - grid-area: 1 / 2 / 2 / 3; - position: static; - padding: 0; - } -} diff --git a/web/templates/right_sidebar.hbs b/web/templates/right_sidebar.hbs index 7d6e80d695..157a64a2f2 100644 --- a/web/templates/right_sidebar.hbs +++ b/web/templates/right_sidebar.hbs @@ -2,22 +2,16 @@
- - - - +
-
- - -
diff --git a/web/tests/activity.test.js b/web/tests/activity.test.js index 0398583adc..ce92e36413 100644 --- a/web/tests/activity.test.js +++ b/web/tests/activity.test.js @@ -30,7 +30,6 @@ const electron_bridge = mock_esm("../src/electron_bridge"); const padded_widget = mock_esm("../src/padded_widget"); const pm_list = mock_esm("../src/pm_list"); const popovers = mock_esm("../src/popovers"); -const resize = mock_esm("../src/resize"); const settings_data = mock_esm("../src/settings_data"); const sidebar_ui = mock_esm("../src/sidebar_ui"); const scroll_util = mock_esm("../src/scroll_util"); @@ -270,13 +269,6 @@ test("presence_list_full_update", ({override, mock_template}) => { assert.equal(presence_rows[0].user_id, me.user_id); }); -function simulate_right_column_buddy_list() { - $("input.user-list-filter").closest = (selector) => { - assert.equal(selector, ".app-main [class^='column-']"); - return $.create("right-sidebar").addClass("column-right"); - }; -} - test("direct_message_update_dom_counts", () => { const $count = $.create("alice-unread-count"); const pm_key = alice.user_id.toString(); @@ -310,8 +302,6 @@ test("handlers", ({override, override_rewire, mock_template}) => { override(padded_widget, "update_padding", noop); override(popovers, "hide_all", noop); override(sidebar_ui, "hide_all", noop); - override(sidebar_ui, "show_userlist_sidebar", noop); - override(resize, "resize_sidebars", noop); // This is kind of weak coverage; we are mostly making sure that // keys and clicks got mapped to functions that don't crash. @@ -370,18 +360,6 @@ test("handlers", ({override, override_rewire, mock_template}) => { handler(e); })(); - (function test_click_header_filter() { - init(); - const e = {}; - const handler = $("#userlist-header-search").get_on_handler("click"); - - simulate_right_column_buddy_list(); - - handler(e); - // and click again - handler(e); - })(); - (function test_enter_key() { init(); diff --git a/web/tests/user_search.test.js b/web/tests/user_search.test.js index 52f7eb2ac6..e372cc2a81 100644 --- a/web/tests/user_search.test.js +++ b/web/tests/user_search.test.js @@ -25,20 +25,13 @@ mock_esm("../src/buddy_list", { }); function mock_setTimeout() { - let set_timeout_function_called = false; set_global("setTimeout", (func) => { - if (set_timeout_function_called) { - // This conditional is needed to avoid indefinite calls. - return; - } - set_timeout_function_called = true; func(); }); } const popovers = mock_esm("../src/popovers"); const presence = mock_esm("../src/presence"); -const resize = mock_esm("../src/resize"); const sidebar_ui = mock_esm("../src/sidebar_ui"); const activity_ui = zrequire("activity_ui"); @@ -103,7 +96,6 @@ test("clear_search", ({override}) => { override(presence, "get_status", () => "active"); override(presence, "get_user_ids", () => all_user_ids); override(popovers, "hide_all", noop); - override(resize, "resize_sidebars", noop); stub_buddy_list_empty_list_message_lengths(); @@ -112,7 +104,6 @@ test("clear_search", ({override}) => { assert.deepEqual(user_ids, {all_user_ids: []}); }); set_input_val("somevalue"); - assert.ok(!$("#user_search_section").hasClass("notdisplayed")); // Now we're clearing the search string and everyone shows up again. override(fake_buddy_list, "populate", (user_ids) => { @@ -121,21 +112,18 @@ test("clear_search", ({override}) => { $("#clear_search_people_button").trigger("click"); assert.equal($("input.user-list-filter").val(), ""); $("#clear_search_people_button").trigger("click"); - assert.ok($("#user_search_section").hasClass("notdisplayed")); }); -test("escape_search", ({override}) => { +test("clear_search", ({override}) => { override(realm, "realm_presence_disabled", true); - override(resize, "resize_sidebars", noop); override(popovers, "hide_all", noop); stub_buddy_list_empty_list_message_lengths(); set_input_val("somevalue"); - activity_ui.escape_search(); + activity_ui.clear_search(); assert.equal($("input.user-list-filter").val(), ""); - activity_ui.escape_search(); - assert.ok($("#user_search_section").hasClass("notdisplayed")); + activity_ui.clear_search(); // We need to reset this because the unit tests aren't isolated from each other. set_input_val(""); @@ -144,7 +132,6 @@ test("escape_search", ({override}) => { test("blur search right", ({override}) => { override(sidebar_ui, "show_userlist_sidebar", noop); override(popovers, "hide_all", noop); - override(resize, "resize_sidebars", noop); mock_setTimeout(); $("input.user-list-filter").closest = (selector) => { @@ -161,7 +148,6 @@ test("blur search right", ({override}) => { test("blur search left", ({override}) => { override(sidebar_ui, "show_streamlist_sidebar", noop); override(popovers, "hide_all", noop); - override(resize, "resize_sidebars", noop); mock_setTimeout(); $("input.user-list-filter").closest = (selector) => { @@ -229,32 +215,6 @@ test("filter_user_ids", ({override}) => { test_filter("fr,al", [alice, fred]); }); -test("click on user header to toggle display", ({override}) => { - const $user_filter = $("input.user-list-filter"); - - override(popovers, "hide_all", noop); - override(sidebar_ui, "show_userlist_sidebar", noop); - override(resize, "resize_sidebars", noop); - - override(realm, "realm_presence_disabled", true); - - assert.ok(!$("#user_search_section").hasClass("notdisplayed")); - - $user_filter.val("bla"); - - $("#userlist-header-search").trigger("click"); - assert.ok($("#user_search_section").hasClass("notdisplayed")); - assert.equal($user_filter.val(), ""); - - $("input.user-list-filter").closest = (selector) => { - assert.equal(selector, ".app-main [class^='column-']"); - return $.create("sidebar").addClass("column-right"); - }; - - $("#userlist-header-search").trigger("click"); - assert.equal($("#user_search_section").hasClass("notdisplayed"), false); -}); - test("searching", () => { assert.equal(activity_ui.searching(), false); $("input.user-list-filter").trigger("focus");