Mostly move inline editing to DiffInline

Summary:
Ref T12616. This doesn't pull over everything (some UI feedback didn't make it yet, and you can't cancel + undo cancelling edits yet) but editing comments technically works.

This is a little shaky, but feels less shaky than every other approach I've tried, so I think I'm finally on a reasonable track here.

Test Plan: Edited some inline comments.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

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

resources/celerity/map.php

@@ -13,7 +13,7 @@ return array(
     'core.pkg.js' => '2ff7879f',
     'darkconsole.pkg.js' => '1f9a31bc',
     'differential.pkg.css' => '58712637',
-    'differential.pkg.js' => '70685319',
+    'differential.pkg.js' => 'aa750623',
     'diffusion.pkg.css' => 'b93d9b8c',
     'diffusion.pkg.js' => '84c8f8fd',
     'favicon.ico' => '30672e08',
@@ -391,14 +391,14 @@ 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' => '80ac3298',
-    'rsrc/js/application/diff/DiffChangesetList.js' => 'a34b9821',
-    'rsrc/js/application/diff/DiffInline.js' => 'f9e76f2d',
+    'rsrc/js/application/diff/DiffChangesetList.js' => 'ecc9542e',
+    'rsrc/js/application/diff/DiffInline.js' => '586c15ff',
     'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
     'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '2e3f9738',
     'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d',
     'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76',
     'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
-    'rsrc/js/application/differential/behavior-edit-inline-comments.js' => 'e7e9551e',
+    'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '974bab6a',
     'rsrc/js/application/differential/behavior-keyboard-nav.js' => '92904457',
     'rsrc/js/application/differential/behavior-populate.js' => '5e41c819',
     'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb',
@@ -624,7 +624,7 @@ return array(
     'javelin-behavior-diff-preview-link' => '051c7832',
     'javelin-behavior-differential-comment-jump' => '4fdb476d',
     'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
-    'javelin-behavior-differential-edit-inline-comments' => 'e7e9551e',
+    'javelin-behavior-differential-edit-inline-comments' => '974bab6a',
     'javelin-behavior-differential-feedback-preview' => 'b064af76',
     'javelin-behavior-differential-keyboard-navigation' => '92904457',
     'javelin-behavior-differential-populate' => '5e41c819',
@@ -786,8 +786,8 @@ return array(
     'phabricator-darkmessage' => 'c48cccdd',
     'phabricator-dashboard-css' => 'fe5b1869',
     'phabricator-diff-changeset' => '80ac3298',
-    'phabricator-diff-changeset-list' => 'a34b9821',
-    'phabricator-diff-inline' => 'f9e76f2d',
+    'phabricator-diff-changeset-list' => 'ecc9542e',
+    'phabricator-diff-inline' => '586c15ff',
     'phabricator-drag-and-drop-file-upload' => '58dea2fa',
     'phabricator-draggable-list' => 'bea6e7f4',
     'phabricator-fatal-config-template-css' => '8f18fa41',
@@ -1347,6 +1347,9 @@ return array(
       'javelin-vector',
       'javelin-dom',
     ),
+    '586c15ff' => array(
+      'javelin-dom',
+    ),
     '58dea2fa' => array(
       'javelin-install',
       'javelin-util',
@@ -1672,6 +1675,14 @@ return array(
       'javelin-mask',
       'phabricator-drag-and-drop-file-upload',
     ),
+    '974bab6a' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-vector',
+      'differential-inline-comment-editor',
+    ),
     '988040b4' => array(
       'javelin-install',
       'javelin-dom',
@@ -1719,9 +1730,6 @@ return array(
       'javelin-dom',
       'javelin-reactor-dom',
     ),
-    'a34b9821' => array(
-      'javelin-install',
-    ),
     'a3a63478' => array(
       'phui-workcard-view-css',
     ),
@@ -2156,14 +2164,6 @@ return array(
       'javelin-workflow',
       'javelin-magical-init',
     ),
-    'e7e9551e' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-vector',
-      'differential-inline-comment-editor',
-    ),
     'e9581f08' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -2171,6 +2171,9 @@ return array(
       'javelin-dom',
       'phabricator-draggable-list',
     ),
+    'ecc9542e' => array(
+      'javelin-install',
+    ),
     'eded9ee8' => array(
       'javelin-behavior',
       'javelin-typeahead-ondemand-source',
@@ -2228,9 +2231,6 @@ return array(
       'javelin-install',
       'javelin-dom',
     ),
-    'f9e76f2d' => array(
-      'javelin-dom',
-    ),
     'fbe497e7' => array(
       'javelin-behavior',
       'javelin-util',

webroot/rsrc/js/application/diff/DiffChangesetList.js

@@ -23,6 +23,12 @@ JX.install('DiffChangesetList', {
 
     var onreveal = JX.bind(this, this._ifawake, this._onreveal);
     JX.Stratcom.listen('click', 'reveal-inline', onreveal);
+
+    var onedit = JX.bind(this, this._ifawake, this._onaction, 'edit');
+    JX.Stratcom.listen(
+      'click',
+      ['differential-inline-comment', 'differential-inline-edit'],
+      onedit);
   },
 
   properties: {
@@ -329,13 +335,32 @@ JX.install('DiffChangesetList', {
     _onhidereveal: function(e, is_hide) {
       e.kill();
 
+      var inline = this._getInlineForEvent(e);
+
+      inline.setHidden(is_hide);
+    },
+
+    _onaction: function(action, e) {
+      // TODO: This can become a kill once things fully switch over..
+      e.prevent();
+
+      var inline = this._getInlineForEvent(e);
+
+      // TODO: For normal operations, highlight the inline range here.
+
+      switch (action) {
+        case 'edit':
+          inline.edit();
+          break;
+      }
+    },
+
+    _getInlineForEvent: function(e) {
       var node = e.getNode('differential-changeset');
       var changeset = this.getChangesetForNode(node);
 
-      var inline_node = e.getNode('inline-row');
-      var inline = changeset.getInlineForRow(inline_node);
-
-      inline.setHidden(is_hide);
+      var inline_row = e.getNode('inline-row');
+      return changeset.getInlineForRow(inline_row);
     }
 
   }

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

@@ -9,13 +9,30 @@ JX.install('DiffInline', {
   construct : function(row) {
     this._row = row;
 
-    var data = JX.Stratcom.getData(row);
-    this._hidden = data.hidden || false;
+    var row_data = JX.Stratcom.getData(row);
+    this._hidden = row_data.hidden || false;
 
     // TODO: Get smarter about this once we do more editing, this is pretty
     // hacky.
     var comment = JX.DOM.find(row, 'div', 'differential-inline-comment');
-    this._id = JX.Stratcom.getData(comment).id;
+    var data = JX.Stratcom.getData(comment);
+
+    this._id = data.id;
+
+    // TODO: This is very, very, very, very, very, very, very hacky.
+    var td = comment.parentNode;
+    var th = td.previousSibling;
+    if (th.parentNode.firstChild != th) {
+      this._displaySide = 'right';
+    } else {
+      this._displaySide = 'left';
+    }
+
+    this._number = data.number;
+    this._length = data.length;
+    this._isNewFile =
+      (this.getDisplaySide() == 'right') ||
+      (data.left != data.right);
   },
 
   properties: {
@@ -26,6 +43,10 @@ JX.install('DiffInline', {
     _id: null,
     _row: null,
     _hidden: false,
+    _number: null,
+    _length: null,
+    _displaySide: null,
+    _isNewFile: null,
 
     setHidden: function(hidden) {
       this._hidden = hidden;
@@ -47,6 +68,159 @@ JX.install('DiffInline', {
         .start();
     },
 
+    edit: function() {
+      var handler = JX.bind(this, this._oneditresponse);
+      var uri = this.getChangeset().getChangesetList().getInlineURI();
+      var data = this._newRequestData();
+
+      // TODO: Set state to "loading".
+
+      new JX.Request(uri, handler)
+        .setData(data)
+        .send();
+    },
+
+    getDisplaySide: function() {
+      return this._displaySide;
+    },
+
+    getLineNumber: function() {
+      return this._number;
+    },
+
+    getLineLength: function() {
+      return this._length;
+    },
+
+    isNewFile: function() {
+      return this._isNewFile;
+    },
+
+    _newRequestData: function() {
+      return {
+        op: 'edit',
+        id: this._id,
+        on_right: ((this.getDisplaySide() == 'right') ? 1 : 0),
+        renderer: this.getChangeset().getRenderer(),
+        number: this.getLineNumber(),
+        length: this.getLineLength(),
+        is_new: this.isNewFile(),
+        replyToCommentPHID: ''
+      };
+    },
+
+    _oneditresponse: function(response) {
+      var rows = JX.$H(response).getNode();
+
+      this._drawEditRows(rows);
+
+      // TODO: Set the row state to "hidden".
+    },
+
+    _drawEditRows: function(rows) {
+      var first_row = JX.DOM.scry(rows, 'tr')[0];
+      var row = first_row;
+      var cursor = this._row;
+
+      while (row) {
+        cursor.parentNode.insertBefore(row, cursor.nextSibling);
+        cursor = row;
+
+        var row_meta = {
+          node: row,
+          type: 'edit',
+          listeners: []
+        };
+
+        row_meta.listeners.push(
+          JX.DOM.listen(
+            row,
+            ['submit', 'didSyntheticSubmit'],
+            'inline-edit-form',
+            JX.bind(this, this._onsubmit, row_meta)));
+
+        row_meta.listeners.push(
+          JX.DOM.listen(
+            row,
+            'click',
+            'inline-edit-cancel',
+            JX.bind(this, this._oncancel, row_meta)));
+
+        row = row.nextSibling;
+      }
+
+      return first_row;
+    },
+
+    _onsubmit: function(row, e) {
+      e.kill();
+
+      var handler = JX.bind(this, this._onsubmitresponse, row);
+
+      JX.Workflow.newFromForm(e.getTarget())
+        .setHandler(handler)
+        .start();
+
+      // TODO: Set state to "loading".
+    },
+
+    _oncancel: function(row, e) {
+      e.kill();
+
+      // TODO: Capture edited text and offer "undo".
+
+      JX.DOM.remove(row.node);
+      this._removeListeners(row.listeners);
+
+      // TODO: Restore state to "normal".
+    },
+
+    _onsubmitresponse: function(row, response) {
+
+     JX.DOM.remove(row.node);
+     this._removeListeners(row.listeners);
+
+     // TODO: Restore state to "normal".
+
+      this._onupdate(response);
+    },
+
+    _onupdate: function(response) {
+      var new_row;
+      if (response.markup) {
+        new_row = this._drawEditRows(JX.$H(response.markup).getNode());
+      }
+
+      // TODO: Save the old row so the action it's undo-able if it was a
+      // delete.
+      var remove_old = true;
+      if (remove_old) {
+        JX.DOM.remove(this._row);
+      }
+
+      this._row = new_row;
+
+      this._didUpdate();
+    },
+
+    _didUpdate: function() {
+      // After making changes to inline comments, refresh the transaction
+      // preview at the bottom of the page.
+
+      // TODO: This isn't the cleanest way to find the preview form, but
+      // rendering no longer has direct access to it.
+      var forms = JX.DOM.scry(document.body, 'form', 'transaction-append');
+      if (forms.length) {
+        JX.DOM.invoke(forms[0], 'shouldRefresh');
+      }
+    },
+
+    _removeListeners: function(listeners) {
+      for (var ii = 0; ii < listeners.length; ii++) {
+        listeners[ii].remove();
+      }
+    },
+
     _getChangesetList: function() {
       var changeset = this.getChangeset();
       return changeset.getChangesetList();

webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js

@@ -310,7 +310,9 @@ JX.behavior('differential-edit-inline-comments', function(config) {
     });
 
   var action_handler = function(op, e) {
-    e.kill();
+    // NOTE: We prevent this event, rather than killing it, because some
+    // actions are now handled by DiffChangesetList.
+    e.prevent();
 
     if (editor) {
       return;
@@ -392,20 +394,25 @@ JX.behavior('differential-edit-inline-comments', function(config) {
       'differential-changeset');
     var view = JX.DiffChangeset.getForNode(changeset_root);
 
-    editor = new JX.DifferentialInlineCommentEditor(config.uri)
-      .setTemplates(view.getUndoTemplates())
-      .setOperation(op)
-      .setID(data.id)
-      .setChangesetID(data.changesetID)
-      .setLineNumber(data.number)
-      .setLength(data.length)
-      .setOnRight(data.on_right)
-      .setOriginalText(original)
-      .setRow(row)
-      .setTable(row.parentNode)
-      .setReplyToCommentPHID(reply_phid)
-      .setRenderer(view.getRenderer())
-      .start();
+    if (op == 'edit') {
+      // This is now handled by DiffChangesetList.
+      editor = true;
+    } else {
+      editor = new JX.DifferentialInlineCommentEditor(config.uri)
+        .setTemplates(view.getUndoTemplates())
+        .setOperation(op)
+        .setID(data.id)
+        .setChangesetID(data.changesetID)
+        .setLineNumber(data.number)
+        .setLength(data.length)
+        .setOnRight(data.on_right)
+        .setOriginalText(original)
+        .setRow(row)
+        .setTable(row.parentNode)
+        .setReplyToCommentPHID(reply_phid)
+        .setRenderer(view.getRenderer())
+        .start();
+    }
 
     set_link_state(true);
   };