From c020f027bbe6c17c1ba4a38150a4935b7fb9668d Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 11 Mar 2019 08:58:46 -0700 Subject: [PATCH] Add an "Sort by Creation Date" filter to workboards and modularize remaining order behaviors Summary: Depends on D20274. Ref T10578. This is en route to an ordering by points, it's just a simpler half-step on the way there. Allow columns to be sorted by creation date, so the newest tasks rise to the top. In this ordering you can never reposition cards, since editing a creation date by dragging makes no sense. This will be true of the "points" ordering too (although we could imagine doing something like prompting the user, some day). Test Plan: Viewed boards by "natural" (allows reordering both when dragging within and between columns), "priority" (reorder only within columns), and "creation date" (reorder never). Dragged cards around between and within columns, got apparently sensible behavior. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T10578 Differential Revision: https://secure.phabricator.com/D20275 --- resources/celerity/map.php | 90 ++++++++++--------- src/__phutil_library_map__.php | 2 + .../PhabricatorProjectBoardViewController.php | 4 + .../PhabricatorProjectColumnCreatedOrder.php | 31 +++++++ .../PhabricatorProjectColumnNaturalOrder.php | 8 ++ .../order/PhabricatorProjectColumnOrder.php | 10 +++ .../PhabricatorProjectColumnOwnerOrder.php | 8 ++ .../PhabricatorProjectColumnPriorityOrder.php | 8 ++ .../js/application/projects/WorkboardBoard.js | 31 ++++++- .../application/projects/WorkboardColumn.js | 10 +-- .../projects/WorkboardOrderTemplate.js | 27 ++++++ .../projects/behavior-project-boards.js | 16 +++- webroot/rsrc/js/core/DraggableList.js | 25 +++++- 13 files changed, 212 insertions(+), 58 deletions(-) create mode 100644 src/applications/project/order/PhabricatorProjectColumnCreatedOrder.php create mode 100644 webroot/rsrc/js/application/projects/WorkboardOrderTemplate.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 33d8b09bc8..014839c14a 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,7 +10,7 @@ return array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', 'core.pkg.css' => '34ce1741', - 'core.pkg.js' => '200a0a61', + 'core.pkg.js' => 'f9c2509b', 'differential.pkg.css' => '8d8360fb', 'differential.pkg.js' => '67e02996', 'diffusion.pkg.css' => '42c75c37', @@ -408,14 +408,15 @@ return array( 'rsrc/js/application/phortune/phortune-credit-card-form.js' => 'd12d214f', 'rsrc/js/application/policy/behavior-policy-control.js' => '0eaa33a9', 'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '9347f172', - 'rsrc/js/application/projects/WorkboardBoard.js' => 'eb55f7e8', + 'rsrc/js/application/projects/WorkboardBoard.js' => '9d59f098', 'rsrc/js/application/projects/WorkboardCard.js' => '0392a5d8', 'rsrc/js/application/projects/WorkboardCardTemplate.js' => '2a61f8d4', - 'rsrc/js/application/projects/WorkboardColumn.js' => 'fd4c2069', + 'rsrc/js/application/projects/WorkboardColumn.js' => 'ec5c5ce0', 'rsrc/js/application/projects/WorkboardController.js' => '42c7a5a7', 'rsrc/js/application/projects/WorkboardHeader.js' => '111bfd2d', 'rsrc/js/application/projects/WorkboardHeaderTemplate.js' => 'b65351bd', - 'rsrc/js/application/projects/behavior-project-boards.js' => '285c337a', + 'rsrc/js/application/projects/WorkboardOrderTemplate.js' => '03e8891f', + 'rsrc/js/application/projects/behavior-project-boards.js' => '412af9d4', 'rsrc/js/application/projects/behavior-project-create.js' => '34c53422', 'rsrc/js/application/projects/behavior-reorder-columns.js' => '8ac32fd9', 'rsrc/js/application/releeph/releeph-preview-branch.js' => '75184d68', @@ -436,7 +437,7 @@ return array( 'rsrc/js/application/uiexample/notification-example.js' => '29819b75', 'rsrc/js/core/Busy.js' => '5202e831', 'rsrc/js/core/DragAndDropFileUpload.js' => '4370900d', - 'rsrc/js/core/DraggableList.js' => '91f40fbf', + 'rsrc/js/core/DraggableList.js' => '8bc7d797', 'rsrc/js/core/Favicon.js' => '7930776a', 'rsrc/js/core/FileUpload.js' => 'ab85e184', 'rsrc/js/core/Hovercard.js' => '074f0783', @@ -656,7 +657,7 @@ return array( 'javelin-behavior-phuix-example' => 'c2c500a7', 'javelin-behavior-policy-control' => '0eaa33a9', 'javelin-behavior-policy-rule-editor' => '9347f172', - 'javelin-behavior-project-boards' => '285c337a', + 'javelin-behavior-project-boards' => '412af9d4', 'javelin-behavior-project-create' => '34c53422', 'javelin-behavior-quicksand-blacklist' => '5a6f6a06', 'javelin-behavior-read-only-warning' => 'b9109f8f', @@ -728,13 +729,14 @@ return array( 'javelin-view-renderer' => '9aae2b66', 'javelin-view-visitor' => '308f9fe4', 'javelin-websocket' => 'fdc13e4e', - 'javelin-workboard-board' => 'eb55f7e8', + 'javelin-workboard-board' => '9d59f098', 'javelin-workboard-card' => '0392a5d8', 'javelin-workboard-card-template' => '2a61f8d4', - 'javelin-workboard-column' => 'fd4c2069', + 'javelin-workboard-column' => 'ec5c5ce0', 'javelin-workboard-controller' => '42c7a5a7', 'javelin-workboard-header' => '111bfd2d', 'javelin-workboard-header-template' => 'b65351bd', + 'javelin-workboard-order-template' => '03e8891f', 'javelin-workflow' => '958e9045', 'maniphest-report-css' => '3d53188b', 'maniphest-task-edit-css' => '272daa84', @@ -759,7 +761,7 @@ return array( 'phabricator-diff-changeset-list' => '04023d82', 'phabricator-diff-inline' => 'a4a14a94', 'phabricator-drag-and-drop-file-upload' => '4370900d', - 'phabricator-draggable-list' => '91f40fbf', + 'phabricator-draggable-list' => '8bc7d797', 'phabricator-fatal-config-template-css' => '20babf50', 'phabricator-favicon' => '7930776a', 'phabricator-feed-css' => 'd8b6e3f8', @@ -912,6 +914,9 @@ return array( '0392a5d8' => array( 'javelin-install', ), + '03e8891f' => array( + 'javelin-install', + ), '04023d82' => array( 'javelin-install', 'phuix-button-view', @@ -1105,15 +1110,6 @@ return array( 'javelin-json', 'phabricator-prefab', ), - '285c337a' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-vector', - 'javelin-stratcom', - 'javelin-workflow', - 'javelin-workboard-controller', - ), '289bf236' => array( 'javelin-install', 'javelin-util', @@ -1231,6 +1227,15 @@ return array( 'javelin-behavior', 'javelin-uri', ), + '412af9d4' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-util', + 'javelin-vector', + 'javelin-stratcom', + 'javelin-workflow', + 'javelin-workboard-controller', + ), '4234f572' => array( 'syntax-default-css', ), @@ -1588,6 +1593,14 @@ return array( 'javelin-dom', 'javelin-typeahead-normalizer', ), + '8bc7d797' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-util', + 'javelin-vector', + 'javelin-magical-init', + ), '8c2ed2bf' => array( 'javelin-behavior', 'javelin-dom', @@ -1635,14 +1648,6 @@ return array( 'javelin-workflow', 'javelin-stratcom', ), - '91f40fbf' => array( - 'javelin-install', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-util', - 'javelin-vector', - 'javelin-magical-init', - ), '92388bae' => array( 'javelin-behavior', 'javelin-scrollbar', @@ -1720,6 +1725,18 @@ return array( 'javelin-uri', 'phabricator-textareautils', ), + '9d59f098' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-workflow', + 'phabricator-draggable-list', + 'javelin-workboard-column', + 'javelin-workboard-header-template', + 'javelin-workboard-card-template', + 'javelin-workboard-order-template', + ), '9f081f05' => array( 'javelin-behavior', 'javelin-dom', @@ -2051,20 +2068,14 @@ return array( 'javelin-install', 'javelin-event', ), - 'eb55f7e8' => array( - 'javelin-install', - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-workflow', - 'phabricator-draggable-list', - 'javelin-workboard-column', - 'javelin-workboard-header-template', - 'javelin-workboard-card-template', - ), 'ec4e31c0' => array( 'phui-timeline-view-css', ), + 'ec5c5ce0' => array( + 'javelin-install', + 'javelin-workboard-card', + 'javelin-workboard-header', + ), 'ee77366f' => array( 'aphront-dialog-view-css', ), @@ -2133,11 +2144,6 @@ return array( 'javelin-magical-init', 'javelin-util', ), - 'fd4c2069' => array( - 'javelin-install', - 'javelin-workboard-card', - 'javelin-workboard-header', - ), 'fdc13e4e' => array( 'javelin-install', ), diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e79ae6ea68..9716e7180e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4050,6 +4050,7 @@ phutil_register_library_map(array( 'PhabricatorProjectColorTransaction' => 'applications/project/xaction/PhabricatorProjectColorTransaction.php', 'PhabricatorProjectColorsConfigType' => 'applications/project/config/PhabricatorProjectColorsConfigType.php', 'PhabricatorProjectColumn' => 'applications/project/storage/PhabricatorProjectColumn.php', + 'PhabricatorProjectColumnCreatedOrder' => 'applications/project/order/PhabricatorProjectColumnCreatedOrder.php', 'PhabricatorProjectColumnDetailController' => 'applications/project/controller/PhabricatorProjectColumnDetailController.php', 'PhabricatorProjectColumnEditController' => 'applications/project/controller/PhabricatorProjectColumnEditController.php', 'PhabricatorProjectColumnHeader' => 'applications/project/order/PhabricatorProjectColumnHeader.php', @@ -10135,6 +10136,7 @@ phutil_register_library_map(array( 'PhabricatorExtendedPolicyInterface', 'PhabricatorConduitResultInterface', ), + 'PhabricatorProjectColumnCreatedOrder' => 'PhabricatorProjectColumnOrder', 'PhabricatorProjectColumnDetailController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectColumnEditController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectColumnHeader' => 'Phobject', diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index 43599fa369..8784847f37 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -631,6 +631,9 @@ final class PhabricatorProjectBoardViewController $header_keys = $ordering->getHeaderKeysForObjects($all_tasks); + $order_maps = array(); + $order_maps[] = $ordering->toDictionary(); + $properties = array(); $behavior_config = array( @@ -642,6 +645,7 @@ final class PhabricatorProjectBoardViewController 'boardPHID' => $project->getPHID(), 'order' => $this->sortKey, + 'orders' => $order_maps, 'headers' => $headers, 'headerKeys' => $header_keys, 'templateMap' => $templates, diff --git a/src/applications/project/order/PhabricatorProjectColumnCreatedOrder.php b/src/applications/project/order/PhabricatorProjectColumnCreatedOrder.php new file mode 100644 index 0000000000..945445aecf --- /dev/null +++ b/src/applications/project/order/PhabricatorProjectColumnCreatedOrder.php @@ -0,0 +1,31 @@ +getDateCreated(), + (int)-$object->getID(), + ); + } + +} diff --git a/src/applications/project/order/PhabricatorProjectColumnNaturalOrder.php b/src/applications/project/order/PhabricatorProjectColumnNaturalOrder.php index 26f4d28601..b21a554715 100644 --- a/src/applications/project/order/PhabricatorProjectColumnNaturalOrder.php +++ b/src/applications/project/order/PhabricatorProjectColumnNaturalOrder.php @@ -9,4 +9,12 @@ final class PhabricatorProjectColumnNaturalOrder return pht('Natural'); } + public function getHasHeaders() { + return false; + } + + public function getCanReorder() { + return true; + } + } diff --git a/src/applications/project/order/PhabricatorProjectColumnOrder.php b/src/applications/project/order/PhabricatorProjectColumnOrder.php index 815ab19f80..f3a1ed86ca 100644 --- a/src/applications/project/order/PhabricatorProjectColumnOrder.php +++ b/src/applications/project/order/PhabricatorProjectColumnOrder.php @@ -68,6 +68,8 @@ abstract class PhabricatorProjectColumnOrder } abstract public function getDisplayName(); + abstract public function getHasHeaders(); + abstract public function getCanReorder(); protected function newColumnTransactions($object, array $header) { return array(); @@ -173,4 +175,12 @@ abstract class PhabricatorProjectColumnOrder ->setOrderKey($this->getColumnOrderKey()); } + final public function toDictionary() { + return array( + 'orderKey' => $this->getColumnOrderKey(), + 'hasHeaders' => $this->getHasHeaders(), + 'canReorder' => $this->getCanReorder(), + ); + } + } diff --git a/src/applications/project/order/PhabricatorProjectColumnOwnerOrder.php b/src/applications/project/order/PhabricatorProjectColumnOwnerOrder.php index c98ee61715..a41b78a11c 100644 --- a/src/applications/project/order/PhabricatorProjectColumnOwnerOrder.php +++ b/src/applications/project/order/PhabricatorProjectColumnOwnerOrder.php @@ -13,6 +13,14 @@ final class PhabricatorProjectColumnOwnerOrder return 'fa-users'; } + public function getHasHeaders() { + return true; + } + + public function getCanReorder() { + return true; + } + protected function newHeaderKeyForObject($object) { return $this->newHeaderKeyForOwnerPHID($object->getOwnerPHID()); } diff --git a/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php b/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php index e08e7c07ef..64a5934e26 100644 --- a/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php +++ b/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php @@ -13,6 +13,14 @@ final class PhabricatorProjectColumnPriorityOrder return 'fa-sort-numeric-asc'; } + public function getHasHeaders() { + return true; + } + + public function getCanReorder() { + return true; + } + protected function newHeaderKeyForObject($object) { return $this->newHeaderKeyForPriority($object->getPriority()); } diff --git a/webroot/rsrc/js/application/projects/WorkboardBoard.js b/webroot/rsrc/js/application/projects/WorkboardBoard.js index a05777e2f2..fa10b2a180 100644 --- a/webroot/rsrc/js/application/projects/WorkboardBoard.js +++ b/webroot/rsrc/js/application/projects/WorkboardBoard.js @@ -9,6 +9,7 @@ * javelin-workboard-column * javelin-workboard-header-template * javelin-workboard-card-template + * javelin-workboard-order-template * @javelin */ @@ -21,6 +22,7 @@ JX.install('WorkboardBoard', { this._headers = {}; this._cards = {}; + this._orders = {}; this._buildColumns(); }, @@ -70,6 +72,14 @@ JX.install('WorkboardBoard', { return this._headers[header_key]; }, + getOrderTemplate: function(order_key) { + if (!this._orders[order_key]) { + this._orders[order_key] = new JX.WorkboardOrderTemplate(order_key); + } + + return this._orders[order_key]; + }, + getHeaderTemplatesForOrder: function(order) { var templates = []; @@ -134,6 +144,10 @@ JX.install('WorkboardBoard', { _setupDragHandlers: function() { var columns = this.getColumns(); + var order_template = this.getOrderTemplate(this.getOrder()); + var has_headers = order_template.getHasHeaders(); + var can_reorder = order_template.getCanReorder(); + var lists = []; for (var k in columns) { var column = columns[k]; @@ -149,8 +163,21 @@ JX.install('WorkboardBoard', { list.setGhostHandler( JX.bind(column, column.handleDragGhost, default_handler)); - if (this.getOrder() !== 'natural') { - list.setCompareHandler(JX.bind(column, column.compareHandler)); + // The "compare handler" locks cards into a specific position in the + // column. + list.setCompareHandler(JX.bind(column, column.compareHandler)); + + // If the view has group headers, we lock cards into the right position + // when moving them between columns, but not within a column. + if (has_headers) { + list.setCompareOnMove(true); + } + + // If we can't reorder cards, we always lock them into their current + // position. + if (!can_reorder) { + list.setCompareOnMove(true); + list.setCompareOnReorder(true); } list.listen('didDrop', JX.bind(this, this._onmovecard, list)); diff --git a/webroot/rsrc/js/application/projects/WorkboardColumn.js b/webroot/rsrc/js/application/projects/WorkboardColumn.js index 271bed467c..709c52016a 100644 --- a/webroot/rsrc/js/application/projects/WorkboardColumn.js +++ b/webroot/rsrc/js/application/projects/WorkboardColumn.js @@ -189,15 +189,7 @@ JX.install('WorkboardColumn', { var board = this.getBoard(); var order = board.getOrder(); - // TODO: This should be modularized into "ProjectColumnOrder" classes, - // but is currently hard-coded. - - switch (order) { - case 'natural': - return false; - } - - return true; + return board.getOrderTemplate(order).getHasHeaders(); }, redraw: function() { diff --git a/webroot/rsrc/js/application/projects/WorkboardOrderTemplate.js b/webroot/rsrc/js/application/projects/WorkboardOrderTemplate.js new file mode 100644 index 0000000000..083dc78b50 --- /dev/null +++ b/webroot/rsrc/js/application/projects/WorkboardOrderTemplate.js @@ -0,0 +1,27 @@ +/** + * @provides javelin-workboard-order-template + * @requires javelin-install + * @javelin + */ + +JX.install('WorkboardOrderTemplate', { + + construct: function(order) { + this._orderKey = order; + }, + + properties: { + hasHeaders: false, + canReorder: false + }, + + members: { + _orderKey: null, + + getOrderKey: function() { + return this._orderKey; + } + + } + +}); diff --git a/webroot/rsrc/js/application/projects/behavior-project-boards.js b/webroot/rsrc/js/application/projects/behavior-project-boards.js index 51c067925f..3aa43722c4 100644 --- a/webroot/rsrc/js/application/projects/behavior-project-boards.js +++ b/webroot/rsrc/js/application/projects/behavior-project-boards.js @@ -87,11 +87,12 @@ JX.behavior('project-boards', function(config, statics) { .setNodeHTMLTemplate(templates[k]); } + var ii; var column_maps = config.columnMaps; for (var column_phid in column_maps) { var column = board.getColumn(column_phid); var column_map = column_maps[column_phid]; - for (var ii = 0; ii < column_map.length; ii++) { + for (ii = 0; ii < column_map.length; ii++) { column.newCard(column_map[ii]); } } @@ -111,8 +112,8 @@ JX.behavior('project-boards', function(config, statics) { } var headers = config.headers; - for (var jj = 0; jj < headers.length; jj++) { - var header = headers[jj]; + for (ii = 0; ii < headers.length; ii++) { + var header = headers[ii]; board.getHeaderTemplate(header.key) .setOrder(header.order) @@ -121,6 +122,15 @@ JX.behavior('project-boards', function(config, statics) { .setEditProperties(header.editProperties); } + var orders = config.orders; + for (ii = 0; ii < orders.length; ii++) { + var order = orders[ii]; + + board.getOrderTemplate(order.orderKey) + .setHasHeaders(order.hasHeaders) + .setCanReorder(order.canReorder); + } + var header_keys = config.headerKeys; for (var header_phid in header_keys) { board.getCardTemplate(header_phid) diff --git a/webroot/rsrc/js/core/DraggableList.js b/webroot/rsrc/js/core/DraggableList.js index 1c7ca766f2..64f57503b8 100644 --- a/webroot/rsrc/js/core/DraggableList.js +++ b/webroot/rsrc/js/core/DraggableList.js @@ -43,7 +43,9 @@ JX.install('DraggableList', { isDropTargetHandler: null, canDragX: false, outerContainer: null, - hasInfiniteHeight: false + hasInfiniteHeight: false, + compareOnMove: false, + compareOnReorder: false }, members : { @@ -501,7 +503,26 @@ JX.install('DraggableList', { var cur_target = false; if (target_list) { - if (compare_handler && (target_list !== this)) { + // Determine if we're going to use the compare handler or not: the + // compare hander locks items into a specific place in the list. For + // example, on Workboards, some operations permit the user to drag + // items between lists, but not to reorder items within a list. + + var should_compare = false; + + var is_reorder = (target_list === this); + var is_move = (target_list !== this); + + if (compare_handler) { + if (is_reorder && this.getCompareOnReorder()) { + should_compare = true; + } + if (is_move && this.getCompareOnMove()) { + should_compare = true; + } + } + + if (should_compare) { cur_target = target_list._getOrderedTarget(this, this._dragging); } else { cur_target = target_list._getCurrentTarget(p); -- 2.51.2