From 3bc54c2041be7b0b55b4c1de6ac03a8354969efd Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 19 Jan 2015 10:14:27 -0800 Subject: [PATCH] Project revamp part 2: Edit Summary: Taking a pass at revamping the edit pages in Projects. Specifically: - Remove EditMainController - Move actions from EditMain to Profile - Move properties from EditMain to Profile - Move timeline from EditMain to Profile - Move Open Tasks from Profile to sidenavicon - Add custom icons and colors to timeline Feel free to bang on this a bit and give feedback, feels generally correct to me. Test Plan: Edit everything I could on various projects. Check links, timelines, actions. Reviewers: btrahan, epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D11421 --- src/__phutil_library_map__.php | 2 - .../PhabricatorFileComposeController.php | 2 +- .../PhabricatorProjectApplication.php | 1 - .../PhabricatorProjectArchiveController.php | 2 +- .../PhabricatorProjectController.php | 24 ++- ...habricatorProjectEditDetailsController.php | 11 +- .../PhabricatorProjectEditIconController.php | 2 +- .../PhabricatorProjectEditMainController.php | 160 ------------------ ...habricatorProjectEditPictureController.php | 2 +- .../PhabricatorProjectProfileController.php | 126 ++++++-------- .../storage/PhabricatorProjectTransaction.php | 50 +++++- src/view/layout/AphrontSideNavFilterView.php | 12 +- 12 files changed, 128 insertions(+), 266 deletions(-) delete mode 100644 src/applications/project/controller/PhabricatorProjectEditMainController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 97cf886ae8..2c67ae32aa 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2204,7 +2204,6 @@ phutil_register_library_map(array( 'PhabricatorProjectDescriptionField' => 'applications/project/customfield/PhabricatorProjectDescriptionField.php', 'PhabricatorProjectEditDetailsController' => 'applications/project/controller/PhabricatorProjectEditDetailsController.php', 'PhabricatorProjectEditIconController' => 'applications/project/controller/PhabricatorProjectEditIconController.php', - 'PhabricatorProjectEditMainController' => 'applications/project/controller/PhabricatorProjectEditMainController.php', 'PhabricatorProjectEditPictureController' => 'applications/project/controller/PhabricatorProjectEditPictureController.php', 'PhabricatorProjectEditorTestCase' => 'applications/project/editor/__tests__/PhabricatorProjectEditorTestCase.php', 'PhabricatorProjectFeedController' => 'applications/project/controller/PhabricatorProjectFeedController.php', @@ -5444,7 +5443,6 @@ phutil_register_library_map(array( 'PhabricatorProjectDescriptionField' => 'PhabricatorProjectStandardCustomField', 'PhabricatorProjectEditDetailsController' => 'PhabricatorProjectController', 'PhabricatorProjectEditIconController' => 'PhabricatorProjectController', - 'PhabricatorProjectEditMainController' => 'PhabricatorProjectController', 'PhabricatorProjectEditPictureController' => 'PhabricatorProjectController', 'PhabricatorProjectEditorTestCase' => 'PhabricatorTestCase', 'PhabricatorProjectFeedController' => 'PhabricatorProjectController', diff --git a/src/applications/files/controller/PhabricatorFileComposeController.php b/src/applications/files/controller/PhabricatorFileComposeController.php index c3351e045f..6903cff5d3 100644 --- a/src/applications/files/controller/PhabricatorFileComposeController.php +++ b/src/applications/files/controller/PhabricatorFileComposeController.php @@ -72,7 +72,7 @@ final class PhabricatorFileComposeController )); if ($project_phid) { - $edit_uri = '/project/edit/'.$project->getID().'/'; + $edit_uri = '/project/profile/'.$project->getID().'/'; $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) diff --git a/src/applications/project/application/PhabricatorProjectApplication.php b/src/applications/project/application/PhabricatorProjectApplication.php index f0e1f1734d..1f2b07cfbf 100644 --- a/src/applications/project/application/PhabricatorProjectApplication.php +++ b/src/applications/project/application/PhabricatorProjectApplication.php @@ -43,7 +43,6 @@ final class PhabricatorProjectApplication extends PhabricatorApplication { '/project/' => array( '(?:query/(?P[^/]+)/)?' => 'PhabricatorProjectListController', 'filter/(?P[^/]+)/' => 'PhabricatorProjectListController', - 'edit/(?P[1-9]\d*)/' => 'PhabricatorProjectEditMainController', 'details/(?P[1-9]\d*)/' => 'PhabricatorProjectEditDetailsController', 'archive/(?P[1-9]\d*)/' diff --git a/src/applications/project/controller/PhabricatorProjectArchiveController.php b/src/applications/project/controller/PhabricatorProjectArchiveController.php index f158783262..fdd34a3ea6 100644 --- a/src/applications/project/controller/PhabricatorProjectArchiveController.php +++ b/src/applications/project/controller/PhabricatorProjectArchiveController.php @@ -26,7 +26,7 @@ final class PhabricatorProjectArchiveController return new Aphront404Response(); } - $edit_uri = $this->getApplicationURI('edit/'.$project->getID().'/'); + $edit_uri = $this->getApplicationURI('profile/'.$project->getID().'/'); if ($request->isFormPost()) { if ($project->isArchived()) { diff --git a/src/applications/project/controller/PhabricatorProjectController.php b/src/applications/project/controller/PhabricatorProjectController.php index f56cd386b5..4955f7fe3f 100644 --- a/src/applications/project/controller/PhabricatorProjectController.php +++ b/src/applications/project/controller/PhabricatorProjectController.php @@ -7,28 +7,27 @@ abstract class PhabricatorProjectController extends PhabricatorController { } public function buildSideNavView($for_app = false) { - $user = $this->getRequest()->getUser(); + $viewer = $this->getViewer(); $nav = new AphrontSideNavFilterView(); $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); $id = null; if ($for_app) { - $user = $this->getRequest()->getUser(); $id = $this->getRequest()->getURIData('id'); if ($id) { $nav->addFilter("profile/{$id}/", pht('Profile')); $nav->addFilter("board/{$id}/", pht('Workboard')); $nav->addFilter("members/{$id}/", pht('Members')); $nav->addFilter("feed/{$id}/", pht('Feed')); - $nav->addFilter("edit/{$id}/", pht('Edit')); + $nav->addFilter("details/{$id}/", pht('Edit Details')); } $nav->addFilter('create', pht('Create Project')); } if (!$id) { id(new PhabricatorProjectSearchEngine()) - ->setViewer($user) + ->setViewer($viewer) ->addNavigationItems($nav->getMenu()); } @@ -38,13 +37,13 @@ abstract class PhabricatorProjectController extends PhabricatorController { } public function buildIconNavView(PhabricatorProject $project) { - $user = $this->getRequest()->getUser(); + $viewer = $this->getViewer(); $id = $project->getID(); $picture = $project->getProfileImageURI(); $name = $project->getName(); $columns = id(new PhabricatorProjectColumnQuery()) - ->setViewer($user) + ->setViewer($viewer) ->withProjectPHIDs(array($project->getPHID())) ->execute(); if ($columns) { @@ -58,9 +57,20 @@ abstract class PhabricatorProjectController extends PhabricatorController { $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); $nav->addIcon("profile/{$id}/", $name, null, $picture); $nav->addIcon("board/{$id}/", pht('Workboard'), $board_icon); + + $class = 'PhabricatorManiphestApplication'; + if (PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) { + $phid = $project->getPHID(); + $query_uri = urisprintf( + '/maniphest/?statuses=%s&allProjects=%s#R', + implode(',', ManiphestTaskStatus::getOpenStatusConstants()), + $phid); + $nav->addIcon(null, pht('Open Tasks'), 'fa-anchor', null, $query_uri); + } + $nav->addIcon("feed/{$id}/", pht('Feed'), 'fa-newspaper-o'); $nav->addIcon("members/{$id}/", pht('Members'), 'fa-group'); - $nav->addIcon("edit/{$id}/", pht('Edit'), 'fa-pencil'); + $nav->addIcon("details/{$id}/", pht('Edit Details'), 'fa-pencil'); return $nav; } diff --git a/src/applications/project/controller/PhabricatorProjectEditDetailsController.php b/src/applications/project/controller/PhabricatorProjectEditDetailsController.php index 7cffebafd4..263260a868 100644 --- a/src/applications/project/controller/PhabricatorProjectEditDetailsController.php +++ b/src/applications/project/controller/PhabricatorProjectEditDetailsController.php @@ -149,13 +149,8 @@ final class PhabricatorProjectEditDetailsController )); } - if ($is_new) { - $redirect_uri = - $this->getApplicationURI('profile/'.$project->getID().'/'); - } else { - $redirect_uri = - $this->getApplicationURI('edit/'.$project->getID().'/'); - } + $redirect_uri = + $this->getApplicationURI('profile/'.$project->getID().'/'); return id(new AphrontRedirectResponse())->setURI($redirect_uri); } catch (PhabricatorApplicationTransactionValidationException $ex) { @@ -304,7 +299,7 @@ final class PhabricatorProjectEditDetailsController if (!$is_new) { $nav = $this->buildIconNavView($project); - $nav->selectFilter("edit/{$id}/"); + $nav->selectFilter("details/{$id}/"); $nav->appendChild($form_box); } else { $nav = array($form_box); diff --git a/src/applications/project/controller/PhabricatorProjectEditIconController.php b/src/applications/project/controller/PhabricatorProjectEditIconController.php index d8fc42eaf7..c21080d595 100644 --- a/src/applications/project/controller/PhabricatorProjectEditIconController.php +++ b/src/applications/project/controller/PhabricatorProjectEditIconController.php @@ -26,7 +26,7 @@ final class PhabricatorProjectEditIconController if (!$project) { return new Aphront404Response(); } - $cancel_uri = $this->getApplicationURI('edit/'.$project->getID().'/'); + $cancel_uri = $this->getApplicationURI('profile/'.$project->getID().'/'); $project_icon = $project->getIcon(); } else { $this->requireApplicationCapability( diff --git a/src/applications/project/controller/PhabricatorProjectEditMainController.php b/src/applications/project/controller/PhabricatorProjectEditMainController.php deleted file mode 100644 index 083d650bac..0000000000 --- a/src/applications/project/controller/PhabricatorProjectEditMainController.php +++ /dev/null @@ -1,160 +0,0 @@ -id = idx($data, 'id'); - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); - $id = $request->getURIData('id'); - - $project = id(new PhabricatorProjectQuery()) - ->setViewer($viewer) - ->withIDs(array($this->id)) - ->needImages(true) - ->executeOne(); - if (!$project) { - return new Aphront404Response(); - } - - $header = id(new PHUIHeaderView()) - ->setHeader(pht('Edit %s', $project->getName())) - ->setUser($viewer) - ->setPolicyObject($project); - - if ($project->getStatus() == PhabricatorProjectStatus::STATUS_ACTIVE) { - $header->setStatus('fa-check', 'bluegrey', pht('Active')); - } else { - $header->setStatus('fa-ban', 'red', pht('Archived')); - } - - $actions = $this->buildActionListView($project); - $properties = $this->buildPropertyListView($project, $actions); - - $object_box = id(new PHUIObjectBoxView()) - ->setHeader($header) - ->addPropertyList($properties); - - $timeline = $this->buildTransactionTimeline( - $project, - new PhabricatorProjectTransactionQuery()); - $timeline->setShouldTerminate(true); - - $nav = $this->buildIconNavView($project); - $nav->selectFilter("edit/{$id}/"); - $nav->appendChild($object_box); - $nav->appendChild($timeline); - - $mnav = $this->buildSideNavView(); - - return $this->buildApplicationPage( - array( - $nav, - ), - array( - 'title' => $project->getName(), - )); - } - - private function buildActionListView(PhabricatorProject $project) { - $request = $this->getRequest(); - $viewer = $request->getUser(); - - $id = $project->getID(); - - $view = id(new PhabricatorActionListView()) - ->setUser($viewer) - ->setObjectURI($request->getRequestURI()); - - $can_edit = PhabricatorPolicyFilter::hasCapability( - $viewer, - $project, - PhabricatorPolicyCapability::CAN_EDIT); - - $view->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Edit Details')) - ->setIcon('fa-pencil') - ->setHref($this->getApplicationURI("details/{$id}/")) - ->setDisabled(!$can_edit) - ->setWorkflow(!$can_edit)); - - $view->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Edit Picture')) - ->setIcon('fa-picture-o') - ->setHref($this->getApplicationURI("picture/{$id}/")) - ->setDisabled(!$can_edit) - ->setWorkflow(!$can_edit)); - - if ($project->isArchived()) { - $view->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Activate Project')) - ->setIcon('fa-check') - ->setHref($this->getApplicationURI("archive/{$id}/")) - ->setDisabled(!$can_edit) - ->setWorkflow(true)); - } else { - $view->addAction( - id(new PhabricatorActionView()) - ->setName(pht('Archive Project')) - ->setIcon('fa-ban') - ->setHref($this->getApplicationURI("archive/{$id}/")) - ->setDisabled(!$can_edit) - ->setWorkflow(true)); - } - - return $view; - } - - private function buildPropertyListView( - PhabricatorProject $project, - PhabricatorActionListView $actions) { - $request = $this->getRequest(); - $viewer = $request->getUser(); - - $view = id(new PHUIPropertyListView()) - ->setUser($viewer) - ->setObject($project) - ->setActionList($actions); - - $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions( - $viewer, - $project); - - $this->loadHandles(array($project->getPHID())); - - $view->addProperty( - pht('Looks Like'), - $this->getHandle($project->getPHID())->renderTag()); - - $view->addProperty( - pht('Visible To'), - $descriptions[PhabricatorPolicyCapability::CAN_VIEW]); - - $view->addProperty( - pht('Editable By'), - $descriptions[PhabricatorPolicyCapability::CAN_EDIT]); - - $view->addProperty( - pht('Joinable By'), - $descriptions[PhabricatorPolicyCapability::CAN_JOIN]); - - return $view; - } - - -} diff --git a/src/applications/project/controller/PhabricatorProjectEditPictureController.php b/src/applications/project/controller/PhabricatorProjectEditPictureController.php index 8e342c3c9a..2a71947f57 100644 --- a/src/applications/project/controller/PhabricatorProjectEditPictureController.php +++ b/src/applications/project/controller/PhabricatorProjectEditPictureController.php @@ -28,7 +28,7 @@ final class PhabricatorProjectEditPictureController return new Aphront404Response(); } - $edit_uri = $this->getApplicationURI('edit/'.$project->getID().'/'); + $edit_uri = $this->getApplicationURI('profile/'.$project->getID().'/'); $view_uri = $this->getApplicationURI('profile/'.$project->getID().'/'); $supported_formats = PhabricatorFile::getTransformableImageFormats(); diff --git a/src/applications/project/controller/PhabricatorProjectProfileController.php b/src/applications/project/controller/PhabricatorProjectProfileController.php index 208c23e098..36f8878698 100644 --- a/src/applications/project/controller/PhabricatorProjectProfileController.php +++ b/src/applications/project/controller/PhabricatorProjectProfileController.php @@ -33,26 +33,12 @@ final class PhabricatorProjectProfileController } $picture = $project->getProfileImageURI(); - require_celerity_resource('phabricator-profile-css'); - $tasks = $this->renderTasksPage($project); - $content = phutil_tag_div('phabricator-project-layout', $tasks); - - $phid = $project->getPHID(); - $create_uri = '/maniphest/task/create/?projects='.$phid; - $icon_new = id(new PHUIIconView()) - ->setIconFont('fa-plus'); - $button_add = id(new PHUIButtonView()) - ->setTag('a') - ->setText(pht('New Task')) - ->setHref($create_uri) - ->setIcon($icon_new); $header = id(new PHUIHeaderView()) ->setHeader($project->getName()) ->setUser($user) ->setPolicyObject($project) - ->setImage($picture) - ->addActionLink($button_add); + ->setImage($picture); if ($project->getStatus() == PhabricatorProjectStatus::STATUS_ACTIVE) { $header->setStatus('fa-check', 'bluegrey', pht('Active')); @@ -67,10 +53,15 @@ final class PhabricatorProjectProfileController ->setHeader($header) ->addPropertyList($properties); + $timeline = $this->buildTransactionTimeline( + $project, + new PhabricatorProjectTransactionQuery()); + $timeline->setShouldTerminate(true); + $nav = $this->buildIconNavView($project); $nav->selectFilter("profile/{$id}/"); $nav->appendChild($object_box); - $nav->appendChild($content); + $nav->appendChild($timeline); return $this->buildApplicationPage( array( @@ -81,66 +72,6 @@ final class PhabricatorProjectProfileController )); } - private function renderTasksPage(PhabricatorProject $project) { - - $user = $this->getRequest()->getUser(); - $limit = 50; - - $query = id(new ManiphestTaskQuery()) - ->setViewer($user) - ->withAnyProjects(array($project->getPHID())) - ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants()) - ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY) - ->needProjectPHIDs(true) - ->setLimit(($limit + 1)); - $tasks = $query->execute(); - $count = count($tasks); - if ($count == ($limit + 1)) { - array_pop($tasks); - } - - $phids = mpull($tasks, 'getOwnerPHID'); - $phids = array_merge( - $phids, - array_mergev(mpull($tasks, 'getProjectPHIDs'))); - $phids = array_filter($phids); - $handles = $this->loadViewerHandles($phids); - - $task_list = new ManiphestTaskListView(); - $task_list->setUser($user); - $task_list->setTasks($tasks); - $task_list->setHandles($handles); - $task_list->setNoDataString(pht('This project has no open tasks.')); - - $phid = $project->getPHID(); - $view_uri = urisprintf( - '/maniphest/?statuses=%s&allProjects=%s#R', - implode(',', ManiphestTaskStatus::getOpenStatusConstants()), - $phid); - $icon = id(new PHUIIconView()) - ->setIconFont('fa-search'); - $button_view = id(new PHUIButtonView()) - ->setTag('a') - ->setText(pht('View Query')) - ->setHref($view_uri) - ->setIcon($icon); - - $header = id(new PHUIHeaderView()) - ->addActionLink($button_view); - - if ($count > $limit) { - $header->setHeader(pht('Highest Priority (some)')); - } else { - $header->setHeader(pht('Highest Priority (all)')); - } - - $content = id(new PHUIObjectBoxView()) - ->setHeader($header) - ->appendChild($task_list); - - return $content; - } - private function buildActionListView(PhabricatorProject $project) { $request = $this->getRequest(); $viewer = $request->getUser(); @@ -159,10 +90,35 @@ final class PhabricatorProjectProfileController $view->addAction( id(new PhabricatorActionView()) - ->setName(pht('Edit Project')) + ->setName(pht('Edit Details')) ->setIcon('fa-pencil') - ->setHref($this->getApplicationURI("edit/{$id}/"))); + ->setHref($this->getApplicationURI("details/{$id}/"))); + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Edit Picture')) + ->setIcon('fa-picture-o') + ->setHref($this->getApplicationURI("picture/{$id}/")) + ->setDisabled(!$can_edit) + ->setWorkflow(!$can_edit)); + + if ($project->isArchived()) { + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Activate Project')) + ->setIcon('fa-check') + ->setHref($this->getApplicationURI("archive/{$id}/")) + ->setDisabled(!$can_edit) + ->setWorkflow(true)); + } else { + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Archive Project')) + ->setIcon('fa-ban') + ->setHref($this->getApplicationURI("archive/{$id}/")) + ->setDisabled(!$can_edit) + ->setWorkflow(true)); + } $action = null; if (!$project->isUserMember($viewer->getPHID())) { @@ -244,6 +200,20 @@ final class PhabricatorProjectProfileController ? $this->renderHandlesForPHIDs($project->getWatcherPHIDs(), ',') : phutil_tag('em', array(), pht('None'))); + $descriptions = PhabricatorPolicyQuery::renderPolicyDescriptions( + $viewer, + $project); + + $this->loadHandles(array($project->getPHID())); + + $view->addProperty( + pht('Looks Like'), + $this->getHandle($project->getPHID())->renderTag()); + + $view->addProperty( + pht('Joinable By'), + $descriptions[PhabricatorPolicyCapability::CAN_JOIN]); + $field_list = PhabricatorCustomField::getObjectFields( $project, PhabricatorCustomField::ROLE_VIEW); diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php index d7ed26c43f..381f4d6cb0 100644 --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -42,6 +42,52 @@ final class PhabricatorProjectTransaction return array_merge($req_phids, parent::getRequiredHandlePHIDs()); } + public function getColor() { + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case PhabricatorProjectTransaction::TYPE_STATUS: + if ($old == 0) { + return 'red'; + } else { + return 'green'; + } + } + return parent::getColor(); + } + + public function getIcon() { + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + switch ($this->getTransactionType()) { + case PhabricatorProjectTransaction::TYPE_STATUS: + if ($old == 0) { + return 'fa-ban'; + } else { + return 'fa-check'; + } + case PhabricatorProjectTransaction::TYPE_LOCKED: + if ($new) { + return 'fa-lock'; + } else { + return 'fa-unlock'; + } + case PhabricatorProjectTransaction::TYPE_ICON: + return $new; + case PhabricatorProjectTransaction::TYPE_IMAGE: + return 'fa-photo'; + case PhabricatorProjectTransaction::TYPE_MEMBERS: + return 'fa-user'; + case PhabricatorProjectTransaction::TYPE_SLUGS: + return 'fa-tag'; + } + return parent::getIcon(); + } + public function getTitle() { $old = $this->getOldValue(); $new = $this->getNewValue(); @@ -63,11 +109,11 @@ final class PhabricatorProjectTransaction case PhabricatorProjectTransaction::TYPE_STATUS: if ($old == 0) { return pht( - '%s closed this project.', + '%s archived this project.', $author_handle); } else { return pht( - '%s reopened this project.', + '%s activated this project.', $author_handle); } case PhabricatorProjectTransaction::TYPE_IMAGE: diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index f82208d5d6..744d5b78e8 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/src/view/layout/AphrontSideNavFilterView.php @@ -100,10 +100,14 @@ final class AphrontSideNavFilterView extends AphrontView { $key, $name, $uri, PHUIListItemView::TYPE_LINK); } - public function addIcon($key, $name, $icon, $image = null) { - $href = clone $this->baseURI; - $href->setPath(rtrim($href->getPath().$key, '/').'/'); - $href = (string)$href; + public function addIcon($key, $name, $icon, $image = null, $uri = null) { + if (!$uri) { + $href = clone $this->baseURI; + $href->setPath(rtrim($href->getPath().$key, '/').'/'); + $href = (string)$href; + } else { + $href = $uri; + } $item = id(new PHUIListItemView()) ->setKey($key)