From ab80d3ef0c6202c760e18ce14c2fa7b1b898ece1 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 20 May 2017 04:27:21 -0700
Subject: [PATCH] (stable) For the diff banner, detect the current changeset
 better

Summary:
Ref T12733. Currently, we detect the changeset which is in the middle of the screen as the current changeset.

This doesn't always get us the most intuitive changeset, particularly after a navigation from the scroll objective list: when you jump to changeset "X", you'd tend to expect "X" to be shown in the header, but the //next// changeset may be shown if "X" is too short.

Instead, select the changeset near the top of the screen (spanning an invisible line slightly below the banner).

Test Plan: Scrolled and jumped through a document with long and short changesets, saw a more intuitive changeset selected by the banner.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17976
---
 resources/celerity/map.php                       | 14 +++++++-------
 .../js/application/diff/DiffChangesetList.js     | 16 ++++++++++------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index e50d8bbd1a..019856296b 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -13,7 +13,7 @@ return array(
     'core.pkg.js' => 'e822b496',
     'darkconsole.pkg.js' => '1f9a31bc',
     'differential.pkg.css' => '4d7dd14e',
-    'differential.pkg.js' => '68a4fa60',
+    'differential.pkg.js' => '6d05ad4c',
     'diffusion.pkg.css' => 'b93d9b8c',
     'diffusion.pkg.js' => '84c8f8fd',
     'favicon.ico' => '30672e08',
@@ -391,7 +391,7 @@ return array(
     'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375',
     'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63',
     'rsrc/js/application/diff/DiffChangeset.js' => 'cf4e2140',
-    'rsrc/js/application/diff/DiffChangesetList.js' => '5c68c40c',
+    'rsrc/js/application/diff/DiffChangesetList.js' => '541206ba',
     'rsrc/js/application/diff/DiffInline.js' => '77e14b60',
     'rsrc/js/application/diff/ScrollObjective.js' => '0eee7a00',
     'rsrc/js/application/diff/ScrollObjectiveList.js' => '1ca4d9db',
@@ -778,7 +778,7 @@ return array(
     'phabricator-darkmessage' => 'c48cccdd',
     'phabricator-dashboard-css' => 'fe5b1869',
     'phabricator-diff-changeset' => 'cf4e2140',
-    'phabricator-diff-changeset-list' => '5c68c40c',
+    'phabricator-diff-changeset-list' => '541206ba',
     'phabricator-diff-inline' => '77e14b60',
     'phabricator-drag-and-drop-file-upload' => '58dea2fa',
     'phabricator-draggable-list' => 'bea6e7f4',
@@ -1316,6 +1316,10 @@ return array(
     '5294060f' => array(
       'phui-theme-css',
     ),
+    '541206ba' => array(
+      'javelin-install',
+      'phabricator-scroll-objective-list',
+    ),
     '54774a28' => array(
       'phui-inline-comment-view-css',
     ),
@@ -1368,10 +1372,6 @@ return array(
       'javelin-stratcom',
       'javelin-dom',
     ),
-    '5c68c40c' => array(
-      'javelin-install',
-      'phabricator-scroll-objective-list',
-    ),
     '5e2634b9' => array(
       'javelin-behavior',
       'javelin-aphlict',
diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js
index e256372852..b615ca79eb 100644
--- a/webroot/rsrc/js/application/diff/DiffChangesetList.js
+++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js
@@ -1358,16 +1358,20 @@ JX.install('DiffChangesetList', {
         return null;
       }
 
-      var v = JX.Vector.getViewport();
+      // We're going to find the changeset which spans an invisible line a
+      // little underneath the bottom of the banner. This makes the header
+      // tick over from "A.txt" to "B.txt" just as "A.txt" scrolls completely
+      // offscreen.
+      var detect_height = 64;
+
       for (var ii = 0; ii < this._changesets.length; ii++) {
         var changeset = this._changesets[ii];
         var c = changeset.getVectors();
 
-        // If the changeset starts above the upper half of the screen...
-        if (c.pos.y < (s.y + (v.y / 2))) {
-          // ...and ends below the lower half of the screen, this is the
-          // current visible changeset.
-          if ((c.pos.y + c.dim.y) > (s.y + (v.y / 2))) {
+        // If the changeset starts above the line...
+        if (c.pos.y <= (s.y + detect_height)) {
+          // ...and ends below the line, this is the current visible changeset.
+          if ((c.pos.y + c.dim.y) >= (s.y + detect_height)) {
             return changeset;
           }
         }