Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

narrow: Clarify some confusing details. 

The update_selection function name was rather misleading, since that
function call is in fact what renders the message list object for the
view.

Also add comments about a few subtle/confusing details that I noticed
while debugging this code path today.
This commit is contained in:
Tim Abbott 2024-01-31 10:39:14 -08:00
parent 7d4ec1f93b
commit 652fea9bdf
2 changed files with 18 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
web/src/message_list.js
View File

@ -286,6 +286,15 @@ export class MessageList {
this.data.set_selected_id(id); this.data.set_selected_id(id);
if (opts.force_rerender) { if (opts.force_rerender) {
// TODO: Because rerender() itself will call
// reselect_selected_id after doing the rendering, we
// actually end up with this function being called
// recursively in this case. The ordering will end up
// being that the message_selected.zulip event for that
// rerender is processed before execution returns here.
//
// The recursive call is unnecessary, so we should figure
// out how to avoid that, both here and in the next block.
this.rerender(); this.rerender();
} else if (!opts.from_rendering) { } else if (!opts.from_rendering) {
this.view.maybe_rerender(); this.view.maybe_rerender();

14
web/src/narrow.js
View File

@ -497,7 +497,7 @@ export function activate(raw_terms, opts) {
anchor, anchor,
cont() { cont() {
if (!select_immediately) { if (!select_immediately) {
update_selection({ render_message_list_with_selected_message({
id_info, id_info,
select_offset: then_select_offset, select_offset: then_select_offset,
msg_list: message_lists.current, msg_list: message_lists.current,
@ -509,14 +509,14 @@ export function activate(raw_terms, opts) {
} }
// Important: We need to consider opening the compose box // Important: We need to consider opening the compose box
// before calling update_selection, so that the logic in // before calling render_message_list_with_selected_message, so that the logic in
// recenter_view for positioning the currently selected // recenter_view for positioning the currently selected
// message can take into account the space consumed by the // message can take into account the space consumed by the
// open compose box. // open compose box.
compose_actions.on_narrow(opts); compose_actions.on_narrow(opts);
if (select_immediately) { if (select_immediately) {
update_selection({ render_message_list_with_selected_message({
id_info, id_info,
select_offset: then_select_offset, select_offset: then_select_offset,
msg_list: message_lists.current, msg_list: message_lists.current,
@ -532,7 +532,6 @@ export function activate(raw_terms, opts) {
handle_post_view_change(msg_list); handle_post_view_change(msg_list);
unread_ui.update_unread_banner(); unread_ui.update_unread_banner();
// It is important to call this after other important updates // It is important to call this after other important updates
@ -740,7 +739,7 @@ export function maybe_add_local_messages(opts) {
return; return;
} }
export function update_selection(opts) { export function render_message_list_with_selected_message(opts) {
if (message_lists.current !== opts.msg_list) { if (message_lists.current !== opts.msg_list) {
// If we navigated away from a view while we were fetching // If we navigated away from a view while we were fetching
// messages for it, don't attempt to move the currently // messages for it, don't attempt to move the currently
@ -768,6 +767,11 @@ export function update_selection(opts) {
const then_scroll = !preserve_pre_narrowing_screen_position; const then_scroll = !preserve_pre_narrowing_screen_position;
// Here we render the actual message list to the DOM with the
// target selected message, using the force_rerender parameter.
//
// TODO: Probably this should accept the offset parameter rather
// than calling `set_message_offset` just after.
message_lists.current.select_id(msg_id, { message_lists.current.select_id(msg_id, {
then_scroll, then_scroll,
use_closest: true, use_closest: true,