Modernized Phriction's Diff Controller

Summary: Refs T2686 - Added additional crumb to link back to History view - Revert buttons hidden for Stub and Move changes, too - added colors to the change set to reflect the colors in the diff Test Plan: looked at various changes, verified correct appearance Reviewers: epriestley, chad, btrahan CC: aran, Korvin Maniphest Tasks: T2686 Differential Revision: https://secure.phabricator.com/D5401
phacility · Mar 22, 2013 · 4c57f8a · 4c57f8a
1 parent cb84cb1
commit 4c57f8a
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 37 deletions.
diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php
@@ -3397,7 +3397,7 @@
   ),
   'phriction-document-css' =>
   array(
-    'uri' => '/res/8d09bd7f/rsrc/css/application/phriction/phriction-document-css.css',
+    'uri' => '/res/e71e4a67/rsrc/css/application/phriction/phriction-document-css.css',
     'type' => 'css',
     'requires' =>
     array(

diff --git a/src/applications/phriction/controller/PhrictionDiffController.php b/src/applications/phriction/controller/PhrictionDiffController.php
@@ -107,6 +107,11 @@ public function processRequest() {
       $crumbs->addCrumb($view);
     }
 
+    $crumbs->addCrumb(
+      id(new PhabricatorCrumbView())
+        ->setName(pht('History'))
+        ->setHref(PhrictionDocument::getSlugURI($slug, 'history')));
+
 
     $title = "Version $l vs $r";
 
@@ -115,7 +120,8 @@ public function processRequest() {
 
     $crumbs->addCrumb(
       id(new PhabricatorCrumbView())
-        ->setName($title));
+        ->setName($title)
+        ->setHref($request->getRequestURI()));
 
 
     $comparison_table = $this->renderComparisonTable(
@@ -166,7 +172,6 @@ public function processRequest() {
     }
 
 
-
     $output = hsprintf(
       '<br><div class="phriction-document-history-diff">'.
         '%s<br /><br />%s'.
@@ -202,9 +207,14 @@ private function renderRevertButton(
     $document_id = $content->getDocumentID();
     $version = $content->getVersion();
 
-    if ($content->getChangeType() == PhrictionChangeType::CHANGE_DELETE) {
-      // Don't show an edit/revert button for changes which deleted the content
-      // since it's silly.
+    $hidden_statuses = array(
+      PhrictionChangeType::CHANGE_DELETE    => true, // Silly
+      PhrictionChangeType::CHANGE_MOVE_AWAY => true, // Plain silly
+      PhrictionChangeType::CHANGE_STUB      => true, // Utterly silly
+    );
+    if (isset($hidden_statuses[$content->getChangeType()])) {
+      // Don't show an edit/revert button for changes which deleted, moved or
+      // stubbed the content since it's silly.
       return null;
     }
 
@@ -236,37 +246,35 @@ private function renderComparisonTable(array $content) {
     $phids = mpull($content, 'getAuthorPHID');
     $handles = $this->loadViewerHandles($phids);
 
-    $rows = array();
+    $list = new PhabricatorObjectItemListView();
+
+    $first = true;
     foreach ($content as $c) {
-      $rows[] = array(
-        phabricator_date($c->getDateCreated(), $user),
-        phabricator_time($c->getDateCreated(), $user),
-        'Version '.$c->getVersion(),
-        $handles[$c->getAuthorPHID()]->renderLink(),
-        $c->getDescription(),
-      );
-    }
+      $author = $handles[$c->getAuthorPHID()]->renderLink();
+      $item = id(new PhabricatorObjectItemView())
+        ->setHeader(pht('%s by %s, %s',
+          PhrictionChangeType::getChangeTypeLabel($c->getChangeType()),
+          $author,
+          pht('Version %s', $c->getVersion())))
+        ->addAttribute(pht('%s %s',
+          phabricator_date($c->getDateCreated(), $user),
+          phabricator_time($c->getDateCreated(), $user)));
+
+      if ($c->getDescription()) {
+        $item->addAttribute($c->getDescription());
+      }
 
+      if ($first == true) {
+        $item->setBarColor('green');
+        $first = false;
+      } else {
+        $item->setBarColor('red');
+      }
 
-    $table = new AphrontTableView($rows);
-    $table->setHeaders(
-      array(
-        pht('Date'),
-        pht('Time'),
-        pht('Version'),
-        pht('Author'),
-        pht('Description'),
-      ));
-    $table->setColumnClasses(
-      array(
-        '',
-        'right',
-        'pri',
-        '',
-        'wide',
-      ));
+      $list->addItem($item);
+    }
 
-    return $table;
+    return $list;
   }
 
 }
diff --git a/webroot/rsrc/css/application/phriction/phriction-document-css.css b/webroot/rsrc/css/application/phriction/phriction-document-css.css
@@ -64,11 +64,8 @@
   font-size: 11px;
 }
 
-
 .phriction-document-history-diff {
-  margin:           0 0 0 2em;
-  padding-bottom:   2em;
-  max-width:        1162px;
+  padding: 0 2em 2em;
 }
 
 .phriction-revert-table {