From 4385005200a194c379b9d4bfa8bee018bffeb99a Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Fri, 10 Jan 2025 07:42:57 +0000 Subject: [PATCH] message_list: Avoid rerender of user sidebar when adding msgs. Added methods to live update user sidebar when messages are added or removed from the message list without doing a complete rerender. --- web/src/activity_ui.ts | 9 +++++++++ web/src/buddy_list.ts | 18 ++++++++++++++++++ web/src/message_list.ts | 21 ++++++++++++--------- web/tests/buddy_data.test.cjs | 9 ++++++++- web/tests/message_list.test.cjs | 4 ++-- 5 files changed, 49 insertions(+), 12 deletions(-) diff --git a/web/src/activity_ui.ts b/web/src/activity_ui.ts index 6c3fd1708b..0527d9a4fb 100644 --- a/web/src/activity_ui.ts +++ b/web/src/activity_ui.ts @@ -10,6 +10,7 @@ import * as buddy_data from "./buddy_data.ts"; import {buddy_list} from "./buddy_list.ts"; import * as keydown_util from "./keydown_util.ts"; import {ListCursor} from "./list_cursor.ts"; +import * as narrow_state from "./narrow_state.ts"; import * as people from "./people.ts"; import * as pm_list from "./pm_list.ts"; import * as popovers from "./popovers.ts"; @@ -96,6 +97,14 @@ export function redraw_user(user_id: number): void { update_presence_indicators(); } +export function rerender_user_sidebar_participants(): void { + if (!narrow_state.stream_id() || !narrow_state.topic()) { + return; + } + + buddy_list.rerender_participants(); +} + export function check_should_redraw_new_user(user_id: number): boolean { if (realm.realm_presence_disabled) { return false; diff --git a/web/src/buddy_list.ts b/web/src/buddy_list.ts index ec6e963107..023d0f2c5e 100644 --- a/web/src/buddy_list.ts +++ b/web/src/buddy_list.ts @@ -983,6 +983,24 @@ export class BuddyList extends BuddyListConf { is_participant_user: all_participant_ids.has(user_id), }); } + + this.display_or_hide_sections(); + this.render_section_headers(); + } + + rerender_participants(): void { + const all_participant_ids = this.render_data.get_all_participant_ids(); + const users_to_remove = this.participant_user_ids.filter( + (user_id) => !all_participant_ids.has(user_id), + ); + const users_to_add = [...all_participant_ids].filter( + (user_id) => !this.participant_user_ids.includes(user_id), + ); + + // We are just moving the users around since we still want to show the + // user in buddy list regardless of if they are a participant, so we + // call `insert_or_move` on both `users_to_remove` and `users_to_add`. + this.insert_or_move([...users_to_remove, ...users_to_add]); } fill_screen_with_content(): void { diff --git a/web/src/message_list.ts b/web/src/message_list.ts index d0d10aea65..2757532c96 100644 --- a/web/src/message_list.ts +++ b/web/src/message_list.ts @@ -8,7 +8,6 @@ import * as compose_tooltips from "./compose_tooltips.ts"; import type {MessageListData} from "./message_list_data.ts"; import * as message_list_tooltips from "./message_list_tooltips.ts"; import {MessageListView} from "./message_list_view.ts"; -import * as message_lists from "./message_lists.ts"; import type {Message} from "./message_store.ts"; import * as narrow_banner from "./narrow_banner.ts"; import * as narrow_state from "./narrow_state.ts"; @@ -216,6 +215,7 @@ export class MessageList { if (interior_messages.length > 0) { this.view.rerender_preserving_scrolltop(true); + this.update_user_sidebar_participants(); return {need_user_to_scroll: true}; } if (top_messages.length > 0) { @@ -244,11 +244,7 @@ export class MessageList { this.select_id(first_unread_message_id, {then_scroll: true, use_closest: true}); } - // Rebuild message list, since we might need to shuffle around the participant users. - if (this === message_lists.current && narrow_state.stream_sub() && narrow_state.topic()) { - activity_ui.build_user_sidebar(); - } - + this.update_user_sidebar_participants(); return render_info; } @@ -503,14 +499,16 @@ export class MessageList { remove_and_rerender(message_ids: number[]): void { const should_rerender = this.data.remove(message_ids); + this.update_user_sidebar_participants(); if (!should_rerender) { return; } this.rerender(); - // Rebuild message list if we're deleting messages from the current list, - // since we might need to remove a participant user. + } + + update_user_sidebar_participants(): void { if (this.is_current_message_list()) { - activity_ui.build_user_sidebar(); + activity_ui.rerender_user_sidebar_participants(); } } @@ -616,6 +614,11 @@ export class MessageList { // But in any case, we need to rerender the list for user muting, // to make sure only the right messages are hidden. this.rerender(); + + // While this can have changed the conversation's visible + // participants, we don't need to call + // this.update_user_sidebar_participants, because changing a + // muted user's state already does a full sidebar redraw. } all_messages(): Message[] { diff --git a/web/tests/buddy_data.test.cjs b/web/tests/buddy_data.test.cjs index f5a8c4dbc7..8d72568e2c 100644 --- a/web/tests/buddy_data.test.cjs +++ b/web/tests/buddy_data.test.cjs @@ -5,15 +5,21 @@ const assert = require("node:assert/strict"); const _ = require("lodash"); const {mock_esm, zrequire} = require("./lib/namespace.cjs"); -const {run_test} = require("./lib/test.cjs"); +const {noop, run_test} = require("./lib/test.cjs"); const {page_params} = require("./lib/zpage_params.cjs"); mock_esm("../src/settings_data", { user_can_access_all_other_users: () => true, }); const timerender = mock_esm("../src/timerender"); +mock_esm("../src/buddy_list", { + buddy_list: { + rerender_participants: noop, + }, +}); const compose_fade_helper = zrequire("compose_fade_helper"); +const activity_ui = zrequire("activity_ui"); const muted_users = zrequire("muted_users"); const narrow_state = zrequire("narrow_state"); const peer_data = zrequire("peer_data"); @@ -435,6 +441,7 @@ test("get_conversation_participants", ({override_rewire}) => { override_rewire(narrow_state, "stream_id", () => rome_sub.stream_id); override_rewire(narrow_state, "topic", () => "Foo"); + activity_ui.rerender_user_sidebar_participants(); assert.deepEqual( buddy_data.get_conversation_participants_callback()(), new Set([selma.user_id]), diff --git a/web/tests/message_list.test.cjs b/web/tests/message_list.test.cjs index b3c43f3f74..669e00200a 100644 --- a/web/tests/message_list.test.cjs +++ b/web/tests/message_list.test.cjs @@ -52,7 +52,7 @@ const current_user = {}; set_current_user(current_user); run_test("basics", ({override}) => { - override(activity_ui, "build_user_sidebar", noop); + override(activity_ui, "rerender_user_sidebar_participants", noop); const filter = new Filter([]); const list = new MessageList({ @@ -451,7 +451,7 @@ run_test("bookend", ({override}) => { }); run_test("add_remove_rerender", ({override}) => { - override(activity_ui, "build_user_sidebar", noop); + override(activity_ui, "rerender_user_sidebar_participants", noop); const filter = new Filter([]); const list = new MessageList({