recent_view: Reduce constant flashing of recent view on initial load.

We completely rerender recent view when we receive a new message,
which is not ideal since it is flashes recent view frequently
during the initial fetch.

Since later parts of initial fetch which trigger the flash, only
load old messages, they cannot change the latest message of
rendered rows in recent view. That means we can just inplace
rerender the rendered rows which got updated and don't have
to worry about order of rows being changed.
This commit is contained in:
Aman Agrawal 2024-05-30 04:20:14 +00:00 committed by Tim Abbott
parent 2fb1199af5
commit 6750b02437
4 changed files with 66 additions and 16 deletions

View File

@ -81,7 +81,7 @@ export type ListWidget<Key, Item = Key> = BaseListWidget & {
get_insert_index: (list: Item[], item: Item) => number, get_insert_index: (list: Item[], item: Item) => number,
) => void; ) => void;
sort: (sorting_function: string, prop?: string) => void; sort: (sorting_function: string, prop?: string) => void;
replace_list_data: (list: Key[]) => void; replace_list_data: (list: Key[], should_redraw?: boolean) => void;
}; };
const DEFAULTS = { const DEFAULTS = {
@ -534,14 +534,16 @@ export function create<Key, Item = Key>(
widget.hard_redraw(); widget.hard_redraw();
}, },
replace_list_data(list) { replace_list_data(list, should_redraw = true) {
/* /*
We mostly use this widget for lists where you are We mostly use this widget for lists where you are
not adding or removing rows, so when you do modify not adding or removing rows, so when you do modify
the list, we have a brute force solution. the list, we have a brute force solution.
*/ */
meta.list = list; meta.list = list;
widget.hard_redraw(); if (should_redraw) {
widget.hard_redraw();
}
}, },
}; };

View File

@ -587,8 +587,8 @@ export function initialize(finished_initial_fetch) {
// Since `all_messages_data` contains continuous message history // Since `all_messages_data` contains continuous message history
// which always contains the latest message, it makes sense for // which always contains the latest message, it makes sense for
// Recent view to display the same data and be in sync. // Recent view to display the same data and be in sync.
all_messages_data.set_add_messages_callback((messages) => { all_messages_data.set_add_messages_callback((messages, rows_order_changed) => {
recent_view_ui.process_messages(messages, all_messages_data); recent_view_ui.process_messages(messages, rows_order_changed, all_messages_data);
}); });
// TODO: Ideally we'd have loading indicators for Recent Conversations // TODO: Ideally we'd have loading indicators for Recent Conversations

View File

@ -44,7 +44,7 @@ export class MessageListData {
_selected_id: number; _selected_id: number;
predicate?: (message: Message) => boolean; predicate?: (message: Message) => boolean;
// This is a callback that is called when messages are added to the message list. // This is a callback that is called when messages are added to the message list.
add_messages_callback?: (messages: Message[]) => void; add_messages_callback?: (messages: Message[], recipient_order_changed: boolean) => void;
// MessageListData is a core data structure for keeping track of a // MessageListData is a core data structure for keeping track of a
// contiguous block of messages matching a given narrow that can // contiguous block of messages matching a given narrow that can
@ -315,7 +315,14 @@ export class MessageListData {
} }
if (this.add_messages_callback) { if (this.add_messages_callback) {
this.add_messages_callback(messages); // If we only added old messages, last message for any of the existing recipients didn't change.
// Although, it maybe have resulted in new recipients being added which should be handled by the caller.
const recipient_order_changed = !(
top_messages.length > 0 &&
bottom_messages.length === 0 &&
interior_messages.length === 0
);
this.add_messages_callback(messages, recipient_order_changed);
} }
const info = { const info = {

View File

@ -464,14 +464,21 @@ export function hide_loading_indicator(): void {
loading.destroy_indicator($("#recent_view_loading_messages_indicator")); loading.destroy_indicator($("#recent_view_loading_messages_indicator"));
} }
export function process_messages(messages: Message[], msg_list_data?: MessageListData): void { export function process_messages(
messages: Message[],
rows_order_changed = true,
msg_list_data?: MessageListData,
): void {
// Always synced with messages in all_messages_data. // Always synced with messages in all_messages_data.
let conversation_data_updated = false; let conversation_data_updated = false;
const updated_rows = new Set<string>();
if (messages.length > 0) { if (messages.length > 0) {
for (const msg of messages) { for (const msg of messages) {
if (recent_view_data.process_message(msg)) { if (recent_view_data.process_message(msg)) {
conversation_data_updated = true; conversation_data_updated = true;
const key = recent_view_util.get_key_from_message(msg);
updated_rows.add(key);
} }
} }
} }
@ -484,7 +491,12 @@ export function process_messages(messages: Message[], msg_list_data?: MessageLis
// Only rerender if conversation data actually changed. // Only rerender if conversation data actually changed.
if (conversation_data_updated) { if (conversation_data_updated) {
complete_rerender(); if (!rows_order_changed) {
// If rows order didn't change, we can just rerender the affected rows.
bulk_inplace_rerender([...updated_rows]);
} else {
complete_rerender();
}
} }
} }
@ -882,7 +894,24 @@ export function filters_should_hide_row(topic_data: ConversationData): boolean {
return false; return false;
} }
export function inplace_rerender(topic_key: string): boolean { export function bulk_inplace_rerender(row_keys: string[]): void {
if (!topics_widget) {
return;
}
// When doing bulk rerender, we assume that order of rows are not going
// to change by default. Row insertion can still change the order but
// we ensure the list remains sorted after insertion.
topics_widget.replace_list_data(get_list_data_for_widget(), false);
topics_widget.filter_and_sort();
for (const key of row_keys) {
inplace_rerender(key, true);
}
setTimeout(revive_current_focus, 0);
}
export function inplace_rerender(topic_key: string, is_bulk_rerender?: boolean): boolean {
if (!recent_view_util.is_visible()) { if (!recent_view_util.is_visible()) {
return false; return false;
} }
@ -897,11 +926,17 @@ export function inplace_rerender(topic_key: string): boolean {
// if a topic is rendered since the `filtered_list` might have // if a topic is rendered since the `filtered_list` might have
// already been updated via other calls. // already been updated via other calls.
const is_topic_rendered = $topic_row.length; const is_topic_rendered = $topic_row.length;
// Resorting the topics_widget is important for the case where we
// are rerendering because of message editing or new messages
// arriving, since those operations often change the sort key.
assert(topics_widget !== undefined); assert(topics_widget !== undefined);
topics_widget.filter_and_sort(); if (!is_bulk_rerender) {
// Resorting the topics_widget is important for the case where we
// are rerendering because of message editing or new messages
// arriving, since those operations often change the sort key.
//
// NOTE: This doesn't add any new entry to the original list but updates the filtered list
// based on the current filters and updated row data.
topics_widget.filter_and_sort();
}
const current_topics_list = topics_widget.get_current_list(); const current_topics_list = topics_widget.get_current_list();
if (is_topic_rendered && filters_should_hide_row(topic_data)) { if (is_topic_rendered && filters_should_hide_row(topic_data)) {
// Since the row needs to be removed from DOM, we need to adjust `row_focus` // Since the row needs to be removed from DOM, we need to adjust `row_focus`
@ -932,7 +967,9 @@ export function inplace_rerender(topic_key: string): boolean {
), ),
); );
} }
setTimeout(revive_current_focus, 0); if (!is_bulk_rerender) {
setTimeout(revive_current_focus, 0);
}
return true; return true;
} }
@ -1205,13 +1242,17 @@ function filter_click_handler(
topics_widget.hard_redraw(); topics_widget.hard_redraw();
} }
function get_list_data_for_widget(): ConversationData[] {
return [...recent_view_data.get_conversations().values()];
}
export function complete_rerender(): void { export function complete_rerender(): void {
if (!recent_view_util.is_visible()) { if (!recent_view_util.is_visible()) {
return; return;
} }
// Show topics list // Show topics list
const mapped_topic_values = [...recent_view_data.get_conversations().values()]; const mapped_topic_values = get_list_data_for_widget();
if (topics_widget) { if (topics_widget) {
topics_widget.replace_list_data(mapped_topic_values); topics_widget.replace_list_data(mapped_topic_values);