From 9dd78c803cddef4d9ee3f03eaafa6d888bc71925 Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulipchat.com>
Date: Thu, 10 Aug 2017 10:35:48 -0400
Subject: [PATCH] Make auto-scrolling less aggressive.

We have code that can automatically scroll an element into "view"
in its container.  We use this for stream sidebar rows inside the
stream list.

Generally the stream sidebar rows are small enough to fit into
the container, and the prior algorithm worked correctly for that
scenario.

If you have lots of topics, however, and a short screen, the
algorithm was being too aggressive.  For example, if the top
wasn't showing, it would scroll the top into view, but at the
cost of scrolling the bottom out of view.

This fix makes the general scrolling algorithm more tame.

Part of the user-facing problem is that the element we pass
into the scrolling code for the stream sidebar rows is bigger
than the part of the row that actually should be shown on
screen.  Nevertheless, it makes sense here to make the general
algorithm more robust.
---
 frontend_tests/node_tests/stream_list.js | 18 ++++++++++++++++++
 static/js/stream_list.js                 | 12 ++++++++++--
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/frontend_tests/node_tests/stream_list.js b/frontend_tests/node_tests/stream_list.js
index a8a6d2ccc6..f3f3295be6 100644
--- a/frontend_tests/node_tests/stream_list.js
+++ b/frontend_tests/node_tests/stream_list.js
@@ -481,6 +481,12 @@ function initialize_stream_data() {
         container_height: 10,
     }));
 
+    assert.equal(0, stream_list.scroll_delta({
+        elem_top: -5,
+        elem_bottom: 15,
+        container_height: 10,
+    }));
+
     // The top is offscreen.
     assert.equal(-3, stream_list.scroll_delta({
         elem_top: -3,
@@ -494,6 +500,12 @@ function initialize_stream_data() {
         container_height: 10,
     }));
 
+    assert.equal(-11, stream_list.scroll_delta({
+        elem_top: -150,
+        elem_bottom: -1,
+        container_height: 10,
+    }));
+
     // The bottom is offscreen.
     assert.equal(3, stream_list.scroll_delta({
         elem_top: 7,
@@ -507,4 +519,10 @@ function initialize_stream_data() {
         container_height: 10,
     }));
 
+    assert.equal(11, stream_list.scroll_delta({
+        elem_top: 11,
+        elem_bottom: 99,
+        container_height: 10,
+    }));
+
 }());
diff --git a/static/js/stream_list.js b/static/js/stream_list.js
index 989917cb51..198abbd6c7 100644
--- a/static/js/stream_list.js
+++ b/static/js/stream_list.js
@@ -616,10 +616,18 @@ exports.scroll_delta = function (opts) {
     var delta = 0;
 
     if (elem_top < 0) {
-        delta = elem_top;
+        delta = Math.max(
+            elem_top,
+            elem_bottom - container_height
+        );
+        delta = Math.min(0, delta);
     } else {
         if (elem_bottom > container_height) {
-            delta = elem_bottom - container_height;
+            delta = Math.min(
+                elem_top,
+                elem_bottom - container_height
+            );
+            delta = Math.max(0, delta);
         }
     }