Move inline "done" checkboxing to DiffInline

Summary:
Ref T12616. This updates clicking the "Done" checkbox for the new stuff.

This one is pretty clean since the "Done" checkbox doesn't do too much weird magic.

Test Plan: Clicked the box a few times.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

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

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' => 'aa750623',
+    'differential.pkg.js' => 'e01579c4',
     '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' => 'ecc9542e',
-    'rsrc/js/application/diff/DiffInline.js' => '586c15ff',
+    'rsrc/js/application/diff/DiffChangesetList.js' => '12575699',
+    'rsrc/js/application/diff/DiffInline.js' => 'bd1b3258',
     'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
-    'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '2e3f9738',
+    'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '9ed8d2b6',
     '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' => '974bab6a',
+    'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '97f363fc',
     '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',
@@ -572,7 +572,7 @@ return array(
     'd3' => 'a11a5ff2',
     'differential-changeset-view-css' => '41af6d25',
     'differential-core-view-css' => '5b7b8ff4',
-    'differential-inline-comment-editor' => '2e3f9738',
+    'differential-inline-comment-editor' => '9ed8d2b6',
     'differential-revision-add-comment-css' => 'c47f8c40',
     'differential-revision-comment-css' => '14b8565a',
     'differential-revision-history-css' => '0e8eb855',
@@ -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' => '974bab6a',
+    'javelin-behavior-differential-edit-inline-comments' => '97f363fc',
     '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' => 'ecc9542e',
-    'phabricator-diff-inline' => '586c15ff',
+    'phabricator-diff-changeset-list' => '12575699',
+    'phabricator-diff-inline' => 'bd1b3258',
     'phabricator-drag-and-drop-file-upload' => '58dea2fa',
     'phabricator-draggable-list' => 'bea6e7f4',
     'phabricator-fatal-config-template-css' => '8f18fa41',
@@ -1000,6 +1000,9 @@ return array(
       'javelin-dom',
       'javelin-typeahead-normalizer',
     ),
+    12575699 => array(
+      'javelin-install',
+    ),
     '1499a8cb' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1115,14 +1118,6 @@ return array(
       'javelin-install',
       'javelin-event',
     ),
-    '2e3f9738' => array(
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-install',
-      'javelin-request',
-      'javelin-workflow',
-    ),
     '2ee659ce' => array(
       'javelin-install',
     ),
@@ -1347,9 +1342,6 @@ return array(
       'javelin-vector',
       'javelin-dom',
     ),
-    '586c15ff' => array(
-      'javelin-dom',
-    ),
     '58dea2fa' => array(
       'javelin-install',
       'javelin-util',
@@ -1675,7 +1667,7 @@ return array(
       'javelin-mask',
       'phabricator-drag-and-drop-file-upload',
     ),
-    '974bab6a' => array(
+    '97f363fc' => array(
       'javelin-behavior',
       'javelin-stratcom',
       'javelin-dom',
@@ -1711,6 +1703,14 @@ return array(
     '9d9685d6' => array(
       'phui-oi-list-view-css',
     ),
+    '9ed8d2b6' => array(
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-install',
+      'javelin-request',
+      'javelin-workflow',
+    ),
     '9f36c42d' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1910,6 +1910,9 @@ return array(
       'javelin-vector',
       'phui-hovercard',
     ),
+    'bd1b3258' => array(
+      'javelin-dom',
+    ),
     'bdaf4d04' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -2171,9 +2174,6 @@ return array(
       'javelin-dom',
       'phabricator-draggable-list',
     ),
-    'ecc9542e' => array(
-      'javelin-install',
-    ),
     'eded9ee8' => array(
       'javelin-behavior',
       'javelin-typeahead-ondemand-source',

src/infrastructure/diff/PhabricatorInlineCommentController.php

@@ -130,12 +130,14 @@ abstract class PhabricatorInlineCommentController
         $inline = $this->loadCommentForDone($this->getCommentID());
 
         $is_draft_state = false;
+        $is_checked = false;
         switch ($inline->getFixedState()) {
           case PhabricatorInlineCommentInterface::STATE_DRAFT:
             $next_state = PhabricatorInlineCommentInterface::STATE_UNDONE;
             break;
           case PhabricatorInlineCommentInterface::STATE_UNDRAFT:
             $next_state = PhabricatorInlineCommentInterface::STATE_DONE;
+            $is_checked = true;
             break;
           case PhabricatorInlineCommentInterface::STATE_DONE:
             $next_state = PhabricatorInlineCommentInterface::STATE_UNDRAFT;
@@ -145,6 +147,7 @@ abstract class PhabricatorInlineCommentController
           case PhabricatorInlineCommentInterface::STATE_UNDONE:
             $next_state = PhabricatorInlineCommentInterface::STATE_DRAFT;
             $is_draft_state = true;
+            $is_checked = true;
             break;
         }
 
@@ -153,6 +156,7 @@ abstract class PhabricatorInlineCommentController
         return id(new AphrontAjaxResponse())
           ->setContent(
             array(
+              'isChecked' => $is_checked,
               'draftState' => $is_draft_state,
             ));
       case 'delete':

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

@@ -29,6 +29,12 @@ JX.install('DiffChangesetList', {
       'click',
       ['differential-inline-comment', 'differential-inline-edit'],
       onedit);
+
+    var ondone = JX.bind(this, this._ifawake, this._onaction, 'done');
+    JX.Stratcom.listen(
+      'click',
+      ['differential-inline-comment', 'differential-inline-done'],
+      ondone);
   },
 
   properties: {
@@ -352,6 +358,9 @@ JX.install('DiffChangesetList', {
         case 'edit':
           inline.edit();
           break;
+        case 'done':
+          inline.toggleDone();
+          break;
       }
     },
 

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

@@ -60,7 +60,7 @@ JX.install('DiffInline', {
         op = 'show';
       }
 
-      var inline_uri = this._getChangesetList().getInlineURI();
+      var inline_uri = this._getInlineURI();
       var comment_id = this._id;
 
       new JX.Workflow(inline_uri, {op: op, ids: comment_id})
@@ -68,9 +68,46 @@ JX.install('DiffInline', {
         .start();
     },
 
+    toggleDone: function() {
+      var uri = this._getInlineURI();
+      var data = {
+        op: 'done',
+        id: this._id
+      };
+
+      var ondone = JX.bind(this, this._ondone);
+
+      new JX.Workflow(uri, data)
+        .setHandler(ondone)
+        .start();
+    },
+
+    _ondone: function(response) {
+      var checkbox = JX.DOM.find(
+        this._row,
+        'input',
+        'differential-inline-done');
+
+      checkbox.checked = (response.isChecked ? 'checked' : null);
+
+      var comment = JX.DOM.findAbove(
+        checkbox,
+        'div',
+        'differential-inline-comment');
+
+      JX.DOM.alterClass(comment, 'inline-is-done', response.isChecked);
+
+      // NOTE: This is marking the inline as having an unsubmitted checkmark,
+      // as opposed to a submitted checkmark. This is different from the
+      // top-level "draft" state of unsubmitted comments.
+      JX.DOM.alterClass(comment, 'inline-state-is-draft', response.draftState);
+
+      this._didUpdate();
+    },
+
     edit: function() {
       var handler = JX.bind(this, this._oneditresponse);
-      var uri = this.getChangeset().getChangesetList().getInlineURI();
+      var uri = this._getInlineURI();
       var data = this._newRequestData();
 
       // TODO: Set state to "loading".
@@ -221,9 +258,10 @@ JX.install('DiffInline', {
       }
     },
 
-    _getChangesetList: function() {
+    _getInlineURI: function() {
       var changeset = this.getChangeset();
-      return changeset.getChangesetList();
+      var list = changeset.getChangesetList();
+      return list.getInlineURI();
     }
   }
 

webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js

@@ -312,28 +312,6 @@ JX.install('DifferentialInlineCommentEditor', {
         .start();
     },
 
-    toggleCheckbox: function(id, checkbox) {
-      var data = {
-        op: 'done',
-        id: id
-      };
-
-      new JX.Workflow(this._uri, data)
-        .setHandler(JX.bind(this, function(r) {
-          checkbox.checked = !checkbox.checked;
-
-          var comment = JX.DOM.findAbove(
-            checkbox,
-            'div',
-            'differential-inline-comment');
-          JX.DOM.alterClass(comment, 'inline-is-done', !!checkbox.checked);
-          JX.DOM.alterClass(comment, 'inline-state-is-draft', r.draftState);
-
-          this._didUpdate();
-        }))
-        .start();
-    },
-
     _didUpdate: function() {
       // After making changes to inline comments, refresh the transaction
       // preview at the bottom of the page.

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

@@ -371,13 +371,6 @@ JX.behavior('differential-edit-inline-comments', function(config) {
       }
     }
 
-    if (op == 'done') {
-      var checkbox = JX.DOM.find(node, 'input', 'differential-inline-done');
-      new JX.DifferentialInlineCommentEditor(config.uri)
-        .toggleCheckbox(data.id, checkbox);
-      return;
-    }
-
     var original = data.original;
     var reply_phid = null;
     if (op == 'reply') {
@@ -417,7 +410,7 @@ JX.behavior('differential-edit-inline-comments', function(config) {
     set_link_state(true);
   };
 
-  for (var op in {'edit': 1, 'delete': 1, 'reply': 1, 'done': 1}) {
+  for (var op in {'edit': 1, 'delete': 1, 'reply': 1}) {
     JX.Stratcom.listen(
       'click',
       ['differential-inline-comment', 'differential-inline-' + op],