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

Allow tasks to be subprioritized by drag-and-drop

Summary:
Like the title says, similar to Facebook Tasks.

Not sure how I really feel about this, but I guess it's kind of OK? I never used
this feature in Facebook Tasks but I think some people like it.

The drag-and-drop to repri across priorities feels okayish.

Because subpriority is a double and we just split the difference when
reprioritizing, you lose ~a bit of precision every time you repri two tasks
against each other and so you can break it by swapping the priorities of two
tasks ~50 times. This case is pretty silly and pathological. We can add some
code to deal with this at some point if necessary.

I think this also fixes the whacky task layout widths once and for all.

(There are a couple of minor UI glitches like headers not vanishing and header
counts not updating that I'm not fixing because I am lazy.)

Test Plan: Dragged and dropped tasks around.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley, mgummelt

Maniphest Tasks: T859

Differential Revision: https://secure.phabricator.com/D1731
This commit is contained in:
epriestley 2012-04-02 12:12:04 -07:00
parent 84c40a732e
commit e7853e4801
16 changed files with 491 additions and 32 deletions

View file

@ -0,0 +1,11 @@
ALTER TABLE phabricator_maniphest.maniphest_task
ADD subpriority DOUBLE NOT NULL;
ALTER TABLE phabricator_maniphest.maniphest_task
ADD KEY (priority, subpriority);
/* Seed the subpriority column with reasonable values that keep order stable. */
UPDATE phabricator_maniphest.maniphest_task
SET subpriority = dateModified;

View file

@ -776,6 +776,21 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/js/application/maniphest/behavior-project-create.js',
),
'javelin-behavior-maniphest-subpriority-editor' =>
array(
'uri' => '/res/7d669df7/rsrc/js/application/maniphest/behavior-subpriorityeditor.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-behavior',
1 => 'javelin-magical-init',
2 => 'javelin-dom',
3 => 'javelin-vector',
4 => 'javelin-stratcom',
5 => 'javelin-workflow',
),
'disk' => '/rsrc/js/application/maniphest/behavior-subpriorityeditor.js',
),
'javelin-behavior-maniphest-transaction-controls' =>
array(
'uri' => '/res/62465554/rsrc/js/application/maniphest/behavior-transaction-controls.js',
@ -1436,7 +1451,7 @@ celerity_register_resource_map(array(
),
'maniphest-task-summary-css' =>
array(
'uri' => '/res/ddb926e4/rsrc/css/application/maniphest/task-summary.css',
'uri' => '/res/d03e96a4/rsrc/css/application/maniphest/task-summary.css',
'type' => 'css',
'requires' =>
array(
@ -2133,7 +2148,7 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/4fbae2af/javelin.pkg.js',
'type' => 'js',
),
'8315b569' =>
'826b7ac2' =>
array(
'name' => 'maniphest.pkg.css',
'symbols' =>
@ -2143,7 +2158,7 @@ celerity_register_resource_map(array(
2 => 'maniphest-task-detail-css',
3 => 'aphront-attached-file-view-css',
),
'uri' => '/res/pkg/8315b569/maniphest.pkg.css',
'uri' => '/res/pkg/826b7ac2/maniphest.pkg.css',
'type' => 'css',
),
'86fc0b0c' =>
@ -2178,7 +2193,7 @@ celerity_register_resource_map(array(
),
'reverse' =>
array(
'aphront-attached-file-view-css' => '8315b569',
'aphront-attached-file-view-css' => '826b7ac2',
'aphront-crumbs-view-css' => '61493db4',
'aphront-dialog-view-css' => '61493db4',
'aphront-form-view-css' => '61493db4',
@ -2244,9 +2259,9 @@ celerity_register_resource_map(array(
'javelin-util' => '4fbae2af',
'javelin-vector' => '4fbae2af',
'javelin-workflow' => '21d01ed8',
'maniphest-task-detail-css' => '8315b569',
'maniphest-task-summary-css' => '8315b569',
'maniphest-transaction-detail-css' => '8315b569',
'maniphest-task-detail-css' => '826b7ac2',
'maniphest-task-summary-css' => '826b7ac2',
'maniphest-transaction-detail-css' => '826b7ac2',
'phabricator-app-buttons-css' => '61493db4',
'phabricator-content-source-view-css' => '18be02e0',
'phabricator-core-buttons-css' => '61493db4',

View file

@ -443,6 +443,7 @@ phutil_register_library_map(array(
'ManiphestExportController' => 'applications/maniphest/controller/export',
'ManiphestReplyHandler' => 'applications/maniphest/replyhandler',
'ManiphestReportController' => 'applications/maniphest/controller/report',
'ManiphestSubpriorityController' => 'applications/maniphest/controller/subpriority',
'ManiphestTask' => 'applications/maniphest/storage/task',
'ManiphestTaskAuxiliaryStorage' => 'applications/maniphest/storage/auxiliary',
'ManiphestTaskDescriptionChangeController' => 'applications/maniphest/controller/descriptionchange',
@ -1301,6 +1302,7 @@ phutil_register_library_map(array(
'ManiphestExportController' => 'ManiphestController',
'ManiphestReplyHandler' => 'PhabricatorMailReplyHandler',
'ManiphestReportController' => 'ManiphestController',
'ManiphestSubpriorityController' => 'ManiphestController',
'ManiphestTask' => 'ManiphestDAO',
'ManiphestTaskAuxiliaryStorage' => 'ManiphestDAO',
'ManiphestTaskDescriptionChangeController' => 'ManiphestController',

View file

@ -206,6 +206,7 @@ class AphrontDefaultApplicationConfiguration
'preview/(?P<id>\d+)/' => 'ManiphestTransactionPreviewController',
),
'export/(?P<key>[^/]+)/' => 'ManiphestExportController',
'subpriority/' => 'ManiphestSubpriorityController',
),
'/T(?P<id>\d+)' => 'ManiphestTaskDetailController',

View file

@ -0,0 +1,79 @@
<?php
/*
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* @group maniphest
*/
final class ManiphestSubpriorityController extends ManiphestController {
public function processRequest() {
$request = $this->getRequest();
if (!$request->validateCSRF()) {
return new Aphront403Response();
}
$task = id(new ManiphestTask())->load($request->getInt('task'));
if (!$task) {
return new Aphront404Response();
}
if ($request->getInt('after')) {
$after_task = id(new ManiphestTask())->load($request->getInt('after'));
if (!$after_task) {
return new Aphront404Response();
}
$after_pri = $after_task->getPriority();
$after_sub = $after_task->getSubpriority();
} else {
$after_pri = $request->getInt('priority');
$after_sub = null;
}
$new_sub = ManiphestTransactionEditor::getNextSubpriority(
$after_pri,
$after_sub);
if ($after_pri != $task->getPriority()) {
$xaction = new ManiphestTransaction();
$xaction->setAuthorPHID($request->getUser()->getPHID());
// TODO: Content source?
$xaction->setTransactionType(ManiphestTransactionType::TYPE_PRIORITY);
$xaction->setNewValue($after_pri);
$editor = new ManiphestTransactionEditor();
$editor->applyTransactions($task, array($xaction));
}
$task->setSubpriority($new_sub);
$task->save();
$pri_class = ManiphestTaskSummaryView::getPriorityClass(
$task->getPriority());
$class = 'maniphest-task-handle maniphest-active-handle '.$pri_class;
$response = array(
'className' => $class,
);
return id(new AphrontAjaxResponse())->setContent($response);
}
}

View file

@ -0,0 +1,22 @@
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'aphront/response/403');
phutil_require_module('phabricator', 'aphront/response/404');
phutil_require_module('phabricator', 'aphront/response/ajax');
phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
phutil_require_module('phabricator', 'applications/maniphest/controller/base');
phutil_require_module('phabricator', 'applications/maniphest/editor/transaction');
phutil_require_module('phabricator', 'applications/maniphest/storage/task');
phutil_require_module('phabricator', 'applications/maniphest/storage/transaction');
phutil_require_module('phabricator', 'applications/maniphest/view/tasksummary');
phutil_require_module('phutil', 'utils');
phutil_require_source('ManiphestSubpriorityController.php');

View file

@ -260,34 +260,65 @@ final class ManiphestTaskListController extends ManiphestController {
$selector = new AphrontNullView();
$group = $query->getParameter('group');
$order = $query->getParameter('order');
$is_draggable =
($group == 'priority') ||
($group == 'none' && $order == 'priority');
$lists = new AphrontNullView();
$lists->appendChild('<div style="padding: 0em 1em;">');
foreach ($tasks as $group => $list) {
$task_list = new ManiphestTaskListView();
$task_list->setShowBatchControls(true);
if ($is_draggable) {
$task_list->setShowSubpriorityControls(true);
}
$task_list->setUser($user);
$task_list->setTasks($list);
$task_list->setHandles($handles);
$count = number_format(count($list));
$selector->appendChild(
'<h1 class="maniphest-task-group-header">'.
phutil_escape_html($group).' ('.$count.')'.
'</h1>');
$selector->appendChild($task_list);
$lists->appendChild(
javelin_render_tag(
'h1',
array(
'class' => 'maniphest-task-group-header',
'sigil' => 'task-group',
'meta' => array(
'priority' => head($list)->getPriority(),
),
),
phutil_escape_html($group).' ('.$count.')'));
$lists->appendChild($task_list);
}
$lists->appendChild('</div>');
$selector->appendChild($lists);
$selector->appendChild($this->renderBatchEditor($query));
$form_id = celerity_generate_unique_node_id();
$selector = phabricator_render_form(
$user,
array(
'method' => 'POST',
'action' => '/maniphest/batch/',
'id' => $form_id,
),
$selector->render());
$list_container->appendChild($selector);
$list_container->appendChild($pager);
Javelin::initBehavior(
'maniphest-subpriority-editor',
array(
'root' => $form_id,
'uri' => '/maniphest/subpriority/',
));
}
$list_container->appendChild('</div>');

View file

@ -41,6 +41,8 @@ final class ManiphestTransactionEditor {
$email_to = array();
$email_to[] = $task->getOwnerPHID();
$pri_changed = $this->isCreate($transactions);
foreach ($transactions as $key => $transaction) {
$type = $transaction->getTransactionType();
$new = $transaction->getNewValue();
@ -151,6 +153,7 @@ final class ManiphestTransactionEditor {
break;
case ManiphestTransactionType::TYPE_PRIORITY:
$task->setPriority($new);
$pri_changed = true;
break;
case ManiphestTransactionType::TYPE_ATTACH:
$task->setAttached($new);
@ -178,6 +181,13 @@ final class ManiphestTransactionEditor {
}
if ($pri_changed) {
$subpriority = ManiphestTransactionEditor::getNextSubpriority(
$task->getPriority(),
null);
$task->setSubpriority($subpriority);
}
$task->save();
foreach ($transactions as $transaction) {
$transaction->setTaskID($task->getID());
@ -383,4 +393,27 @@ final class ManiphestTransactionEditor {
return array_unique($tags);
}
public static function getNextSubpriority($pri, $sub) {
if ($sub === null) {
$next = id(new ManiphestTask())->loadOneWhere(
'priority = %d ORDER BY subpriority ASC LIMIT 1',
$pri);
if ($next) {
return $next->getSubpriority() - ((double)(2 << 16));
}
} else {
$next = id(new ManiphestTask())->loadOneWhere(
'priority = %d AND subpriority > %s ORDER BY subpriority ASC LIMIT 1',
$pri,
$sub);
if ($next) {
return ($sub + $next->getSubpriority()) / 2;
}
}
return (double)(2 << 32);
}
}

View file

@ -11,6 +11,7 @@ phutil_require_module('phabricator', 'applications/feed/publisher');
phutil_require_module('phabricator', 'applications/maniphest/constants/action');
phutil_require_module('phabricator', 'applications/maniphest/constants/status');
phutil_require_module('phabricator', 'applications/maniphest/constants/transactiontype');
phutil_require_module('phabricator', 'applications/maniphest/storage/task');
phutil_require_module('phabricator', 'applications/maniphest/view/transactiondetail');
phutil_require_module('phabricator', 'applications/metamta/constants/notificationtype');
phutil_require_module('phabricator', 'applications/metamta/storage/mail');

View file

@ -443,6 +443,7 @@ final class ManiphestTaskQuery {
switch ($this->orderBy) {
case self::ORDER_PRIORITY:
$order[] = 'priority';
$order[] = 'subpriority';
$order[] = 'dateModified';
break;
case self::ORDER_CREATED:
@ -463,6 +464,7 @@ final class ManiphestTaskQuery {
foreach ($order as $k => $column) {
switch ($column) {
case 'subpriority':
case 'ownerOrdering':
$order[$k] = "task.{$column} ASC";
break;

View file

@ -28,6 +28,7 @@ final class ManiphestTask extends ManiphestDAO {
protected $status;
protected $priority;
protected $subpriority;
protected $title;
protected $description;

View file

@ -25,6 +25,7 @@ final class ManiphestTaskListView extends ManiphestView {
private $handles;
private $user;
private $showBatchControls;
private $showSubpriorityControls;
public function setTasks(array $tasks) {
$this->tasks = $tasks;
@ -46,6 +47,11 @@ final class ManiphestTaskListView extends ManiphestView {
return $this;
}
public function setShowSubpriorityControls($show_subpriority_controls) {
$this->showSubpriorityControls = $show_subpriority_controls;
return $this;
}
public function render() {
$views = array();
@ -53,6 +59,7 @@ final class ManiphestTaskListView extends ManiphestView {
$view = new ManiphestTaskSummaryView();
$view->setTask($task);
$view->setShowBatchControls($this->showBatchControls);
$view->setShowSubpriorityControls($this->showSubpriorityControls);
$view->setUser($this->user);
$view->setHandles($this->handles);
$views[] = $view->render();

View file

@ -25,6 +25,7 @@ final class ManiphestTaskSummaryView extends ManiphestView {
private $handles;
private $user;
private $showBatchControls;
private $showSubpriorityControls;
public function setTask(ManiphestTask $task) {
$this->task = $task;
@ -46,6 +47,24 @@ final class ManiphestTaskSummaryView extends ManiphestView {
return $this;
}
public function setShowSubpriorityControls($show_subpriority_controls) {
$this->showSubpriorityControls = $show_subpriority_controls;
return $this;
}
public static function getPriorityClass($priority) {
$classes = array(
ManiphestTaskPriority::PRIORITY_UNBREAK_NOW => 'pri-unbreak',
ManiphestTaskPriority::PRIORITY_TRIAGE => 'pri-triage',
ManiphestTaskPriority::PRIORITY_HIGH => 'pri-high',
ManiphestTaskPriority::PRIORITY_NORMAL => 'pri-normal',
ManiphestTaskPriority::PRIORITY_LOW => 'pri-low',
ManiphestTaskPriority::PRIORITY_WISH => 'pri-wish',
);
return idx($classes, $priority);
}
public function render() {
if (!$this->user) {
@ -57,16 +76,7 @@ final class ManiphestTaskSummaryView extends ManiphestView {
require_celerity_resource('maniphest-task-summary-css');
$classes = array(
ManiphestTaskPriority::PRIORITY_UNBREAK_NOW => 'pri-unbreak',
ManiphestTaskPriority::PRIORITY_TRIAGE => 'pri-triage',
ManiphestTaskPriority::PRIORITY_HIGH => 'pri-high',
ManiphestTaskPriority::PRIORITY_NORMAL => 'pri-normal',
ManiphestTaskPriority::PRIORITY_LOW => 'pri-low',
ManiphestTaskPriority::PRIORITY_WISH => 'pri-wish',
);
$pri_class = idx($classes, $task->getPriority());
$pri_class = self::getPriorityClass($task->getPriority());
$status_map = ManiphestTaskStatus::getTaskStatusMap();
$batch = null;
@ -91,15 +101,32 @@ final class ManiphestTaskSummaryView extends ManiphestView {
$this->handles,
$task->getProjectPHIDs()));
$control_class = null;
$control_sigil = null;
if ($this->showSubpriorityControls) {
$control_class = 'maniphest-active-handle';
$control_sigil = 'maniphest-task-handle';
}
$handle = javelin_render_tag(
'td',
array(
'class' => 'maniphest-task-handle '.$pri_class.' '.$control_class,
'sigil' => $control_sigil,
),
'');
return javelin_render_tag(
'table',
array(
'class' => 'maniphest-task-summary',
'sigil' => 'maniphest-task',
'meta' => array(
'taskID' => $task->getID(),
),
),
'<tr>'.
'<td class="maniphest-task-handle '.$pri_class.'">'.
'</td>'.
$handle.
$batch.
'<td class="maniphest-task-number">'.
'T'.$task->getID().

View file

@ -5,7 +5,7 @@
.maniphest-task-summary {
width: 100%;
margin: 4px 0;
border-collapse: collapse;
border-collapse: separate;
font-size: 12px;
color: #222222;
@ -42,6 +42,7 @@
padding-right: 0px;
width: 16px;
text-align: center;
overflow: hidden;
}
.maniphest-task-summary td.maniphest-task-batch,
@ -70,9 +71,9 @@
}
.maniphest-task-summary td.maniphest-task-name {
overflow: hidden;
font-weight: bold;
white-space: normal;
overflow: hidden;
}
.maniphest-task-summary td.maniphest-task-projects {
@ -95,32 +96,32 @@
.maniphest-task-summary .pri-unbreak {
border-color: #ff0000;
background: #ff0000;
background-color: #ff0000;
}
.maniphest-task-summary .pri-triage {
border-color: #ee00ee;
background: #ee00ee;
background-color: #ee00ee;
}
.maniphest-task-summary .pri-high {
border-color: #ff6622;
background: #ff6622;
background-color: #ff6622;
}
.maniphest-task-summary .pri-normal {
border-color: #ffaa66;
background: #ffaa66;
background-color: #ffaa66;
}
.maniphest-task-summary .pri-low {
border-color: #eecc66;
background: #eecc66;
background-color: #eecc66;
}
.maniphest-task-summary .pri-wish {
border-color: #0099ff;
background: #0099ff;
background-color: #0099ff;
}
.maniphest-task-group-header {
@ -178,3 +179,26 @@
.maniphest-list-container {
padding: 0 1em;
}
td.maniphest-active-handle {
cursor: move;
background-image: url('/rsrc/image/grippy_texture.png');
background-position: 3px 0px;
background-repeat: repeat-y;
}
.maniphest-subpriority-target {
position: relative;
border: 1px dashed #aaaaaa;
background: #f9f9f9;
}
.maniphest-task-loading {
opacity: 0.5;
}
.maniphest-task-dragging {
position: relative;
opacity: 0.5;
z-index: 5;
}

Binary file not shown.

After

Width:  |  Height:  |  Size: 138 B

View file

@ -0,0 +1,203 @@
/**
* @provides javelin-behavior-maniphest-subpriority-editor
* @requires javelin-behavior
* javelin-magical-init
* javelin-dom
* javelin-vector
* javelin-stratcom
* javelin-workflow
*/
JX.behavior('maniphest-subpriority-editor', function(config) {
var dragging = null;
var sending = null;
var origin = null;
var targets = null;
var target = null;
var droptarget = JX.$N('div', {className: 'maniphest-subpriority-target'});
var ondrag = function(e) {
if (dragging || sending) {
return;
}
dragging = e.getNode('maniphest-task');
origin = JX.$V(e);
var tasks = JX.DOM.scry(JX.$(config.root), 'table', 'maniphest-task');
var heads = JX.DOM.scry(JX.$(config.root), 'h1', 'task-group');
var nodes = tasks.concat(heads);
targets = [];
for (var ii = 0; ii < nodes.length; ii++) {
targets.push({
node: nodes[ii],
y: JX.$V(nodes[ii]).y + (JX.Vector.getDim(nodes[ii]).y / 2)
});
}
targets.sort(function(u, v) { return v.y - u.y; });
JX.DOM.alterClass(dragging, 'maniphest-task-dragging', true);
droptarget.style.height = JX.Vector.getDim(dragging).y + 'px';
e.kill();
};
var onmove = function(e) {
if (!dragging) {
return;
}
var p = JX.$V(e);
// Compute the size and position of the drop target indicator, because we
// need to update our static position computations to account for it.
var adjust_h = JX.Vector.getDim(droptarget).y;
var adjust_y = JX.$V(droptarget).y;
// Find the node we're dragging the task underneath. This is the first
// node in the list that's above the cursor. If that node is the node
// we're dragging or its predecessor, don't select a target, because the
// operation would be a no-op.
var cur_target = null;
for (var ii = 0; ii < targets.length; ii++) {
// If the drop target indicator is above the target, we need to adjust
// the target's trigger height down accordingly. This makes dragging
// items down the list smoother, because the target doesn't jump to the
// next item while the cursor is over it.
var trigger = targets[ii].y;
if (adjust_y <= trigger) {
trigger += adjust_h;
}
// If the cursor is above this target, we aren't dropping underneath it.
if (trigger >= p.y) {
continue;
}
// Don't choose the dragged row or its predecessor as targets.
cur_target = targets[ii].node;
if (cur_target == dragging) {
cur_target = null;
}
if (targets[ii - 1] && targets[ii - 1].node == dragging) {
cur_target = null;
}
break;
}
// If we've selected a new target, update the UI to show where we're
// going to drop the row.
if (cur_target != target) {
if (target) {
JX.DOM.remove(droptarget);
}
if (cur_target) {
if (cur_target.nextSibling) {
cur_target.parentNode.insertBefore(
droptarget,
cur_target.nextSibling);
} else {
cur_target.parentNode.appendChild(droptarget);
}
}
target = cur_target;
if (target) {
// If we've changed where the droptarget is, update the adjustments
// so we accurately reflect document state when we tweak things below.
// This avoids a flash of bad state as the mouse is dragged upward
// across the document.
adjust_h = JX.Vector.getDim(droptarget).y;
adjust_y = JX.$V(droptarget).y;
}
}
// If the drop target indicator is above the cursor in the document, adjust
// the cursor position for the change in node document position. Do this
// before choosing a new target to avoid a flash of nonsense.
if (target) {
if (adjust_y <= origin.y) {
p.y -= adjust_h;
}
}
p.x = 0;
p.y -= origin.y;
p.setPos(dragging);
e.kill();
};
var ondrop = function(e) {
if (!dragging) {
return;
}
JX.DOM.alterClass(dragging, 'maniphest-task-dragging', false);
JX.$V(0, 0).setPos(dragging);
if (!target) {
dragging = null;
return;
}
var data = {
task: JX.Stratcom.getData(dragging).taskID
};
if (JX.DOM.isType(target, 'h1')) {
data.priority = JX.Stratcom.getData(target).priority;
} else {
data.after = JX.Stratcom.getData(target).taskID;
}
target = null;
JX.DOM.remove(dragging);
JX.DOM.replace(droptarget, dragging);
sending = dragging;
dragging = null;
JX.DOM.alterClass(sending, 'maniphest-task-loading', true);
var onresponse = function(r) {
JX.DOM.alterClass(sending, 'maniphest-task-loading', false);
var handle = JX.DOM.find(sending, 'td', 'maniphest-task-handle');
handle.className = r.className;
sending = null;
};
new JX.Workflow(config.uri, data)
.setHandler(onresponse)
.start();
e.kill();
};
// NOTE: Javelin does not dispatch mousemove by default.
JX.enableDispatch(document.body, 'mousemove');
JX.Stratcom.listen('mousedown', 'maniphest-task-handle', ondrag);
JX.Stratcom.listen('mousemove', null, onmove);
JX.Stratcom.listen('mouseup', null, ondrop);
});