Remove 40-streams criterion for flagging dormant streams.

Before this change, we would move "dormant" streams to the bottom
of your stream sidebar, but only if you had 40+ streams.

Now we do this in all cases to be more consistent.

This commit also changes the redraw strategy when we remove rows.
Before this change, we were doing incremental updates, but now we
call build_stream_list to do a complete rebuild.  This was partly
motivated by adding the new divider, which would have complicated
the incrememental approach when you removed the last remaining
dormant stream.

frontend_tests/node_tests/stream_list.js

@@ -165,6 +165,24 @@ function clear_filters() {
 
     stream_list.build_stream_list();
 
+    global.stream_data.is_active = function (stream_name) {
+        return stream_name !== 'social';
+    };
+
+    var streams = global.stream_sort.get_streams().slice(0, 6);
+
+    assert.deepEqual(streams, [
+        // three groups: pinned, normal, dormant
+        'devel',
+        'Rome',
+        'test',
+        //
+        'announce',
+        'Denmark',
+        //
+        'social',
+    ]);
+
     // verify pinned streams are sorted by lowercase stream name
     var devel_li = stream_list.stream_sidebar.get_row(develSub.stream_id).get_li();
     assert.equal(devel_li.next().find('[ data-name="Rome"]').length, 1);
@@ -177,60 +195,4 @@ function clear_filters() {
     var Denmark_li = stream_list.stream_sidebar.get_row(DenmarkSub.stream_id).get_li();
     assert.equal(Denmark_li.next().find('[ data-name="social"]').length, 1);
 
-    // verify pinned streams are sorted before unpinned streams
-    // i.e. the last pinned stream (testSub) is before the first unpinned stream (announceSub)
-    var test_li = stream_list.stream_sidebar.get_row(testSub.stream_id).get_li();
-    assert.equal(test_li.nextAll().find('[ data-name="announce"]').length, 1);
-
-    // add another 40 dummy unpinned streams since sort_recent is set to true only when
-    // there are more than 40 subscribed streams
-    for (var i = 1; i <= 40; i += 1) {
-        var dummyName = 'dummy' + i;
-        var dummySub = {
-            name: dummyName,
-            stream_id: 7000 + i,
-            color: 'green',
-            id: 11 + i,
-            pin_to_top: false,
-            subscribed: true,
-        };
-        stream_list.create_sidebar_row(dummySub);
-        global.stream_data.add_sub(dummyName, dummySub);
-    }
-
-    stream_data.populate_stream_topics_for_tests(
-        // testSub is pinned, DenmarkSub and socialSub are unpinned
-        _.object([testSub.name, socialSub.name, DenmarkSub.name], [testSub, socialSub, DenmarkSub])
-    );
-
-    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);
-    assert.equal(Rome_li.next().find('[ data-name="test"]').length, 1);
-
-    // verify DenmarkSub and socialSub are sorted at the top of unpinned streams
-    // because they are active
-    assert.equal(Denmark_li.next().find('[ data-name="social"]').length, 1);
-    var social_li = stream_list.stream_sidebar.get_row(socialSub.stream_id).get_li();
-    assert.equal(social_li.next().find('[ data-name="announce"]').length, 1);
-
-    // verify inactive unpinned streams are still sorted by lowercase stream name
-    assert.equal(announce_li.next().find('[ data-name="dummy1"]').length, 1);
-
-    // verify pinned streams are still sorted before unpinned streams
-    // i.e. the last pinned stream (testSub) is before the first unpinned stream (socialSub)
-    assert.equal(test_li.nextAll().find('[ data-name="social"]').length, 1);
 }());

static/js/stream_list.js

@@ -43,21 +43,11 @@ exports.stream_sidebar = (function () {
     };
 
     self.remove_row = function (stream_id) {
-        var widget = self.rows.get(stream_id);
-        if (!widget) {
-            blueslip.warn('Cannot remove stream id ' + stream_id);
-            return;
-        }
-
-        widget.remove();
-
-        // This <hr> separates pinned streams from unpinned streams,
-        // so when removing a row, we check whether this removed the
-        // last pinned stream, and thus we no longer need the divider.
-        var $pinned_streams_hr = $("#stream_filters hr.pinned-stream-split");
-        if ($pinned_streams_hr.prev().length === 0) {
-            $pinned_streams_hr.remove();
-        }
+        // This only removes the row from our data structure.
+        // Our caller should use build_stream_list() to re-draw
+        // the sidebar, so that we don't have to deal with edge
+        // cases like removing the last pinned stream (and removing
+        // the divider).
 
         self.rows.del(stream_id);
     };
@@ -73,13 +63,7 @@ function get_search_term() {
 
 exports.remove_sidebar_row = function (stream_id) {
     exports.stream_sidebar.remove_row(stream_id);
-
-    // 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.build_stream_list();
 };
 
 exports.create_initial_sidebar_rows = function () {
@@ -117,9 +101,7 @@ exports.build_stream_list = function () {
     function add_sidebar_li(stream) {
         var sub = stream_data.get_sub(stream);
         var sidebar_row = exports.stream_sidebar.get_row(sub.stream_id);
-        if (stream_groups.sort_recent) {
-            sidebar_row.update_whether_active();
-        }
+        sidebar_row.update_whether_active();
         elems.push(sidebar_row.get_li().get(0));
     }
 
@@ -128,7 +110,7 @@ exports.build_stream_list = function () {
     _.each(stream_groups.pinned_streams, add_sidebar_li);
 
     if (stream_groups.pinned_streams.length > 0) {
-        elems.push($('<hr class="pinned-stream-split">').get(0));
+        elems.push($('<hr class="stream-split">').get(0));
     }
 
     _.each(stream_groups.normal_streams, add_sidebar_li);
@@ -166,7 +148,7 @@ function zoom_in() {
     $(".stream-filters-label").each(function () {
         $(this).hide();
     });
-    $(".pinned-stream-split").each(function () {
+    $(".stream-split").each(function () {
         $(this).hide();
     });
 
@@ -189,7 +171,7 @@ function zoom_out(options) {
     $(".stream-filters-label").each(function () {
         $(this).show();
     });
-    $(".pinned-stream-split").each(function () {
+    $(".stream-split").each(function () {
         $(this).show();
     });
 

static/js/stream_sort.js

@@ -43,10 +43,8 @@ exports.sort_groups = function (search_term) {
 
     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);
+        return stream_data.is_active(stream);
     }
 
     var pinned_streams = [];
@@ -87,7 +85,6 @@ exports.sort_groups = function (search_term) {
         pinned_streams: pinned_streams,
         normal_streams: normal_streams,
         dormant_streams: dormant_streams,
-        sort_recent: sort_recent,
     };
 };