Transactions - make edit transactions that are grouped work nicely

Summary: ...the key is to move a layer lower and beam down the updated comment. There is a wee bit of Javascript gymnastics going on here. Fixes T4608.

Test Plan: made a comment + resolve. clicked edit and made changes. noted transaction updated correctly and "history" link worked. edited again to a deletion and noted the "this is deleted" looked right and history link still worked

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin, chad

Maniphest Tasks: T4608

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

resources/celerity/map.php

@@ -8,7 +8,7 @@ return array(
   'names' =>
   array(
     'core.pkg.css' => 'ab5dd633',
-    'core.pkg.js' => '57d0e1c4',
+    'core.pkg.js' => '68839013',
     'darkconsole.pkg.js' => 'ca8671ce',
     'differential.pkg.css' => '9578e86b',
     'differential.pkg.js' => '11a5b750',
@@ -83,7 +83,7 @@ return array(
     'rsrc/css/application/phame/phame.css' => '450826e1',
     'rsrc/css/application/pholio/pholio-edit.css' => 'b9e59b6d',
     'rsrc/css/application/pholio/pholio-inline-comments.css' => '52be33f0',
-    'rsrc/css/application/pholio/pholio.css' => 'd23ace50',
+    'rsrc/css/application/pholio/pholio.css' => '2fa97dbe',
     'rsrc/css/application/phortune/phortune-credit-card-form.css' => 'b25b4beb',
     'rsrc/css/application/phrequent/phrequent.css' => 'ffc185ad',
     'rsrc/css/application/phriction/phriction-document-css.css' => '7d7f0071',
@@ -147,7 +147,7 @@ return array(
     'rsrc/css/phui/phui-status.css' => '2f562399',
     'rsrc/css/phui/phui-tag-view.css' => '295d81c4',
     'rsrc/css/phui/phui-text.css' => '23e9b4b7',
-    'rsrc/css/phui/phui-timeline-view.css' => '7dcd5354',
+    'rsrc/css/phui/phui-timeline-view.css' => '23fe340a',
     'rsrc/css/phui/phui-workboard-view.css' => 'bf70dd2e',
     'rsrc/css/phui/phui-workpanel-view.css' => '97b69459',
     'rsrc/css/sprite-actions.css' => '969ad0e5',
@@ -412,7 +412,7 @@ return array(
     'rsrc/js/application/search/behavior-reorder-queries.js' => '37871df4',
     'rsrc/js/application/slowvote/behavior-slowvote-embed.js' => 'a51fdb2e',
     'rsrc/js/application/transactions/behavior-transaction-comment-form.js' => '9084a36f',
-    'rsrc/js/application/transactions/behavior-transaction-list.js' => '3c918aa8',
+    'rsrc/js/application/transactions/behavior-transaction-list.js' => '925c9bab',
     'rsrc/js/application/uiexample/JavelinViewExample.js' => 'd4a14807',
     'rsrc/js/application/uiexample/ReactorButtonExample.js' => '44524435',
     'rsrc/js/application/uiexample/ReactorCheckboxExample.js' => '7ba325ee',
@@ -451,7 +451,7 @@ return array(
     'rsrc/js/core/behavior-error-log.js' => 'a5d7cf86',
     'rsrc/js/core/behavior-fancy-datepicker.js' => '5d584426',
     'rsrc/js/core/behavior-file-tree.js' => 'c8728c70',
-    'rsrc/js/core/behavior-form.js' => '27d4da3f',
+    'rsrc/js/core/behavior-form.js' => 'a9aaba0c',
     'rsrc/js/core/behavior-gesture.js' => 'fe2e0ba4',
     'rsrc/js/core/behavior-global-drag-and-drop.js' => '8fd76bab',
     'rsrc/js/core/behavior-history-install.js' => '7ee2b591',
@@ -532,7 +532,7 @@ return array(
     'javelin-behavior-aphront-basic-tokenizer' => 'b3a4b884',
     'javelin-behavior-aphront-crop' => 'b98fc918',
     'javelin-behavior-aphront-drag-and-drop-textarea' => '4a11ea9c',
-    'javelin-behavior-aphront-form-disable-on-submit' => '27d4da3f',
+    'javelin-behavior-aphront-form-disable-on-submit' => 'a9aaba0c',
     'javelin-behavior-aphront-more' => '9b9197be',
     'javelin-behavior-audio-source' => '59b251eb',
     'javelin-behavior-audit-preview' => 'be81801d',
@@ -603,7 +603,7 @@ return array(
     'javelin-behavior-phabricator-show-all-transactions' => '7c273581',
     'javelin-behavior-phabricator-tooltips' => '48db4145',
     'javelin-behavior-phabricator-transaction-comment-form' => '9084a36f',
-    'javelin-behavior-phabricator-transaction-list' => '3c918aa8',
+    'javelin-behavior-phabricator-transaction-list' => '925c9bab',
     'javelin-behavior-phabricator-watch-anchor' => '06e05112',
     'javelin-behavior-phame-post-preview' => '61d927ec',
     'javelin-behavior-pholio-mock-edit' => '1e1e8bb0',
@@ -731,7 +731,7 @@ return array(
     'phabricator-uiexample-reactor-sendproperties' => '551add57',
     'phabricator-zindex-css' => '0fd29d49',
     'phame-css' => '450826e1',
-    'pholio-css' => 'd23ace50',
+    'pholio-css' => '2fa97dbe',
     'pholio-edit-css' => 'b9e59b6d',
     'pholio-inline-comments-css' => '52be33f0',
     'phortune-credit-card-form' => '2290aeef',
@@ -762,7 +762,7 @@ return array(
     'phui-status-list-view-css' => '2f562399',
     'phui-tag-view-css' => '295d81c4',
     'phui-text-css' => '23e9b4b7',
-    'phui-timeline-view-css' => '7dcd5354',
+    'phui-timeline-view-css' => '23fe340a',
     'phui-workboard-view-css' => 'bf70dd2e',
     'phui-workpanel-view-css' => '97b69459',
     'policy-css' => '957ea14c',
@@ -988,12 +988,6 @@ return array(
       7 => 'javelin-typeahead-preloaded-source',
       8 => 'javelin-json',
     ),
-    '27d4da3f' =>
-    array(
-      0 => 'javelin-behavior',
-      1 => 'javelin-stratcom',
-      2 => 'javelin-dom',
-    ),
     '2a2dba85' =>
     array(
       0 => 'javelin-behavior',
@@ -1055,14 +1049,6 @@ return array(
       1 => 'javelin-dom',
       2 => 'phortune-credit-card-form',
     ),
-    '3c918aa8' =>
-    array(
-      0 => 'javelin-behavior',
-      1 => 'javelin-stratcom',
-      2 => 'javelin-workflow',
-      3 => 'javelin-dom',
-      4 => 'javelin-fx',
-    ),
     '403a3dce' =>
     array(
       0 => 'javelin-install',
@@ -1404,6 +1390,15 @@ return array(
       4 => 'javelin-request',
       5 => 'phabricator-shaped-request',
     ),
+    '925c9bab' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-stratcom',
+      2 => 'javelin-workflow',
+      3 => 'javelin-dom',
+      4 => 'javelin-fx',
+      5 => 'javelin-util',
+    ),
     '938aed89' =>
     array(
       0 => 'javelin-behavior',
@@ -1497,6 +1492,12 @@ return array(
       1 => 'javelin-dom',
       2 => 'javelin-stratcom',
     ),
+    'a9aaba0c' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-stratcom',
+      2 => 'javelin-dom',
+    ),
     'ad7a69ca' =>
     array(
       0 => 'javelin-install',

src/applications/transactions/view/PhabricatorApplicationTransactionView.php

@@ -146,9 +146,12 @@ class PhabricatorApplicationTransactionView extends AphrontView {
       Javelin::initBehavior(
         'phabricator-transaction-list',
         array(
-          'listID'      => $list_id,
+          'listID'          => $list_id,
-          'objectPHID'  => $this->getObjectPHID(),
+          'objectPHID'      => $this->getObjectPHID(),
-          'nextAnchor'  => $this->anchorOffset + count($events),
+          'nextAnchor'      => $this->anchorOffset + count($events),
+          'historyLink'     => '/transactions/history/',
+          'historyLinkText' => pht('Edited'),
+          'linkDelimiter'   => PHUITimelineEventView::DELIMITER,
         ));
     }
 
@@ -202,12 +205,22 @@ class PhabricatorApplicationTransactionView extends AphrontView {
 
     if ($comment) {
       if ($comment->getIsDeleted()) {
-        return phutil_tag(
+        return javelin_tag(
-          'em',
+          'span',
-          array(),
+          array(
+            'class' => 'comment-deleted',
+            'sigil' => 'transaction-comment',
+            'meta'  => array('phid' => $comment->getTransactionPHID()),
+          ),
           pht('This comment has been deleted.'));
       } else if ($xaction->hasComment()) {
-        return $engine->getOutput($comment, $field);
+        return javelin_tag(
+          'span',
+          array(
+            'sigil' => 'transaction-comment',
+            'meta'  => array('phid' => $comment->getTransactionPHID()),
+          ),
+          $engine->getOutput($comment, $field));
       } else {
         // This is an empty, non-deleted comment. Usually this happens when
         // rendering previews.
@@ -333,9 +346,9 @@ class PhabricatorApplicationTransactionView extends AphrontView {
       }
     }
 
-    $content = $this->renderTransactionContent($xaction);
+    $comment = $this->renderTransactionContent($xaction);
-    if ($content) {
+    if ($comment) {
-      $event->appendChild($content);
+      $event->appendChild($comment);
     }
 
     return $event;

src/view/phui/PHUITimelineEventView.php

@@ -2,6 +2,8 @@
 
 final class PHUITimelineEventView extends AphrontView {
 
+  const DELIMITER = " \xC2\xB7 ";
+
   private $userHandle;
   private $title;
   private $icon;
@@ -146,12 +148,13 @@ final class PHUITimelineEventView extends AphrontView {
       }
       $extra = array_reverse($extra);
       $extra = array_mergev($extra);
-      $extra = phutil_tag(
+      $extra = javelin_tag(
         'span',
         array(
+          'sigil' => 'timeline-extra',
           'class' => 'phui-timeline-extra',
         ),
-        phutil_implode_html(" \xC2\xB7 ", $extra));
+        phutil_implode_html(self::DELIMITER, $extra));
     } else {
       $extra = null;
     }
@@ -342,7 +345,7 @@ final class PHUITimelineEventView extends AphrontView {
           'a',
           array(
             'href'  => '/transactions/history/'.$xaction_phid.'/',
-            'sigil' => 'workflow',
+            'sigil' => 'workflow transaction-edit-history',
           ),
           pht('Edited'));
       }

webroot/rsrc/css/application/pholio/pholio.css

@@ -191,7 +191,7 @@
   padding-bottom: 2px;
 }
 
-.pholio-transaction-inline-comment div {
+.pholio-transaction-inline-comment span {
   display: table-cell;
   vertical-align: middle;
   padding-left: 8px;

webroot/rsrc/css/phui/phui-timeline-view.css

@@ -160,6 +160,10 @@
   overflow-x: auto;
 }
 
+.phui-timeline-core-content .comment-deleted {
+  font-style: italic;
+}
+
 .device .phui-timeline-event-view {
   min-height: 23px;
   position: relative;

webroot/rsrc/js/application/transactions/behavior-transaction-list.js

@@ -5,6 +5,7 @@
  *           javelin-workflow
  *           javelin-dom
  *           javelin-fx
+ *           javelin-util
  */
 
 JX.behavior('phabricator-transaction-list', function(config) {
@@ -62,14 +63,49 @@ JX.behavior('phabricator-transaction-list', function(config) {
     }
   }
 
+  function edittransaction(transaction, response) {
+    // NOTE: this is for 1 transaction only
+    for (var phid in response.xactions) {
+      var new_node = JX.$H(response.xactions[phid]).getFragment().firstChild;
+      var new_comment = JX.DOM.find(new_node, 'span', 'transaction-comment');
+      var old_comments = JX.DOM.scry(
+        transaction,
+        'span',
+        'transaction-comment');
+      var old_comment = old_comments[0];
+      JX.DOM.replace(old_comment, new_comment);
+      var edit_history = JX.DOM.scry(
+          transaction,
+          'a',
+          'transaction-edit-history');
+      if (!edit_history.length) {
+        var transaction_phid = JX.Stratcom.getData(new_comment).phid;
+        var history_link = JX.$N(
+          'a',
+          { sigil : 'transaction-edit-history',
+            href  : config.historyLink + transaction_phid + '/' },
+          config.historyLinkText);
+        JX.Stratcom.addSigil(history_link, 'workflow');
+        var timeline_extra = JX.DOM.find(transaction, 'span', 'timeline-extra');
+        var old_content = JX.$H(timeline_extra.innerHTML);
+        JX.DOM.setContent(
+          timeline_extra,
+          [history_link, config.linkDelimiter, old_content]);
+      }
+      new JX.FX(transaction).setDuration(500).start({opacity: [0, 1]});
+    }
+  }
+
   JX.DOM.listen(list, 'click', 'transaction-edit', function(e) {
     if (!e.isNormalClick()) {
       return;
     }
 
+    var transaction = e.getNode('transaction');
+
     JX.Workflow.newFromLink(e.getTarget())
       .setData({anchor: e.getNodeData('transaction').anchor})
-      .setHandler(ontransactions)
+      .setHandler(JX.bind(null, edittransaction, transaction))
       .start();
 
     e.kill();