From 9a6c912ef6cbc8f9c49d46f4f5973e19339c4473 Mon Sep 17 00:00:00 2001 From: Anh Nhan Nguyen Date: Tue, 26 Mar 2013 14:05:45 -0700 Subject: [PATCH] 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 --- .../DifferentialRevisionListController.php | 27 +---- .../PhabricatorFlagEditController.php | 4 +- .../PhabricatorFlagListController.php | 54 +++++++-- .../flag/query/PhabricatorFlagQuery.php | 35 +++++- .../flag/view/PhabricatorFlagListView.php | 108 ++++++++---------- webroot/rsrc/css/application/flag/flag.css | 7 ++ 6 files changed, 137 insertions(+), 98 deletions(-) diff --git a/src/applications/differential/controller/DifferentialRevisionListController.php b/src/applications/differential/controller/DifferentialRevisionListController.php index ad1be4bf00..e54143a466 100644 --- 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(); diff --git a/src/applications/flag/controller/PhabricatorFlagEditController.php b/src/applications/flag/controller/PhabricatorFlagEditController.php index 69968ba678..ff8c356da7 100644 --- 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( "

%s


", - 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))); } diff --git a/src/applications/flag/controller/PhabricatorFlagListController.php b/src/applications/flag/controller/PhabricatorFlagListController.php index f4ebca574f..9b43507f92 100644 --- 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, )); } diff --git a/src/applications/flag/query/PhabricatorFlagQuery.php b/src/applications/flag/query/PhabricatorFlagQuery.php index 8ee7b33753..20fb19e416 100644 --- 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) { diff --git a/src/applications/flag/view/PhabricatorFlagListView.php b/src/applications/flag/view/PhabricatorFlagListView.php index 9e43887216..1ee5b2ed48 100644 --- 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; } } diff --git a/webroot/rsrc/css/application/flag/flag.css b/webroot/rsrc/css/application/flag/flag.css index 5ab0b12ea0..b5a1a8c17b 100644 --- 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;