Modernizing Flag Application

Summary: Fixes a weird string in the flag dialog Migrated to ObjectItemView (Cards) Removed filters from side nav (unnecessary) Go away, go away, panel. Nobody will miss you. Adding sorting/ordering to Flag (separation like in Maniphest ordering still remaining) Hacking our way in DifferentialRevisionListController thanks to panels (who added `->setFlush()` btw? It's awesome!) Test Plan: I would not dare to stand here if it did not work. Reviewers: epriestley, chad, btrahan Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D5449
2024-11-23 07:12:41 +01:00 · 2013-03-26 14:05:45 -07:00 · 2013-03-26 14:05:45 -07:00 · 9a6c912ef6
commit 9a6c912ef6
parent 8fc94b02a9
6 changed files with 137 additions and 98 deletions
--- a/src/applications/differential/controller/DifferentialRevisionListController.php
+++ b/src/applications/differential/controller/DifferentialRevisionListController.php
@ -146,9 +146,7 @@ final class DifferentialRevisionListController extends DifferentialController {
      $handles = $this->loadViewerHandles($phids);

      foreach ($views as $view) {
-        if (empty($view['special'])) {
-          $view['view']->setHandles($handles);
-        }
+        $view['view']->setHandles($handles);
        $panel = new AphrontPanelView();
        $panel->setHeader($view['title']);
        $panel->appendChild($view['view']);
@ -460,29 +458,6 @@ final class DifferentialRevisionListController extends DifferentialController {
          'view'  => $view,
        );

-        // Flags are sort of private, so only show the flag panel if you're
-        // looking at your own requests.
-        if (in_array($user->getPHID(), $user_phids)) {
-          $flags = id(new PhabricatorFlagQuery())
-            ->setViewer($user)
-            ->withOwnerPHIDs(array($user->getPHID()))
-            ->withTypes(array(PhabricatorPHIDConstants::PHID_TYPE_DREV))
-            ->needHandles(true)
-            ->execute();
-
-          if ($flags) {
-            $view = id(new PhabricatorFlagListView())
-              ->setFlags($flags)
-              ->setUser($user);
-
-            $views[] = array(
-              'title'   => pht('Flagged Revisions'),
-              'view'    => $view,
-              'special' => true,
-            );
-          }
-        }
-
        $view = id(clone $template)
          ->setRevisions($waiting)
          ->loadAssets();
--- a/src/applications/flag/controller/PhabricatorFlagEditController.php
+++ b/src/applications/flag/controller/PhabricatorFlagEditController.php
@ -54,8 +54,8 @@ final class PhabricatorFlagEditController extends PhabricatorFlagController {
      $form
        ->appendChild(hsprintf(
          "<p>%s</p><br />",
-          pht('You can flag this %s if you want to remember to look ".
-            "at it later.',
+          pht('You can flag this %s if you want to remember to look '.
+            'at it later.',
            $type_name)));
    }

--- a/src/applications/flag/controller/PhabricatorFlagListController.php
+++ b/src/applications/flag/controller/PhabricatorFlagListController.php
@ -7,33 +7,67 @@ final class PhabricatorFlagListController extends PhabricatorFlagController {
    $user = $request->getUser();

    $nav = new AphrontSideNavFilterView();
-    $nav->setBaseURI(new PhutilURI('/flag/view/'));
-    $nav->addLabel(pht('Flags'));
-    $nav->addFilter('all', pht('Your Flags'));
-    $nav->selectFilter('all', 'all');
+
+    $filter_form = new AphrontFormView();
+    $filter_form->setUser($user);
+    $filter_form->appendChild(
+      id(new AphrontFormToggleButtonsControl())
+        ->setName('o')
+        ->setLabel(pht('Sort Order'))
+        ->setBaseURI($request->getRequestURI(), 'o')
+        ->setValue($request->getStr('o', 'n'))
+        ->setButtons(
+          array(
+            'n'   => pht('Date'),
+            'c'   => pht('Color'),
+            'o'   => pht('Object Type'),
+            'r'   => pht('Reason'),
+          )));
+
+    $filter = new AphrontListFilterView();
+    $filter->appendChild($filter_form);

    $query = new PhabricatorFlagQuery();
    $query->withOwnerPHIDs(array($user->getPHID()));
    $query->setViewer($user);
    $query->needHandles(true);

+    switch ($request->getStr('o', 'n')) {
+      case 'n':
+        $order = PhabricatorFlagQuery::ORDER_ID;
+        break;
+      case 'c':
+        $order = PhabricatorFlagQuery::ORDER_COLOR;
+        break;
+      case 'o':
+        $order = PhabricatorFlagQuery::ORDER_OBJECT;
+        break;
+      case 'r':
+        $order = PhabricatorFlagQuery::ORDER_REASON;
+        break;
+      default:
+        throw new Exception("Unknown order!");
+    }
+    $query->withOrder($order);
+
    $flags = $query->execute();

    $view = new PhabricatorFlagListView();
    $view->setFlags($flags);
    $view->setUser($user);

-    $panel = new AphrontPanelView();
-    $panel->setHeader(pht('Flags'));
-    $panel->appendChild($view);
-    $panel->setNoBackground();
+    $header = new PhabricatorHeaderView();
+    $header->setHeader(pht('Flags'));

-    $nav->appendChild($panel);
+    $nav->appendChild($header);
+    $nav->appendChild($filter);
+    $nav->appendChild($view);

-    return $this->buildStandardPageResponse(
+    return $this->buildApplicationPage(
      $nav,
      array(
        'title' => pht('Flags'),
+        'dust'  => true,
      ));
  }

--- a/src/applications/flag/query/PhabricatorFlagQuery.php
+++ b/src/applications/flag/query/PhabricatorFlagQuery.php
@ -13,6 +13,12 @@ final class PhabricatorFlagQuery {
  private $needObjects;
  private $viewer;

+  private $order     = 'order-id';
+  const ORDER_ID     = 'order-id';
+  const ORDER_COLOR  = 'order-color';
+  const ORDER_OBJECT = 'order-object';
+  const ORDER_REASON = 'order-reason';
+
  public function setViewer($viewer) {
    $this->viewer = $viewer;
    return $this;
@ -33,6 +39,11 @@ final class PhabricatorFlagQuery {
    return $this;
  }

+  public function withOrder($order) {
+    $this->order = $order;
+    return $this;
+  }
+
  public function needHandles($need) {
    $this->needHandles = $need;
    return $this;
@ -143,7 +154,29 @@ final class PhabricatorFlagQuery {
  }

  private function buildOrderClause($conn_r) {
-    return 'ORDER BY id DESC';
+    return qsprintf($conn_r,
+      'ORDER BY %Q',
+      $this->getOrderColumn($conn_r));
+  }
+
+  private function getOrderColumn($conn_r) {
+    switch ($this->order) {
+      case self::ORDER_ID:
+        return 'id DESC';
+        break;
+      case self::ORDER_COLOR:
+        return 'color DESC';
+        break;
+      case self::ORDER_OBJECT:
+        return 'type ASC';
+        break;
+      case self::ORDER_REASON:
+        return 'reasonPHID DESC';
+        break;
+      default:
+        throw new Exception("Unknown order {$this->order}!");
+        break;
+    }
  }

  private function buildLimitClause($conn_r) {
--- a/src/applications/flag/view/PhabricatorFlagListView.php
+++ b/src/applications/flag/view/PhabricatorFlagListView.php
@ -3,6 +3,7 @@
 final class PhabricatorFlagListView extends AphrontView {

  private $flags;
+  private $flush = false;

  public function setFlags(array $flags) {
    assert_instances_of($flags, 'PhabricatorFlag');
@ -10,75 +11,64 @@ final class PhabricatorFlagListView extends AphrontView {
    return $this;
  }

+  public function setFlush($flush) {
+    $this->flush = $flush;
+    return $this;
+  }
+
  public function render() {
    $user = $this->user;

    require_celerity_resource('phabricator-flag-css');

+    $list = new PhabricatorObjectItemListView();
+    $list->setCards(true);
+    $list->setNoDataString(pht('No flags.'));
+    $list->setFlush($this->flush);
+
    $rows = array();
    foreach ($this->flags as $flag) {
      $class = PhabricatorFlagColor::getCSSClass($flag->getColor());

-      $rows[] = array(
-        phutil_tag(
-          'div',
-          array(
-            'class' => 'phabricator-flag-icon '.$class,
-          ),
-          ''),
-        $flag->getHandle()->renderLink(),
-        $flag->getNote(),
-        phabricator_datetime($flag->getDateCreated(), $user),
-        phabricator_form(
-          $user,
-          array(
-            'method' => 'POST',
-            'action' => '/flag/edit/'.$flag->getObjectPHID().'/',
-            'sigil'  => 'workflow',
-          ),
-          phutil_tag(
-            'button',
-            array(
-              'class' => 'small grey',
-            ),
-            pht('Edit Flag'))),
-        phabricator_form(
-          $user,
-          array(
-            'method' => 'POST',
-            'action' => '/flag/delete/'.$flag->getID().'/',
-            'sigil'  => 'workflow',
-          ),
-          phutil_tag(
-            'button',
-            array(
-              'class' => 'small grey',
-            ),
-            pht('Remove Flag'))),
-      );
+      $flag_icon = phutil_tag(
+        'div',
+        array(
+          'class' => 'phabricator-flag-icon '.$class,
+        ),
+        '');
+
+      $edit_link = javelin_tag(
+        'a',
+        array(
+          'href' => '/flag/edit/'.$flag->getObjectPHID().'/',
+          'sigil' => 'workflow',
+        ),
+        pht('Edit'));
+
+      $remove_link = javelin_tag(
+        'a',
+        array(
+          'href' => '/flag/delete/'.$flag->getID().'/',
+          'sigil' => 'workflow',
+        ),
+        pht('Remove'));
+
+      $item = new PhabricatorObjectItemView();
+      $item->addIcon('edit', $edit_link);
+      $item->addIcon('delete', $remove_link);
+
+      $item->setHeader(hsprintf('%s %s',
+        $flag_icon, $flag->getHandle()->renderLink()));
+
+      $item->addAttribute(phabricator_datetime($flag->getDateCreated(), $user));
+
+      if ($flag->getNote()) {
+        $item->addAttribute($flag->getNote());
+      }
+
+      $list->addItem($item);
    }

-    $table = new AphrontTableView($rows);
-    $table->setHeaders(
-      array(
-        '',
-        pht('Flagged Object'),
-        pht('Note'),
-        pht('Flagged On'),
-        '',
-        '',
-      ));
-    $table->setColumnClasses(
-      array(
-        'narrow',
-        'wrap pri',
-        'wrap',
-        'narrow',
-        'narrow action',
-        'narrow action',
-      ));
-    $table->setNoDataString(pht('No flags.'));
-
-    return $table->render();
+    return $list;
  }
 }
--- a/webroot/rsrc/css/application/flag/flag.css
+++ b/webroot/rsrc/css/application/flag/flag.css
@ -7,6 +7,13 @@
  background: transparent 0 0 no-repeat;
 }

+.phabricator-object-item .phabricator-flag-icon {
+  float: left;
+  margin: 2px;
+  margin-right: 8px;
+  margin-bottom: 0;
+}
+
 .phabricator-flag-radio {
  padding-left: 18px;
  margin: 1px;