Update Differential diff view

Summary:
Ref T603.

  - Primarily, this gets rid of a `DifferentialRevisionListData` callsite.
  - Also modernize and clean up some UI stuff.

Test Plan:
{F48260}
{F48261}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T603

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

src/__celerity_resource_map__.php

@@ -859,7 +859,7 @@ celerity_register_resource_map(array(
   ),
   'aphront-panel-view-css' =>
   array(
-    'uri' => '/res/4031ea99/rsrc/css/aphront/panel-view.css',
+    'uri' => '/res/f55024c3/rsrc/css/aphront/panel-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -4073,7 +4073,7 @@ celerity_register_resource_map(array(
 ), array(
   'packages' =>
   array(
-    'b939ab89' =>
+    '60bbab00' =>
     array(
       'name' => 'core.pkg.css',
       'symbols' =>
@@ -4121,7 +4121,7 @@ celerity_register_resource_map(array(
         40 => 'phabricator-property-list-view-css',
         41 => 'phabricator-tag-view-css',
       ),
-      'uri' => '/res/pkg/b939ab89/core.pkg.css',
+      'uri' => '/res/pkg/60bbab00/core.pkg.css',
       'type' => 'css',
     ),
     'f2ad0683' =>
@@ -4315,16 +4315,16 @@ celerity_register_resource_map(array(
   'reverse' =>
   array(
     'aphront-attached-file-view-css' => 'adc3c36d',
-    'aphront-dialog-view-css' => 'b939ab89',
-    'aphront-error-view-css' => 'b939ab89',
-    'aphront-form-view-css' => 'b939ab89',
-    'aphront-list-filter-view-css' => 'b939ab89',
-    'aphront-pager-view-css' => 'b939ab89',
-    'aphront-panel-view-css' => 'b939ab89',
-    'aphront-table-view-css' => 'b939ab89',
-    'aphront-tokenizer-control-css' => 'b939ab89',
-    'aphront-tooltip-css' => 'b939ab89',
-    'aphront-typeahead-control-css' => 'b939ab89',
+    'aphront-dialog-view-css' => '60bbab00',
+    'aphront-error-view-css' => '60bbab00',
+    'aphront-form-view-css' => '60bbab00',
+    'aphront-list-filter-view-css' => '60bbab00',
+    'aphront-pager-view-css' => '60bbab00',
+    'aphront-panel-view-css' => '60bbab00',
+    'aphront-table-view-css' => '60bbab00',
+    'aphront-tokenizer-control-css' => '60bbab00',
+    'aphront-tooltip-css' => '60bbab00',
+    'aphront-typeahead-control-css' => '60bbab00',
     'differential-changeset-view-css' => 'dd27a69b',
     'differential-core-view-css' => 'dd27a69b',
     'differential-inline-comment-editor' => '9488bb69',
@@ -4338,7 +4338,7 @@ celerity_register_resource_map(array(
     'differential-table-of-contents-css' => 'dd27a69b',
     'diffusion-commit-view-css' => 'c8ce2d88',
     'diffusion-icons-css' => 'c8ce2d88',
-    'global-drag-and-drop-css' => 'b939ab89',
+    'global-drag-and-drop-css' => '60bbab00',
     'inline-comment-summary-css' => 'dd27a69b',
     'javelin-aphlict' => 'f2ad0683',
     'javelin-behavior' => 'a9f14d76',
@@ -4412,55 +4412,55 @@ celerity_register_resource_map(array(
     'javelin-util' => 'a9f14d76',
     'javelin-vector' => 'a9f14d76',
     'javelin-workflow' => 'a9f14d76',
-    'lightbox-attachment-css' => 'b939ab89',
+    'lightbox-attachment-css' => '60bbab00',
     'maniphest-task-summary-css' => 'adc3c36d',
     'maniphest-transaction-detail-css' => 'adc3c36d',
-    'phabricator-action-list-view-css' => 'b939ab89',
-    'phabricator-application-launch-view-css' => 'b939ab89',
+    'phabricator-action-list-view-css' => '60bbab00',
+    'phabricator-application-launch-view-css' => '60bbab00',
     'phabricator-busy' => 'f2ad0683',
     'phabricator-content-source-view-css' => 'dd27a69b',
-    'phabricator-core-css' => 'b939ab89',
-    'phabricator-crumbs-view-css' => 'b939ab89',
+    'phabricator-core-css' => '60bbab00',
+    'phabricator-crumbs-view-css' => '60bbab00',
     'phabricator-drag-and-drop-file-upload' => '9488bb69',
     'phabricator-dropdown-menu' => 'f2ad0683',
     'phabricator-file-upload' => 'f2ad0683',
-    'phabricator-filetree-view-css' => 'b939ab89',
-    'phabricator-flag-css' => 'b939ab89',
-    'phabricator-form-view-css' => 'b939ab89',
-    'phabricator-header-view-css' => 'b939ab89',
+    'phabricator-filetree-view-css' => '60bbab00',
+    'phabricator-flag-css' => '60bbab00',
+    'phabricator-form-view-css' => '60bbab00',
+    'phabricator-header-view-css' => '60bbab00',
     'phabricator-hovercard' => 'f2ad0683',
-    'phabricator-jump-nav' => 'b939ab89',
+    'phabricator-jump-nav' => '60bbab00',
     'phabricator-keyboard-shortcut' => 'f2ad0683',
     'phabricator-keyboard-shortcut-manager' => 'f2ad0683',
-    'phabricator-main-menu-view' => 'b939ab89',
+    'phabricator-main-menu-view' => '60bbab00',
     'phabricator-menu-item' => 'f2ad0683',
-    'phabricator-nav-view-css' => 'b939ab89',
+    'phabricator-nav-view-css' => '60bbab00',
     'phabricator-notification' => 'f2ad0683',
-    'phabricator-notification-css' => 'b939ab89',
-    'phabricator-notification-menu-css' => 'b939ab89',
-    'phabricator-object-item-list-view-css' => 'b939ab89',
+    'phabricator-notification-css' => '60bbab00',
+    'phabricator-notification-menu-css' => '60bbab00',
+    'phabricator-object-item-list-view-css' => '60bbab00',
     'phabricator-object-selector-css' => 'dd27a69b',
     'phabricator-phtize' => 'f2ad0683',
     'phabricator-prefab' => 'f2ad0683',
     'phabricator-project-tag-css' => 'adc3c36d',
-    'phabricator-property-list-view-css' => 'b939ab89',
-    'phabricator-remarkup-css' => 'b939ab89',
+    'phabricator-property-list-view-css' => '60bbab00',
+    'phabricator-remarkup-css' => '60bbab00',
     'phabricator-shaped-request' => '9488bb69',
-    'phabricator-side-menu-view-css' => 'b939ab89',
-    'phabricator-standard-page-view' => 'b939ab89',
-    'phabricator-tag-view-css' => 'b939ab89',
+    'phabricator-side-menu-view-css' => '60bbab00',
+    'phabricator-standard-page-view' => '60bbab00',
+    'phabricator-tag-view-css' => '60bbab00',
     'phabricator-textareautils' => 'f2ad0683',
     'phabricator-tooltip' => 'f2ad0683',
-    'phabricator-transaction-view-css' => 'b939ab89',
-    'phabricator-zindex-css' => 'b939ab89',
-    'phui-button-css' => 'b939ab89',
-    'phui-form-css' => 'b939ab89',
-    'phui-icon-view-css' => 'b939ab89',
-    'phui-spacing-css' => 'b939ab89',
-    'sprite-apps-large-css' => 'b939ab89',
-    'sprite-gradient-css' => 'b939ab89',
-    'sprite-icons-css' => 'b939ab89',
-    'sprite-menu-css' => 'b939ab89',
-    'syntax-highlighting-css' => 'b939ab89',
+    'phabricator-transaction-view-css' => '60bbab00',
+    'phabricator-zindex-css' => '60bbab00',
+    'phui-button-css' => '60bbab00',
+    'phui-form-css' => '60bbab00',
+    'phui-icon-view-css' => '60bbab00',
+    'phui-spacing-css' => '60bbab00',
+    'sprite-apps-large-css' => '60bbab00',
+    'sprite-gradient-css' => '60bbab00',
+    'sprite-icons-css' => '60bbab00',
+    'sprite-menu-css' => '60bbab00',
+    'syntax-highlighting-css' => '60bbab00',
   ),
 ));

src/applications/differential/controller/DifferentialDiffViewController.php

@@ -10,6 +10,7 @@ final class DifferentialDiffViewController extends DifferentialController {
 
   public function processRequest() {
     $request = $this->getRequest();
+    $viewer = $request->getUser();
 
     $diff = id(new DifferentialDiff())->load($this->id);
     if (!$diff) {
@@ -17,31 +18,19 @@ final class DifferentialDiffViewController extends DifferentialController {
     }
 
     if ($diff->getRevisionID()) {
-      $top_panel = new AphrontPanelView();
-      $top_panel->setWidth(AphrontPanelView::WIDTH_WIDE);
-      $link = phutil_tag(
-        'a',
-        array(
-          'href' => PhabricatorEnv::getURI('/D'.$diff->getRevisionID()),
-        ),
-        'D'.$diff->getRevisionID());
-      $top_panel->appendChild(phutil_tag(
-        'h1',
-        array(),
-        pht('This diff belongs to revision %s', $link)));
+      $top_part = id(new AphrontErrorView())
+        ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
+        ->appendChild(
+          pht(
+            'This diff belongs to revision %s.',
+            phutil_tag(
+              'a',
+              array(
+                'href' => '/D'.$diff->getRevisionID(),
+              ),
+              'D'.$diff->getRevisionID())));
     } else {
-      $action_panel = new AphrontPanelView();
-      $action_panel->setHeader('Preview Diff');
-      $action_panel->setWidth(AphrontPanelView::WIDTH_WIDE);
-      $action_panel->appendChild(hsprintf(
-        '<p class="aphront-panel-instructions">%s</p>',
-        pht(
-          'Review the diff for correctness. When you are satisfied, either '.
-          '<strong>create a new revision</strong> or <strong>update '.
-          'an existing revision</strong>.',
-          hsprintf(''))));
-
-      // TODO: implmenent optgroup support in AphrontFormSelectControl?
+      // TODO: implement optgroup support in AphrontFormSelectControl?
       $select = array();
       $select[] = hsprintf('<optgroup label="%s">', pht('Create New Revision'));
       $select[] = hsprintf(
@@ -49,10 +38,11 @@ final class DifferentialDiffViewController extends DifferentialController {
         pht('Create a new Revision...'));
       $select[] = hsprintf('</optgroup>');
 
-      $revision_data = new DifferentialRevisionListData(
-        DifferentialRevisionListData::QUERY_OPEN_OWNED,
-        array($request->getUser()->getPHID()));
-      $revisions = $revision_data->loadRevisions();
+      $revisions = id(new DifferentialRevisionQuery())
+        ->setViewer($viewer)
+        ->withAuthors(array($viewer->getPHID()))
+        ->withStatus(DifferentialRevisionQuery::STATUS_OPEN)
+        ->execute();
 
       if ($revisions) {
         $select[] = hsprintf(
@@ -64,7 +54,7 @@ final class DifferentialDiffViewController extends DifferentialController {
             array(
               'value' => $revision->getID(),
             ),
-            $revision->getTitle());
+            'D'.$revision->getID().' '.$revision->getTitle());
         }
         $select[] = hsprintf('</optgroup>');
       }
@@ -74,12 +64,16 @@ final class DifferentialDiffViewController extends DifferentialController {
         array('name' => 'revisionID'),
         $select);
 
-      $action_form = new AphrontFormView();
-      $action_form
+      $form = id(new AphrontFormView())
         ->setUser($request->getUser())
         ->setAction('/differential/revision/edit/')
         ->addHiddenInput('diffID', $diff->getID())
         ->addHiddenInput('viaDiffView', 1)
+        ->setFlexible(true)
+        ->appendRemarkupInstructions(
+          pht(
+            'Review the diff for correctness. When you are satisfied, either '.
+            '**create a new revision** or **update an existing revision**.'))
         ->appendChild(
           id(new AphrontFormMarkupControl())
           ->setLabel(pht('Attach To'))
@@ -88,9 +82,7 @@ final class DifferentialDiffViewController extends DifferentialController {
           id(new AphrontFormSubmitControl())
           ->setValue(pht('Continue')));
 
-      $action_panel->appendChild($action_form);
-
-      $top_panel = $action_panel;
+      $top_part = $form;
     }
 
     $props = id(new DifferentialDiffProperty())->loadAllWhere(
@@ -120,6 +112,9 @@ final class DifferentialDiffViewController extends DifferentialController {
       }
     }
 
+    $property_head = id(new PhabricatorHeaderView())
+      ->setHeader(pht('Properties'));
+
     $property_view = new PhabricatorPropertyListView();
     foreach ($dict as $key => $value) {
       $property_view->addProperty($key, $value);
@@ -147,17 +142,23 @@ final class DifferentialDiffViewController extends DifferentialController {
       ->setTitle(pht('Diff %d', $diff->getID()))
       ->setUser($request->getUser());
 
-    return $this->buildStandardPageResponse(
-      id(new DifferentialPrimaryPaneView())
-        ->appendChild(
-          array(
-            $top_panel->render(),
-            $property_view,
-            $table_of_contents->render(),
-            $details->render(),
-          )),
+    $crumbs = $this->buildApplicationCrumbs();
+    $crumbs->addCrumb(
+      id(new PhabricatorCrumbView())
+        ->setName(pht('Diff %d', $diff->getID())));
+
+    return $this->buildApplicationPage(
+      array(
+        $crumbs,
+        $top_part,
+        $property_head,
+        $property_view,
+        $table_of_contents,
+        $details,
+      ),
       array(
         'title' => pht('Diff View'),
+        'dust' => true,
       ));
   }
 

webroot/rsrc/css/aphront/panel-view.css

@@ -52,11 +52,6 @@
   float: right;
 }
 
-.aphront-panel-view p.aphront-panel-instructions {
-  margin: .5em 2em .75em;
-  font-size: 13px;
-}
-
 .device-desktop .aphront-panel-width-form {
   width: 720px;
   margin-right: auto;