Improve design of PhabricatorObjectListView

Summary: See discussion in T2014. Aligns this element more closely with @chad's `frame_v3.psd` mock, and implements the icon/label element. Removes "details".

Test Plan: {F27062} {F27063} {F27064} {F27065}

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2014

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

src/applications/calendar/controller/PhabricatorCalendarViewStatusController.php

@@ -68,14 +68,19 @@ final class PhabricatorCalendarViewStatusController
       }
       $from = phabricator_datetime($status->getDateFrom(), $user);
       $to   = phabricator_datetime($status->getDateTo(), $user);
+
+      $color = ($status->getStatus() == PhabricatorUserStatus::STATUS_AWAY)
+        ? 'red'
+        : 'yellow';
+
       $item = id(new PhabricatorObjectItemView())
         ->setHeader($status->getTerseSummary($user))
         ->setHref($href)
-        ->addDetail(
-          pht('Description'),
-          phutil_escape_html($status->getDescription()))
-        ->addAttribute(pht('From %s', $from))
-        ->addAttribute(pht('To %s', $to));
+        ->setBarColor($color)
+        ->addAttribute(pht('From %s to %s', $from, $to))
+        ->addAttribute(
+          phutil_escape_html(
+            phutil_utf8_shorten($status->getDescription(), 64)));
 
       $list->addItem($item);
     }

src/applications/macro/controller/PhabricatorMacroListController.php

@@ -78,39 +78,34 @@ final class PhabricatorMacroListController
 
     $nav->appendChild($filter_view);
 
-    if ($macros) {
-      $pinboard = new PhabricatorPinboardView();
-      foreach ($macros as $macro) {
-        $file_phid = $macro->getFilePHID();
-        $file = idx($files_map, $file_phid);
+    $pinboard = new PhabricatorPinboardView();
+    $pinboard->setNoDataString($nodata);
+    foreach ($macros as $macro) {
+      $file_phid = $macro->getFilePHID();
+      $file = idx($files_map, $file_phid);
 
-        $item = new PhabricatorPinboardItemView();
-        if ($file) {
-          $item->setImageURI($file->getThumb220x165URI());
-          $item->setImageSize(220, 165);
-          if ($file->getAuthorPHID()) {
-            $author_handle = $this->getHandle($file->getAuthorPHID());
-            $item->appendChild(
-              'Created by '.$author_handle->renderLink());
-          }
-          $datetime = phabricator_date($file->getDateCreated(), $viewer);
+      $item = new PhabricatorPinboardItemView();
+      if ($file) {
+        $item->setImageURI($file->getThumb220x165URI());
+        $item->setImageSize(220, 165);
+        if ($file->getAuthorPHID()) {
+          $author_handle = $this->getHandle($file->getAuthorPHID());
           $item->appendChild(
-            phutil_render_tag(
-              'div',
-              array(),
-              'Created on '.$datetime));
+            'Created by '.$author_handle->renderLink());
         }
-        $item->setURI($this->getApplicationURI('/view/'.$macro->getID().'/'));
-        $item->setHeader($macro->getName());
-
-        $pinboard->addItem($item);
+        $datetime = phabricator_date($file->getDateCreated(), $viewer);
+        $item->appendChild(
+          phutil_render_tag(
+            'div',
+            array(),
+            'Created on '.$datetime));
       }
-      $nav->appendChild($pinboard);
-    } else {
-      $list = new PhabricatorObjectItemListView();
-      $list->setNoDataString($nodata);
-      $nav->appendChild($list);
+      $item->setURI($this->getApplicationURI('/view/'.$macro->getID().'/'));
+      $item->setHeader($macro->getName());
+
+      $pinboard->addItem($item);
     }
+    $nav->appendChild($pinboard);
 
     if (!strlen($filter)) {
       $nav->appendChild($pager);

src/applications/paste/controller/PhabricatorPasteListController.php

@@ -39,11 +39,17 @@ final class PhabricatorPasteListController extends PhabricatorPasteController {
     $pastes = $query->executeWithCursorPager($pager);
 
     $list = $this->buildPasteList($pastes);
-    $list->setHeader($title);
     $list->setPager($pager);
     $list->setNoDataString($nodata);
 
-    $nav->appendChild($list);
+    $header = id(new PhabricatorHeaderView())
+      ->setHeader($title);
+
+    $nav->appendChild(
+      array(
+        $header,
+        $list,
+      ));
 
     $crumbs = $this
       ->buildApplicationCrumbs($nav)
@@ -71,17 +77,16 @@ final class PhabricatorPasteListController extends PhabricatorPasteController {
     $this->loadHandles(mpull($pastes, 'getAuthorPHID'));
 
     $list = new PhabricatorObjectItemListView();
+    $list->setViewer($user);
     foreach ($pastes as $paste) {
-      $created = phabricator_datetime($paste->getDateCreated(), $user);
+      $created = phabricator_date($paste->getDateCreated(), $user);
+      $author = $this->getHandle($paste->getAuthorPHID())->renderLink();
 
       $item = id(new PhabricatorObjectItemView())
         ->setHeader($paste->getFullName())
         ->setHref('/P'.$paste->getID())
-        ->addDetail(
-          pht('Author'),
-          $this->getHandle($paste->getAuthorPHID())->renderLink())
-        ->addAttribute(pht('Created %s', $created));
-
+        ->setObject($paste)
+        ->addAttribute(pht('Created %s by %s', $created, $author));
       $list->addItem($item);
     }
 

src/applications/phame/controller/PhameController.php

@@ -41,30 +41,40 @@ abstract class PhameController extends PhabricatorController {
     assert_instances_of($posts, 'PhamePost');
 
     $list = id(new PhabricatorObjectItemListView())
+      ->setViewer($user)
       ->setNoDataString($nodata);
 
     foreach ($posts as $post) {
-      $item = id(new PhabricatorObjectItemView())
-        ->setHeader($post->getTitle())
-        ->setHref($this->getApplicationURI('post/view/'.$post->getID().'/'))
-        ->addDetail(
-          pht('Blogger'),
-          $this->getHandle($post->getBloggerPHID())->renderLink())
-        ->addDetail(
-          pht('Blog'),
-          $post->getBlog()
-            ? $this->getHandle($post->getBlog()->getPHID())->renderLink()
-            : '-');
+      $blogger = $this->getHandle($post->getBloggerPHID())->renderLink();
 
-      if ($post->isDraft()) {
-        $item->addAttribute(pht('Draft'));
-      } else {
-        $date_published = phabricator_datetime(
-          $post->getDatePublished(),
-          $user);
-        $item->addAttribute(pht('Published on %s', $date_published));
+      $blog = null;
+      if ($post->getBlog()) {
+        $blog = $this->getHandle($post->getBlog()->getPHID())->renderLink();
       }
 
+      $published = null;
+      if ($post->getDatePublished()) {
+        $published = phabricator_date($post->getDatePublished(), $user);
+      }
+
+      $draft = $post->isDraft();
+
+      $item = id(new PhabricatorObjectItemView())
+        ->setObject($post)
+        ->setHeader($post->getTitle())
+        ->setHref($this->getApplicationURI('post/view/'.$post->getID().'/'));
+
+      if ($blog) {
+        $item->addAttribute($blog);
+      }
+
+      if ($draft) {
+        $desc = pht('Draft by %s', $blogger);
+      } else {
+        $desc = pht('Published on %s by %s', $published, $blogger);
+      }
+      $item->addAttribute($desc);
+
       $list->addItem($item);
     }
 

src/applications/phame/controller/blog/PhameBlogListController.php

@@ -71,14 +71,13 @@ final class PhameBlogListController extends PhameController {
 
     $view = new PhabricatorObjectItemListView();
     $view->setNoDataString($nodata);
+    $view->setViewer($user);
     foreach ($blogs as $blog) {
 
       $item = id(new PhabricatorObjectItemView())
         ->setHeader($blog->getName())
         ->setHref($this->getApplicationURI('blog/view/'.$blog->getID().'/'))
-        ->addDetail(
-          pht('Custom Domain'),
-          phutil_escape_html($blog->getDomain()));
+        ->setObject($blog);
 
       $view->addItem($item);
     }

src/applications/ponder/controller/PonderFeedController.php

@@ -90,35 +90,22 @@ final class PonderFeedController extends PonderController {
     $user = $this->getRequest()->getUser();
 
     $view = new PhabricatorObjectItemListView();
+    $view->setViewer($user);
     $view->setNoDataString(pht('No matching questions.'));
     foreach ($questions as $question) {
       $item = new PhabricatorObjectItemView();
       $item->setHeader('Q'.$question->getID().' '.$question->getTitle());
       $item->setHref('/Q'.$question->getID());
+      $item->setObject($question);
 
-      $desc = $question->getContent();
-      if ($desc) {
-        $item->addDetail(
-          pht('Description'),
-          phutil_escape_html(phutil_utf8_shorten($desc, 128)));
-      }
+      $item->addAttribute(
+        pht(
+          'Asked by %s on %s',
+          $this->getHandle($question->getAuthorPHID())->renderLink(),
+          phabricator_date($question->getDateCreated(), $user)));
 
-      $item->addDetail(
-        pht('Author'),
-        $this->getHandle($question->getAuthorPHID())->renderLink());
-
-      $item->addDetail(
-        pht('Votes'),
-        $question->getVoteCount());
-
-      $item->addDetail(
-        pht('Answers'),
-        $question->getAnswerCount());
-
-      $created = pht(
-        'Created %s',
-        phabricator_date($question->getDateCreated(), $user));
-      $item->addAttribute($created);
+      $item->addAttribute(
+        pht('%d Answer(s)', $question->getAnswerCount()));
 
       $view->addItem($item);
     }

src/applications/ponder/view/PonderUserProfileView.php

@@ -61,6 +61,7 @@ final class PonderUserProfileView extends AphrontView {
     $answers = $apagebuttons->sliceResults($answers);
 
     $view = new PhabricatorObjectItemListView();
+    $view->setViewer($user);
     $view->setNoDataString(pht('No matching answers.'));
 
     foreach ($answers as $answer) {
@@ -68,6 +69,7 @@ final class PonderUserProfileView extends AphrontView {
       $author_phid = $question->getAuthorPHID();
 
       $item = new PhabricatorObjectItemView();
+      $item->setObject($answer);
       $href = id(new PhutilURI('/Q' . $question->getID()))
         ->setFragment('A' . $answer->getID());
       $item->setHeader(
@@ -75,19 +77,20 @@ final class PonderUserProfileView extends AphrontView {
       );
       $item->setHref($href);
 
-      $item->addDetail(
-        pht('Votes'),
-        $answer->getVoteCount()
-      );
+      $item->addAttribute(
+        pht('Created %s', phabricator_date($answer->getDateCreated(), $user)));
 
-      $item->addDetail(
-        pht('Question'),
-        self::abbreviate($question->getTitle())
-      );
+      $item->addAttribute(pht('%d Vote(s)', $answer->getVoteCount()));
 
       $item->addAttribute(
-        pht('Created %s', phabricator_date($answer->getDateCreated(), $user))
-      );
+        pht(
+          'Answer to %s',
+          phutil_render_tag(
+            'a',
+            array(
+              'href' => '/Q'.$question->getID(),
+            ),
+            self::abbreviate($question->getTitle()))));
 
       $view->addItem($item);
     }

src/view/layout/PhabricatorObjectItemListView.php

@@ -6,6 +6,16 @@ final class PhabricatorObjectItemListView extends AphrontView {
   private $items;
   private $pager;
   private $noDataString;
+  private $viewer;
+
+  public function setViewer($viewer) {
+    $this->viewer = $viewer;
+    return $this;
+  }
+
+  public function getViewer() {
+    return $this->viewer;
+  }
 
   public function setHeader($header) {
     $this->header = $header;
@@ -30,12 +40,15 @@ final class PhabricatorObjectItemListView extends AphrontView {
   public function render() {
     require_celerity_resource('phabricator-object-item-list-view-css');
 
-    $header = phutil_render_tag(
-      'h1',
-      array(
-        'class' => 'phabricator-object-item-list-header',
-      ),
-      phutil_escape_html($this->header));
+    $header = null;
+    if (strlen($this->header)) {
+      $header = phutil_render_tag(
+        'h1',
+        array(
+          'class' => 'phabricator-object-item-list-header',
+        ),
+        phutil_escape_html($this->header));
+    }
 
     if ($this->items) {
       $items = $this->renderSingleView($this->items);

src/view/layout/PhabricatorObjectItemView.php

@@ -7,6 +7,18 @@ final class PhabricatorObjectItemView extends AphrontView {
   private $attributes = array();
   private $details = array();
   private $dates = array();
+  private $icons = array();
+  private $barColor;
+  private $object;
+
+  public function setObject($object) {
+    $this->object = $object;
+    return $this;
+  }
+
+  public function getObject() {
+    return $this->object;
+  }
 
   public function setHref($href) {
     $this->href = $href;
@@ -26,14 +38,23 @@ final class PhabricatorObjectItemView extends AphrontView {
     return $this->header;
   }
 
-  public function addDetail($name, $value, $class = null) {
-    $this->details[] = array(
-      'name'  => $name,
-      'value' => $value,
+  public function addIcon($icon, $label = null) {
+    $this->icons[] = array(
+      'icon'  => $icon,
+      'label' => $label,
     );
     return $this;
   }
 
+  public function setBarColor($bar_color) {
+    $this->barColor = $bar_color;
+    return $this;
+  }
+
+  public function getBarColor() {
+    return $this->barColor;
+  }
+
   public function addAttribute($attribute) {
     $this->attributes[] = $attribute;
     return $this;
@@ -48,32 +69,61 @@ final class PhabricatorObjectItemView extends AphrontView {
       ),
       phutil_escape_html($this->header));
 
-    $details = null;
-    if ($this->details) {
-      $details = array();
-      foreach ($this->details as $detail) {
-        $details[] =
-          '<dt class="phabricator-object-detail-key">'.
-            phutil_escape_html($detail['name']).
-          '</dt>';
-        $details[] =
-          '<dd class="phabricator-object-detail-value">'.
-            $detail['value'].
-          '</dt>';
+    $icons = null;
+    if ($this->icons) {
+      $icon_list = array();
+      foreach ($this->icons as $spec) {
+        $icon = $spec['icon'];
+
+        $icon = phutil_render_tag(
+          'span',
+          array(
+            'class' => 'phabricator-object-item-icon-image '.
+                       'sprite-icon action-'.$icon,
+          ),
+          '');
+
+        $label = phutil_render_tag(
+          'span',
+          array(
+            'class' => 'phabricator-object-item-icon-label',
+          ),
+          phutil_escape_html($spec['label']));
+
+        $icon_list[] = phutil_render_tag(
+          'li',
+          array(
+            'class' => 'phabricator-object-item-icon',
+          ),
+          $label.$icon);
       }
-      $details = phutil_render_tag(
-        'dl',
+
+      $icons = phutil_render_tag(
+        'ul',
         array(
-          'class' => 'phabricator-object-detail-list',
+          'class' => 'phabricator-object-item-icons',
         ),
-        implode('', $details));
+        implode('', $icon_list));
     }
 
     $attrs = null;
     if ($this->attributes) {
       $attrs = array();
+      $spacer = phutil_render_tag(
+        'span',
+        array(
+          'class' => 'phabricator-object-item-attribute-spacer',
+        ),
+        '&middot;');
+      $first = true;
       foreach ($this->attributes as $attribute) {
-        $attrs[] = '<li>'.$attribute.'</li>';
+        $attrs[] = phutil_render_tag(
+          'li',
+          array(
+            'class' => 'phabricator-object-item-attribute',
+          ),
+          ($first ? null : $spacer).$attribute);
+        $first = false;
       }
       $attrs = phutil_render_tag(
         'ul',
@@ -83,12 +133,18 @@ final class PhabricatorObjectItemView extends AphrontView {
         implode('', $attrs));
     }
 
+    $classes = array();
+    $classes[] = 'phabricator-object-item';
+    if ($this->barColor) {
+      $classes[] = 'phabricator-object-item-bar-color-'.$this->barColor;
+    }
+
     return phutil_render_tag(
       'div',
       array(
-        'class' => 'phabricator-object-item',
+        'class' => implode(' ', $classes),
       ),
-      $header.$details.$attrs);
+      $icons.$header.$attrs);
   }
 
 }

src/view/layout/PhabricatorPinboardView.php

@@ -3,6 +3,12 @@
 final class PhabricatorPinboardView extends AphrontView {
 
   private $items = array();
+  private $noDataString;
+
+  public function setNoDataString($no_data_string) {
+    $this->noDataString = $no_data_string;
+    return $this;
+  }
 
   public function addItem(PhabricatorPinboardItemView $item) {
     $this->items[] = $item;
@@ -12,6 +18,14 @@ final class PhabricatorPinboardView extends AphrontView {
   public function render() {
     require_celerity_resource('phabricator-pinboard-view-css');
 
+    if (!$this->items) {
+      $string = nonempty($this->noDataString, pht('No data.'));
+      return id(new AphrontErrorView())
+        ->setSeverity(AphrontErrorView::SEVERITY_NODATA)
+        ->appendChild(phutil_escape_html($string))
+        ->render();
+    }
+
     return phutil_render_tag(
       'div',
       array(

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

@@ -3,100 +3,140 @@
  */
 
 .phabricator-object-item-list-view {
-  padding: 0 2%;
-  margin: 1em 0;
+  padding: 8px 6px;
+  background: #f4f5f6;
 }
 
-.phabricator-object-item-list-header {
-  color: #333333;
-  font-size: 15px;
-  margin-bottom: 12px;
-}
-
-
 .phabricator-object-item {
-  background: #f9f9f9;
-  border: 1px solid #dbdbdb;
-  border-radius: 2px;
-  margin: 6px 0;
+  background: #ffffff;
+  border-style: solid;
+  border-color: #d7d7d7 #e4e3e4 #bcbcbd #d7d7d7;
+  border-width: 1px 1px 1px 3px;
+  margin: 3px 0;
 
   overflow: hidden;
-  box-shadow: 0 1px 2px rgba(0, 0, 0, 0.10);
+  box-shadow: 0px 1px 1px rgba(0, 0, 0, 0.10);
 }
 
 .phabricator-object-item-name {
   display: block;
-  background: #e9e9e9;
-  padding: 2px 1em;
+  padding: 4px 8px 4px;
+  margin-right: 20px;
   font-weight: bold;
 }
 
-.device-desktop .phabricator-object-item-name {
-
-  border: solid #dbdbdb;
-  border-width: 0 0 1px 0;
-}
-
-/* - Detail List ------------------------------------------------------------
-
-  Object details, used to render things like authors, reviewers, etc.
-
-*/
-
-
-.phabricator-object-detail-list {
-  font-size: 11px;
-  width: 55%;
-  padding: 2px 1%;
-}
-
-.device-desktop .phabricator-object-detail-list,
-.device-tablet .phabricator-object-detail-list {
-  float: left;
-}
-
-.phabricator-object-detail-key {
-  color: #555555;
-  float: left;
-  clear: left;
-  width: 20%;
-  text-align: right;
-  padding: 2px 6px;
-  overflow: hidden;
-}
-
-.phabricator-object-detail-value {
-  color: #444444;
-  float: left;
-
-  padding: 2px 6px;
-}
-
-
-/* - Attribute List ------------------------------------------------------------
+/* - Attribute List ------------------------------
 
   Object attributes, commonly used to render created date, etc.
 
 */
 
+
 .phabricator-object-item-attributes {
+  padding: 0px 8px 4px;
+}
+
+.phabricator-object-item-attribute {
+  display: inline;
   color: #555555;
-  text-align: right;
   font-size: 11px;
-
-  padding: 2px 1%;
 }
 
-.device-desktop .phabricator-object-item-attributes,
-.device-tablet .phabricator-object-item-attributes {
-  width: 40%;
+.phabricator-object-item-attribute-spacer {
+  padding: 0 4px;
+}
+
+
+/* - Attribute List ------------------------------
+
+  Object attributes, commonly used to render created date, etc.
+
+*/
+
+.phabricator-object-item-icons {
   float: right;
+  padding: 2px 4px 0;
 }
 
-.device-phone .phabricator-object-item-attributes {
-  clear: both;
+.device-desktop .phabricator-object-item-icons {
+  width: 120px;
 }
 
-.phabricator-object-item-attributes li {
-  padding: 2px 1%;
+.device .phabricator-object-item-icons {
+  width: 18px;
+}
+
+.device .phabricator-object-item-icon-label {
+  display: none;
+  vertical-align: middle;
+}
+
+.phabricator-object-item-icon {
+  position: relative;
+  font-size: 11px;
+  color: #666666;
+  text-align: right;
+  white-space: nowrap;
+  overflow: hidden;
+
+  min-height: 18px;
+  line-height: 18px;
+}
+
+.device-desktop .phabricator-object-item-icon {
+  padding-right: 22px;
+}
+
+.phabricator-object-item-icon-image {
+  position: absolute;
+  right: 2px;
+  top: 2px;
+  width: 14px;
+  height: 14px;
+}
+
+
+/* - Bar Colors --------------------------------
+
+
+*/
+
+.phabricator-object-item-bar-color-red {
+  border-left-color: #cc0000;
+}
+
+.phabricator-object-item-bar-color-orange {
+  border-left-color: #cc7300;
+}
+
+.phabricator-object-item-bar-color-yellow {
+  border-left-color: #ccc000;
+}
+
+.phabricator-object-item-bar-color-green {
+  border-left-color: #009b2d;
+}
+
+.phabricator-object-item-bar-color-sky {
+  border-left-color: #6498cf;
+}
+
+.phabricator-object-item-bar-color-blue {
+  border-left-color: #00659a;
+}
+
+.phabricator-object-item-bar-color-indigo {
+  border-left-color: #3a00cc;
+}
+
+.phabricator-object-item-bar-color-violet {
+  border-left-color: #67009b;
+}
+
+.phabricator-object-item-bar-color-grey {
+  border-left-color: #999999;
+}
+
+.phabricator-object-item-bar-color-black {
+  border-left-color: #333333;
 }