Fix a diff objective issue where objectives could appear in the wrong place

Summary:
Ref T12733. When creating a new comment, the objective could appear to far up in the scrollbar because we were anchoring it to an invisible row.

Instead, anchor to the "edit" row while editing.

Test Plan: Created a new comment at the very top of a file, saw "File, Star" icons instead of "Star, File".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17978

resources/celerity/map.php

@@ -13,7 +13,7 @@ return array(
     'core.pkg.js' => '599698a7',
     'darkconsole.pkg.js' => '1f9a31bc',
     'differential.pkg.css' => '7d4cfa59',
-    'differential.pkg.js' => 'f94e941c',
+    'differential.pkg.js' => 'fc6a23eb',
     'diffusion.pkg.css' => 'b93d9b8c',
     'diffusion.pkg.js' => '84c8f8fd',
     'favicon.ico' => '30672e08',
@@ -392,8 +392,8 @@ return array(
     'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63',
     'rsrc/js/application/diff/DiffChangeset.js' => 'cf4e2140',
     'rsrc/js/application/diff/DiffChangesetList.js' => 'a716ca27',
-    'rsrc/js/application/diff/DiffInline.js' => 'fa07d36e',
+    'rsrc/js/application/diff/DiffInline.js' => '93cbb03f',
-    'rsrc/js/application/diff/ScrollObjective.js' => '2e069f79',
+    'rsrc/js/application/diff/ScrollObjective.js' => '9df4e4e2',
     'rsrc/js/application/diff/ScrollObjectiveList.js' => '085dd101',
     'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
     'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07',
@@ -779,7 +779,7 @@ return array(
     'phabricator-dashboard-css' => 'fe5b1869',
     'phabricator-diff-changeset' => 'cf4e2140',
     'phabricator-diff-changeset-list' => 'a716ca27',
-    'phabricator-diff-inline' => 'fa07d36e',
+    'phabricator-diff-inline' => '93cbb03f',
     'phabricator-drag-and-drop-file-upload' => '58dea2fa',
     'phabricator-draggable-list' => 'bea6e7f4',
     'phabricator-fatal-config-template-css' => '8f18fa41',
@@ -799,7 +799,7 @@ return array(
     'phabricator-phtize' => 'd254d646',
     'phabricator-prefab' => 'c5af80a2',
     'phabricator-remarkup-css' => 'd1a5e11e',
-    'phabricator-scroll-objective' => '2e069f79',
+    'phabricator-scroll-objective' => '9df4e4e2',
     'phabricator-scroll-objective-list' => '085dd101',
     'phabricator-search-results-css' => 'f87d23ad',
     'phabricator-shaped-request' => '7cbe244b',
@@ -1113,13 +1113,6 @@ return array(
       'javelin-install',
       'javelin-event',
     ),
-    '2e069f79' => array(
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-install',
-      'javelin-workflow',
-    ),
     '2ee659ce' => array(
       'javelin-install',
     ),
@@ -1611,6 +1604,9 @@ return array(
       'javelin-stratcom',
       'javelin-dom',
     ),
+    '93cbb03f' => array(
+      'javelin-dom',
+    ),
     '93d0c9e3' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1675,6 +1671,13 @@ return array(
     '9d9685d6' => array(
       'phui-oi-list-view-css',
     ),
+    '9df4e4e2' => array(
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-install',
+      'javelin-workflow',
+    ),
     '9f36c42d' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -2206,9 +2209,6 @@ return array(
       'javelin-install',
       'javelin-dom',
     ),
-    'fa07d36e' => array(
-      'javelin-dom',
-    ),
     'fbe497e7' => array(
       'javelin-behavior',
       'javelin-util',

webroot/rsrc/js/application/diff/DiffInline.js

@@ -74,9 +74,10 @@ JX.install('DiffInline', {
 
       this._changesetID = data.changesetID;
 
-      this.updateObjective();
       this.setInvisible(false);
 
+      this.updateObjective();
+
       return this;
     },
 
@@ -166,9 +167,11 @@ JX.install('DiffInline', {
       this._changeset = changeset;
 
       var objectives = changeset.getChangesetList().getObjectives();
+
+      // Create this inline's objective, but don't show it yet.
       this._objective = objectives.newObjective()
-        .setCallback(JX.bind(this, this._onobjective));
+        .setCallback(JX.bind(this, this._onobjective))
-      this.updateObjective();
+        .hide();
 
       return this;
     },
@@ -206,11 +209,17 @@ JX.install('DiffInline', {
       var icon = 'fa-comment';
       var color = 'bluegrey';
       var tooltip = null;
+      var anchor = this._row;
 
       if (this._isEditing) {
         icon = 'fa-star';
         color = 'pink';
         tooltip = pht('Editing Comment');
+
+        // If we're editing, anchor to the row with the editor instead of the
+        // actual comment row (which is invisible and can have a misleading
+        // position).
+        anchor = this._row.nextSibling;
       } else if (this._isDraft) {
         // This inline is an unsubmitted draft.
         icon = 'fa-pencil';
@@ -225,6 +234,7 @@ JX.install('DiffInline', {
       }
 
       objective
+        .setAnchor(anchor)
         .setIcon(icon)
         .setColor(color)
         .setTooltip(tooltip)

webroot/rsrc/js/application/diff/ScrollObjective.js

@@ -111,6 +111,7 @@ JX.install('ScrollObjective', {
 
     hide: function() {
       this._visible = false;
+      return this;
     },
 
     isVisible: function() {