[Rough Sketch] Differential ObjectItemView Smexyness

Summary:
Tried out `PhabricatorObjectItemView` for Differential. It looks smexy and smooth.

Refs T2014

- Title and Date as Maniphest
- Author in the handle icon
- Bar color reflects revision status (Needs Review, Accepted, Abandoned etc.) @chad looking for non-blue is faster than keeping watch for everything that's not "Closed" in old table form
- Some status information are in footer icons; currently only stale/old status display as well as saved drafts, maybe more in future; these come into my mind:
  - No reviewer warning
  - Push Blocking Priority (T2730)
  - Trivial, fast review guaranteed
  - Sketch / Just looking for advice/help
  - Arcanist Project (T2614)
  - Denote "Public Send-in" (T1476)

{F37662}
{F37663}
{F37664}
{F37665}

Some flaws:

- Date and reviewers on every entry the same?
- No respect for Differential fields (for some reason, every entry appeared the same, so broke it to parts)
- Plenty of (potential) increase in height - advise reducing paging length from 100 to 50 - or just ignore me

Suggestions for the future:

- Expand the meta information regarding revisions; e.g. the various status displays above
- Uh... T2543, T1279, T793, T731 and what else I want for Differential, because they are awesome!
- T793 should be in particular easy appearance-wise, just copy-paste from Maniphest

Test Plan: By looking at it, of course. Verified there are no errors or crashed

Reviewers: epriestley, chad, btrahan, liguobig

Reviewed By: chad

CC: aran, Korvin, edward, nh

Maniphest Tasks: T2014

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

Conflicts:
	src/__celerity_resource_map__.php

src/__celerity_resource_map__.php

@@ -805,7 +805,7 @@ celerity_register_resource_map(array(
   ),
   'aphront-error-view-css' =>
   array(
-    'uri' => '/res/a574aa01/rsrc/css/aphront/error-view.css',
+    'uri' => '/res/e2bb50c4/rsrc/css/aphront/error-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -3249,7 +3249,7 @@ celerity_register_resource_map(array(
   ),
   'phabricator-object-item-list-view-css' =>
   array(
-    'uri' => '/res/8d18c133/rsrc/css/layout/phabricator-object-item-list-view.css',
+    'uri' => '/res/d58ecb3c/rsrc/css/layout/phabricator-object-item-list-view.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -4073,7 +4073,7 @@ celerity_register_resource_map(array(
 ), array(
   'packages' =>
   array(
-    'a1d34b1a' =>
+    '0ad4a08f' =>
     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/a1d34b1a/core.pkg.css',
+      'uri' => '/res/pkg/0ad4a08f/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' => 'a1d34b1a',
+    'aphront-dialog-view-css' => '0ad4a08f',
-    'aphront-error-view-css' => 'a1d34b1a',
+    'aphront-error-view-css' => '0ad4a08f',
-    'aphront-form-view-css' => 'a1d34b1a',
+    'aphront-form-view-css' => '0ad4a08f',
-    'aphront-list-filter-view-css' => 'a1d34b1a',
+    'aphront-list-filter-view-css' => '0ad4a08f',
-    'aphront-pager-view-css' => 'a1d34b1a',
+    'aphront-pager-view-css' => '0ad4a08f',
-    'aphront-panel-view-css' => 'a1d34b1a',
+    'aphront-panel-view-css' => '0ad4a08f',
-    'aphront-table-view-css' => 'a1d34b1a',
+    'aphront-table-view-css' => '0ad4a08f',
-    'aphront-tokenizer-control-css' => 'a1d34b1a',
+    'aphront-tokenizer-control-css' => '0ad4a08f',
-    'aphront-tooltip-css' => 'a1d34b1a',
+    'aphront-tooltip-css' => '0ad4a08f',
-    'aphront-typeahead-control-css' => 'a1d34b1a',
+    'aphront-typeahead-control-css' => '0ad4a08f',
     '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' => 'a1d34b1a',
+    'global-drag-and-drop-css' => '0ad4a08f',
     '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' => 'a1d34b1a',
+    'lightbox-attachment-css' => '0ad4a08f',
     'maniphest-task-summary-css' => 'adc3c36d',
     'maniphest-transaction-detail-css' => 'adc3c36d',
-    'phabricator-action-list-view-css' => 'a1d34b1a',
+    'phabricator-action-list-view-css' => '0ad4a08f',
-    'phabricator-application-launch-view-css' => 'a1d34b1a',
+    'phabricator-application-launch-view-css' => '0ad4a08f',
     'phabricator-busy' => 'f2ad0683',
     'phabricator-content-source-view-css' => 'dd27a69b',
-    'phabricator-core-css' => 'a1d34b1a',
+    'phabricator-core-css' => '0ad4a08f',
-    'phabricator-crumbs-view-css' => 'a1d34b1a',
+    'phabricator-crumbs-view-css' => '0ad4a08f',
     'phabricator-drag-and-drop-file-upload' => '9488bb69',
     'phabricator-dropdown-menu' => 'f2ad0683',
     'phabricator-file-upload' => 'f2ad0683',
-    'phabricator-filetree-view-css' => 'a1d34b1a',
+    'phabricator-filetree-view-css' => '0ad4a08f',
-    'phabricator-flag-css' => 'a1d34b1a',
+    'phabricator-flag-css' => '0ad4a08f',
-    'phabricator-form-view-css' => 'a1d34b1a',
+    'phabricator-form-view-css' => '0ad4a08f',
-    'phabricator-header-view-css' => 'a1d34b1a',
+    'phabricator-header-view-css' => '0ad4a08f',
     'phabricator-hovercard' => 'f2ad0683',
-    'phabricator-jump-nav' => 'a1d34b1a',
+    'phabricator-jump-nav' => '0ad4a08f',
     'phabricator-keyboard-shortcut' => 'f2ad0683',
     'phabricator-keyboard-shortcut-manager' => 'f2ad0683',
-    'phabricator-main-menu-view' => 'a1d34b1a',
+    'phabricator-main-menu-view' => '0ad4a08f',
     'phabricator-menu-item' => 'f2ad0683',
-    'phabricator-nav-view-css' => 'a1d34b1a',
+    'phabricator-nav-view-css' => '0ad4a08f',
     'phabricator-notification' => 'f2ad0683',
-    'phabricator-notification-css' => 'a1d34b1a',
+    'phabricator-notification-css' => '0ad4a08f',
-    'phabricator-notification-menu-css' => 'a1d34b1a',
+    'phabricator-notification-menu-css' => '0ad4a08f',
-    'phabricator-object-item-list-view-css' => 'a1d34b1a',
+    'phabricator-object-item-list-view-css' => '0ad4a08f',
     'phabricator-object-selector-css' => 'dd27a69b',
     'phabricator-phtize' => 'f2ad0683',
     'phabricator-prefab' => 'f2ad0683',
     'phabricator-project-tag-css' => 'adc3c36d',
-    'phabricator-property-list-view-css' => 'a1d34b1a',
+    'phabricator-property-list-view-css' => '0ad4a08f',
-    'phabricator-remarkup-css' => 'a1d34b1a',
+    'phabricator-remarkup-css' => '0ad4a08f',
     'phabricator-shaped-request' => '9488bb69',
-    'phabricator-side-menu-view-css' => 'a1d34b1a',
+    'phabricator-side-menu-view-css' => '0ad4a08f',
-    'phabricator-standard-page-view' => 'a1d34b1a',
+    'phabricator-standard-page-view' => '0ad4a08f',
-    'phabricator-tag-view-css' => 'a1d34b1a',
+    'phabricator-tag-view-css' => '0ad4a08f',
     'phabricator-textareautils' => 'f2ad0683',
     'phabricator-tooltip' => 'f2ad0683',
-    'phabricator-transaction-view-css' => 'a1d34b1a',
+    'phabricator-transaction-view-css' => '0ad4a08f',
-    'phabricator-zindex-css' => 'a1d34b1a',
+    'phabricator-zindex-css' => '0ad4a08f',
-    'phui-button-css' => 'a1d34b1a',
+    'phui-button-css' => '0ad4a08f',
-    'phui-form-css' => 'a1d34b1a',
+    'phui-form-css' => '0ad4a08f',
-    'phui-icon-view-css' => 'a1d34b1a',
+    'phui-icon-view-css' => '0ad4a08f',
-    'phui-spacing-css' => 'a1d34b1a',
+    'phui-spacing-css' => '0ad4a08f',
-    'sprite-apps-large-css' => 'a1d34b1a',
+    'sprite-apps-large-css' => '0ad4a08f',
-    'sprite-gradient-css' => 'a1d34b1a',
+    'sprite-gradient-css' => '0ad4a08f',
-    'sprite-icons-css' => 'a1d34b1a',
+    'sprite-icons-css' => '0ad4a08f',
-    'sprite-menu-css' => 'a1d34b1a',
+    'sprite-menu-css' => '0ad4a08f',
-    'syntax-highlighting-css' => 'a1d34b1a',
+    'syntax-highlighting-css' => '0ad4a08f',
   ),
 ));

src/__phutil_library_map__.php

@@ -2319,6 +2319,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionListController' => 'DifferentialController',
     'DifferentialRevisionListView' => 'AphrontView',
     'DifferentialRevisionMailReceiver' => 'PhabricatorObjectMailReceiver',
+    'DifferentialRevisionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'DifferentialRevisionStatsView' => 'AphrontView',
     'DifferentialRevisionStatusFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialRevisionUpdateHistoryView' => 'AphrontView',

src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php

@@ -138,30 +138,13 @@ final class DifferentialReviewersFieldSpecification
   public function renderValueForRevisionList(DifferentialRevision $revision) {
     $primary_reviewer = $revision->getPrimaryReviewer();
     if ($primary_reviewer) {
-      $other_reviewers = array_flip($revision->getReviewers());
+      $names = array();
-      unset($other_reviewers[$primary_reviewer]);
+
-      if ($other_reviewers) {
+      foreach ($revision->getReviewers() as $reviewer) {
-        $names = array();
+        $names[] = $this->getHandle($reviewer)->renderLink();
-        foreach ($other_reviewers as $reviewer => $_) {
-          $names[] = $this->getHandle($reviewer)->getLinkName();
-        }
-        $suffix = javelin_tag(
-          'abbr',
-          array(
-            'sigil' => 'has-tooltip',
-            'meta'  => array(
-              'tip'   => implode(', ', $names),
-              'align' => 'E',
-            ),
-          ),
-          '(+'.(count($names)).')');
-      } else {
-        $suffix = null;
       }
-      return hsprintf(
+
-        '%s %s',
+      return phutil_implode_html(', ', $names);
-        $this->getHandle($primary_reviewer)->renderLink(),
-        $suffix);
     } else {
       return phutil_tag('em', array(), 'None');
     }

src/applications/differential/view/DifferentialRevisionListView.php

@@ -76,23 +76,18 @@ final class DifferentialRevisionListView extends AphrontView {
       throw new Exception("Call setUser() before render()!");
     }
 
-    $fresh = null;
+    $fresh = PhabricatorEnv::getEnvConfig('differential.days-fresh');
-    $stale = null;
+    if ($fresh) {
+      $fresh = PhabricatorCalendarHoliday::getNthBusinessDay(
+        time(),
+        -$fresh);
+    }
 
-    if ($this->highlightAge) {
+    $stale = PhabricatorEnv::getEnvConfig('differential.days-stale');
-      $fresh = PhabricatorEnv::getEnvConfig('differential.days-fresh');
+    if ($stale) {
-      if ($fresh) {
+      $stale = PhabricatorCalendarHoliday::getNthBusinessDay(
-        $fresh = PhabricatorCalendarHoliday::getNthBusinessDay(
+        time(),
-          time(),
+        -$stale);
-          -$fresh);
-      }
-
-      $stale = PhabricatorEnv::getEnvConfig('differential.days-stale');
-      if ($stale) {
-        $stale = PhabricatorCalendarHoliday::getNthBusinessDay(
-          time(),
-          -$stale);
-      }
     }
 
     Javelin::initBehavior('phabricator-tooltips', array());
@@ -101,50 +96,30 @@ final class DifferentialRevisionListView extends AphrontView {
     $flagged = mpull($this->flags, null, 'getObjectPHID');
 
     foreach ($this->fields as $field) {
-      $field->setUser($this->user);
       $field->setHandles($this->handles);
     }
 
-    $cell_classes = array();
+    $list = new PhabricatorObjectItemListView();
-    $rows = array();
+    $list->setCards(true);
+    $list->setFlush(true);
+
     foreach ($this->revisions as $revision) {
+      $item = new PhabricatorObjectItemView();
+      $rev_fields = array();
+      $icons = array();
+
       $phid = $revision->getPHID();
-      $flag = '';
       if (isset($flagged[$phid])) {
-        $class = PhabricatorFlagColor::getCSSClass($flagged[$phid]->getColor());
+        $icons['flag'] = array(
-        $note = $flagged[$phid]->getNote();
+          'icon' => 'flag-'.$flagged[$phid]->getColor(),
-        $flag = javelin_tag(
+        );
-          'div',
+      }
-          $note ? array(
+      if (array_key_exists($revision->getID(), $this->drafts)) {
-            'class' => 'phabricator-flag-icon '.$class,
+        $icons['draft'] = array(
-            'sigil' => 'has-tooltip',
+          'icon' => 'file-white',
-            'meta'  => array(
+          'href' => '/D'.$revision->getID().'#comment-preview',
-              'tip'   => $note,
+        );
-              'align' => 'N',
-              'size'  => 240,
-            ),
-          ) : array(
-            'class' => 'phabricator-flag-icon '.$class,
-          ),
-          '');
-
-      } else if (array_key_exists($revision->getID(), $this->drafts)) {
-        $src = '/rsrc/image/icon/fatcow/page_white_edit.png';
-        $flag = hsprintf(
-          '<a href="%s">%s</a>',
-          '/D'.$revision->getID().'#comment-preview',
-          phutil_tag(
-            'img',
-            array(
-              'src' => celerity_get_resource_uri($src),
-              'width' => 16,
-              'height' => 16,
-              'alt' => 'Draft',
-              'title' => pht('Draft Comment'),
-            )));
       }
-
-      $row = array($flag);
 
       $modified = $revision->getDateModified();
 
@@ -152,38 +127,79 @@ final class DifferentialRevisionListView extends AphrontView {
         if (($fresh || $stale) &&
             $field instanceof DifferentialDateModifiedFieldSpecification) {
           if ($stale && $modified < $stale) {
-            $class = 'revision-age-old';
+            $days = floor((time() - $modified) / 60 / 60 / 24);
+            $icons['age'] = array(
+              'icon' => 'warning-white',
+              'label' => pht('Old (%d days)', $days),
+            );
           } else if ($fresh && $modified < $fresh) {
-            $class = 'revision-age-stale';
+            $days = floor((time() - $modified) / 60 / 60 / 24);
+            $icons['age'] = array(
+              'icon' => 'perflab-white',
+              'label' => pht('Stale (%d days)', $days),
+            );
           } else {
-            $class = 'revision-age-fresh';
+            // Fresh, noOp();
           }
-          $cell_classes[count($rows)][count($row)] = $class;
         }
 
-        $row[] = $field->renderValueForRevisionList($revision);
+        $rev_header = $field->renderHeaderForRevisionList();
+        $rev_fields[$rev_header] = $field
+          ->renderValueForRevisionList($revision);
       }
 
-      $rows[] = $row;
+      $status = $revision->getStatus();
+      $status_name =
+        ArcanistDifferentialRevisionStatus::getNameForRevisionStatus($status);
+
+      $flag_icon = null;
+      if (isset($icons['flag'])) {
+        $flag_icon = $icons['flag']['icon'];
+      }
+
+      $item->setObjectName('D'.$revision->getID());
+      $item->setHeader(phutil_tag('a',
+        array('href' => '/D'.$revision->getID()),
+        $revision->getTitle()));
+      $item->addAttribute($status_name);
+
+      // Author
+      $author_handle = $this->handles[$revision->getAuthorPHID()];
+      $item->addByline(pht('Author: %s', $author_handle->renderLink()));
+
+      // Reviewers
+      $item->addAttribute(pht('Reviewers: %s', $rev_fields['Reviewers']));
+
+      $do_not_display_age = array(
+        ArcanistDifferentialRevisionStatus::CLOSED => true,
+        ArcanistDifferentialRevisionStatus::ABANDONED => true,
+      );
+      if (isset($icons['age']) && !isset($do_not_display_age[$status])) {
+        $item->addFootIcon($icons['age']['icon'], $icons['age']['label']);
+      }
+      if (isset($icons['draft'])) {
+        $item->addFootIcon($icons['draft']['icon'], pht('Saved Draft'),
+          $icons['draft']['href']);
+      }
+
+      // Updated on
+      $item->addIcon($flag_icon ? $flag_icon : 'none',
+        hsprintf('%s', $rev_fields['Updated']));
+
+      // First remove the fields we already have
+      $count = 7;
+      $rev_fields = array_slice($rev_fields, $count);
+
+      // Then add each one of them
+      // TODO: Add render-to-foot-icon support
+      foreach ($rev_fields as $header => $field) {
+        $item->addAttribute(pht('%s: %s', $header, $field));
+      }
+
+      $list->addItem($item);
     }
 
-    $headers = array('');
+    return $list;
-    $classes = array('');
-    foreach ($this->fields as $field) {
-      $headers[] = $field->renderHeaderForRevisionList();
-      $classes[] = $field->getColumnClassForRevisionList();
-    }
-
-    $table = new AphrontTableView($rows);
-    $table->setHeaders($headers);
-    $table->setColumnClasses($classes);
-    $table->setCellClasses($cell_classes);
-
-    $table->setNoDataString(pht('No revisions found.'));
-
-    require_celerity_resource('differential-revision-history-css');
-
-    return $table->render();
   }
 
   public static function getDefaultFields(PhabricatorUser $user) {

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

@@ -74,3 +74,8 @@ form.aphront-dialog-view .aphront-error-view {
   background: #f3f3f3;
   color: #666;
 }
+
+.make-me-sneeze .aphront-error-severity-nodata {
+  background: #fff;
+  border-color: #e6e6e6;
+}

webroot/rsrc/css/layout/phabricator-object-item-list-view.css

@@ -14,6 +14,10 @@
   padding: 0;
 }
 
+.phabricator-object-list-flush .aphront-error-view {
+  margin: 0;
+}
+
 .phabricator-object-item {
   background: #ffffff;
   border-style: solid;
@@ -308,6 +312,14 @@
   line-height: 18px;
 }
 
+/*
+ * Items with icon 'none' still have on mobile, thus creating a weird vertical
+ * margin for elements which follow
+ */
+.device-phone .phabricator-object-item-icon-none {
+  display: none;
+}
+
 .device-desktop .phabricator-object-item-icon {
   padding-right: 22px;
 }