From 231aa098cb19cec028a189dded01ea9191021e40 Mon Sep 17 00:00:00 2001 From: evykassirer Date: Tue, 22 Aug 2023 17:18:53 -0700 Subject: [PATCH] buddy list: Create section in buddy list for users from narrow. Fixes #21285. --- tools/test-js-with-node | 1 + web/e2e-tests/message-basics.test.ts | 26 +- web/src/activity_ui.js | 5 +- web/src/buddy_data.ts | 55 +- web/src/buddy_list.js | 537 ++++++++++++++++-- web/src/click_handlers.js | 22 +- web/src/list_util.ts | 1 + web/src/narrow.js | 2 + web/src/sidebar_ui.ts | 18 + web/src/tippyjs.ts | 16 +- web/src/ui_init.js | 12 +- web/src/user_card_popover.js | 4 +- web/src/views_util.js | 5 + web/styles/app_variables.css | 6 + web/styles/dark_theme.css | 5 - web/styles/right_sidebar.css | 86 ++- web/templates/buddy_list/section_header.hbs | 4 + web/templates/buddy_list/title_tooltip.hbs | 4 + .../buddy_list/view_all_subscribers.hbs | 1 + web/templates/buddy_list/view_all_users.hbs | 1 + web/templates/right_sidebar.hbs | 12 +- web/tests/activity.test.js | 347 ++++++----- web/tests/buddy_data.test.js | 59 ++ web/tests/buddy_list.test.js | 255 +++++++-- web/tests/lib/buddy_list.js | 47 ++ web/tests/narrow_activate.test.js | 13 + web/tests/recent_view.test.js | 13 +- web/tests/user_search.test.js | 17 +- 28 files changed, 1294 insertions(+), 280 deletions(-) create mode 100644 web/templates/buddy_list/section_header.hbs create mode 100644 web/templates/buddy_list/title_tooltip.hbs create mode 100644 web/templates/buddy_list/view_all_subscribers.hbs create mode 100644 web/templates/buddy_list/view_all_users.hbs create mode 100644 web/tests/lib/buddy_list.js diff --git a/tools/test-js-with-node b/tools/test-js-with-node index d80949d188..4b3b48ecb5 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -62,6 +62,7 @@ EXEMPT_FILES = make_set( "web/src/blueslip.ts", "web/src/blueslip_stacktrace.ts", "web/src/browser_history.ts", + "web/src/buddy_list.js", "web/src/click_handlers.js", "web/src/compose.js", "web/src/compose_actions.js", diff --git a/web/e2e-tests/message-basics.test.ts b/web/e2e-tests/message-basics.test.ts index b910790812..15f1eb5f2f 100644 --- a/web/e2e-tests/message-basics.test.ts +++ b/web/e2e-tests/message-basics.test.ts @@ -422,24 +422,21 @@ async function test_stream_search_filters_stream_list(page: Page): Promise async function test_users_search(page: Page): Promise { console.log("Search users using right sidebar"); async function assert_in_list(page: Page, name: string): Promise { - await page.waitForSelector( - `#buddy-list-users-matching-view li [data-name="${CSS.escape(name)}"]`, - { - visible: true, - }, - ); + await page.waitForSelector(`#buddy-list-other-users li [data-name="${CSS.escape(name)}"]`, { + visible: true, + }); } async function assert_selected(page: Page, name: string): Promise { await page.waitForSelector( - `#buddy-list-users-matching-view li.highlighted_user [data-name="${CSS.escape(name)}"]`, + `#buddy-list-other-users li.highlighted_user [data-name="${CSS.escape(name)}"]`, {visible: true}, ); } async function assert_not_selected(page: Page, name: string): Promise { await page.waitForSelector( - `#buddy-list-users-matching-view li.highlighted_user [data-name="${CSS.escape(name)}"]`, + `#buddy-list-other-users li.highlighted_user [data-name="${CSS.escape(name)}"]`, {hidden: true}, ); } @@ -451,9 +448,7 @@ async function test_users_search(page: Page): Promise { // Enter the search box and test selected suggestion navigation await page.click("#user_filter_icon"); - await page.waitForSelector("#buddy-list-users-matching-view .highlighted_user", { - visible: true, - }); + 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"); await assert_not_selected(page, "King Hamlet"); @@ -475,12 +470,9 @@ async function test_users_search(page: Page): Promise { await arrow(page, "Down"); // Now Iago must be highlighted - await page.waitForSelector( - '#buddy-list-users-matching-view li.highlighted_user [data-name="Iago"]', - { - visible: true, - }, - ); + await page.waitForSelector('#buddy-list-other-users li.highlighted_user [data-name="Iago"]', { + visible: true, + }); await assert_not_selected(page, "King Hamlet"); await assert_not_selected(page, "aaron"); await assert_not_selected(page, "Desdemona"); diff --git a/web/src/activity_ui.js b/web/src/activity_ui.js index 0d3ecb34e1..b28fd68d06 100644 --- a/web/src/activity_ui.js +++ b/web/src/activity_ui.js @@ -93,13 +93,14 @@ export function build_user_sidebar() { return undefined; } - const filter_text = get_filter_text(); + const filter_text = user_filter.text(); const all_user_ids = buddy_data.get_filtered_and_sorted_user_ids(filter_text); buddy_list.populate({all_user_ids}); - render_empty_user_list_message_if_needed(buddy_list.$container); + render_empty_user_list_message_if_needed(buddy_list.$users_matching_view_container); + render_empty_user_list_message_if_needed(buddy_list.$other_users_container); return all_user_ids; // for testing } diff --git a/web/src/buddy_data.ts b/web/src/buddy_data.ts index 92bbfdc29c..717da6f253 100644 --- a/web/src/buddy_data.ts +++ b/web/src/buddy_data.ts @@ -3,9 +3,12 @@ import * as compose_fade_users from "./compose_fade_users"; import * as hash_util from "./hash_util"; import {$t} from "./i18n"; import * as muted_users from "./muted_users"; +import * as narrow_state from "./narrow_state"; import * as people from "./people"; import * as presence from "./presence"; import {realm} from "./state_data"; +import * as stream_data from "./stream_data"; +import type {StreamSubscription} from "./sub_store"; import * as timerender from "./timerender"; import * as unread from "./unread"; import {user_settings} from "./user_settings"; @@ -24,6 +27,11 @@ import * as util from "./util"; export const max_size_before_shrinking = 600; let is_searching_users = false; + +export function get_is_searching_users(): boolean { + return is_searching_users; +} + export function set_is_searching_users(val: boolean): void { is_searching_users = val; } @@ -71,7 +79,35 @@ export function level(user_id: number): number { } } -export function compare_function(a: number, b: number): number { +export function user_matches_narrow( + user_id: number, + pm_ids: Set, + stream_id?: number | null, +): boolean { + if (stream_id) { + return stream_data.is_user_subscribed(stream_id, user_id); + } + if (pm_ids.size > 0) { + return pm_ids.has(user_id) || people.is_my_user_id(user_id); + } + return false; +} + +export function compare_function( + a: number, + b: number, + current_sub: StreamSubscription | undefined, + pm_ids: Set, +): number { + const a_would_receive_message = user_matches_narrow(a, pm_ids, current_sub?.stream_id); + const b_would_receive_message = user_matches_narrow(b, pm_ids, current_sub?.stream_id); + if (a_would_receive_message && !b_would_receive_message) { + return -1; + } + if (!a_would_receive_message && b_would_receive_message) { + return 1; + } + const level_a = level(a); const level_b = level(b); const diff = level_a - level_b; @@ -91,7 +127,9 @@ export function compare_function(a: number, b: number): number { export function sort_users(user_ids: number[]): number[] { // TODO sort by unread count first, once we support that - user_ids.sort(compare_function); + const current_sub = narrow_state.stream_sub(); + const pm_ids_set = narrow_state.pm_ids_set(); + user_ids.sort((a, b) => compare_function(a, b, current_sub, pm_ids_set)); return user_ids; } @@ -276,7 +314,11 @@ function maybe_shrink_list(user_ids: number[], user_filter_text: string): number return user_ids; } - user_ids = user_ids.filter((user_id) => user_is_recently_active(user_id)); + // We want to always show PM recipients even if they're inactive. + const pm_ids_set = narrow_state.pm_ids_set(); + user_ids = user_ids.filter( + (user_id) => user_is_recently_active(user_id) || user_matches_narrow(user_id, pm_ids_set), + ); return user_ids; } @@ -340,6 +382,13 @@ function get_filtered_user_id_list(user_filter_text: string): number[] { if (!base_user_id_list.includes(my_user_id)) { base_user_id_list = [my_user_id, ...base_user_id_list]; } + + // We want to always show PM recipients even if they're inactive. + const pm_ids_set = narrow_state.pm_ids_set(); + if (pm_ids_set.size) { + const base_user_id_set = new Set([...base_user_id_list, ...pm_ids_set]); + base_user_id_list = [...base_user_id_set]; + } } const user_ids = filter_user_ids(user_filter_text, base_user_id_list); diff --git a/web/src/buddy_list.js b/web/src/buddy_list.js index a21b1eb5ed..99faa27053 100644 --- a/web/src/buddy_list.js +++ b/web/src/buddy_list.js @@ -1,16 +1,40 @@ import $ from "jquery"; +import tippy from "tippy.js"; +import render_section_header from "../templates/buddy_list/section_header.hbs"; +import render_view_all_subscribers from "../templates/buddy_list/view_all_subscribers.hbs"; +import render_view_all_users from "../templates/buddy_list/view_all_users.hbs"; +import render_empty_list_widget_for_list from "../templates/empty_list_widget_for_list.hbs"; import render_presence_row from "../templates/presence_row.hbs"; import render_presence_rows from "../templates/presence_rows.hbs"; import * as blueslip from "./blueslip"; import * as buddy_data from "./buddy_data"; +import {media_breakpoints_num} from "./css_variables"; +import * as hash_util from "./hash_util"; +import {$t} from "./i18n"; import * as message_viewport from "./message_viewport"; +import * as narrow_state from "./narrow_state"; import * as padded_widget from "./padded_widget"; +import * as peer_data from "./peer_data"; +import * as people from "./people"; import * as scroll_util from "./scroll_util"; +import * as stream_data from "./stream_data"; +import {INTERACTIVE_HOVER_DELAY} from "./tippyjs"; +import {user_settings} from "./user_settings"; + +function get_formatted_sub_count(sub_count) { + if (sub_count < 1000) { + return sub_count; + } + return new Intl.NumberFormat(user_settings.default_language, {notation: "compact"}).format( + sub_count, + ); +} class BuddyListConf { - container_selector = "#buddy-list-users-matching-view"; + matching_view_list_selector = "#buddy-list-users-matching-view"; + other_user_list_selector = "#buddy-list-other-users"; scroll_container_selector = "#buddy_list_wrapper"; item_selector = "li.user_sidebar_entry"; padding_selector = "#buddy_list_wrapper_padding"; @@ -27,8 +51,10 @@ class BuddyListConf { get_li_from_user_id(opts) { const user_id = opts.user_id; - const $container = $(this.container_selector); - return $container.find(`${this.item_selector}[data-user-id='${CSS.escape(user_id)}']`); + const $buddy_list_container = $("#buddy_list_wrapper"); + return $buddy_list_container.find( + `${this.item_selector}[data-user-id='${CSS.escape(user_id)}']`, + ); } get_user_id_from_li(opts) { @@ -55,16 +81,260 @@ class BuddyListConf { export class BuddyList extends BuddyListConf { all_user_ids = []; + users_matching_view_ids = []; + other_user_ids = []; + users_matching_view_is_collapsed = false; + other_users_is_collapsed = false; + + initialize_tooltips() { + $("#right-sidebar").on("mouseenter", ".buddy-list-heading", (e) => { + e.stopPropagation(); + const $elem = $(e.currentTarget); + let placement = "left"; + if (window.innerWidth < media_breakpoints_num.md) { + // On small devices display tooltips based on available space. + // This will default to "bottom" placement for this tooltip. + placement = "auto"; + } + const get_total_subscriber_count = this.total_subscriber_count; + tippy($elem[0], { + // Because the buddy list subheadings are potential click targets + // for purposes having nothing to do with the subscriber count + // (collapsing/expanding), we delay showing the tooltip until the + // user lingers a bit longer. + delay: INTERACTIVE_HOVER_DELAY, + // Don't show tooltip on touch devices (99% mobile) since touch + // pressing on users in the left or right sidebar leads to narrow + // being changed and the sidebar is hidden. So, there is no user + // displayed to show tooltip for. It is safe to show the tooltip + // on long press but it not worth the inconvenience of having a + // tooltip hanging around on a small mobile screen if anything + // going wrong. + touch: false, + arrow: true, + placement, + showOnCreate: true, + onShow(instance) { + let tooltip_text; + const current_sub = narrow_state.stream_sub(); + const pm_ids_set = narrow_state.pm_ids_set(); + const subscriber_count = get_total_subscriber_count(current_sub, pm_ids_set); + const elem_id = $elem.attr("id"); + if (elem_id === "buddy-list-users-matching-view-section-heading") { + if (current_sub) { + tooltip_text = $t( + { + defaultMessage: + "{N, plural, one {# subscriber} other {# subscribers}}", + }, + {N: subscriber_count}, + ); + } else { + tooltip_text = $t( + { + defaultMessage: + "{N, plural, one {# participant} other {# participants}}", + }, + {N: subscriber_count}, + ); + } + } else { + const total_user_count = people.get_active_human_count(); + const other_users_count = total_user_count - subscriber_count; + tooltip_text = $t( + { + defaultMessage: + "{N, plural, one {# other user} other {# other users}}", + }, + {N: other_users_count}, + ); + } + instance.setContent(tooltip_text); + }, + onHidden(instance) { + instance.destroy(); + }, + appendTo: () => document.body, + }); + }); + } populate(opts) { this.render_count = 0; - this.$container.empty(); + this.$users_matching_view_container.empty(); + this.users_matching_view_ids = []; + this.$other_users_container.empty(); + this.other_user_ids = []; // We rely on our caller to give us items // in already-sorted order. this.all_user_ids = opts.all_user_ids; this.fill_screen_with_content(); + + // We do a handful of things once we're done rendering all the users, + // and each of these tasks need shared data that we'll compute first. + const current_sub = narrow_state.stream_sub(); + const pm_ids_set = narrow_state.pm_ids_set(); + + // If we have only "other users" and aren't in a stream/DM view + // then we don't show section headers and only show one untitled + // section. + const hide_headers = this.should_hide_headers(current_sub, pm_ids_set); + const subscriber_count = this.total_subscriber_count(current_sub, pm_ids_set); + const total_user_count = people.get_active_human_count(); + const other_users_count = total_user_count - subscriber_count; + const has_inactive_users_matching_view = + subscriber_count > this.users_matching_view_ids.length; + const has_inactive_other_users = other_users_count > this.other_user_ids.length; + + const data = { + current_sub, + hide_headers, + subscriber_count, + other_users_count, + has_inactive_users_matching_view, + has_inactive_other_users, + }; + + $("#buddy-list-users-matching-view-container .view-all-subscribers-link").remove(); + $("#buddy-list-other-users-container .view-all-users-link").remove(); + if (!buddy_data.get_is_searching_users()) { + this.render_view_user_list_links(data); + } + this.render_section_headers(data); + if (!hide_headers) { + this.update_empty_list_placeholders(data); + } + } + + update_empty_list_placeholders({has_inactive_users_matching_view, has_inactive_other_users}) { + let matching_view_empty_list_message; + let other_users_empty_list_message; + + if (buddy_data.get_is_searching_users()) { + matching_view_empty_list_message = $t({defaultMessage: "No matching users."}); + other_users_empty_list_message = $t({defaultMessage: "No matching users."}); + } else { + if (has_inactive_users_matching_view) { + matching_view_empty_list_message = $t({defaultMessage: "No active users."}); + } else { + matching_view_empty_list_message = $t({defaultMessage: "None."}); + } + + if (has_inactive_other_users) { + other_users_empty_list_message = $t({defaultMessage: "No active users."}); + } else { + other_users_empty_list_message = $t({defaultMessage: "None."}); + } + } + + $("#buddy-list-users-matching-view").data( + "search-results-empty", + matching_view_empty_list_message, + ); + if ($("#buddy-list-users-matching-view .empty-list-message").length) { + const empty_list_widget = render_empty_list_widget_for_list({ + matching_view_empty_list_message, + }); + $("#buddy-list-users-matching-view").empty(); + $("#buddy-list-users-matching-view").append(empty_list_widget); + } + + $("#buddy-list-other-users").data("search-results-empty", other_users_empty_list_message); + if ($("#buddy-list-other-users .empty-list-message").length) { + const empty_list_widget = render_empty_list_widget_for_list({ + other_users_empty_list_message, + }); + $("#buddy-list-other-users").empty(); + $("#buddy-list-other-users").append(empty_list_widget); + } + } + + render_section_headers({current_sub, hide_headers, subscriber_count, other_users_count}) { + $("#buddy-list-users-matching-view-container .buddy-list-subsection-header").empty(); + $("#buddy-list-other-users-container .buddy-list-subsection-header").empty(); + + $("#buddy-list-users-matching-view-container").toggleClass("no-display", hide_headers); + + // 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 total_user_count = people.get_active_human_count(); + const formatted_count = get_formatted_sub_count(total_user_count); + 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) { + header_text = $t({defaultMessage: "In this stream"}); + } else { + header_text = $t({defaultMessage: "In this conversation"}); + } + + $("#buddy-list-users-matching-view-container .buddy-list-subsection-header").append( + render_section_header({ + id: "buddy-list-users-matching-view-section-heading", + header_text, + user_count: get_formatted_sub_count(subscriber_count), + toggle_class: "toggle-users-matching-view", + is_collapsed: this.users_matching_view_is_collapsed, + }), + ); + + $("#buddy-list-other-users-container .buddy-list-subsection-header").append( + render_section_header({ + id: "buddy-list-other-users-section-heading", + header_text: $t({defaultMessage: "Others"}), + user_count: get_formatted_sub_count(other_users_count), + toggle_class: "toggle-other-users", + is_collapsed: this.other_users_is_collapsed, + }), + ); + } + + toggle_users_matching_view_section() { + this.users_matching_view_is_collapsed = !this.users_matching_view_is_collapsed; + $("#buddy-list-users-matching-view-container").toggleClass( + "collapsed", + this.users_matching_view_is_collapsed, + ); + $("#buddy-list-users-matching-view-container .toggle-users-matching-view").toggleClass( + "fa-caret-down", + !this.users_matching_view_is_collapsed, + ); + $("#buddy-list-users-matching-view-container .toggle-users-matching-view").toggleClass( + "fa-caret-right", + this.users_matching_view_is_collapsed, + ); + + // Collapsing and uncollapsing sections has a similar effect to + // scrolling, so we make sure to fill screen with content here as well. + this.fill_screen_with_content(); + } + + toggle_other_users_section() { + this.other_users_is_collapsed = !this.other_users_is_collapsed; + $("#buddy-list-other-users-container").toggleClass( + "collapsed", + this.other_users_is_collapsed, + ); + $("#buddy-list-other-users-container .toggle-other-users").toggleClass( + "fa-caret-down", + !this.other_users_is_collapsed, + ); + $("#buddy-list-other-users-container .toggle-other-users").toggleClass( + "fa-caret-right", + this.other_users_is_collapsed, + ); + + // Collapsing and uncollapsing sections has a similar effect to + // scrolling, so we make sure to fill screen with content here as well. + this.fill_screen_with_content(); } render_more(opts) { @@ -80,12 +350,57 @@ export class BuddyList extends BuddyListConf { } const items = this.get_data_from_user_ids(more_user_ids); + const subscribed_users = []; + const other_users = []; + const current_sub = narrow_state.stream_sub(); + const pm_ids_set = narrow_state.pm_ids_set(); - const html = this.items_to_html({ - items, + for (const item of items) { + if (buddy_data.user_matches_narrow(item.user_id, pm_ids_set, current_sub?.stream_id)) { + subscribed_users.push(item); + this.users_matching_view_ids.push(item.user_id); + } else { + other_users.push(item); + this.other_user_ids.push(item.user_id); + } + } + + // Remove the empty list message before adding users + if ( + $(`${this.matching_view_list_selector} .empty-list-message`).length > 0 && + subscribed_users.length + ) { + this.$users_matching_view_container.empty(); + } + const subscribed_users_html = this.items_to_html({ + items: subscribed_users, + }); + this.$users_matching_view_container = $(this.matching_view_list_selector); + this.$users_matching_view_container.append(subscribed_users_html); + + // Remove the empty list message before adding users + if ( + $(`${this.other_user_list_selector} .empty-list-message`).length > 0 && + other_users.length + ) { + this.$other_users_container.empty(); + } + const other_users_html = this.items_to_html({ + items: other_users, + }); + this.$other_users_container = $(this.other_user_list_selector); + this.$other_users_container.append(other_users_html); + + const hide_headers = this.should_hide_headers(current_sub, pm_ids_set); + const subscriber_count = this.total_subscriber_count(current_sub, pm_ids_set); + const total_user_count = people.get_active_human_count(); + const other_users_count = total_user_count - subscriber_count; + this.render_section_headers({ + current_sub, + hide_headers, + subscriber_count, + other_users_count, }); - this.$container = $(this.container_selector); - this.$container.append(html); // Invariant: more_user_ids.length >= items.length. // (Usually they're the same, but occasionally user ids @@ -98,44 +413,162 @@ export class BuddyList extends BuddyListConf { } get_items() { - const $obj = this.$container.find(`${this.item_selector}`); - return $obj.map((_i, elem) => $(elem)); + const $user_matching_view_obj = this.$users_matching_view_container.find( + `${this.item_selector}`, + ); + const $users_matching_view_elems = $user_matching_view_obj.map((_i, elem) => $(elem)); + + const $other_user_obj = this.$other_users_container.find(`${this.item_selector}`); + const $other_user_elems = $other_user_obj.map((_i, elem) => $(elem)); + + return [...$users_matching_view_elems, ...$other_user_elems]; + } + + should_hide_headers(current_sub, pm_ids_set) { + // If we have only "other users" and aren't in a stream/DM view + // then we don't show section headers and only show one untitled + // section. + return this.users_matching_view_ids.length === 0 && !current_sub && !pm_ids_set.size; + } + + total_subscriber_count(current_sub, pm_ids_set) { + // Includes inactive users who might not show up in the buddy list. + if (current_sub) { + return peer_data.get_subscriber_count(current_sub.stream_id, false); + } else if (pm_ids_set.size) { + const pm_ids_list = [...pm_ids_set]; + // Plus one for the "me" user, who isn't in the recipients list (except + // for when it's a private message conversation with only "me" in it). + if (pm_ids_list.length === 1 && people.is_my_user_id(pm_ids_list[0])) { + return 1; + } + return pm_ids_list.length + 1; + } + return 0; + } + + render_view_user_list_links({ + current_sub, + has_inactive_users_matching_view, + has_inactive_other_users, + }) { + // For stream views, we show a link at the bottom of the list of subscribed users that + // lets a user find the full list of subscribed users and information about them. + if ( + current_sub && + stream_data.can_view_subscribers(current_sub) && + has_inactive_users_matching_view + ) { + const stream_edit_hash = hash_util.stream_edit_url(current_sub, "subscribers"); + $("#buddy-list-users-matching-view-container").append( + render_view_all_subscribers({ + stream_edit_hash, + }), + ); + } + + // We give a link to view the list of all users to help reduce confusion about + // there being hidden (inactive) "other" users. + if (has_inactive_other_users) { + $("#buddy-list-other-users-container").append(render_view_all_users()); + } } // From `type List`, where the key is a user_id. first_key() { - return this.all_user_ids[0]; + if (this.users_matching_view_ids.length) { + return this.users_matching_view_ids[0]; + } + if (this.other_user_ids.length) { + return this.other_user_ids[0]; + } + return undefined; } // From `type List`, where the key is a user_id. prev_key(key) { - const i = this.all_user_ids.indexOf(key); - - if (i <= 0) { + let i = this.users_matching_view_ids.indexOf(key); + // This would be the middle of the list of users matching view, + // moving to a prev user matching the view. + if (i > 0) { + return this.users_matching_view_ids[i - 1]; + } + // If it's the first user matching the view, we don't move the selection. + if (i === 0) { return undefined; } - return this.all_user_ids[i - 1]; + // This would be the middle of the other users list moving to a prev other user. + i = this.other_user_ids.indexOf(key); + if (i > 0) { + return this.other_user_ids[i - 1]; + } + // The key before the first other user is the last user matching view, if that exists, + // and if it doesn't then we don't move the selection. + if (i === 0) { + if (this.users_matching_view_ids.length > 0) { + return this.users_matching_view_ids.at(-1); + } + return undefined; + } + // The only way we reach here is if the key isn't found in either list, + // which shouldn't happen. + blueslip.error("Couldn't find key in buddy list", { + key, + users_matching_view_ids: this.users_matching_view_ids, + other_user_ids: this.other_user_ids, + }); + return undefined; } // From `type List`, where the key is a user_id. next_key(key) { - const i = this.all_user_ids.indexOf(key); - - if (i < 0) { + let i = this.users_matching_view_ids.indexOf(key); + // Moving from users matching the view to the list of other users, + // if they exist, otherwise do nothing. + if (i >= 0 && i === this.users_matching_view_ids.length - 1) { + if (this.other_user_ids.length > 0) { + return this.other_user_ids[0]; + } return undefined; } + // This is a regular move within the list of users matching the view. + if (i >= 0) { + return this.users_matching_view_ids[i + 1]; + } - return this.all_user_ids[i + 1]; + i = this.other_user_ids.indexOf(key); + // If we're at the end of other users, we don't do anything. + if (i >= 0 && i === this.other_user_ids.length - 1) { + return undefined; + } + // This is a regular move within other users. + if (i >= 0) { + return this.other_user_ids[i + 1]; + } + + // The only way we reach here is if the key isn't found in either list, + // which shouldn't happen. + blueslip.error("Couldn't find key in buddy list", { + key, + users_matching_view_ids: this.users_matching_view_ids, + other_user_ids: this.other_user_ids, + }); + return undefined; } maybe_remove_user_id(opts) { - const pos = this.all_user_ids.indexOf(opts.user_id); - - if (pos < 0) { - return; + let pos = this.users_matching_view_ids.indexOf(opts.user_id); + if (pos >= 0) { + this.users_matching_view_ids.splice(pos, 1); + } else { + pos = this.other_user_ids.indexOf(opts.user_id); + if (pos < 0) { + return; + } + this.other_user_ids.splice(pos, 1); } - + pos = this.all_user_ids.indexOf(opts.user_id); this.all_user_ids.splice(pos, 1); if (pos < this.render_count) { @@ -150,15 +583,20 @@ export class BuddyList extends BuddyListConf { const user_id = opts.user_id; let i; - for (i = 0; i < this.all_user_ids.length; i += 1) { - const list_user_id = this.all_user_ids[i]; + const user_id_list = opts.user_id_list; - if (this.compare_function(user_id, list_user_id) < 0) { + const current_sub = narrow_state.stream_sub(); + const pm_ids_set = narrow_state.pm_ids_set(); + + for (i = 0; i < user_id_list.length; i += 1) { + const list_user_id = user_id_list[i]; + + if (this.compare_function(user_id, list_user_id, current_sub, pm_ids_set) < 0) { return i; } } - return this.all_user_ids.length; + return user_id_list.length; } force_render(opts) { @@ -199,6 +637,8 @@ export class BuddyList extends BuddyListConf { return $li; } + // We reference all_user_ids to see if we've rendered + // it yet. const pos = this.all_user_ids.indexOf(user_id); if (pos < 0) { @@ -221,12 +661,18 @@ export class BuddyList extends BuddyListConf { insert_new_html(opts) { const user_id_following_insertion = opts.new_user_id; const html = opts.html; - const new_pos_in_all_users = opts.pos; + const new_pos_in_all_users = opts.new_pos_in_all_users; + const is_subscribed_user = opts.is_subscribed_user; + // This means we're inserting at the end if (user_id_following_insertion === undefined) { if (new_pos_in_all_users === this.render_count) { this.render_count += 1; - this.$container.append(html); + if (is_subscribed_user) { + this.$users_matching_view_container.append(html); + } else { + this.$other_users_container.append(html); + } this.update_padding(); } return; @@ -246,22 +692,40 @@ export class BuddyList extends BuddyListConf { this.maybe_remove_user_id({user_id}); - const pos = this.find_position({ + const new_pos_in_all_users = this.find_position({ user_id, + user_id_list: this.all_user_ids, + }); + + const current_sub = narrow_state.stream_sub(); + const pm_ids_set = narrow_state.pm_ids_set(); + const is_subscribed_user = buddy_data.user_matches_narrow( + user_id, + pm_ids_set, + current_sub?.stream_id, + ); + const user_id_list = is_subscribed_user + ? this.users_matching_view_ids + : this.other_user_ids; + const new_pos_in_user_list = this.find_position({ + user_id, + user_id_list, }); // Order is important here--get the new_user_id // before mutating our list. An undefined value // corresponds to appending. - const new_user_id = this.all_user_ids[pos]; + const new_user_id = user_id_list[new_pos_in_user_list]; - this.all_user_ids.splice(pos, 0, user_id); + user_id_list.splice(new_pos_in_user_list, 0, user_id); + this.all_user_ids.splice(new_pos_in_all_users, 0, user_id); const html = this.item_to_html({item}); this.insert_new_html({ - pos, + new_pos_in_all_users, html, new_user_id, + is_subscribed_user, }); } @@ -293,7 +757,8 @@ export class BuddyList extends BuddyListConf { // This is a bit of a hack to make sure we at least have // an empty list to start, before we get the initial payload. - $container = $(this.container_selector); + $users_matching_view_container = $(this.matching_view_list_selector); + $other_users_container = $(this.other_user_list_selector); start_scroll_handler() { // We have our caller explicitly call this to make @@ -309,7 +774,7 @@ export class BuddyList extends BuddyListConf { padded_widget.update_padding({ shown_rows: this.render_count, total_rows: this.all_user_ids.length, - content_selector: this.container_selector, + content_selector: "#buddy_list_wrapper", padding_selector: this.padding_selector, }); } diff --git a/web/src/click_handlers.js b/web/src/click_handlers.js index b4201c0b5b..36acd73ab2 100644 --- a/web/src/click_handlers.js +++ b/web/src/click_handlers.js @@ -474,18 +474,16 @@ export function initialize() { }); // SIDEBARS - $("#buddy-list-users-matching-view") - .expectOne() - .on("click", ".selectable_sidebar_block", (e) => { - const $li = $(e.target).parents("li"); + $(".buddy-list-section").on("click", ".selectable_sidebar_block", (e) => { + const $li = $(e.target).parents("li"); - activity_ui.narrow_for_user({$li}); + activity_ui.narrow_for_user({$li}); - e.preventDefault(); - e.stopPropagation(); - sidebar_ui.hide_userlist_sidebar(); - $(".tooltip").remove(); - }); + e.preventDefault(); + e.stopPropagation(); + sidebar_ui.hide_userlist_sidebar(); + $(".tooltip").remove(); + }); // Doesn't show tooltip on touch devices. function do_render_buddy_list_tooltip( @@ -549,7 +547,7 @@ export function initialize() { } // BUDDY LIST TOOLTIPS (not displayed on touch devices) - $("#buddy-list-users-matching-view").on("mouseenter", ".selectable_sidebar_block", (e) => { + $(".buddy-list-section").on("mouseenter", ".selectable_sidebar_block", (e) => { e.stopPropagation(); const $elem = $(e.currentTarget).closest(".user_sidebar_entry").find(".user-presence-link"); const user_id_string = $elem.attr("data-user-id"); @@ -557,7 +555,7 @@ export function initialize() { // `target_node` is the `ul` element since it stays in DOM even after updates. function get_target_node() { - return document.querySelector("#buddy-list-users-matching-view"); + return $(e.target).parents(".buddy-list-section")[0]; } function check_reference_removed(mutation, instance) { diff --git a/web/src/list_util.ts b/web/src/list_util.ts index e64f16379f..f2d1289c9f 100644 --- a/web/src/list_util.ts +++ b/web/src/list_util.ts @@ -4,6 +4,7 @@ const list_selectors = [ "#stream_filters", "#left-sidebar-navigation-list", "#buddy-list-users-matching-view", + "#buddy-list-other-users", "#send_later_options", ]; diff --git a/web/src/narrow.js b/web/src/narrow.js index 05afdec2ad..03a3b5fdad 100644 --- a/web/src/narrow.js +++ b/web/src/narrow.js @@ -2,6 +2,7 @@ import * as Sentry from "@sentry/browser"; import $ from "jquery"; import assert from "minimalistic-assert"; +import * as activity_ui from "./activity_ui"; import {all_messages_data} from "./all_messages_data"; import * as blueslip from "./blueslip"; import * as browser_history from "./browser_history"; @@ -1031,6 +1032,7 @@ function handle_post_view_change(msg_list) { left_sidebar_navigation_area.handle_narrow_activated(filter); stream_list.handle_narrow_activated(filter); pm_list.handle_narrow_activated(filter); + activity_ui.build_user_sidebar(); } function handle_post_narrow_deactivate_processes(msg_list) { diff --git a/web/src/sidebar_ui.ts b/web/src/sidebar_ui.ts index b71e47a954..9b6141788f 100644 --- a/web/src/sidebar_ui.ts +++ b/web/src/sidebar_ui.ts @@ -3,6 +3,7 @@ import $ from "jquery"; import render_left_sidebar from "../templates/left_sidebar.hbs"; import render_right_sidebar from "../templates/right_sidebar.hbs"; +import {buddy_list} from "./buddy_list"; import {page_params} from "./page_params"; import * as rendered_markdown from "./rendered_markdown"; import * as resize from "./resize"; @@ -161,6 +162,9 @@ export function initialize_right_sidebar(): void { }); $("#right-sidebar-container").html(rendered_sidebar); + + buddy_list.initialize_tooltips(); + update_invite_user_option(); if (page_params.is_spectator) { rendered_markdown.update_elements( @@ -187,4 +191,18 @@ export function initialize_right_sidebar(): void { } } }); + + $("#buddy-list-users-matching-view-container").on( + "click", + ".buddy-list-subsection-header", + (e) => { + e.stopPropagation(); + buddy_list.toggle_users_matching_view_section(); + }, + ); + + $("#buddy-list-other-users-container").on("click", ".buddy-list-subsection-header", (e) => { + e.stopPropagation(); + buddy_list.toggle_other_users_section(); + }); } diff --git a/web/src/tippyjs.ts b/web/src/tippyjs.ts index c666bba5c6..d960f75e61 100644 --- a/web/src/tippyjs.ts +++ b/web/src/tippyjs.ts @@ -2,10 +2,13 @@ import $ from "jquery"; import assert from "minimalistic-assert"; import tippy, {delegate} from "tippy.js"; +import render_buddy_list_title_tooltip from "../templates/buddy_list/title_tooltip.hbs"; import render_tooltip_templates from "../templates/tooltip_templates.hbs"; import {$t} from "./i18n"; +import * as people from "./people"; import * as popovers from "./popovers"; +import * as ui_util from "./ui_util"; import {user_settings} from "./user_settings"; // For tooltips without data-tippy-content, we use the HTML content of @@ -240,7 +243,6 @@ export function initialize(): void { delegate("body", { target: [ "#streams_header .streams-tooltip-target", - "#userlist-title", "#user_filter_icon", "#scroll-to-bottom-button-clickable-area", ".spectator_narrow_login_button", @@ -561,4 +563,16 @@ export function initialize(): void { }, appendTo: () => document.body, }); + + delegate("body", { + target: "#userlist-header", + placement: "top", + appendTo: () => document.body, + onShow(instance) { + const total_user_count = people.get_active_human_count(); + instance.setContent( + ui_util.parse_html(render_buddy_list_title_tooltip({total_user_count})), + ); + }, + }); } diff --git a/web/src/ui_init.js b/web/src/ui_init.js index 4bf7b35632..02fc4f889e 100644 --- a/web/src/ui_init.js +++ b/web/src/ui_init.js @@ -747,6 +747,7 @@ export function initialize_everything(state_data) { sidebar_ui.hide_all(); popovers.hide_all(); narrow.by("stream", sub.name, {trigger}); + activity_ui.build_user_sidebar(); }, }); stream_list_sort.initialize(); @@ -792,7 +793,6 @@ export function initialize_everything(state_data) { search.initialize({ on_narrow_search: narrow.activate, }); - tutorial.initialize(); desktop_notifications.initialize(); audible_notifications.initialize(); compose_notifications.initialize({ @@ -814,9 +814,6 @@ export function initialize_everything(state_data) { settings_toggle.initialize(); about_zulip.initialize(); - // All overlays must be initialized before hashchange.js - hashchange.initialize(); - initialize_unread_ui(); activity.initialize(); activity_ui.initialize({ @@ -824,6 +821,13 @@ export function initialize_everything(state_data) { narrow.by("dm", email, {trigger: "sidebar"}); }, }); + // This needs to happen after activity_ui.initialize, so that user_filter + // is defined. + tutorial.initialize(); + + // All overlays, and also activity_ui, must be initialized before hashchange.js + hashchange.initialize(); + emoji_picker.initialize(); user_group_popover.initialize(); user_card_popover.initialize(); diff --git a/web/src/user_card_popover.js b/web/src/user_card_popover.js index 2844f1b98d..052e444a72 100644 --- a/web/src/user_card_popover.js +++ b/web/src/user_card_popover.js @@ -810,13 +810,13 @@ function register_click_handlers() { $("body").on("click", ".update_status_text", open_user_status_modal); // Clicking on one's own status emoji should open the user status modal. - $("#buddy-list-users-matching-view").on( + $(".buddy-list-section").on( "click", ".user_sidebar_entry_me .status-emoji", open_user_status_modal, ); - $("#buddy-list-users-matching-view").on("click", ".user-list-sidebar-menu-icon", (e) => { + $(".buddy-list-section").on("click", ".user-list-sidebar-menu-icon", (e) => { e.stopPropagation(); const $target = $(e.currentTarget).closest("li"); diff --git a/web/src/views_util.js b/web/src/views_util.js index d317cba798..7857c70414 100644 --- a/web/src/views_util.js +++ b/web/src/views_util.js @@ -1,5 +1,6 @@ import $ from "jquery"; +import * as activity_ui from "./activity_ui"; import * as compose_actions from "./compose_actions"; import * as compose_recipient from "./compose_recipient"; import * as dropdown_widget from "./dropdown_widget"; @@ -88,6 +89,10 @@ export function show(opts) { opts.complete_rerender(); compose_actions.on_show_navigation_view(); + // This has to happen after resetting the current narrow filter, so + // that the buddy list is rendered with the correct narrow state. + activity_ui.build_user_sidebar(); + // Misc. if (opts.is_recent_view) { resize.update_recent_view_filters_height(); diff --git a/web/styles/app_variables.css b/web/styles/app_variables.css index f673283c35..6a6ca2783e 100644 --- a/web/styles/app_variables.css +++ b/web/styles/app_variables.css @@ -197,6 +197,7 @@ --color-background-tab-picker-tab-option-hover: hsl(0deg 0% 100% / 60%); --color-background-popover: hsl(0deg 0% 100%); --color-background-alert-word: hsl(18deg 100% 84%); + --color-buddy-list-highlighted-user: hsl(120deg 12.3% 71.4% / 38%); /* Compose box colors */ --color-compose-send-button-icon-color: hsl(0deg 0% 100%); @@ -247,6 +248,8 @@ --color-text-personal-menu-some-status: hsl(0deg 0% 40%); --color-text-sidebar-heading: hsl(0deg 0% 43%); --color-text-sidebar-popover-menu: hsl(0deg 0% 20%); + --color-text-url: hsl(200deg 100% 40%); + --color-text-url-hover: hsl(200deg 100% 25%); /* Markdown code colors */ --color-markdown-code-text: hsl(0deg 0% 0%); @@ -484,6 +487,7 @@ --color-outline-tab-picker-tab-option: hsl(0deg 0% 100% / 12%); --color-background-tab-picker-tab-option-hover: hsl(0deg 0% 100% / 5%); --color-background-alert-word: hsl(22deg 70% 35%); + --color-buddy-list-highlighted-user: hsl(136deg 25% 73% / 20%); /* Compose box colors */ --color-compose-send-button-focus-shadow: hsl(0deg 0% 100% / 80%); @@ -532,6 +536,7 @@ --color-text-search: hsl(0deg 0% 100% / 75%); --color-text-search-hover: hsl(0deg 0% 100%); --color-text-search-placeholder: hsl(0deg 0% 100% / 50%); + --color-text-empty-list-message: hsl(0deg 0% 67%); --color-text-dropdown-menu: hsl(0deg 0% 100% / 80%); --color-text-full-name: hsl(0deg 0% 100%); --color-text-item: hsl(0deg 0% 50%); @@ -547,6 +552,7 @@ hsl(0deg 0% 75%) 75%, hsl(0deg 0% 11%) ); + --color-text-url-hover: hsl(200deg 79% 66%); /* Markdown code colors */ /* Note that Markdown code-link colors are identical diff --git a/web/styles/dark_theme.css b/web/styles/dark_theme.css index 1f5a7c9005..3a92645f71 100644 --- a/web/styles/dark_theme.css +++ b/web/styles/dark_theme.css @@ -687,11 +687,6 @@ } } - #buddy-list-users-matching-view li:hover, - #buddy-list-users-matching-view li.highlighted_user { - background-color: hsl(136deg 25% 73% / 20%); - } - .group-row.active, .stream-row.active { background-color: hsl(0deg 0% 0% / 20%); diff --git a/web/styles/right_sidebar.css b/web/styles/right_sidebar.css index 8ed92a9f6f..1b99d75451 100644 --- a/web/styles/right_sidebar.css +++ b/web/styles/right_sidebar.css @@ -20,8 +20,44 @@ $user_status_emoji_width: 24px; overflow: auto; } -#buddy-list-users-matching-view { - max-width: 95%; +.toggle-narrow-users, +.toggle-other-users { + width: 7px; +} + +#buddy-list-users-matching-view-container, +#buddy-list-other-users-container { + margin-bottom: 10px; + + &.no-display { + display: none; + } + + .view-all-subscribers-link, + .view-all-users-link { + margin-left: 15px; + } +} + +#buddy-list-users-matching-view-container.collapsed { + #buddy-list-users-matching-view, + .view-all-subscribers-link { + display: none; + } +} + +#buddy-list-other-users-container.collapsed { + #buddy-list-other-users, + .view-all-users-link { + display: none; + } +} + +#buddy-list-users-matching-view, +#buddy-list-other-users { + width: 90%; + margin-left: 10px; + margin-bottom: 0; overflow-x: hidden; list-style-position: inside; /* Draw the bullets inside our box */ @@ -31,7 +67,6 @@ $user_status_emoji_width: 24px; text-overflow: ellipsis; list-style-type: none; border-radius: 4px; - padding-right: 15px; padding-top: 1px; padding-bottom: 2px; @@ -81,7 +116,7 @@ $user_status_emoji_width: 24px; &:hover, &.highlighted_user { - background-color: hsl(120deg 12.3% 71.4% / 38%); + background-color: var(--color-buddy-list-highlighted-user); } } @@ -93,19 +128,60 @@ $user_status_emoji_width: 24px; } .empty-list-message { - font-size: 1.2em; + font-style: italic; + color: var(--color-text-empty-list-message); + /* Overwrite default empty list font size, to look better under the subheaders. */ + font-size: 14px; + /* Override .empty-list-message !important styling */ + padding: 0 !important; + margin-left: 5px; + text-align: left; &:hover { background-color: inherit; } } + /* Overwrite some stray list rules (including one in left_sidebar.css) to turn color + back to the bootstrap default. */ + .view-all-subscribers-link, + .view-all-users-link { + color: var(--color-text-url); + + &:hover { + color: var(--color-text-url-hover); + } + } + & a { color: inherit; margin-left: 0; } } +.buddy-list-subsection-header { + display: flex; + align-items: center; + cursor: pointer; + background-color: var(--color-background); + line-height: 1; + position: sticky; + top: 0; + z-index: 1; + color: var(--color-text-default); +} + +.buddy-list-heading { + user-select: none; + font-weight: 600; + margin: 0; + padding: 5px; +} + +.buddy-list-subsection-header.no-display { + display: none; +} + .user-presence-link, .user_sidebar_entry .selectable_sidebar_block { display: flex; diff --git a/web/templates/buddy_list/section_header.hbs b/web/templates/buddy_list/section_header.hbs new file mode 100644 index 0000000000..f51dcad8fe --- /dev/null +++ b/web/templates/buddy_list/section_header.hbs @@ -0,0 +1,4 @@ +
+ {{header_text}} ({{user_count}}) +
+ diff --git a/web/templates/buddy_list/title_tooltip.hbs b/web/templates/buddy_list/title_tooltip.hbs new file mode 100644 index 0000000000..2026bb335f --- /dev/null +++ b/web/templates/buddy_list/title_tooltip.hbs @@ -0,0 +1,4 @@ +{{#tr}} +Search {total_user_count, plural, =1 {1 person} other {# people}} +{{/tr}} +{{tooltip_hotkey_hints "W"}} diff --git a/web/templates/buddy_list/view_all_subscribers.hbs b/web/templates/buddy_list/view_all_subscribers.hbs new file mode 100644 index 0000000000..119ba95035 --- /dev/null +++ b/web/templates/buddy_list/view_all_subscribers.hbs @@ -0,0 +1 @@ +View all subscribers diff --git a/web/templates/buddy_list/view_all_users.hbs b/web/templates/buddy_list/view_all_users.hbs new file mode 100644 index 0000000000..41b2b58346 --- /dev/null +++ b/web/templates/buddy_list/view_all_users.hbs @@ -0,0 +1 @@ +View all users diff --git a/web/templates/right_sidebar.hbs b/web/templates/right_sidebar.hbs index dc8fc5911c..d6cc493047 100644 --- a/web/templates/right_sidebar.hbs +++ b/web/templates/right_sidebar.hbs @@ -2,8 +2,7 @@
-
-
    +
    +
    +
      +
      +
      +
      +
        +
        diff --git a/web/tests/activity.test.js b/web/tests/activity.test.js index 13a1bd5ab0..ad0954b0cb 100644 --- a/web/tests/activity.test.js +++ b/web/tests/activity.test.js @@ -2,6 +2,13 @@ const {strict: assert} = require("assert"); +const { + clear_buddy_list, + override_user_matches_narrow, + buddy_list_add_user_matching_view, + buddy_list_add_other_user, + stub_buddy_list_elements, +} = require("./lib/buddy_list"); const {mock_esm, set_global, with_overrides, zrequire} = require("./lib/namespace"); const {run_test, noop} = require("./lib/test"); const blueslip = require("./lib/zblueslip"); @@ -19,7 +26,6 @@ const _document = { }; const channel = mock_esm("../src/channel"); -const compose_state = mock_esm("../src/compose_state"); const padded_widget = mock_esm("../src/padded_widget"); const pm_list = mock_esm("../src/pm_list"); const popovers = mock_esm("../src/popovers"); @@ -32,7 +38,6 @@ const watchdog = mock_esm("../src/watchdog"); set_global("document", _document); const huddle_data = zrequire("huddle_data"); -const compose_fade = zrequire("compose_fade"); const keydown_util = zrequire("keydown_util"); const muted_users = zrequire("muted_users"); const presence = zrequire("presence"); @@ -41,7 +46,11 @@ const buddy_data = zrequire("buddy_data"); const {buddy_list} = zrequire("buddy_list"); const activity = zrequire("activity"); const activity_ui = zrequire("activity_ui"); +const stream_data = zrequire("stream_data"); +const narrow_state = zrequire("narrow_state"); +const peer_data = zrequire("peer_data"); const util = zrequire("util"); +const {Filter} = zrequire("../src/filter"); const me = { email: "me@zulip.com", @@ -90,10 +99,13 @@ people.add_active_user(zoe); people.add_active_user(me); people.initialize_current_user(me.user_id); -function clear_buddy_list() { - buddy_list.populate({ - all_user_ids: [], - }); +const $alice_stub = $.create("alice stub"); +const $fred_stub = $.create("fred stub"); + +const rome_sub = {name: "Rome", subscribed: true, stream_id: 1001}; +function add_sub_and_set_as_current_narrow(sub) { + stream_data.add_sub(sub); + narrow_state.set_current_filter(new Filter([{operator: "stream", operand: sub.name}])); } function test(label, f) { @@ -109,6 +121,10 @@ function test(label, f) { }); }); + stub_buddy_list_elements(); + helpers.override(buddy_list, "render_section_headers", noop); + helpers.override(buddy_list, "render_view_user_list_links", noop); + presence.presence_info.set(alice.user_id, {status: "active"}); presence.presence_info.set(fred.user_id, {status: "active"}); presence.presence_info.set(jill.user_id, {status: "active"}); @@ -117,7 +133,7 @@ function test(label, f) { presence.presence_info.set(zoe.user_id, {status: "active"}); presence.presence_info.set(me.user_id, {status: "active"}); - clear_buddy_list(); + clear_buddy_list(buddy_list); muted_users.set_muted_users([]); activity.clear_for_testing(); @@ -213,14 +229,12 @@ test("huddle_data.process_loaded_messages", () => { test("presence_list_full_update", ({override, mock_template}) => { override(padded_widget, "update_padding", noop); + let presence_rows = []; mock_template("presence_rows.hbs", false, (data) => { - assert.equal(data.presence_rows.length, 7); - assert.equal(data.presence_rows[0].user_id, me.user_id); + presence_rows = [...presence_rows, ...data.presence_rows]; }); $(".user-list-filter").trigger("focus"); - compose_state.private_message_recipient = () => fred.email; - compose_fade.set_focused_recipient("private"); const user_ids = activity_ui.build_user_sidebar(); @@ -233,6 +247,9 @@ test("presence_list_full_update", ({override, mock_template}) => { zoe.user_id, mark.user_id, ]); + + assert.equal(presence_rows.length, 7); + assert.equal(presence_rows[0].user_id, me.user_id); }); function simulate_right_column_buddy_list() { @@ -242,20 +259,11 @@ function simulate_right_column_buddy_list() { }; } -function buddy_list_add(user_id, $stub) { - if ($stub.attr) { - $stub.attr("data-user-id", user_id); - } - $stub.length = 1; - const sel = `li.user_sidebar_entry[data-user-id='${CSS.escape(user_id)}']`; - $("#buddy-list-users-matching-view").set_find_results(sel, $stub); -} - test("direct_message_update_dom_counts", () => { const $count = $.create("alice-unread-count"); const pm_key = alice.user_id.toString(); const $li = $.create("alice stub"); - buddy_list_add(pm_key, $li); + buddy_list_add_user_matching_view(pm_key, $li); $li.set_find_results(".unread_count", $count); $count.set_parents_result("li", $li); @@ -289,9 +297,22 @@ test("handlers", ({override, override_rewire, mock_template}) => { // This is kind of weak coverage; we are mostly making sure that // keys and clicks got mapped to functions that don't crash. - let $me_li; - let $alice_li; - let $fred_li; + const $me_li = $.create("me stub"); + const $alice_li = $.create("alice stub"); + const $fred_li = $.create("fred stub"); + + // Simulate a small window by having the + // fill_screen_with_content render the entire + // list in one pass. We will do more refined + // testing in the buddy_list node tests. + override(buddy_list, "fill_screen_with_content", () => { + buddy_list.render_more({ + chunk_size: 100, + }); + buddy_list_add_user_matching_view(me.user_id, $me_li); + buddy_list_add_user_matching_view(alice.user_id, $alice_li); + buddy_list_add_user_matching_view(fred.user_id, $fred_li); + }); let narrowed; @@ -302,24 +323,16 @@ test("handlers", ({override, override_rewire, mock_template}) => { function init() { $.clear_all_elements(); - buddy_list.populate({ - all_user_ids: [me.user_id, alice.user_id, fred.user_id], - }); + stub_buddy_list_elements(); buddy_list.start_scroll_handler = noop; override_rewire(util, "call_function_periodically", noop); override_rewire(activity, "send_presence_to_server", noop); activity_ui.initialize({narrow_by_email}); - $("#buddy-list-users-matching-view").empty = noop; - - $me_li = $.create("me stub"); - $alice_li = $.create("alice stub"); - $fred_li = $.create("fred stub"); - - buddy_list_add(me.user_id, $me_li); - buddy_list_add(alice.user_id, $alice_li); - buddy_list_add(fred.user_id, $fred_li); + buddy_list.populate({ + all_user_ids: [me.user_id, alice.user_id, fred.user_id], + }); } (function test_filter_keys() { @@ -382,79 +395,80 @@ test("handlers", ({override, override_rewire, mock_template}) => { })(); }); -test("first/prev/next", ({override, mock_template}) => { - let rendered_alice; - let rendered_fred; - user_settings.user_list_style = 2; - - mock_template("presence_row.hbs", false, (data) => { - switch (data.user_id) { - case alice.user_id: - rendered_alice = true; - assert.deepEqual(data, { - faded: true, - href: "#narrow/dm/1-Alice-Smith", - is_current_user: false, - name: "Alice Smith", - num_unread: 0, - user_circle_class: "user_circle_green", - user_id: alice.user_id, - status_emoji_info: undefined, - status_text: undefined, - user_list_style: { - COMPACT: false, - WITH_STATUS: true, - WITH_AVATAR: false, - }, - should_add_guest_user_indicator: false, - }); - break; - case fred.user_id: - rendered_fred = true; - assert.deepEqual(data, { - href: "#narrow/dm/2-Fred-Flintstone", - name: "Fred Flintstone", - user_id: fred.user_id, - is_current_user: false, - num_unread: 0, - user_circle_class: "user_circle_green", - faded: false, - status_emoji_info: undefined, - status_text: undefined, - user_list_style: { - COMPACT: false, - WITH_STATUS: true, - WITH_AVATAR: false, - }, - should_add_guest_user_indicator: false, - }); - break; - /* istanbul ignore next */ - default: - throw new Error(`we did not expect to have to render a row for ${data.name}`); - } - }); - +test("first/prev/next", ({override, override_rewire, mock_template}) => { + override_rewire(buddy_data, "user_matches_narrow", override_user_matches_narrow); + mock_template("presence_rows.hbs", false, noop); override(padded_widget, "update_padding", noop); + stub_buddy_list_elements(); + // empty list assert.equal(buddy_list.first_key(), undefined); + blueslip.reset(); + blueslip.expect("error", "Couldn't find key in buddy list"); + assert.equal(buddy_list.prev_key(alice.user_id), undefined); + blueslip.reset(); + blueslip.expect("error", "Couldn't find key in buddy list"); + assert.equal(buddy_list.next_key(alice.user_id), undefined); + blueslip.reset(); + + // one user matching the view + clear_buddy_list(buddy_list); + buddy_list_add_user_matching_view(alice.user_id, $alice_stub); + buddy_list.populate({ + all_user_ids: [alice.user_id], + }); + assert.equal(buddy_list.first_key(), alice.user_id); assert.equal(buddy_list.prev_key(alice.user_id), undefined); assert.equal(buddy_list.next_key(alice.user_id), undefined); - override(buddy_list.$container, "append", noop); - - activity_ui.redraw_user(alice.user_id); - activity_ui.redraw_user(fred.user_id); - + // two users matching the view + clear_buddy_list(buddy_list); + buddy_list_add_user_matching_view(alice.user_id, $alice_stub); + buddy_list_add_user_matching_view(fred.user_id, $fred_stub); + buddy_list.populate({ + all_user_ids: [alice.user_id, fred.user_id], + }); assert.equal(buddy_list.first_key(), alice.user_id); assert.equal(buddy_list.prev_key(alice.user_id), undefined); assert.equal(buddy_list.prev_key(fred.user_id), alice.user_id); - assert.equal(buddy_list.next_key(alice.user_id), fred.user_id); assert.equal(buddy_list.next_key(fred.user_id), undefined); - assert.ok(rendered_alice); - assert.ok(rendered_fred); + // one other user + clear_buddy_list(buddy_list); + buddy_list_add_other_user(fred.user_id, $fred_stub); + buddy_list.populate({ + all_user_ids: [fred.user_id], + }); + assert.equal(buddy_list.first_key(), fred.user_id); + assert.equal(buddy_list.prev_key(fred.user_id), undefined); + assert.equal(buddy_list.next_key(fred.user_id), undefined); + + // two other users + clear_buddy_list(buddy_list); + buddy_list_add_other_user(alice.user_id, $alice_stub); + buddy_list_add_other_user(fred.user_id, $fred_stub); + buddy_list.populate({ + all_user_ids: [alice.user_id, fred.user_id], + }); + assert.equal(buddy_list.first_key(), alice.user_id); + assert.equal(buddy_list.prev_key(alice.user_id), undefined); + assert.equal(buddy_list.prev_key(fred.user_id), alice.user_id); + assert.equal(buddy_list.next_key(alice.user_id), fred.user_id); + assert.equal(buddy_list.next_key(fred.user_id), undefined); + + // one user matching the view, and one other user + clear_buddy_list(buddy_list); + buddy_list_add_user_matching_view(alice.user_id, $alice_stub); + buddy_list_add_other_user(alice.user_id, $fred_stub); + buddy_list.populate({ + all_user_ids: [alice.user_id, fred.user_id], + }); + assert.equal(buddy_list.first_key(), alice.user_id); + assert.equal(buddy_list.prev_key(alice.user_id), undefined); + assert.equal(buddy_list.prev_key(fred.user_id), alice.user_id); + assert.equal(buddy_list.next_key(alice.user_id), fred.user_id); + assert.equal(buddy_list.next_key(fred.user_id), undefined); }); test("render_empty_user_list_message", ({override, mock_template}) => { @@ -481,15 +495,17 @@ test("render_empty_user_list_message", ({override, mock_template}) => { test("insert_one_user_into_empty_list", ({override, mock_template}) => { user_settings.user_list_style = 2; + + override(padded_widget, "update_padding", noop); mock_template("presence_row.hbs", true, (data, html) => { assert.deepEqual(data, { + faded: false, href: "#narrow/dm/1-Alice-Smith", name: "Alice Smith", user_id: 1, is_current_user: false, num_unread: 0, user_circle_class: "user_circle_green", - faded: true, status_emoji_info: undefined, status_text: undefined, user_list_style: { @@ -503,51 +519,69 @@ test("insert_one_user_into_empty_list", ({override, mock_template}) => { return html; }); - override(padded_widget, "update_padding", noop); - - let appended_html; - override(buddy_list.$container, "append", (html) => { - appended_html = html; + let users_matching_view_appended_html; + override(buddy_list.$users_matching_view_container, "append", (html) => { + users_matching_view_appended_html = html; + }); + let other_users_appended_html; + override(buddy_list.$other_users_container, "append", (html) => { + other_users_appended_html = html; }); + add_sub_and_set_as_current_narrow(rome_sub); + + buddy_list_add_user_matching_view(alice.user_id, $alice_stub); + peer_data.set_subscribers(rome_sub.stream_id, [alice.user_id]); activity_ui.redraw_user(alice.user_id); - assert.ok(appended_html.indexOf('data-user-id="1"') > 0); - assert.ok(appended_html.indexOf("user_circle_green") > 0); + assert.ok(users_matching_view_appended_html.indexOf('data-user-id="1"') > 0); + assert.ok(users_matching_view_appended_html.indexOf("user_circle_green") > 0); + + clear_buddy_list(buddy_list); + buddy_list_add_other_user(alice.user_id, $alice_stub); + peer_data.set_subscribers(rome_sub.stream_id, []); + activity_ui.redraw_user(alice.user_id); + assert.ok(other_users_appended_html.indexOf('data-user-id="1"') > 0); + assert.ok(other_users_appended_html.indexOf("user_circle_green") > 0); }); test("insert_alice_then_fred", ({override, mock_template}) => { mock_template("presence_row.hbs", true, (_data, html) => html); - let appended_html; - override(buddy_list.$container, "append", (html) => { - appended_html = html; + let other_users_appended_html; + override(buddy_list.$other_users_container, "append", (html) => { + other_users_appended_html = html; }); override(padded_widget, "update_padding", noop); activity_ui.redraw_user(alice.user_id); - assert.ok(appended_html.indexOf('data-user-id="1"') > 0); - assert.ok(appended_html.indexOf("user_circle_green") > 0); + assert.ok(other_users_appended_html.indexOf('data-user-id="1"') > 0); + assert.ok(other_users_appended_html.indexOf("user_circle_green") > 0); activity_ui.redraw_user(fred.user_id); - assert.ok(appended_html.indexOf('data-user-id="2"') > 0); - assert.ok(appended_html.indexOf("user_circle_green") > 0); + assert.ok(other_users_appended_html.indexOf('data-user-id="2"') > 0); + assert.ok(other_users_appended_html.indexOf("user_circle_green") > 0); }); -test("insert_fred_then_alice_then_rename", ({override, mock_template}) => { +test("insert_fred_then_alice_then_rename, both as users matching view", ({ + override, + mock_template, +}) => { mock_template("presence_row.hbs", true, (_data, html) => html); - let appended_html; - override(buddy_list.$container, "append", (html) => { - appended_html = html; + add_sub_and_set_as_current_narrow(rome_sub); + peer_data.set_subscribers(rome_sub.stream_id, [alice.user_id, fred.user_id]); + + let users_matching_view_appended_html; + override(buddy_list.$users_matching_view_container, "append", (html) => { + users_matching_view_appended_html = html; }); override(padded_widget, "update_padding", noop); + buddy_list_add_user_matching_view(alice.user_id, $alice_stub); + buddy_list_add_user_matching_view(fred.user_id, $fred_stub); activity_ui.redraw_user(fred.user_id); - assert.ok(appended_html.indexOf('data-user-id="2"') > 0); - assert.ok(appended_html.indexOf("user_circle_green") > 0); - - const $fred_stub = $.create("fred-first"); - buddy_list_add(fred.user_id, $fred_stub); + assert.ok(users_matching_view_appended_html.indexOf('data-user-id="2"') > 0); + assert.ok(users_matching_view_appended_html.indexOf("user_circle_green") > 0); let inserted_html; $fred_stub.before = (html) => { @@ -571,8 +605,58 @@ test("insert_fred_then_alice_then_rename", ({override, mock_template}) => { }; people.add_active_user(fred_with_new_name); - const $alice_stub = $.create("alice-first"); - buddy_list_add(alice.user_id, $alice_stub); + $alice_stub.before = (html) => { + inserted_html = html; + }; + + activity_ui.redraw_user(fred_with_new_name.user_id); + assert.ok(fred_removed); + assert.ok(users_matching_view_appended_html.indexOf('data-user-id="2"') > 0); + + // restore old Fred data + people.add_active_user(fred); +}); + +test("insert_fred_then_alice_then_rename, both as other users", ({override, mock_template}) => { + mock_template("presence_row.hbs", true, (_data, html) => html); + + add_sub_and_set_as_current_narrow(rome_sub); + peer_data.set_subscribers(rome_sub.stream_id, []); + + let other_users_appended_html; + override(buddy_list.$other_users_container, "append", (html) => { + other_users_appended_html = html; + }); + override(padded_widget, "update_padding", noop); + + buddy_list_add_other_user(alice.user_id, $alice_stub); + buddy_list_add_other_user(fred.user_id, $fred_stub); + + activity_ui.redraw_user(fred.user_id); + assert.ok(other_users_appended_html.indexOf('data-user-id="2"') > 0); + assert.ok(other_users_appended_html.indexOf("user_circle_green") > 0); + + let inserted_html; + $fred_stub.before = (html) => { + inserted_html = html; + }; + + let fred_removed; + $fred_stub.remove = () => { + fred_removed = true; + }; + + activity_ui.redraw_user(alice.user_id); + assert.ok(inserted_html.indexOf('data-user-id="1"') > 0); + assert.ok(inserted_html.indexOf("user_circle_green") > 0); + + // Next rename fred to Aaron. + const fred_with_new_name = { + email: fred.email, + user_id: fred.user_id, + full_name: "Aaron", + }; + people.add_active_user(fred_with_new_name); $alice_stub.before = (html) => { inserted_html = html; @@ -580,7 +664,7 @@ test("insert_fred_then_alice_then_rename", ({override, mock_template}) => { activity_ui.redraw_user(fred_with_new_name.user_id); assert.ok(fred_removed); - assert.ok(appended_html.indexOf('data-user-id="2"') > 0); + assert.ok(other_users_appended_html.indexOf('data-user-id="2"') > 0); // restore old Fred data people.add_active_user(fred); @@ -624,7 +708,7 @@ test("update_presence_info", ({override}) => { }; const $alice_li = $.create("alice stub"); - buddy_list_add(alice.user_id, $alice_li); + buddy_list_add_user_matching_view(alice.user_id, $alice_li); let inserted; override(buddy_list, "insert_or_move", () => { @@ -661,10 +745,9 @@ test("update_presence_info", ({override}) => { }); test("initialize", ({override, mock_template}) => { - mock_template("presence_rows.hbs", false, noop); - override(padded_widget, "update_padding", noop); override(pm_list, "update_private_messages", noop); override(watchdog, "check_for_unsuspend", noop); + override(buddy_list, "fill_screen_with_content", noop); let payload; override(channel, "post", (arg) => { @@ -677,9 +760,13 @@ test("initialize", ({override, mock_template}) => { function clear() { $.clear_all_elements(); - buddy_list.$container = $("#buddy-list-users-matching-view"); - buddy_list.$container.append = noop; - clear_buddy_list(); + buddy_list.$users_matching_view_container = $("#buddy-list-users-matching-view"); + buddy_list.$users_matching_view_container.append = noop; + buddy_list.$other_users_container = $("#buddy-list-other-users"); + buddy_list.$other_users_container.append = noop; + stub_buddy_list_elements(); + mock_template("empty_list_widget_for_list.hbs", false, noop); + clear_buddy_list(buddy_list); page_params.presences = {}; } diff --git a/web/tests/buddy_data.test.js b/web/tests/buddy_data.test.js index 9bedb95d0f..e91e8a25de 100644 --- a/web/tests/buddy_data.test.js +++ b/web/tests/buddy_data.test.js @@ -13,6 +13,7 @@ const timerender = mock_esm("../src/timerender"); const compose_fade_helper = zrequire("compose_fade_helper"); const muted_users = zrequire("muted_users"); +const narrow_state = zrequire("narrow_state"); const peer_data = zrequire("peer_data"); const people = zrequire("people"); const presence = zrequire("presence"); @@ -421,6 +422,12 @@ test("always show me", () => { assert.deepEqual(buddy_data.get_filtered_and_sorted_user_ids(""), [me.user_id]); }); +test("always show pm users", ({override_rewire}) => { + people.add_active_user(selma); + override_rewire(narrow_state, "pm_ids_set", () => new Set([selma.user_id])); + assert.deepEqual(buddy_data.get_filtered_and_sorted_user_ids(""), [me.user_id, selma.user_id]); +}); + test("level", () => { realm.server_presence_offline_threshold_seconds = 200; @@ -450,6 +457,58 @@ test("level", () => { assert.equal(buddy_data.level(selma.user_id), 3); }); +test("compare_function", () => { + const first_user_shown_higher = -1; + const second_user_shown_higher = 1; + + const stream_id = 1001; + const sub = {name: "Rome", subscribed: true, stream_id}; + stream_data.add_sub(sub); + people.add_active_user(alice); + people.add_active_user(fred); + + // Alice is higher because of alphabetical sorting. + peer_data.set_subscribers(stream_id, []); + assert.equal( + second_user_shown_higher, + buddy_data.compare_function(fred.user_id, alice.user_id, sub, new Set()), + ); + + // Fred is higher because they're in the narrow and Alice isn't. + peer_data.set_subscribers(stream_id, [fred.user_id]); + assert.equal( + first_user_shown_higher, + buddy_data.compare_function(fred.user_id, alice.user_id, sub, new Set()), + ); + assert.equal( + second_user_shown_higher, + buddy_data.compare_function(alice.user_id, fred.user_id, sub, new Set()), + ); + + // Fred is higher because they're in the DM conversation and Alice isn't. + assert.equal( + first_user_shown_higher, + buddy_data.compare_function( + fred.user_id, + alice.user_id, + undefined, + new Set([fred.user_id]), + ), + ); + + // Alice is higher because of alphabetical sorting. + assert.equal( + second_user_shown_higher, + buddy_data.compare_function(fred.user_id, alice.user_id, undefined, new Set()), + ); + + // The user is part of a DM conversation, though that's not explicitly in the DM list. + assert.equal( + first_user_shown_higher, + buddy_data.compare_function(me.user_id, alice.user_id, undefined, new Set([fred.user_id])), + ); +}); + test("user_last_seen_time_status", ({override}) => { set_presence(selma.user_id, "active"); set_presence(me.user_id, "active"); diff --git a/web/tests/buddy_list.test.js b/web/tests/buddy_list.test.js index 8d60a383ff..038602927f 100644 --- a/web/tests/buddy_list.test.js +++ b/web/tests/buddy_list.test.js @@ -4,6 +4,13 @@ const {strict: assert} = require("assert"); const _ = require("lodash"); +const { + clear_buddy_list, + override_user_matches_narrow, + buddy_list_add_user_matching_view, + buddy_list_add_other_user, + stub_buddy_list_elements, +} = require("./lib/buddy_list"); const {mock_esm, zrequire} = require("./lib/namespace"); const {run_test, noop} = require("./lib/test"); const blueslip = require("./lib/zblueslip"); @@ -12,8 +19,9 @@ const $ = require("./lib/zjquery"); const padded_widget = mock_esm("../src/padded_widget"); const message_viewport = mock_esm("../src/message_viewport"); -const people = zrequire("people"); +const buddy_data = zrequire("buddy_data"); const {BuddyList} = zrequire("buddy_list"); +const people = zrequire("people"); function init_simulated_scrolling() { const elem = { @@ -35,54 +43,43 @@ const alice = { full_name: "Alice Smith", }; people.add_active_user(alice); +const bob = { + email: "bob@zulip.com", + user_id: 15, + full_name: "Bob Smith", +}; +people.add_active_user(bob); +const chris = { + email: "chris@zulip.com", + user_id: 20, + full_name: "Chris Smith", +}; +people.add_active_user(chris); +const $alice_li = $.create("alice-stub"); +const $bob_li = $.create("bob-stub"); -run_test("get_items", () => { - const buddy_list = new BuddyList(); - - // We don't make $alice_li an actual jQuery stub, - // because our test only cares that it comes - // back from get_items. - const $alice_li = "alice stub"; - const sel = "li.user_sidebar_entry"; - const $container = $.create("get_items container", { - children: [{to_$: () => $alice_li}], - }); - buddy_list.$container.set_find_results(sel, $container); - - const items = buddy_list.get_items(); - assert.deepEqual(items, [$alice_li]); -}); - -run_test("basics", ({override}) => { +run_test("basics", ({override, mock_template}) => { const buddy_list = new BuddyList(); init_simulated_scrolling(); - override(buddy_list, "get_data_from_user_ids", (user_ids) => { - assert.deepEqual(user_ids, [alice.user_id]); - return "data-stub"; - }); - - override(buddy_list, "items_to_html", (opts) => { - const items = opts.items; - assert.equal(items, "data-stub"); - return "html-stub"; - }); - + override(buddy_list, "items_to_html", () => "html-stub"); override(message_viewport, "height", () => 550); override(padded_widget, "update_padding", noop); + stub_buddy_list_elements(); + mock_template("buddy_list/view_all_users.hbs", false, noop); - let appended; + let appended_to_users_matching_view; $("#buddy-list-users-matching-view").append = (html) => { assert.equal(html, "html-stub"); - appended = true; + appended_to_users_matching_view = true; }; buddy_list.populate({ all_user_ids: [alice.user_id], }); - assert.ok(appended); + assert.ok(appended_to_users_matching_view); - const $alice_li = {length: 1}; + const $alice_li = "alice-stub"; override(buddy_list, "get_li_from_user_id", (opts) => { const user_id = opts.user_id; @@ -97,14 +94,98 @@ run_test("basics", ({override}) => { assert.equal($li, $alice_li); }); -run_test("big_list", ({override}) => { +run_test("split list", ({override, override_rewire, mock_template}) => { + const buddy_list = new BuddyList(); + init_simulated_scrolling(); + stub_buddy_list_elements(); + mock_template("buddy_list/section_header.hbs", false, noop); + mock_template("buddy_list/view_all_users.hbs", false, noop); + + override_rewire(buddy_data, "user_matches_narrow", override_user_matches_narrow); + + override(buddy_list, "items_to_html", (opts) => { + if (opts.items.length > 0) { + return "html-stub"; + } + return "empty-list"; + }); + override(message_viewport, "height", () => 550); + override(padded_widget, "update_padding", noop); + + let appended_to_users_matching_view = false; + $("#buddy-list-users-matching-view").append = (html) => { + if (html === "html-stub") { + appended_to_users_matching_view = true; + } + }; + + let appended_to_other_users = false; + $("#buddy-list-other-users").append = (html) => { + if (html === "html-stub") { + appended_to_other_users = true; + } + }; + + // one user matching the view + buddy_list_add_user_matching_view(alice.user_id, $alice_li); + buddy_list.populate({ + all_user_ids: [alice.user_id], + }); + assert.ok(appended_to_users_matching_view); + assert.ok(!appended_to_other_users); + appended_to_users_matching_view = false; + + // one other user + clear_buddy_list(buddy_list); + buddy_list_add_other_user(alice.user_id, $alice_li); + buddy_list.populate({ + all_user_ids: [alice.user_id], + }); + assert.ok(!appended_to_users_matching_view); + assert.ok(appended_to_other_users); + appended_to_other_users = false; + + // a user matching the view, and an other user + clear_buddy_list(buddy_list); + buddy_list_add_user_matching_view(alice.user_id, $alice_li); + buddy_list_add_other_user(bob.user_id, $bob_li); + buddy_list.populate({ + all_user_ids: [alice.user_id, bob.user_id], + }); + assert.ok(appended_to_users_matching_view); + assert.ok(appended_to_other_users); +}); + +run_test("find_li", ({override, mock_template}) => { + const buddy_list = new BuddyList(); + + override(buddy_list, "fill_screen_with_content", noop); + mock_template("buddy_list/view_all_users.hbs", false, noop); + stub_buddy_list_elements(); + + clear_buddy_list(buddy_list); + buddy_list_add_user_matching_view(alice.user_id, $alice_li); + buddy_list_add_other_user(bob.user_id, $bob_li); + + let $li = buddy_list.find_li({ + key: alice.user_id, + }); + assert.equal($li, $alice_li); + + $li = buddy_list.find_li({ + key: bob.user_id, + }); + assert.equal($li, $bob_li); +}); + +run_test("fill_screen_with_content early break on big list", ({override, mock_template}) => { + stub_buddy_list_elements(); const buddy_list = new BuddyList(); const elem = init_simulated_scrolling(); + stub_buddy_list_elements(); + mock_template("buddy_list/view_all_users.hbs", false, noop); - // Don't actually render, but do simulate filling up - // the screen. let chunks_inserted = 0; - override(buddy_list, "render_more", () => { elem.scrollHeight += 100; chunks_inserted += 1; @@ -112,7 +193,9 @@ run_test("big_list", ({override}) => { override(message_viewport, "height", () => 550); // We will have more than enough users, but still - // only do 6 chunks of data. + // only do 6 chunks of data (20 users per chunk) + // because of exiting early from fill_screen_with_content + // because of not scrolling enough to fetch more users. const num_users = 300; const user_ids = []; @@ -130,9 +213,72 @@ run_test("big_list", ({override}) => { all_user_ids: user_ids, }); + // Only 6 chunks, even though that's 120 users instead of the full 300. assert.equal(chunks_inserted, 6); }); +run_test("big_list", ({override, override_rewire, mock_template}) => { + const buddy_list = new BuddyList(); + init_simulated_scrolling(); + + stub_buddy_list_elements(); + override(padded_widget, "update_padding", noop); + override(message_viewport, "height", () => 550); + override_rewire(buddy_data, "user_matches_narrow", override_user_matches_narrow); + mock_template("empty_list_widget_for_list.hbs", false, noop); + mock_template("buddy_list/section_header.hbs", false, noop); + mock_template("buddy_list/view_all_users.hbs", false, noop); + + let items_to_html_call_count = 0; + override(buddy_list, "items_to_html", () => { + items_to_html_call_count += 1; + return "html-stub"; + }); + + const num_users = 300; + const user_ids = []; + + // This isn't a great way of testing this, but this is here for + // the sake of code coverage. Essentially, for a very long list, + // these buddy list sections can collect empty messages in the middle + // of populating (i.e. once a chunk is rendered) which later might need + // to be removed to add users from future chunks. + // + // For example: chunk1 populates only users in the list of users matching, + // the view and the empty list says "None", but chunk2 adds users to the + // other list so the "None" message should be removed. + // + // Here we're just saying both lists are rendered as empty from start, + // which doesn't actually happen, since I don't know how to properly + // get it set in the middle of buddy_list.populate(). + $("#buddy-list-users-matching-view .empty-list-message").length = 1; + $("#buddy-list-other-users .empty-list-message").length = 1; + + _.times(num_users, (i) => { + const person = { + email: "foo" + i + "@zulip.com", + user_id: 100 + i, + full_name: "Somebody " + i, + }; + people.add_active_user(person); + if (i < 100 || i % 2 === 0) { + buddy_list_add_user_matching_view(person.user_id, $.create("stub" + i)); + } else { + buddy_list_add_other_user(person.user_id, $.create("stub" + i)); + } + user_ids.push(person.user_id); + }); + + buddy_list.populate({ + all_user_ids: user_ids, + }); + + // Chunks are default size 20, so there should be 300/20 = 15 chunks + const expected_chunks_inserted = 15; + // Two calls per chunk: one for users_matching_view and one for other_users. + assert.equal(items_to_html_call_count, 2 * expected_chunks_inserted); +}); + run_test("force_render", ({override}) => { const buddy_list = new BuddyList(); buddy_list.render_count = 50; @@ -159,10 +305,10 @@ run_test("find_li w/force_render", ({override}) => { const buddy_list = new BuddyList(); // If we call find_li w/force_render set, and the - // key is not already rendered in DOM, then the - // widget will call show_key to force-render it. + // user_id is not already rendered in DOM, then the + // widget will force-render it. const user_id = "999"; - const $stub_li = {length: 0}; + const $stub_li = "stub-li"; override(buddy_list, "get_li_from_user_id", (opts) => { assert.equal(opts.user_id, user_id); @@ -178,10 +324,10 @@ run_test("find_li w/force_render", ({override}) => { shown = true; }); - const $empty_li = buddy_list.find_li({ + const $hidden_li = buddy_list.find_li({ key: user_id, }); - assert.equal($empty_li, $stub_li); + assert.equal($hidden_li, $stub_li); assert.ok(!shown); const $li = buddy_list.find_li({ @@ -195,7 +341,7 @@ run_test("find_li w/force_render", ({override}) => { run_test("find_li w/bad key", ({override}) => { const buddy_list = new BuddyList(); - override(buddy_list, "get_li_from_user_id", () => ({length: 0})); + override(buddy_list, "get_li_from_user_id", () => "stub-li"); const $undefined_li = buddy_list.find_li({ key: "not-there", @@ -205,23 +351,20 @@ run_test("find_li w/bad key", ({override}) => { assert.deepEqual($undefined_li, []); }); -run_test("scrolling", ({override}) => { +run_test("scrolling", ({override, mock_template}) => { const buddy_list = new BuddyList(); - init_simulated_scrolling(); - - override(message_viewport, "height", () => 550); - - buddy_list.populate({ - all_user_ids: [], - }); - let tried_to_fill; - override(buddy_list, "fill_screen_with_content", () => { tried_to_fill = true; }); + mock_template("buddy_list/view_all_users.hbs", false, noop); + stub_buddy_list_elements(); + init_simulated_scrolling(); + stub_buddy_list_elements(); - assert.ok(!tried_to_fill); + clear_buddy_list(buddy_list); + assert.ok(tried_to_fill); + tried_to_fill = false; buddy_list.start_scroll_handler(); $(buddy_list.scroll_container_selector).trigger("scroll"); diff --git a/web/tests/lib/buddy_list.js b/web/tests/lib/buddy_list.js new file mode 100644 index 0000000000..a4c6319535 --- /dev/null +++ b/web/tests/lib/buddy_list.js @@ -0,0 +1,47 @@ +"use strict"; + +const {noop} = require("./test"); +const $ = require("./zjquery"); + +let users_matching_view = []; +exports.buddy_list_add_user_matching_view = (user_id, $stub) => { + if ($stub.attr) { + $stub.attr("data-user-id", user_id); + } + $stub.length = 1; + users_matching_view.push(user_id); + const sel = `li.user_sidebar_entry[data-user-id='${CSS.escape(user_id)}']`; + $("#buddy_list_wrapper").set_find_results(sel, $stub); +}; + +let other_users = []; +exports.buddy_list_add_other_user = (user_id, $stub) => { + if ($stub.attr) { + $stub.attr("data-user-id", user_id); + } + $stub.length = 1; + other_users.push(user_id); + const sel = `li.user_sidebar_entry[data-user-id='${CSS.escape(user_id)}']`; + $("#buddy_list_wrapper").set_find_results(sel, $stub); +}; + +exports.override_user_matches_narrow = (user_id) => users_matching_view.includes(user_id); + +exports.clear_buddy_list = (buddy_list) => { + buddy_list.populate({ + all_user_ids: [], + }); + users_matching_view = []; + other_users = []; +}; + +exports.stub_buddy_list_elements = () => { + // Set to an empty list since we're not testing CSS. + $("#buddy-list-users-matching-view").children = () => []; + $("#buddy-list-other-users").children = () => []; + $("#buddy-list-users-matching-view .empty-list-message").length = 0; + $("#buddy-list-other-users .empty-list-message").length = 0; + $("#buddy-list-other-users-container .view-all-users-link").length = 0; + $("#buddy-list-users-matching-view-container .view-all-subscribers-link").remove = noop; + $("#buddy-list-other-users-container .view-all-users-link").remove = noop; +}; diff --git a/web/tests/narrow_activate.test.js b/web/tests/narrow_activate.test.js index ac24eb1d94..446dcba6c5 100644 --- a/web/tests/narrow_activate.test.js +++ b/web/tests/narrow_activate.test.js @@ -72,9 +72,12 @@ mock_esm("../src/user_topics", { is_topic_muted: () => false, }); +const {buddy_list} = zrequire("buddy_list"); +const activity_ui = zrequire("activity_ui"); const narrow_state = zrequire("narrow_state"); const stream_data = zrequire("stream_data"); const narrow = zrequire("narrow"); +const people = zrequire("people"); const denmark = { subscribed: false, @@ -156,6 +159,16 @@ function stub_message_list() { run_test("basics", ({override}) => { stub_message_list(); + activity_ui.set_cursor_and_filter(); + + const me = { + email: "me@zulip.com", + user_id: 999, + full_name: "Me Myself", + }; + people.add_active_user(me); + people.initialize_current_user(me.user_id); + override(buddy_list, "populate", noop); const helper = test_helper({override}); const terms = [{operator: "stream", operand: "Denmark"}]; diff --git a/web/tests/recent_view.test.js b/web/tests/recent_view.test.js index dd88f6ac7b..b01eb7a9ef 100644 --- a/web/tests/recent_view.test.js +++ b/web/tests/recent_view.test.js @@ -189,6 +189,8 @@ dropdown_widget.DropdownWidget = function DropdownWidget() { }; const {all_messages_data} = zrequire("all_messages_data"); +const {buddy_list} = zrequire("buddy_list"); +const activity_ui = zrequire("activity_ui"); const people = zrequire("people"); const rt = zrequire("recent_view_ui"); const recent_view_util = zrequire("recent_view_util"); @@ -444,7 +446,7 @@ function test(label, f) { }); } -test("test_recent_view_show", ({mock_template}) => { +test("test_recent_view_show", ({override, mock_template}) => { // Note: unread count and urls are fake, // since they are generated in external libraries // and are not to be tested here. @@ -458,6 +460,8 @@ test("test_recent_view_show", ({mock_template}) => { is_spectator: false, }; + activity_ui.set_cursor_and_filter(); + mock_template("recent_view_table.hbs", false, (data) => { assert.deepEqual(data, expected); return ""; @@ -465,6 +469,11 @@ test("test_recent_view_show", ({mock_template}) => { mock_template("recent_view_row.hbs", false, noop); + let buddy_list_populated = false; + override(buddy_list, "populate", () => { + buddy_list_populated = true; + }); + stub_out_filter_buttons(); // We don't test the css calls; we just skip over them. $("#mark_read_on_scroll_state_banner").toggleClass = noop; @@ -474,6 +483,8 @@ test("test_recent_view_show", ({mock_template}) => { rt.show(); + assert.ok(buddy_list_populated); + // incorrect topic_key assert.equal(rt.inplace_rerender("stream_unknown:topic_unknown"), false); }); diff --git a/web/tests/user_search.test.js b/web/tests/user_search.test.js index da9b4e4e0b..91d902fa93 100644 --- a/web/tests/user_search.test.js +++ b/web/tests/user_search.test.js @@ -9,7 +9,10 @@ const {realm} = require("./lib/zpage_params"); const fake_buddy_list = { scroll_container_selector: "#whatever", - $container: { + $users_matching_view_container: { + data() {}, + }, + $other_users_container: { data() {}, }, find_li() {}, @@ -58,7 +61,7 @@ const all_user_ids = [alice.user_id, fred.user_id, jill.user_id, me.user_id]; const ordered_user_ids = [me.user_id, alice.user_id, fred.user_id, jill.user_id]; function test(label, f) { - run_test(label, ({override}) => { + run_test(label, (opts) => { people.init(); people.add_active_user(alice); people.add_active_user(fred); @@ -67,7 +70,7 @@ function test(label, f) { people.initialize_current_user(me.user_id); muted_users.set_muted_users([]); activity_ui.set_cursor_and_filter(); - f({override}); + f(opts); }); } @@ -76,12 +79,19 @@ function set_input_val(val) { $(".user-list-filter").trigger("input"); } +function stub_buddy_list_empty_list_message_lengths() { + $("#buddy-list-users-matching-view .empty-list-message").length = 0; + $("#buddy-list-other-users .empty-list-message").length = 0; +} + 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(); + // Empty because no users match this search string. override(fake_buddy_list, "populate", (user_ids) => { assert.deepEqual(user_ids, {all_user_ids: []}); @@ -104,6 +114,7 @@ test("escape_search", ({override}) => { 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();