From e0557046f3e79d543b6672a173ab0a5711487bad Mon Sep 17 00:00:00 2001
From: Steve Howell <showell@zulipchat.com>
Date: Thu, 3 May 2018 12:34:29 +0000
Subject: [PATCH] refactor: Extract narrow.update_selection().

The maybe_select_closest helper, when first introduced, was
tiny and close to its callers.

As it's grown, it's become kind of a big hurdle to reading
narrow.activate(), because it's out of chronological order
and it's hard to tell at a glance which variables it's closing
on.

Now we just move it out to module scope.

It's mostly moving code, with these minor changes:

        * we pass in opts for the old closure vars
        * we rename then_select_offset -> select_offset
        * we early-exit on empty lists
---
 static/js/narrow.js | 77 ++++++++++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 33 deletions(-)

diff --git a/static/js/narrow.js b/static/js/narrow.js
index 9c9210bf6f..a43503fade 100644
--- a/static/js/narrow.js
+++ b/static/js/narrow.js
@@ -172,37 +172,6 @@ exports.activate = function (raw_operators, opts) {
     message_list.narrowed = msg_list;
     current_msg_list = message_list.narrowed;
 
-    function maybe_select_closest() {
-        if (! message_list.narrowed.empty()) {
-            var msg_id;
-
-            if (select_strategy.flavor === 'exact') {
-                msg_id = select_strategy.msg_id;
-            } else if (select_strategy.flavor === 'first_unread') {
-                msg_id = message_list.narrowed.first_unread_message_id();
-            }
-
-            var preserve_pre_narrowing_screen_position =
-                (message_list.narrowed.get(msg_id) !== undefined) &&
-                (then_select_offset !== undefined);
-
-            var then_scroll = !preserve_pre_narrowing_screen_position;
-
-            message_list.narrowed.select_id(msg_id, {then_scroll: then_scroll,
-                                                     use_closest: true,
-                                                     force_rerender: true,
-                                                    });
-
-            if (preserve_pre_narrowing_screen_position) {
-                // Scroll so that the selected message is in the same
-                // position in the viewport as it was prior to
-                // narrowing
-                message_list.narrowed.view.set_message_offset(then_select_offset);
-            }
-            unread_ops.process_visible();
-        }
-    }
-
     // Populate the message list if we can apply our filter locally (i.e.
     // with no backend help) and we have the message we want to select.
     if (narrow_state.get_current_filter().can_apply_locally()) {
@@ -236,7 +205,10 @@ exports.activate = function (raw_operators, opts) {
             use_first_unread_anchor: use_first_unread,
             cont: function () {
                 if (defer_selecting_closest) {
-                    maybe_select_closest();
+                    exports.update_selection({
+                        select_strategy: select_strategy,
+                        select_offset: then_select_offset,
+                    });
                 }
                 msg_list.network_time = new Date();
                 maybe_report_narrow_time(msg_list);
@@ -246,7 +218,10 @@ exports.activate = function (raw_operators, opts) {
 
     if (! defer_selecting_closest) {
         message_scroll.hide_indicators();
-        maybe_select_closest();
+        exports.update_selection({
+            select_strategy: select_strategy,
+            select_offset: then_select_offset,
+        });
     } else {
         message_scroll.show_loading_older();
     }
@@ -279,6 +254,42 @@ exports.activate = function (raw_operators, opts) {
     }, 0);
 };
 
+exports.update_selection = function (opts) {
+    if (message_list.narrowed.empty()) {
+        return;
+    }
+
+    var select_strategy = opts.select_strategy;
+    var select_offset = opts.select_offset;
+
+    var msg_id;
+
+    if (select_strategy.flavor === 'exact') {
+        msg_id = select_strategy.msg_id;
+    } else if (select_strategy.flavor === 'first_unread') {
+        msg_id = message_list.narrowed.first_unread_message_id();
+    }
+
+    var preserve_pre_narrowing_screen_position =
+        (message_list.narrowed.get(msg_id) !== undefined) &&
+        (select_offset !== undefined);
+
+    var then_scroll = !preserve_pre_narrowing_screen_position;
+
+    message_list.narrowed.select_id(msg_id, {then_scroll: then_scroll,
+                                             use_closest: true,
+                                             force_rerender: true,
+                                            });
+
+    if (preserve_pre_narrowing_screen_position) {
+        // Scroll so that the selected message is in the same
+        // position in the viewport as it was prior to
+        // narrowing
+        message_list.narrowed.view.set_message_offset(select_offset);
+    }
+    unread_ops.process_visible();
+};
+
 exports.stream_topic = function () {
     // This function returns the stream/topic that we most
     // specifically care about, according (mostly) to the