diff --git a/frontend_tests/node_tests/recent_topics.js b/frontend_tests/node_tests/recent_topics.js index 5b490380bf..14bda58e15 100644 --- a/frontend_tests/node_tests/recent_topics.js +++ b/frontend_tests/node_tests/recent_topics.js @@ -63,7 +63,7 @@ const ListWidget = mock_esm("../../static/js/list_widget", { }, hard_redraw: noop, - render_item: (item) => ListWidget.modifier(item), + filter_and_sort: noop, replace_list_data: (data) => { assert.notEqual( expected_data_to_replace_in_list_widget, @@ -111,9 +111,6 @@ mock_esm("../../static/js/pm_list", { update_private_messages: noop, handle_narrow_deactivated: noop, }); -mock_esm("../../static/js/popovers", { - any_active: () => false, -}); mock_esm("../../static/js/recent_senders", { get_topic_recent_senders: () => [1, 2], }); @@ -141,6 +138,7 @@ mock_esm("../../static/js/sub_store", { color: "", invite_only: false, is_web_public: true, + subscribed: true, }; }, }); @@ -425,7 +423,10 @@ test("test_filter_all", ({mock_template}) => { i = row_data.length; rt.set_default_focus(); $(".home-page-input").trigger("focus"); - assert.equal(rt.inplace_rerender("1:topic-1"), true); + assert.equal( + rt.filters_should_hide_topic({last_msg_id: 1, participated: true, type: "stream"}), + false, + ); }); test("test_filter_unread", ({mock_template}) => { @@ -479,7 +480,10 @@ test("test_filter_unread", ({mock_template}) => { stub_out_filter_buttons(); rt.process_messages(messages); $(".home-page-input").trigger("focus"); - assert.equal(rt.inplace_rerender("1:topic-1"), true); + assert.equal( + rt.filters_should_hide_topic({last_msg_id: 1, participated: true, type: "stream"}), + false, + ); $("#recent_topics_filter_buttons").removeClass("btn-recent-selected"); @@ -598,11 +602,17 @@ test("test_filter_participated", ({mock_template}) => { rt.process_messages(messages); $(".home-page-input").trigger("focus"); - assert.equal(rt.inplace_rerender("1:topic-4"), true); + assert.equal( + rt.filters_should_hide_topic({last_msg_id: 4, participated: true, type: "stream"}), + false, + ); // Set muted filter rt.set_filter("muted"); - assert.equal(rt.inplace_rerender("1:topic-7"), true); + assert.equal( + rt.filters_should_hide_topic({last_msg_id: 7, participated: true, type: "stream"}), + false, + ); // remove muted filter rt.set_filter("muted"); @@ -677,7 +687,8 @@ test("test_update_unread_count", () => { rt.update_topic_unread_count(messages[9]); }); -test("basic assertions", ({mock_template}) => { +test("basic assertions", ({mock_template, override_rewire}) => { + override_rewire(rt, "inplace_rerender", noop); rt.clear_for_tests(); mock_template("recent_topics_table.hbs", false, () => {}); diff --git a/static/js/list_widget.js b/static/js/list_widget.js index 90ba95c07c..cd9254fffa 100644 --- a/static/js/list_widget.js +++ b/static/js/list_widget.js @@ -172,7 +172,9 @@ export function create($container, list, opts) { return undefined; } - const widget = {}; + const widget = { + meta, + }; widget.get_current_list = function () { return meta.filtered_list; diff --git a/static/js/recent_topics_ui.js b/static/js/recent_topics_ui.js index 9356cf6327..8b1d2ed4f2 100644 --- a/static/js/recent_topics_ui.js +++ b/static/js/recent_topics_ui.js @@ -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; }