Make drag-and-drop on workboards interact with priority column headers

Summary:
Ref T10333. Ref T8135. Depends on D20247. Allow users to drag-and-drop cards on a priority-sorted workboard under headers, even if the header has no other cards.

As of D20247, headers show up but they aren't really interactive. Now, you can drag cards directly underneath a header (instead of only between other cards). For example, if a column has only one "Wishlist" task, you may drag it under the "High", "Normal", or "Low" priority headers to select a specific priority.

(Some of this code still feels a little rough, but I think it will generalize once other types of sorting are available.)

Test Plan: Dragged cards within and between priority groups, saw appropriate priority edits applied in every case I could come up with.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T10333, T8135

Differential Revision: https://secure.phabricator.com/D20248

resources/celerity/map.php

@@ -409,13 +409,13 @@ 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' => 'e4e2d107',
-    'rsrc/js/application/projects/WorkboardCard.js' => 'c23ddfde',
-    'rsrc/js/application/projects/WorkboardColumn.js' => 'fd9cb972',
+    'rsrc/js/application/projects/WorkboardBoard.js' => 'a4f1e85d',
+    'rsrc/js/application/projects/WorkboardCard.js' => '887ef74f',
+    'rsrc/js/application/projects/WorkboardColumn.js' => 'ca444dca',
     'rsrc/js/application/projects/WorkboardController.js' => '42c7a5a7',
-    'rsrc/js/application/projects/WorkboardHeader.js' => '354c5c0e',
-    'rsrc/js/application/projects/WorkboardHeaderTemplate.js' => '9b86cd0d',
-    'rsrc/js/application/projects/behavior-project-boards.js' => 'a3f6b67f',
+    'rsrc/js/application/projects/WorkboardHeader.js' => '6e75daea',
+    'rsrc/js/application/projects/WorkboardHeaderTemplate.js' => '2d641f7d',
+    'rsrc/js/application/projects/behavior-project-boards.js' => 'e2730b90',
     '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',
@@ -657,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' => 'a3f6b67f',
+    'javelin-behavior-project-boards' => 'e2730b90',
     'javelin-behavior-project-create' => '34c53422',
     'javelin-behavior-quicksand-blacklist' => '5a6f6a06',
     'javelin-behavior-read-only-warning' => 'b9109f8f',
@@ -729,12 +729,12 @@ return array(
     'javelin-view-renderer' => '9aae2b66',
     'javelin-view-visitor' => '308f9fe4',
     'javelin-websocket' => 'fdc13e4e',
-    'javelin-workboard-board' => 'e4e2d107',
-    'javelin-workboard-card' => 'c23ddfde',
-    'javelin-workboard-column' => 'fd9cb972',
+    'javelin-workboard-board' => 'a4f1e85d',
+    'javelin-workboard-card' => '887ef74f',
+    'javelin-workboard-column' => 'ca444dca',
     'javelin-workboard-controller' => '42c7a5a7',
-    'javelin-workboard-header' => '354c5c0e',
-    'javelin-workboard-header-template' => '9b86cd0d',
+    'javelin-workboard-header' => '6e75daea',
+    'javelin-workboard-header-template' => '2d641f7d',
     'javelin-workflow' => '958e9045',
     'maniphest-report-css' => '3d53188b',
     'maniphest-task-edit-css' => '272daa84',
@@ -1125,6 +1125,9 @@ return array(
       'javelin-dom',
       'phabricator-keyboard-shortcut',
     ),
+    '2d641f7d' => array(
+      'javelin-install',
+    ),
     '2e255291' => array(
       'javelin-install',
       'javelin-util',
@@ -1163,9 +1166,6 @@ return array(
       'javelin-stratcom',
       'javelin-workflow',
     ),
-    '354c5c0e' => array(
-      'javelin-install',
-    ),
     '37b8a04a' => array(
       'javelin-install',
       'javelin-util',
@@ -1458,6 +1458,9 @@ return array(
       'javelin-install',
       'javelin-util',
     ),
+    '6e75daea' => array(
+      'javelin-install',
+    ),
     70245195 => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1566,6 +1569,9 @@ return array(
       'javelin-install',
       'javelin-dom',
     ),
+    '887ef74f' => array(
+      'javelin-install',
+    ),
     '89a1ae3a' => array(
       'javelin-dom',
       'javelin-util',
@@ -1701,9 +1707,6 @@ return array(
       'javelin-dom',
       'javelin-stratcom',
     ),
-    '9b86cd0d' => array(
-      'javelin-install',
-    ),
     '9cec214e' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1728,15 +1731,6 @@ return array(
     'a241536a' => array(
       'javelin-install',
     ),
-    'a3f6b67f' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-vector',
-      'javelin-stratcom',
-      'javelin-workflow',
-      'javelin-workboard-controller',
-    ),
     'a4356cde' => array(
       'javelin-install',
       'javelin-dom',
@@ -1762,6 +1756,16 @@ return array(
       'javelin-request',
       'javelin-util',
     ),
+    'a4f1e85d' => array(
+      'javelin-install',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-workflow',
+      'phabricator-draggable-list',
+      'javelin-workboard-column',
+      'javelin-workboard-header-template',
+    ),
     'a5257c4e' => array(
       'javelin-install',
       'javelin-dom',
@@ -1906,9 +1910,6 @@ return array(
       'javelin-stratcom',
       'javelin-uri',
     ),
-    'c23ddfde' => array(
-      'javelin-install',
-    ),
     'c2c500a7' => array(
       'javelin-install',
       'javelin-dom',
@@ -1959,6 +1960,11 @@ return array(
       'javelin-util',
       'phabricator-keyboard-shortcut-manager',
     ),
+    'ca444dca' => array(
+      'javelin-install',
+      'javelin-workboard-card',
+      'javelin-workboard-header',
+    ),
     'cf32921f' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -2025,15 +2031,14 @@ return array(
       'javelin-dom',
       'javelin-history',
     ),
-    'e4e2d107' => array(
-      'javelin-install',
+    'e2730b90' => array(
+      'javelin-behavior',
       'javelin-dom',
       'javelin-util',
+      'javelin-vector',
       'javelin-stratcom',
       'javelin-workflow',
-      'phabricator-draggable-list',
-      'javelin-workboard-column',
-      'javelin-workboard-header-template',
+      'javelin-workboard-controller',
     ),
     'e562708c' => array(
       'javelin-install',
@@ -2136,11 +2141,6 @@ return array(
       'javelin-magical-init',
       'javelin-util',
     ),
-    'fd9cb972' => array(
-      'javelin-install',
-      'javelin-workboard-card',
-      'javelin-workboard-header',
-    ),
     'fdc13e4e' => array(
       'javelin-install',
     ),

src/applications/maniphest/storage/ManiphestTask.php

@@ -252,6 +252,7 @@ final class ManiphestTask extends ManiphestDAO
     return array(
       PhabricatorProjectColumn::ORDER_PRIORITY => array(
         (int)-$this->getPriority(),
+        PhabricatorProjectColumn::NODETYPE_CARD,
         (double)-$this->getSubpriority(),
         (int)-$this->getID(),
       ),

src/applications/project/controller/PhabricatorProjectBoardViewController.php

@@ -651,11 +651,15 @@ final class PhabricatorProjectBoardViewController
         ));
 
       $headers[] = array(
-        'order' => 'priority',
+        'order' => PhabricatorProjectColumn::ORDER_PRIORITY,
         'key' => $header_key,
         'template' => hsprintf('%s', $template),
         'vector' => array(
           (int)-$priority,
+          PhabricatorProjectColumn::NODETYPE_HEADER,
+        ),
+        'editProperties' => array(
+          PhabricatorProjectColumn::ORDER_PRIORITY => (int)$priority,
         ),
       );
     }

src/applications/project/controller/PhabricatorProjectMoveController.php

@@ -15,6 +15,14 @@ final class PhabricatorProjectMoveController
     $before_phid = $request->getStr('beforePHID');
     $order = $request->getStr('order', PhabricatorProjectColumn::DEFAULT_ORDER);
 
+    $edit_header = null;
+    $raw_header = $request->getStr('header');
+    if (strlen($raw_header)) {
+      $edit_header = phutil_json_decode($raw_header);
+    } else {
+      $edit_header = array();
+    }
+
     $project = id(new PhabricatorProjectQuery())
       ->setViewer($viewer)
       ->requireCapabilities(
@@ -87,10 +95,14 @@ final class PhabricatorProjectMoveController
         ));
 
     if ($order == PhabricatorProjectColumn::ORDER_PRIORITY) {
+      $header_priority = idx(
+        $edit_header,
+        PhabricatorProjectColumn::ORDER_PRIORITY);
       $priority_xactions = $this->getPriorityTransactions(
         $object,
         $after_phid,
-        $before_phid);
+        $before_phid,
+        $header_priority);
       foreach ($priority_xactions as $xaction) {
         $xactions[] = $xaction;
       }
@@ -110,13 +122,33 @@ final class PhabricatorProjectMoveController
   private function getPriorityTransactions(
     ManiphestTask $task,
     $after_phid,
-    $before_phid) {
+    $before_phid,
+    $header_priority) {
+
+    $xactions = array();
+    $must_move = false;
+
+    if ($header_priority !== null) {
+      if ($task->getPriority() !== $header_priority) {
+        $task = id(clone $task)
+          ->setPriority($header_priority);
+
+        $keyword_map = ManiphestTaskPriority::getTaskPriorityKeywordsMap();
+        $keyword = head(idx($keyword_map, $header_priority));
+
+        $xactions[] = id(new ManiphestTransaction())
+          ->setTransactionType(
+            ManiphestTaskPriorityTransaction::TRANSACTIONTYPE)
+          ->setNewValue($keyword);
+
+        $must_move = true;
+      }
+    }
 
     list($after_task, $before_task) = $this->loadPriorityTasks(
       $after_phid,
       $before_phid);
 
-    $must_move = false;
     if ($after_task && !$task->isLowerPriorityThan($after_task)) {
       $must_move = true;
     }
@@ -125,10 +157,10 @@ final class PhabricatorProjectMoveController
       $must_move = true;
     }
 
-    // The move doesn't require a priority change to be valid, so don't
-    // change the priority since we are not being forced to.
+    // The move doesn't require a subpriority change to be valid, so don't
+    // change the subpriority since we are not being forced to.
     if (!$must_move) {
-      return array();
+      return $xactions;
     }
 
     $try = array(
@@ -139,28 +171,41 @@ final class PhabricatorProjectMoveController
     $pri = null;
     $sub = null;
     foreach ($try as $spec) {
-      list($task, $is_after) = $spec;
+      list($nearby_task, $is_after) = $spec;
 
-      if (!$task) {
+      if (!$nearby_task) {
         continue;
       }
 
       list($pri, $sub) = ManiphestTransactionEditor::getAdjacentSubpriority(
-        $task,
+        $nearby_task,
         $is_after);
 
+      // If we drag under a "Low" header between a "Normal" task and a "Low"
+      // task, we don't want to accept a subpriority assignment which changes
+      // our priority to "Normal". Only accept a subpriority that keeps us in
+      // the right primary priority.
+      if ($header_priority !== null) {
+        if ($pri !== $header_priority) {
+          continue;
+        }
+      }
+
       // If we find a priority on the first try, don't keep going.
       break;
     }
 
-    $keyword_map = ManiphestTaskPriority::getTaskPriorityKeywordsMap();
-    $keyword = head(idx($keyword_map, $pri));
-
-    $xactions = array();
     if ($pri !== null) {
-      $xactions[] = id(new ManiphestTransaction())
-        ->setTransactionType(ManiphestTaskPriorityTransaction::TRANSACTIONTYPE)
-        ->setNewValue($keyword);
+      if ($header_priority === null) {
+        $keyword_map = ManiphestTaskPriority::getTaskPriorityKeywordsMap();
+        $keyword = head(idx($keyword_map, $pri));
+
+        $xactions[] = id(new ManiphestTransaction())
+          ->setTransactionType(
+            ManiphestTaskPriorityTransaction::TRANSACTIONTYPE)
+          ->setNewValue($keyword);
+      }
+
       $xactions[] = id(new ManiphestTransaction())
         ->setTransactionType(
           ManiphestTaskSubpriorityTransaction::TRANSACTIONTYPE)

src/applications/project/storage/PhabricatorProjectColumn.php

@@ -16,6 +16,9 @@ final class PhabricatorProjectColumn
   const ORDER_NATURAL = 'natural';
   const ORDER_PRIORITY = 'priority';
 
+  const NODETYPE_HEADER = 0;
+  const NODETYPE_CARD = 1;
+
   protected $name;
   protected $status;
   protected $projectPHID;

webroot/rsrc/js/application/projects/WorkboardBoard.js

@@ -161,11 +161,15 @@ JX.install('WorkboardBoard', {
 
         var list = new JX.DraggableList('project-card', column.getRoot())
           .setOuterContainer(this.getRoot())
-          .setFindItemsHandler(JX.bind(column, column.getCardNodes))
+          .setFindItemsHandler(JX.bind(column, column.getDropTargetNodes))
           .setCanDragX(true)
           .setHasInfiniteHeight(true)
           .setIsDropTargetHandler(JX.bind(column, column.setIsDropTarget));
 
+        var default_handler = list.getGhostHandler();
+        list.setGhostHandler(
+          JX.bind(column, column.handleDragGhost, default_handler));
+
         if (this.getOrder() !== 'natural') {
           list.setCompareHandler(JX.bind(column, column.compareHandler));
         }
@@ -198,16 +202,39 @@ JX.install('WorkboardBoard', {
         order: this.getOrder()
       };
 
-      if (after_node) {
-        data.afterPHID = JX.Stratcom.getData(after_node).objectPHID;
+      var after_data;
+      var after_card = after_node;
+      while (after_card) {
+        after_data = JX.Stratcom.getData(after_card);
+        if (after_data.objectPHID) {
+          break;
+        }
+        after_card = after_card.previousSibling;
       }
 
-      var before_node = item.nextSibling;
-      if (before_node) {
-        var before_phid = JX.Stratcom.getData(before_node).objectPHID;
-        if (before_phid) {
-          data.beforePHID = before_phid;
+      if (after_data) {
+        data.afterPHID = after_data.objectPHID;
+      }
+
+      var before_data;
+      var before_card = item.nextSibling;
+      while (before_card) {
+        before_data = JX.Stratcom.getData(before_card);
+        if (before_data.objectPHID) {
+          break;
         }
+        before_card = before_card.nextSibling;
+      }
+
+      if (before_data) {
+        data.beforePHID = before_data.objectPHID;
+      }
+
+      var header_key = JX.Stratcom.getData(after_node).headerKey;
+      if (header_key) {
+        var properties = this.getHeaderTemplate(header_key)
+          .getEditProperties();
+        data.header = JX.JSON.stringify(properties);
       }
 
       var visible_phids = [];

webroot/rsrc/js/application/projects/WorkboardCard.js

@@ -55,6 +55,10 @@ JX.install('WorkboardCard', {
       return this._root;
     },
 
+    isWorkboardHeader: function() {
+      return false;
+    },
+
     redraw: function() {
       var old_node = this._root;
       this._root = null;

webroot/rsrc/js/application/projects/WorkboardColumn.js

@@ -52,6 +52,10 @@ JX.install('WorkboardColumn', {
       return this._cards;
     },
 
+    _getObjects: function() {
+      return this._objects;
+    },
+
     getCard: function(phid) {
       return this._cards[phid];
     },
@@ -126,12 +130,13 @@ JX.install('WorkboardColumn', {
       return this;
     },
 
-    getCardNodes: function() {
-      var cards = this.getCards();
+    getDropTargetNodes: function() {
+      var objects = this._getObjects();
 
       var nodes = [];
-      for (var k in cards) {
-        nodes.push(cards[k].getNode());
+      for (var ii = 0; ii < objects.length; ii++) {
+        var object = objects[ii];
+        nodes.push(object.getNode());
       }
 
       return nodes;
@@ -160,6 +165,32 @@ JX.install('WorkboardColumn', {
       return this._headers[key];
     },
 
+    handleDragGhost: function(default_handler, ghost, node) {
+      // If the column has headers, don't let the user drag a card above
+      // the topmost header: for example, you can't change a task to have
+      // a priority higher than the highest possible priority.
+
+      if (this._hasColumnHeaders()) {
+        if (!node) {
+          return false;
+        }
+      }
+
+      return default_handler(ghost, node);
+    },
+
+    _hasColumnHeaders: function() {
+      var board = this.getBoard();
+      var order = board.getOrder();
+
+      switch (order) {
+        case 'natural':
+          return false;
+      }
+
+      return true;
+    },
+
     _getCardHeaderKey: function(card, order) {
       switch (order) {
         case 'priority':
@@ -174,18 +205,16 @@ JX.install('WorkboardColumn', {
       var order = board.getOrder();
 
       var list;
-      var has_headers;
       if (order == 'natural') {
         list = this._getCardsSortedNaturally();
-        has_headers = false;
       } else {
         list = this._getCardsSortedByKey(order);
-        has_headers = true;
       }
 
       var ii;
       var objects = [];
 
+      var has_headers = this._hasColumnHeaders();
       var header_keys = [];
       var seen_headers = {};
       if (has_headers) {
@@ -245,15 +274,23 @@ JX.install('WorkboardColumn', {
       var board = this.getBoard();
       var order = board.getOrder();
 
-      var src_phid = JX.Stratcom.getData(src_node).objectPHID;
-      var dst_phid = JX.Stratcom.getData(dst_node).objectPHID;
-
-      var u_vec = board.getOrderVector(src_phid, order);
-      var v_vec = board.getOrderVector(dst_phid, order);
+      var u_vec = this._getNodeOrderVector(src_node, order);
+      var v_vec = this._getNodeOrderVector(dst_node, order);
 
       return board.compareVectors(u_vec, v_vec);
     },
 
+    _getNodeOrderVector: function(node, order) {
+      var board = this.getBoard();
+      var data = JX.Stratcom.getData(node);
+
+      if (data.objectPHID) {
+        return board.getOrderVector(data.objectPHID, order);
+      }
+
+      return board.getHeaderTemplate(data.headerKey).getVector();
+    },
+
     setIsDropTarget: function(is_target) {
       var node = this.getWorkpanelNode();
       JX.DOM.alterClass(node, 'workboard-column-drop-target', is_target);

webroot/rsrc/js/application/projects/WorkboardHeader.js

@@ -30,8 +30,14 @@ JX.install('WorkboardHeader', {
         var board = this.getColumn().getBoard();
         var template = board.getHeaderTemplate(header_key).getTemplate();
         this._root = JX.$H(template).getFragment().firstChild;
+
+        JX.Stratcom.getData(this._root).headerKey = header_key;
       }
       return this._root;
+    },
+
+    isWorkboardHeader: function() {
+      return true;
     }
   }
 

webroot/rsrc/js/application/projects/WorkboardHeaderTemplate.js

@@ -13,7 +13,8 @@ JX.install('WorkboardHeaderTemplate', {
   properties: {
     template: null,
     order: null,
-    vector: null
+    vector: null,
+    editProperties: null
   },
 
   members: {

webroot/rsrc/js/application/projects/behavior-project-boards.js

@@ -112,7 +112,8 @@ JX.behavior('project-boards', function(config, statics) {
     board.getHeaderTemplate(header.key)
       .setOrder(header.order)
       .setTemplate(header.template)
-      .setVector(header.vector);
+      .setVector(header.vector)
+      .setEditProperties(header.editProperties);
   }
 
   board.start();