paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-06 15:03:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

recent_topics: Improve behaviour of inplace rerender.

Browse Source 
We used to hide and show topic rows in the DOM when topics are
updated. This resulted in incorrect calculations in the length of
visible topics. As a consequence, focus is sometimes set to hidden topic.
Removing hidden topics from DOM helps us keep
the calculations correct.

The fixes bugs related to focus being lost when trying to mute
or mark as read the last row.
This commit is contained in:
Aman Agrawal
2022-10-29 13:55:36 +00:00
committed by Tim Abbott
parent ef067eafad
commit c41c94e36e
3 changed files with 68 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
static/js/recent_topics_ui.js
View File

@@ -498,15 +498,54 @@ export function inplace_rerender(topic_key) {
}
const topic_data = topics.get(topic_key);
topics_widget.render_item(topic_data);
const topic_row = get_topic_row(topic_data);
if (filters_should_hide_topic(topic_data)) {
topic_row.hide();
// 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.
topics_widget.filter_and_sort();
const current_topics_list = topics_widget.get_current_list();
if (topic_row.length && filters_should_hide_topic(topic_data)) {
const row_is_focused = get_focused_row_message().id === topic_data.last_msg_id;
if (row_is_focused && row_focus >= current_topics_list.length) {
row_focus = current_topics_list.length - 1;
}
topic_row.remove();
// We removed a rendered row, so we need to reduce one offset.
// TODO: This is correct, but a list_widget abstractions violation.
topics_widget.meta.offset -= 1;
} else if (!topic_row.length && filters_should_hide_topic(topic_data)) {
// In case `topic_row` is not present, our job is already done here
// since it has not been rendered yet and we already removed it from
// the filtered list in `topic_widget`. So, it won't be displayed in
// the future too.
} else if (topic_row.length && !filters_should_hide_topic(topic_data)) {
// Only a re-render is required in this case.
topics_widget.render_item(topic_data);
} else {
topic_row.show();
// Final case: !topic_row.length && !filters_should_hide_topic(topic_data).
if (current_topics_list.length <= 2) {
// Avoids edge cases for us and could be faster too.
topics_widget.clean_redraw();
revive_current_focus();
return true;
}
// We need to insert the row for it to be displayed at the
// correct position. current_topics_list must contain the
// topic_item, since we know !filters_should_hide_topic(topic_data).
const topic_insert_index = current_topics_list.findIndex(
(topic_item) => topic_item.last_msg_id === topic_data.last_msg_id,
);
// Rows greater than `offset` are not rendered in the DOM by list_widget;
// for those, there's nothing to update.
// TODO: This is correct, but a list_widget abstractions violation.
if (topic_insert_index <= topics_widget.meta.offset) {
const rendered_row = render_recent_topic_row(format_conversation(topic_data));
const $target_row = $(`#recent_topics_table table tbody tr:eq(${topic_insert_index})`);
$target_row.before(rendered_row);
topics_widget.meta.offset += 1;
}
}
revive_current_focus();
setTimeout(revive_current_focus, 0);
return true;
}