1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00

When dragging nodes between different columns on an ordered board, don't reorder them by making secondary edits

Summary:
Ref T10334. When a workboard is ordered by priority, dragging from column "A" to a particular place in column "B" currently means "move this task to column B, and adjust its priority so that it naturally sorts into the location under my mouse cursor".

Users frequently find this confusing / undesirable.

To begin improving this, make "drag from column A to column B" and "drag from somewhere in column A to somewhere else in column A" into different operations. The first operation, a movement between columns, no longer implies an ordering change. The second action still does.

So if you actually want to change the priority of a task, you drag it within its current column. If you just want to move it to a different column, you drag it between columns.

This creates some possible problems:

  - Some users may love the current behavior and just not be very vocal about it. I doubt it, but presumably we'll hear from them if we break it.
  - If you actualy want to move + reorder, it's a bit more cumbersome now. We could possibly add something like "shift + drag" for this if there's feedback.
  - The new behavior is probably less surprising, but may not be much more obvious. Future changes (for example, in T10335) should help make it more clear.
  - When you mouse cursor goes over column B, the card dashed-rectangle preview target thing jumps to the correct position in the column -- but that may not be under your mouse cursor. This feels pretty much fine if the whole column fits on screen. It may not be so great if the column does not fit on screen and the dashed-rectangle-thing has vanished. This is just a UI feedback issue and we could refine this later (scroll/highlight the column).

Test Plan:
  - Created several tasks at different priority levels, sorted a board by priority, dragged tasks between columns. Dragging from "A" to "B" no longer causes a priority edit.
  - Also, dragged within a column. This still performs priority edits.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T10334

Differential Revision: https://secure.phabricator.com/D20242
This commit is contained in:
epriestley 2019-03-01 15:13:25 -08:00
parent c1bff3b801
commit 2260738de9
4 changed files with 84 additions and 35 deletions

View file

@ -10,7 +10,7 @@ return array(
'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.css' => '3c8a0668',
'conpherence.pkg.js' => '020aebcf', 'conpherence.pkg.js' => '020aebcf',
'core.pkg.css' => '34ce1741', 'core.pkg.css' => '34ce1741',
'core.pkg.js' => '2cda17a4', 'core.pkg.js' => '9eb1254b',
'differential.pkg.css' => '1755a478', 'differential.pkg.css' => '1755a478',
'differential.pkg.js' => '67e02996', 'differential.pkg.js' => '67e02996',
'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.css' => '42c75c37',
@ -409,9 +409,9 @@ return array(
'rsrc/js/application/phortune/phortune-credit-card-form.js' => 'd12d214f', '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-control.js' => '0eaa33a9',
'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '9347f172', 'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '9347f172',
'rsrc/js/application/projects/WorkboardBoard.js' => '45d0b2b1', 'rsrc/js/application/projects/WorkboardBoard.js' => '3a8c42a3',
'rsrc/js/application/projects/WorkboardCard.js' => '9a513421', 'rsrc/js/application/projects/WorkboardCard.js' => '9a513421',
'rsrc/js/application/projects/WorkboardColumn.js' => '8573dc1b', 'rsrc/js/application/projects/WorkboardColumn.js' => 'b451fd4c',
'rsrc/js/application/projects/WorkboardController.js' => '42c7a5a7', 'rsrc/js/application/projects/WorkboardController.js' => '42c7a5a7',
'rsrc/js/application/projects/behavior-project-boards.js' => '05c74d65', 'rsrc/js/application/projects/behavior-project-boards.js' => '05c74d65',
'rsrc/js/application/projects/behavior-project-create.js' => '34c53422', 'rsrc/js/application/projects/behavior-project-create.js' => '34c53422',
@ -434,7 +434,7 @@ return array(
'rsrc/js/application/uiexample/notification-example.js' => '29819b75', 'rsrc/js/application/uiexample/notification-example.js' => '29819b75',
'rsrc/js/core/Busy.js' => '5202e831', 'rsrc/js/core/Busy.js' => '5202e831',
'rsrc/js/core/DragAndDropFileUpload.js' => '4370900d', 'rsrc/js/core/DragAndDropFileUpload.js' => '4370900d',
'rsrc/js/core/DraggableList.js' => '3c6bd549', 'rsrc/js/core/DraggableList.js' => 'd594c805',
'rsrc/js/core/Favicon.js' => '7930776a', 'rsrc/js/core/Favicon.js' => '7930776a',
'rsrc/js/core/FileUpload.js' => 'ab85e184', 'rsrc/js/core/FileUpload.js' => 'ab85e184',
'rsrc/js/core/Hovercard.js' => '074f0783', 'rsrc/js/core/Hovercard.js' => '074f0783',
@ -727,9 +727,9 @@ return array(
'javelin-view-renderer' => '9aae2b66', 'javelin-view-renderer' => '9aae2b66',
'javelin-view-visitor' => '308f9fe4', 'javelin-view-visitor' => '308f9fe4',
'javelin-websocket' => 'fdc13e4e', 'javelin-websocket' => 'fdc13e4e',
'javelin-workboard-board' => '45d0b2b1', 'javelin-workboard-board' => '3a8c42a3',
'javelin-workboard-card' => '9a513421', 'javelin-workboard-card' => '9a513421',
'javelin-workboard-column' => '8573dc1b', 'javelin-workboard-column' => 'b451fd4c',
'javelin-workboard-controller' => '42c7a5a7', 'javelin-workboard-controller' => '42c7a5a7',
'javelin-workflow' => '958e9045', 'javelin-workflow' => '958e9045',
'maniphest-report-css' => '3d53188b', 'maniphest-report-css' => '3d53188b',
@ -755,7 +755,7 @@ return array(
'phabricator-diff-changeset-list' => '04023d82', 'phabricator-diff-changeset-list' => '04023d82',
'phabricator-diff-inline' => 'a4a14a94', 'phabricator-diff-inline' => 'a4a14a94',
'phabricator-drag-and-drop-file-upload' => '4370900d', 'phabricator-drag-and-drop-file-upload' => '4370900d',
'phabricator-draggable-list' => '3c6bd549', 'phabricator-draggable-list' => 'd594c805',
'phabricator-fatal-config-template-css' => '20babf50', 'phabricator-fatal-config-template-css' => '20babf50',
'phabricator-favicon' => '7930776a', 'phabricator-favicon' => '7930776a',
'phabricator-feed-css' => 'd8b6e3f8', 'phabricator-feed-css' => 'd8b6e3f8',
@ -1188,18 +1188,19 @@ return array(
'javelin-install', 'javelin-install',
'javelin-dom', 'javelin-dom',
), ),
'3a8c42a3' => array(
'javelin-install',
'javelin-dom',
'javelin-util',
'javelin-stratcom',
'javelin-workflow',
'phabricator-draggable-list',
'javelin-workboard-column',
),
'3b4899b0' => array( '3b4899b0' => array(
'javelin-behavior', 'javelin-behavior',
'phabricator-prefab', 'phabricator-prefab',
), ),
'3c6bd549' => array(
'javelin-install',
'javelin-dom',
'javelin-stratcom',
'javelin-util',
'javelin-vector',
'javelin-magical-init',
),
'3dc5ad43' => array( '3dc5ad43' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-stratcom', 'javelin-stratcom',
@ -1256,15 +1257,6 @@ return array(
'43bc9360' => array( '43bc9360' => array(
'javelin-install', 'javelin-install',
), ),
'45d0b2b1' => array(
'javelin-install',
'javelin-dom',
'javelin-util',
'javelin-stratcom',
'javelin-workflow',
'phabricator-draggable-list',
'javelin-workboard-column',
),
'46116c01' => array( '46116c01' => array(
'javelin-request', 'javelin-request',
'javelin-behavior', 'javelin-behavior',
@ -1565,10 +1557,6 @@ return array(
'javelin-workflow', 'javelin-workflow',
'phabricator-draggable-list', 'phabricator-draggable-list',
), ),
'8573dc1b' => array(
'javelin-install',
'javelin-workboard-card',
),
'87428eb2' => array( '87428eb2' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-diffusion-locate-file-source', 'javelin-diffusion-locate-file-source',
@ -1855,6 +1843,10 @@ return array(
'b347a301' => array( 'b347a301' => array(
'javelin-behavior', 'javelin-behavior',
), ),
'b451fd4c' => array(
'javelin-install',
'javelin-workboard-card',
),
'b517bfa0' => array( 'b517bfa0' => array(
'phui-oi-list-view-css', 'phui-oi-list-view-css',
), ),
@ -1994,6 +1986,14 @@ return array(
'd3799cb4' => array( 'd3799cb4' => array(
'javelin-install', 'javelin-install',
), ),
'd594c805' => array(
'javelin-install',
'javelin-dom',
'javelin-stratcom',
'javelin-util',
'javelin-vector',
'javelin-magical-init',
),
'd8a86cfb' => array( 'd8a86cfb' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-dom', 'javelin-dom',

View file

@ -118,6 +118,10 @@ JX.install('WorkboardBoard', {
.setCanDragX(true) .setCanDragX(true)
.setHasInfiniteHeight(true); .setHasInfiniteHeight(true);
if (this.getOrder() !== 'natural') {
list.setCompareHandler(JX.bind(column, column.compareHandler));
}
list.listen('didDrop', JX.bind(this, this._onmovecard, list)); list.listen('didDrop', JX.bind(this, this._onmovecard, list));
lists.push(list); lists.push(list);

View file

@ -175,6 +175,19 @@ JX.install('WorkboardColumn', {
this._dirty = false; this._dirty = false;
}, },
compareHandler: function(src_list, src_node, dst_list, dst_node) {
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 = this.getBoard().getOrderVector(src_phid, order);
var v_vec = this.getBoard().getOrderVector(dst_phid, order);
return this._compareVectors(u_vec, v_vec);
},
_getCardsSortedNaturally: function() { _getCardsSortedNaturally: function() {
var list = []; var list = [];
@ -200,15 +213,19 @@ JX.install('WorkboardColumn', {
}, },
_sortCards: function(order, u, v) { _sortCards: function(order, u, v) {
var ud = this.getBoard().getOrderVector(u.getPHID(), order); var u_vec = this.getBoard().getOrderVector(u.getPHID(), order);
var vd = this.getBoard().getOrderVector(v.getPHID(), order); var v_vec = this.getBoard().getOrderVector(v.getPHID(), order);
for (var ii = 0; ii < ud.length; ii++) { return this._compareVectors(u_vec, v_vec);
if (ud[ii] > vd[ii]) { },
_compareVectors: function(u_vec, v_vec) {
for (var ii = 0; ii < u_vec.length; ii++) {
if (u_vec[ii] > v_vec[ii]) {
return 1; return 1;
} }
if (ud[ii] < vd[ii]) { if (u_vec[ii] < v_vec[ii]) {
return -1; return -1;
} }
} }

View file

@ -39,6 +39,7 @@ JX.install('DraggableList', {
properties : { properties : {
findItemsHandler: null, findItemsHandler: null,
compareHandler: null,
canDragX: false, canDragX: false,
outerContainer: null, outerContainer: null,
hasInfiniteHeight: false hasInfiniteHeight: false
@ -367,8 +368,29 @@ JX.install('DraggableList', {
return this; return this;
}, },
_getOrderedTarget: function(src_list, src_node) {
var targets = this._getTargets();
// NOTE: The targets are ordered from the bottom of the column to the
// top, so we're looking for the first node that we sort below. If we
// don't find one, we'll sort to the head of the column.
for (var ii = 0; ii < targets.length; ii++) {
var target = targets[ii];
if (this._compareTargets(src_list, src_node, target.item) > 0) {
return target.item;
}
}
return null;
},
_compareTargets: function(src_list, src_node, dst_node) {
var dst_list = this;
return this.getCompareHandler()(src_list, src_node, dst_list, dst_node);
},
_getCurrentTarget : function(p) { _getCurrentTarget : function(p) {
var ghost = this.getGhostNode();
var targets = this._getTargets(); var targets = this._getTargets();
var dragging = this._dragging; var dragging = this._dragging;
@ -461,9 +483,15 @@ JX.install('DraggableList', {
// Compute the size and position of the drop target indicator, because we // Compute the size and position of the drop target indicator, because we
// need to update our static position computations to account for it. // need to update our static position computations to account for it.
var compare_handler = this.getCompareHandler();
var cur_target = false; var cur_target = false;
if (target_list) { if (target_list) {
cur_target = target_list._getCurrentTarget(p); if (compare_handler && (target_list !== this)) {
cur_target = target_list._getOrderedTarget(this, this._dragging);
} else {
cur_target = target_list._getCurrentTarget(p);
}
} }
// If we've selected a new target, update the UI to show where we're // If we've selected a new target, update the UI to show where we're