From 300910f462a36acb94b5102090ae13aa31b7d7e9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 15 Aug 2014 11:16:08 -0700 Subject: [PATCH] Allow columns to have a point limit Summary: Fixes T5885. This implements optional soft point limits for workboard columns, per traditional Kanban. - Allow columns to have a point limit set. - When a column has a point limit, show it in the header. - If a column has too many points in it, show the column and point count in red. @chad, this could probably use some design tweaks. In particular: - I changed the color of "hidden" columns to avoid confusion with "overfull" columns. We might be able to find a better color. - UI hints for overfull columns might need adjustment. (After T4427, we'll let you sum some custom field instead of total number of tasks, which is why this is called "points" rather than "number of tasks".) Test Plan: {F190914} Note that: - "Pre-planning" has a limit, so it shows "4/12". - "Planning" has a limit and is overfull, so it shows "5 / 4". - Other columns do not have limits. - "Post-planning" is a hidden column. This might be too muted now. Transactions: {F190915} Error messages / edit screen: {F190916} Reviewers: btrahan, chad Reviewed By: btrahan Subscribers: chad, epriestley Maniphest Tasks: T5885 Differential Revision: https://secure.phabricator.com/D10276 --- resources/celerity/map.php | 24 ++++++------ src/__phutil_library_map__.php | 4 +- .../PhabricatorProjectApplication.php | 2 +- .../PhabricatorProjectBoardViewController.php | 17 ++++++--- ...abricatorProjectColumnDetailController.php | 6 +++ ...habricatorProjectColumnEditController.php} | 38 +++++++++++++++---- ...bricatorProjectColumnTransactionEditor.php | 25 ++++++++++++ .../storage/PhabricatorProjectColumn.php | 31 +++++++++++++-- .../PhabricatorProjectColumnTransaction.php | 19 ++++++++++ src/view/phui/PHUIWorkpanelView.php | 14 +++++++ webroot/rsrc/css/phui/phui-workpanel-view.css | 13 +++++-- .../projects/behavior-project-boards.js | 28 ++++++++++---- 12 files changed, 178 insertions(+), 43 deletions(-) rename src/applications/project/controller/{PhabricatorProjectBoardEditController.php => PhabricatorProjectColumnEditController.php} (79%) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 51d0a60ac7..0396eb33a3 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -145,7 +145,7 @@ return array( 'rsrc/css/phui/phui-text.css' => '23e9b4b7', 'rsrc/css/phui/phui-timeline-view.css' => 'bbd990d0', 'rsrc/css/phui/phui-workboard-view.css' => '2bf82d00', - 'rsrc/css/phui/phui-workpanel-view.css' => 'e26044fa', + 'rsrc/css/phui/phui-workpanel-view.css' => '198c7e6c', 'rsrc/css/sprite-apps-large.css' => '20ec0cc0', 'rsrc/css/sprite-apps.css' => 'd5baed0f', 'rsrc/css/sprite-conpherence.css' => '3b4a0487', @@ -412,7 +412,7 @@ return array( 'rsrc/js/application/policy/behavior-policy-rule-editor.js' => 'fe9a552f', 'rsrc/js/application/ponder/behavior-votebox.js' => '4e9b766b', 'rsrc/js/application/projects/behavior-boards-dropdown.js' => '0ec56e1d', - 'rsrc/js/application/projects/behavior-project-boards.js' => 'e4b6c65a', + 'rsrc/js/application/projects/behavior-project-boards.js' => 'a6c6a058', 'rsrc/js/application/projects/behavior-project-create.js' => '065227cc', 'rsrc/js/application/projects/behavior-reorder-columns.js' => 'e1d25dfb', 'rsrc/js/application/releeph/releeph-preview-branch.js' => 'b2b4fbaf', @@ -636,7 +636,7 @@ return array( 'javelin-behavior-policy-control' => 'f3fef818', 'javelin-behavior-policy-rule-editor' => 'fe9a552f', 'javelin-behavior-ponder-votebox' => '4e9b766b', - 'javelin-behavior-project-boards' => 'e4b6c65a', + 'javelin-behavior-project-boards' => 'a6c6a058', 'javelin-behavior-project-create' => '065227cc', 'javelin-behavior-refresh-csrf' => '7814b593', 'javelin-behavior-releeph-preview-branch' => 'b2b4fbaf', @@ -792,7 +792,7 @@ return array( 'phui-text-css' => '23e9b4b7', 'phui-timeline-view-css' => 'bbd990d0', 'phui-workboard-view-css' => '2bf82d00', - 'phui-workpanel-view-css' => 'e26044fa', + 'phui-workpanel-view-css' => '198c7e6c', 'phuix-action-list-view' => 'b5c256b8', 'phuix-action-view' => '6e8cefa4', 'phuix-dropdown-menu' => 'bd4c8dca', @@ -1471,6 +1471,14 @@ return array( 'a5d7cf86' => array( 'javelin-dom', ), + 'a6c6a058' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-workflow', + 'phabricator-draggable-list', + ), 'a80d0378' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1770,14 +1778,6 @@ return array( 'javelin-dom', 'javelin-uri', ), - 'e4b6c65a' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-workflow', - 'phabricator-draggable-list', - ), 'e566f52c' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 74cd4b0c7c..94949bc310 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1930,12 +1930,12 @@ phutil_register_library_map(array( 'PhabricatorProjectArchiveController' => 'applications/project/controller/PhabricatorProjectArchiveController.php', 'PhabricatorProjectBoardController' => 'applications/project/controller/PhabricatorProjectBoardController.php', 'PhabricatorProjectBoardDeleteController' => 'applications/project/controller/PhabricatorProjectBoardDeleteController.php', - 'PhabricatorProjectBoardEditController' => 'applications/project/controller/PhabricatorProjectBoardEditController.php', 'PhabricatorProjectBoardImportController' => 'applications/project/controller/PhabricatorProjectBoardImportController.php', 'PhabricatorProjectBoardReorderController' => 'applications/project/controller/PhabricatorProjectBoardReorderController.php', 'PhabricatorProjectBoardViewController' => 'applications/project/controller/PhabricatorProjectBoardViewController.php', 'PhabricatorProjectColumn' => 'applications/project/storage/PhabricatorProjectColumn.php', 'PhabricatorProjectColumnDetailController' => 'applications/project/controller/PhabricatorProjectColumnDetailController.php', + 'PhabricatorProjectColumnEditController' => 'applications/project/controller/PhabricatorProjectColumnEditController.php', 'PhabricatorProjectColumnPHIDType' => 'applications/project/phid/PhabricatorProjectColumnPHIDType.php', 'PhabricatorProjectColumnPosition' => 'applications/project/storage/PhabricatorProjectColumnPosition.php', 'PhabricatorProjectColumnPositionQuery' => 'applications/project/query/PhabricatorProjectColumnPositionQuery.php', @@ -4771,7 +4771,6 @@ phutil_register_library_map(array( 'PhabricatorProjectArchiveController' => 'PhabricatorProjectController', 'PhabricatorProjectBoardController' => 'PhabricatorProjectController', 'PhabricatorProjectBoardDeleteController' => 'PhabricatorProjectBoardController', - 'PhabricatorProjectBoardEditController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectBoardImportController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectBoardReorderController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectBoardViewController' => 'PhabricatorProjectBoardController', @@ -4781,6 +4780,7 @@ phutil_register_library_map(array( 'PhabricatorDestructibleInterface', ), 'PhabricatorProjectColumnDetailController' => 'PhabricatorProjectBoardController', + 'PhabricatorProjectColumnEditController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectColumnPHIDType' => 'PhabricatorPHIDType', 'PhabricatorProjectColumnPosition' => array( 'PhabricatorProjectDAO', diff --git a/src/applications/project/application/PhabricatorProjectApplication.php b/src/applications/project/application/PhabricatorProjectApplication.php index c5d484157d..059113acc6 100644 --- a/src/applications/project/application/PhabricatorProjectApplication.php +++ b/src/applications/project/application/PhabricatorProjectApplication.php @@ -66,7 +66,7 @@ final class PhabricatorProjectApplication extends PhabricatorApplication { 'move/(?P[1-9]\d*)/' => 'PhabricatorProjectMoveController', 'board/(?P[1-9]\d*)/' => array( 'edit/(?:(?P\d+)/)?' - => 'PhabricatorProjectBoardEditController', + => 'PhabricatorProjectColumnEditController', 'delete/(?:(?P\d+)/)?' => 'PhabricatorProjectBoardDeleteController', 'column/(?:(?P\d+)/)?' diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index ef2c882c07..00c5f5065d 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -225,7 +225,16 @@ final class PhabricatorProjectBoardViewController $panel = id(new PHUIWorkpanelView()) ->setHeader($column->getDisplayName()) - ->setHeaderColor($column->getHeaderColor()); + ->addSigil('workpanel'); + + $header_icon = $column->getHeaderIcon(); + if ($header_icon) { + $panel->setHeaderIcon($header_icon); + } + + if ($column->isHidden()) { + $panel->addClass('project-panel-hidden'); + } $column_menu = $this->buildColumnMenu($project, $column); $panel->addHeaderAction($column_menu); @@ -252,6 +261,7 @@ final class PhabricatorProjectBoardViewController 'columnPHID' => $column->getPHID(), 'countTagID' => $tag_id, 'countTagContentID' => $tag_content_id, + 'pointLimit' => $column->getPointLimit(), )); foreach ($column_tasks as $task) { @@ -268,11 +278,6 @@ final class PhabricatorProjectBoardViewController ->getItem()); } $panel->setCards($cards); - - if (!$column_tasks) { - $cards->addClass('project-column-empty'); - } - $board->addPanel($panel); } diff --git a/src/applications/project/controller/PhabricatorProjectColumnDetailController.php b/src/applications/project/controller/PhabricatorProjectColumnDetailController.php index ae519c70cb..7c7c05bab2 100644 --- a/src/applications/project/controller/PhabricatorProjectColumnDetailController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnDetailController.php @@ -160,6 +160,12 @@ final class PhabricatorProjectColumnDetailController pht('Editable By'), $descriptions[PhabricatorPolicyCapability::CAN_EDIT]); + + $limit = $column->getPointLimit(); + $properties->addProperty( + pht('Point Limit'), + $limit ? $limit : pht('No Limit')); + return $properties; } diff --git a/src/applications/project/controller/PhabricatorProjectBoardEditController.php b/src/applications/project/controller/PhabricatorProjectColumnEditController.php similarity index 79% rename from src/applications/project/controller/PhabricatorProjectBoardEditController.php rename to src/applications/project/controller/PhabricatorProjectColumnEditController.php index af674898e3..84a5e2d590 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardEditController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnEditController.php @@ -1,6 +1,6 @@ getPointLimit(); + $v_name = $column->getName(); + $validation_exception = null; $base_uri = '/board/'.$this->projectID.'/'; if ($is_new) { @@ -60,7 +65,8 @@ final class PhabricatorProjectBoardEditController } if ($request->isFormPost()) { - $new_name = $request->getStr('name'); + $v_name = $request->getStr('name'); + $v_limit = $request->getStr('limit'); if ($is_new) { $column->setProjectPHID($project->getPHID()); @@ -79,10 +85,17 @@ final class PhabricatorProjectBoardEditController $column->setSequence($new_sequence); } + $xactions = array(); + $type_name = PhabricatorProjectColumnTransaction::TYPE_NAME; - $xactions = array(id(new PhabricatorProjectColumnTransaction()) + $xactions[] = id(new PhabricatorProjectColumnTransaction()) ->setTransactionType($type_name) - ->setNewValue($new_name)); + ->setNewValue($v_name); + + $type_limit = PhabricatorProjectColumnTransaction::TYPE_LIMIT; + $xactions[] = id(new PhabricatorProjectColumnTransaction()) + ->setTransactionType($type_limit) + ->setNewValue($v_limit); try { $editor = id(new PhabricatorProjectColumnTransactionEditor()) @@ -93,20 +106,31 @@ final class PhabricatorProjectBoardEditController return id(new AphrontRedirectResponse())->setURI($view_uri); } catch (PhabricatorApplicationTransactionValidationException $ex) { $e_name = $ex->getShortMessage($type_name); + $e_limit = $ex->getShortMessage($type_limit); $validation_exception = $ex; } } $form = new AphrontFormView(); - $form->setUser($request->getUser()) + $form + ->setUser($request->getUser()) ->appendChild( id(new AphrontFormTextControl()) - ->setValue($column->getName()) + ->setValue($v_name) ->setLabel(pht('Name')) ->setName('name') ->setError($e_name) ->setCaption( - pht('This will be displayed as the header of the column.'))); + pht('This will be displayed as the header of the column.'))) + ->appendChild( + id(new AphrontFormTextControl()) + ->setValue($v_limit) + ->setLabel(pht('Point Limit')) + ->setName('limit') + ->setError($e_limit) + ->setCaption( + pht('Maximum number of points of tasks allowed in the column.'))); + if ($is_new) { $title = pht('Create Column'); diff --git a/src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php index 7eea56e338..08b0355ce5 100644 --- a/src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectColumnTransactionEditor.php @@ -16,6 +16,7 @@ final class PhabricatorProjectColumnTransactionEditor $types[] = PhabricatorProjectColumnTransaction::TYPE_NAME; $types[] = PhabricatorProjectColumnTransaction::TYPE_STATUS; + $types[] = PhabricatorProjectColumnTransaction::TYPE_LIMIT; return $types; } @@ -29,6 +30,9 @@ final class PhabricatorProjectColumnTransactionEditor return $object->getName(); case PhabricatorProjectColumnTransaction::TYPE_STATUS: return $object->getStatus(); + case PhabricatorProjectColumnTransaction::TYPE_LIMIT: + return $object->getPointLimit(); + } return parent::getCustomTransactionOldValue($object, $xaction); @@ -42,6 +46,11 @@ final class PhabricatorProjectColumnTransactionEditor case PhabricatorProjectColumnTransaction::TYPE_NAME: case PhabricatorProjectColumnTransaction::TYPE_STATUS: return $xaction->getNewValue(); + case PhabricatorProjectColumnTransaction::TYPE_LIMIT: + if ($xaction->getNewValue()) { + return (int)$xaction->getNewValue(); + } + return null; } return parent::getCustomTransactionNewValue($object, $xaction); @@ -58,6 +67,9 @@ final class PhabricatorProjectColumnTransactionEditor case PhabricatorProjectColumnTransaction::TYPE_STATUS: $object->setStatus($xaction->getNewValue()); return; + case PhabricatorProjectColumnTransaction::TYPE_LIMIT: + $object->setPointLimit($xaction->getNewValue()); + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -70,6 +82,7 @@ final class PhabricatorProjectColumnTransactionEditor switch ($xaction->getTransactionType()) { case PhabricatorProjectColumnTransaction::TYPE_NAME: case PhabricatorProjectColumnTransaction::TYPE_STATUS: + case PhabricatorProjectColumnTransaction::TYPE_LIMIT: return; } @@ -84,6 +97,18 @@ final class PhabricatorProjectColumnTransactionEditor $errors = parent::validateTransaction($object, $type, $xactions); switch ($type) { + case PhabricatorProjectColumnTransaction::TYPE_LIMIT: + foreach ($xactions as $xaction) { + $value = $xaction->getNewValue(); + if (strlen($value) && !preg_match('/^\d+\z/', $value)) { + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht('Column point limit must be empty, or a positive integer.'), + $xaction); + } + } + break; case PhabricatorProjectColumnTransaction::TYPE_NAME: $missing = $this->validateIsEmptyTextField( $object->getName(), diff --git a/src/applications/project/storage/PhabricatorProjectColumn.php b/src/applications/project/storage/PhabricatorProjectColumn.php index eec88dbcbc..e37a28dce3 100644 --- a/src/applications/project/storage/PhabricatorProjectColumn.php +++ b/src/applications/project/storage/PhabricatorProjectColumn.php @@ -71,16 +71,30 @@ final class PhabricatorProjectColumn return pht('Unnamed Column'); } - public function getHeaderColor() { + public function getHeaderIcon() { + $icon = null; + if ($this->isHidden()) { - return PHUIActionHeaderView::HEADER_LIGHTRED; + $icon = 'fa-eye-slash'; + $text = pht('Hidden'); } if ($this->isDefaultColumn()) { - return PHUIActionHeaderView::HEADER_DARK_GREY; + $icon = 'fa-archive'; + $text = pht('Default'); } - return PHUIActionHeaderView::HEADER_GREY; + if ($icon) { + return id(new PHUIIconView()) + ->setIconFont($icon) + ->addSigil('has-tooltip') + ->setMetadata( + array( + 'tip' => $text, + ));; + } + + return null; } public function getProperty($key, $default = null) { @@ -92,6 +106,15 @@ final class PhabricatorProjectColumn return $this; } + public function getPointLimit() { + return $this->getProperty('pointLimit'); + } + + public function setPointLimit($limit) { + $this->setProperty('pointLimit', $limit); + return $this; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/project/storage/PhabricatorProjectColumnTransaction.php b/src/applications/project/storage/PhabricatorProjectColumnTransaction.php index e89672c070..dd4f95d5f6 100644 --- a/src/applications/project/storage/PhabricatorProjectColumnTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectColumnTransaction.php @@ -5,6 +5,7 @@ final class PhabricatorProjectColumnTransaction const TYPE_NAME = 'project:col:name'; const TYPE_STATUS = 'project:col:status'; + const TYPE_LIMIT = 'project:col:limit'; public function getApplicationName() { return 'project'; @@ -43,6 +44,24 @@ final class PhabricatorProjectColumnTransaction $author_handle); } } + case PhabricatorProjectColumnTransaction::TYPE_LIMIT: + if (!$old) { + return pht( + '%s set the point limit for this column to %s.', + $author_handle, + $new); + } else if (!$new) { + return pht( + '%s removed the point limit for this column.', + $author_handle); + } else { + return pht( + '%s changed point limit for this column from %s to %s.', + $author_handle, + $old, + $new); + } + case PhabricatorProjectColumnTransaction::TYPE_STATUS: switch ($new) { case PhabricatorProjectColumn::STATUS_ACTIVE: diff --git a/src/view/phui/PHUIWorkpanelView.php b/src/view/phui/PHUIWorkpanelView.php index 20b77a8685..ef420b7056 100644 --- a/src/view/phui/PHUIWorkpanelView.php +++ b/src/view/phui/PHUIWorkpanelView.php @@ -8,6 +8,16 @@ final class PHUIWorkpanelView extends AphrontTagView { private $headerColor = PHUIActionHeaderView::HEADER_GREY; private $headerActions = array(); private $headerTag; + private $headerIcon; + + public function setHeaderIcon(PHUIIconView $header_icon) { + $this->headerIcon = $header_icon; + return $this; + } + + public function getHeaderIcon() { + return $this->headerIcon; + } public function setCards(PHUIObjectItemListView $cards) { $this->cards[] = $cards; @@ -65,6 +75,10 @@ final class PHUIWorkpanelView extends AphrontTagView { ->setHeaderTitle($this->header) ->setHeaderColor($this->headerColor); + if ($this->headerIcon) { + $header->setHeaderIcon($this->headerIcon); + } + if ($this->headerTag) { $header->setTag($this->headerTag); } diff --git a/webroot/rsrc/css/phui/phui-workpanel-view.css b/webroot/rsrc/css/phui/phui-workpanel-view.css index 0a5f4e9e4d..07fab3b746 100644 --- a/webroot/rsrc/css/phui/phui-workpanel-view.css +++ b/webroot/rsrc/css/phui/phui-workpanel-view.css @@ -93,25 +93,30 @@ width: auto; } -.project-column-empty { +.project-panel-hidden { + opacity: 0.75; +} + +.project-panel-empty .phui-object-item-list-view { background: rgba(255,255,255,.4); border-radius: 3px; margin-bottom: 4px; border: 1px dashed #fff; } -.project-column-empty .drag-ghost { +.project-panel-empty .phui-object-item-list-view .drag-ghost { display: none; } -.project-column-empty.drag-target-list { +.project-panel-empty .phui-object-item-list-view.drag-target-list { background: rgba(255,255,255,.7); } -.phui-workpanel-view .phui-workpanel-lightred .phui-action-header { +.project-panel-over-limit .phui-action-header { border-top: 1px solid {$redborder}; border-left: 1px solid {$redborder}; border-right: 1px solid {$redborder}; + background: {$lightredbackground}; } /* - Workpanel Cards ----------------------------------------------------------- diff --git a/webroot/rsrc/js/application/projects/behavior-project-boards.js b/webroot/rsrc/js/application/projects/behavior-project-boards.js index 3bf136046e..21fb4735c9 100644 --- a/webroot/rsrc/js/application/projects/behavior-project-boards.js +++ b/webroot/rsrc/js/application/projects/behavior-project-boards.js @@ -18,9 +18,6 @@ JX.behavior('project-boards', function(config) { var data = JX.Stratcom.getData(col); var cards = finditems(col); - // Add the "empty" CSS class if the column has nothing in it. - JX.DOM.alterClass(col, 'project-column-empty', !cards.length); - // Update the count of tasks in the column header. if (!data.countTagNode) { data.countTagNode = JX.$(data.countTagID); @@ -33,17 +30,34 @@ JX.behavior('project-boards', function(config) { sum += 1; } - JX.DOM.setContent(JX.$(data.countTagContentID), sum); - // TODO: This is a little bit hacky, but we don't have a PHUIX version of // this element yet. + var over_limit = (data.pointLimit && (sum > data.pointLimit)); + + var display_value = sum; + if (data.pointLimit) { + display_value = sum + ' / ' + data.pointLimit; + } + JX.DOM.setContent(JX.$(data.countTagContentID), display_value); + + + var panel_map = { + 'project-panel-empty': !cards.length, + 'project-panel-over-limit': over_limit + }; + var panel = JX.DOM.findAbove(col, 'div', 'workpanel'); + for (var k in panel_map) { + JX.DOM.alterClass(panel, k, !!panel_map[k]); + } + var color_map = { 'phui-tag-shade-disabled': (sum === 0), - 'phui-tag-shade-blue': (sum > 0) + 'phui-tag-shade-blue': (sum > 0 && !over_limit), + 'phui-tag-shade-red': (over_limit) }; for (var k in color_map) { - JX.DOM.alterClass(data.countTagNode, k, color_map[k]); + JX.DOM.alterClass(data.countTagNode, k, !!color_map[k]); } }