diff --git a/.eslintrc.json b/.eslintrc.json index 0f970db39f..2e414f1674 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -92,6 +92,7 @@ "notifications": false, "message_flags": false, "bot_data": false, + "stream_sort": false, "stream_list": false, "stream_popover": false, "narrow": false, diff --git a/frontend_tests/node_tests/stream_list.js b/frontend_tests/node_tests/stream_list.js index a23708cf48..df7ad68c5b 100644 --- a/frontend_tests/node_tests/stream_list.js +++ b/frontend_tests/node_tests/stream_list.js @@ -9,6 +9,7 @@ add_dependencies({ narrow: 'js/narrow', stream_color: 'js/stream_color', stream_data: 'js/stream_data', + stream_sort: 'js/stream_sort', subs: 'js/subs', templates: 'js/templates', unread: 'js/unread', @@ -204,6 +205,17 @@ function clear_filters() { stream_list.build_stream_list(); + var streams = global.stream_sort.get_streams().slice(0, 6); + + assert.deepEqual(streams, [ + 'devel', + 'Rome', + 'test', + 'Denmark', + 'social', + 'announce', + ]); + // verify pinned streams are still sorted by lowercase name // i.e. not affected by sort_recent set to true assert.equal(devel_li.next().find('[ data-name="Rome"]').length, 1); diff --git a/static/js/stream_list.js b/static/js/stream_list.js index 14b1c801a6..1e09fc7292 100644 --- a/static/js/stream_list.js +++ b/static/js/stream_list.js @@ -3,8 +3,6 @@ var stream_list = (function () { var exports = {}; var zoomed_stream = ''; -var previous_sort_order; -var previous_unpinned_order; function update_count_in_dom(unread_count_elem, count) { var count_span = unread_count_elem.find('.count'); @@ -27,32 +25,6 @@ function update_count_in_dom(unread_count_elem, count) { value_span.text(count); } -function filter_streams_by_search(streams) { - var search_box = $(".stream-list-filter"); - - var search_term = search_box.expectOne().val().trim(); - - if (search_term === '') { - return streams; - } - - var search_terms = search_term.toLowerCase().split(","); - search_terms = _.map(search_terms, function (s) { - return s.trim(); - }); - - var filtered_streams = _.filter(streams, function (stream) { - return _.any(search_terms, function (search_term) { - var lower_stream_name = stream.toLowerCase().split(" "); - return _.any(lower_stream_name, function (name) { - return name.indexOf(search_term) === 0; - }); - }); - }); - - return filtered_streams; -} - exports.stream_sidebar = (function () { var self = {}; @@ -93,11 +65,21 @@ exports.stream_sidebar = (function () { return self; }()); +function get_search_term() { + var search_box = $(".stream-list-filter"); + var search_term = search_box.expectOne().val().trim(); + return search_term; +} + exports.remove_sidebar_row = function (stream_id) { exports.stream_sidebar.remove_row(stream_id); - // We need to make sure we resort if the removed sub gets added again - previous_sort_order = undefined; - previous_unpinned_order = undefined; + + // We call sort_groups to update our list of streams, + // even though we don't use the result, since we can + // just remove the row. (The list of streams will + // be used by things like typeahead and hotkeys to + // advance to the next stream.) + stream_sort.sort_groups(get_search_term()); }; exports.create_initial_sidebar_rows = function () { @@ -121,63 +103,39 @@ exports.build_stream_list = function () { return; } - streams = filter_streams_by_search(streams); + // The main logic to build the list is in stream_sort.js, and + // we get three lists of streams (pinned/normal/dormant). + var stream_groups = stream_sort.sort_groups(get_search_term()); + + if (stream_groups.same_as_before) { + return; + } - var sort_recent = (streams.length > 40); - var pinned_streams = []; - var unpinned_streams = []; var parent = $('#stream_filters'); var elems = []; function add_sidebar_li(stream) { var sub = stream_data.get_sub(stream); var sidebar_row = exports.stream_sidebar.get_row(sub.stream_id); - if (sort_recent) { + if (stream_groups.sort_recent) { sidebar_row.update_whether_active(); } elems.push(sidebar_row.get_li().get(0)); } - _.each(streams, function (stream) { - var pinned = stream_data.get_sub(stream).pin_to_top; - if (pinned) { - pinned_streams.push(stream); - } else { - unpinned_streams.push(stream); - } - }); - - pinned_streams.sort(util.strcmp); - - unpinned_streams.sort(function (a, b) { - if (sort_recent) { - if (stream_data.is_active(b) && ! stream_data.is_active(a)) { - return 1; - } else if (! stream_data.is_active(b) && stream_data.is_active(a)) { - return -1; - } - } - return util.strcmp(a, b); - }); - - streams = pinned_streams.concat(unpinned_streams); - - if (previous_sort_order !== undefined && - util.array_compare(previous_sort_order, streams) && - util.array_compare(previous_unpinned_order, unpinned_streams)) { - return; - } - previous_sort_order = streams; - previous_unpinned_order = unpinned_streams; parent.empty(); - if (pinned_streams.length > 0) { - _.each(pinned_streams, add_sidebar_li); + _.each(stream_groups.pinned_streams, add_sidebar_li); + + if (stream_groups.pinned_streams.length > 0) { elems.push($('
').get(0)); } - if (unpinned_streams.length > 0) { - _.each(unpinned_streams, add_sidebar_li); - } + + _.each(stream_groups.normal_streams, add_sidebar_li); + + // TODO: Add a divider here if there are dormant streams. + + _.each(stream_groups.dormant_streams, add_sidebar_li); $(elems).appendTo(parent); }; diff --git a/static/js/stream_sort.js b/static/js/stream_sort.js new file mode 100644 index 0000000000..8f700650d2 --- /dev/null +++ b/static/js/stream_sort.js @@ -0,0 +1,98 @@ +var stream_sort = (function () { + +var exports = {}; + +var previous_pinned; +var previous_normal; +var previous_dormant; +var all_streams = []; + +exports.get_streams = function () { + // Right now this is only used for testing, but we should + // use it for things like hotkeys that cycle through streams. + return all_streams; +}; + +function filter_streams_by_search(streams, search_term) { + if (search_term === '') { + return streams; + } + + var search_terms = search_term.toLowerCase().split(","); + search_terms = _.map(search_terms, function (s) { + return s.trim(); + }); + + var filtered_streams = _.filter(streams, function (stream) { + return _.any(search_terms, function (search_term) { + var lower_stream_name = stream.toLowerCase().split(" "); + return _.any(lower_stream_name, function (name) { + return name.indexOf(search_term) === 0; + }); + }); + }); + + return filtered_streams; +} + +exports.sort_groups = function (search_term) { + var streams = stream_data.subscribed_streams(); + if (streams.length === 0) { + return; + } + + streams = filter_streams_by_search(streams, search_term); + + var sort_recent = (streams.length > 40); + + function is_normal(stream) { + return !sort_recent || stream_data.is_active(stream); + } + + var pinned_streams = []; + var normal_streams = []; + var dormant_streams = []; + + _.each(streams, function (stream) { + var pinned = stream_data.get_sub(stream).pin_to_top; + if (pinned) { + pinned_streams.push(stream); + } else if (is_normal(stream)) { + normal_streams.push(stream); + } else { + dormant_streams.push(stream); + } + }); + + pinned_streams.sort(util.strcmp); + normal_streams.sort(util.strcmp); + dormant_streams.sort(util.strcmp); + + var same_as_before = ( + previous_pinned !== undefined && + util.array_compare(previous_pinned, pinned_streams) && + util.array_compare(previous_normal, normal_streams) && + util.array_compare(previous_dormant, dormant_streams)); + + if (!same_as_before) { + previous_pinned = pinned_streams; + previous_normal = normal_streams; + previous_dormant = dormant_streams; + + all_streams = pinned_streams.concat(normal_streams, dormant_streams); + } + + return { + same_as_before: same_as_before, + pinned_streams: pinned_streams, + normal_streams: normal_streams, + dormant_streams: dormant_streams, + sort_recent: sort_recent, + }; +}; + +return exports; +}()); +if (typeof module !== 'undefined') { + module.exports = stream_sort; +} diff --git a/zproject/settings.py b/zproject/settings.py index e90124bdf1..a60e6d23e0 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -846,6 +846,7 @@ JS_SPECS = { 'js/unread.js', 'js/topic_list.js', 'js/pm_list.js', + 'js/stream_sort.js', 'js/stream_list.js', 'js/filter.js', 'js/message_list_view.js',