topic list: Use vdom techniques.

We avoid complicated code to update unread counts
by just using vdom.js.

One small change here is that if click on "more
topics", we replace it with the spinner instead
of putting the spinner after it.  This saves us
a redraw under the new scheme.
This commit is contained in:
Steve Howell 2020-01-31 19:15:46 +00:00 committed by Tim Abbott
parent 3a533dbe8f
commit f0e18b3b3e
5 changed files with 105 additions and 185 deletions

View File

@ -3,8 +3,8 @@ set_global('$', global.make_zjquery());
set_global('blueslip', global.make_zblueslip()); set_global('blueslip', global.make_zblueslip());
set_global('i18n', global.stub_i18n); set_global('i18n', global.stub_i18n);
const FoldDict = zrequire('fold_dict').FoldDict;
const IntDict = zrequire('int_dict').IntDict; const IntDict = zrequire('int_dict').IntDict;
zrequire('unread_ui'); zrequire('unread_ui');
zrequire('Filter', 'js/filter'); zrequire('Filter', 'js/filter');
zrequire('util'); zrequire('util');
@ -435,7 +435,7 @@ run_test('narrowing', () => {
}; };
stream_list.handle_narrow_deactivated(); stream_list.handle_narrow_deactivated();
assert.equal(removed_classes, 'active-filter active-sub-filter'); assert.equal(removed_classes, 'active-filter');
assert(topics_closed); assert(topics_closed);
}); });
@ -663,28 +663,6 @@ run_test('update_count_in_dom', () => {
stream_list.update_dom_with_unread_counts(counts); stream_list.update_dom_with_unread_counts(counts);
assert.equal($('<stream-value>').text(), '99'); assert.equal($('<stream-value>').text(), '99');
assert(stream_li.hasClass('stream-with-count')); assert(stream_li.hasClass('stream-with-count'));
let topic_results;
topic_list.set_count = function (stream_id, topic, count) {
topic_results = {
stream_id: stream_id,
topic: topic,
count: count,
};
};
const topic_count = new FoldDict();
topic_count.set('lunch', '555');
counts.topic_count.set(stream_id, topic_count);
stream_list.update_dom_with_unread_counts(counts);
assert.deepEqual(topic_results, {
stream_id: stream_id,
topic: 'lunch',
count: 555,
});
}); });
narrow_state.active = () => false; narrow_state.active = () => false;

View File

@ -320,26 +320,6 @@ exports.update_dom_with_unread_counts = function (counts) {
for (const [stream_id, count] of counts.stream_count) { for (const [stream_id, count] of counts.stream_count) {
set_stream_unread_count(stream_id, count); set_stream_unread_count(stream_id, count);
} }
// counts.topic_count maps streams to hashes of topics to counts
for (const [stream_id, topic_hash] of counts.topic_count) {
// Because the topic_list data structure doesn't keep track of
// which topics the "more topics" unread count came from, we
// need to compute the correct value from scratch here.
let more_topics_total = 0;
for (const [topic, count] of topic_hash) {
const in_more_topics = topic_list.set_count(stream_id, topic, count);
if (in_more_topics === true) {
more_topics_total += count;
}
}
if (topic_list.active_stream_id() === stream_id) {
// Update the "more topics" unread count; we communicate
// this to the `topic_list` library by passing `null` as
// the topic.
topic_list.set_count(stream_id, null, more_topics_total);
}
}
}; };
exports.rename_stream = function (sub) { exports.rename_stream = function (sub) {
@ -398,7 +378,7 @@ exports.get_sidebar_stream_topic_info = function (filter) {
}; };
function deselect_stream_items() { function deselect_stream_items() {
$("ul#stream_filters li").removeClass('active-filter active-sub-filter'); $("ul#stream_filters li").removeClass('active-filter');
} }
exports.update_stream_sidebar_for_narrow = function (filter) { exports.update_stream_sidebar_for_narrow = function (filter) {

View File

@ -1,7 +1,6 @@
const render_more_topics = require('../templates/more_topics.hbs'); const render_more_topics = require('../templates/more_topics.hbs');
const render_more_topics_spinner = require('../templates/more_topics_spinner.hbs'); const render_more_topics_spinner = require('../templates/more_topics_spinner.hbs');
const render_topic_list_item = require('../templates/topic_list_item.hbs'); const render_topic_list_item = require('../templates/topic_list_item.hbs');
const FoldDict = require('./fold_dict').FoldDict;
const IntDict = require('./int_dict').IntDict; const IntDict = require('./int_dict').IntDict;
const topic_list_data = require('./topic_list_data'); const topic_list_data = require('./topic_list_data');
@ -18,6 +17,12 @@ const active_widgets = new IntDict();
// We know whether we're zoomed or not. // We know whether we're zoomed or not.
let zoomed = false; let zoomed = false;
exports.update = function () {
for (const widget of active_widgets.values()) {
widget.build();
}
};
exports.remove_expanded_topics = function () { exports.remove_expanded_topics = function () {
stream_popover.hide_topic_popover(); stream_popover.hide_topic_popover();
@ -50,43 +55,73 @@ exports.zoom_out = function () {
exports.rebuild(parent_widget, stream_id); exports.rebuild(parent_widget, stream_id);
}; };
function update_unread_count(unread_count_elem, count) { exports.keyed_topic_li = (convo) => {
// unread_count_elem is a jquery element...we expect DOM const render = () => {
// to look like this: return render_topic_list_item(convo);
// <div class="topic-unread-count {{#if is_zero}}zero_count{{/if}}"> };
// <div class="value">{{unread}}</div>
// </div>
const value_span = unread_count_elem.find('.value');
if (value_span.length === 0) { const eq = (other) => {
blueslip.error('malformed dom for unread count'); return _.isEqual(convo, other.convo);
return; };
}
if (count === 0) { const key = 't:' + convo.topic_name;
unread_count_elem.addClass("zero_count");
value_span.text('');
} else {
unread_count_elem.removeClass("zero_count");
unread_count_elem.show();
value_span.text(count);
}
}
exports.set_count = function (stream_id, topic, count) { return {
const widget = active_widgets.get(stream_id); key: key,
render: render,
convo: convo,
eq: eq,
};
};
if (widget === undefined) { exports.more_li = (more_topics_unreads) => {
return false; const render = () => {
} return render_more_topics({
more_topics_unreads: more_topics_unreads,
});
};
return widget.set_count(topic, count); const eq = (other) => {
return other.more_items &&
more_topics_unreads === other.more_topics_unreads;
};
const key = 'more';
return {
key: key,
more_items: true,
more_topics_unreads: more_topics_unreads,
render: render,
eq: eq,
};
};
exports.spinner_li = () => {
const render = () => {
return render_more_topics_spinner();
};
const eq = (other) => {
return other.spinner;
};
const key = 'more';
return {
key: key,
spinner: true,
render: render,
eq: eq,
};
}; };
exports.widget = function (parent_elem, my_stream_id) { exports.widget = function (parent_elem, my_stream_id) {
const self = {}; const self = {};
self.build_list = function () { self.prior_dom = undefined;
self.build_list = function (spinner) {
const list_info = topic_list_data.get_list_info( const list_info = topic_list_data.get_list_info(
my_stream_id, zoomed); my_stream_id, zoomed);
@ -97,35 +132,25 @@ exports.widget = function (parent_elem, my_stream_id) {
list_info.items.length === num_possible_topics && list_info.items.length === num_possible_topics &&
topic_data.is_complete_for_stream_id(my_stream_id); topic_data.is_complete_for_stream_id(my_stream_id);
const ul = $('<ul class="topic-list">'); const attrs = [
['class', 'topic-list'],
];
self.topic_items = new FoldDict(); const nodes = _.map(
list_info.items, exports.keyed_topic_li);
// This is the main list of topics: if (spinner) {
// topic1 nodes.push(exports.spinner_li());
// topic2 } else if (!is_showing_all_possible_topics) {
// topic3 nodes.push(exports.more_li(more_topics_unreads));
_.each(list_info.items, (topic_info) => {
const li = $(render_topic_list_item(topic_info));
self.topic_items.set(topic_info.topic_name, li);
ul.append(li);
});
// Now, we decide whether we need to show the "more topics"
// widget. We need it if there are at least 5 topics in the
// frontend's cache, or if we (possibly) don't have all
// historical topics in the browser's cache.
if (!is_showing_all_possible_topics) {
const show_more_html = render_more_topics({
more_topics_unreads: more_topics_unreads,
});
ul.append($(show_more_html));
const spinner = render_more_topics_spinner();
ul.append($(spinner));
} }
return ul;
const dom = vdom.ul({
attrs: attrs,
keyed_nodes: nodes,
});
return dom;
}; };
self.get_parent = function () { self.get_parent = function () {
@ -136,94 +161,26 @@ exports.widget = function (parent_elem, my_stream_id) {
return my_stream_id; return my_stream_id;
}; };
self.get_dom = function () {
return self.dom;
};
self.remove = function () { self.remove = function () {
self.dom.remove(); parent_elem.find('.topic-list').remove();
self.prior_dom = undefined;
}; };
self.num_items = function () { self.build = function (spinner) {
return self.topic_items.size; const new_dom = self.build_list(spinner);
};
self.set_count = function (topic, count) { function replace_content(html) {
let unread_count_elem; self.remove();
if (topic === null) { parent_elem.append(html);
// null is used for updating the "more topics" count.
if (zoomed) {
return false;
}
const unread_count_parent = $(".show-more-topics");
if (unread_count_parent.length === 0) {
// If no show-more-topics element is present in the
// DOM, there are two possibilities. The most likely
// is that there are simply no unreads on that topic
// and there should continue to not be a "more topics"
// button; we can check this by looking at count.
if (count === 0) {
return false;
} }
// The alternative is that there is these new messages function find() {
// create the need for a "more topics" widget with a return parent_elem.find('.topic-list');
// nonzero unread count, and we need to create one and
// add it to the DOM.
//
// With our current implementation, this code path
// will always have its results overwritten shortly
// after, because (1) the can only happen when we just
// added unread counts, (not removing them), and (2)
// when learning about new (unread) messages,
// stream_list.update_dom_with_unread_count is always
// immediately followed by
// stream_list.update_streams_sidebar, which will
// rebuilds the topic list from scratch anyway.
//
// So this code mostly exists to document this corner
// case if in the future we adjust the model for
// managing unread counts. The code for updating this
// element would look something like the following:
//
// var show_more = self.build_more_topics_section(count);
// var topic_list_ul = exports.get_stream_li().find(".topic-list").expectOne();
// topic_list_ul.append(show_more);
return false;
}
unread_count_elem = unread_count_parent.find(".topic-unread-count");
update_unread_count(unread_count_elem, count);
return false;
} }
if (!self.topic_items.has(topic)) { vdom.update(replace_content, find, new_dom, self.prior_dom);
// `topic_li` may not exist if the topic is behind "more
// topics"; We need to update the "more topics" count
// instead in that case; we do this by returning true to
// notify the caller to accumulate these.
if (muting.is_topic_muted(my_stream_id, topic)) {
// But we don't count unreads in muted topics.
return false;
}
return true;
}
const topic_li = self.topic_items.get(topic); self.prior_dom = new_dom;
unread_count_elem = topic_li.find('.topic-unread-count');
update_unread_count(unread_count_elem, count);
return false;
};
self.show_spinner = function () {
// The spinner will go away once we get results and redraw
// the whole list.
const spinner = self.dom.find('.searching-for-more-topics');
spinner.show();
};
self.build = function () {
self.dom = self.build_list();
parent_elem.append(self.dom);
}; };
return self; return self;
@ -251,6 +208,13 @@ exports.get_stream_li = function () {
}; };
exports.rebuild = function (stream_li, stream_id) { exports.rebuild = function (stream_li, stream_id) {
const active_widget = active_widgets.get(stream_id);
if (active_widget) {
active_widget.build();
return;
}
exports.remove_expanded_topics(); exports.remove_expanded_topics();
const widget = exports.widget(stream_li, stream_id); const widget = exports.widget(stream_li, stream_id);
widget.build(); widget.build();
@ -286,13 +250,14 @@ exports.zoom_in = function () {
return; return;
} }
const widget = active_widgets.get(stream_id); active_widget.build();
exports.rebuild(widget.get_parent(), stream_id);
} }
ui.get_scroll_element($('#stream-filters-container')).scrollTop(0); ui.get_scroll_element($('#stream-filters-container')).scrollTop(0);
active_widget.show_spinner();
const spinner = true;
active_widget.build(spinner);
topic_data.get_server_history(stream_id, on_success); topic_data.get_server_history(stream_id, on_success);
}; };

View File

@ -51,6 +51,7 @@ exports.update_unread_counts = function () {
top_left_corner.update_dom_with_unread_counts(res); top_left_corner.update_dom_with_unread_counts(res);
stream_list.update_dom_with_unread_counts(res); stream_list.update_dom_with_unread_counts(res);
pm_list.update_dom_with_unread_counts(res); pm_list.update_dom_with_unread_counts(res);
topic_list.update();
notifications.update_pm_count(res.private_message_count); notifications.update_pm_count(res.private_message_count);
const notifiable_unread_count = unread.calculate_notifiable_count(res); const notifiable_unread_count = unread.calculate_notifiable_count(res);
notifications.update_title_count(notifiable_unread_count); notifications.update_title_count(notifiable_unread_count);

View File

@ -518,10 +518,6 @@ li.expanded_private_message a {
visibility: hidden; visibility: hidden;
} }
.searching-for-more-topics {
display: none;
}
.searching-for-more-topics img { .searching-for-more-topics img {
height: 16px; height: 16px;
margin-left: 6px; margin-left: 6px;