From 282b3f79880c2cfd53f82f00f3d28d6b608d5aaa Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 30 May 2013 18:55:25 -0700 Subject: [PATCH] Allow Maniphest tasks to be edited from the list view Summary: Fixes T1945. Ref T2947. At various times, installs (Disqus, Dropbox, etc.) have asked for a way to edit tasks more quickly. Provide edit-from-lists. {F44700} {F44701} The one rough edge on this is that if you change the task priority we update it inline but don't move it. It's probably infeasible to actually move it, but maybe we could give it some sort of visual style to indicate that it's dirty. Test Plan: Edited tasks normally and via this action thing. Reviewers: chad, btrahan Reviewed By: chad CC: tido, deuresti, ahoffer, aran Maniphest Tasks: T1945, T2947 Differential Revision: https://secure.phabricator.com/D6086 --- src/__celerity_resource_map__.php | 61 ++++++++++++------- .../controller/ManiphestController.php | 23 +++++++ .../ManiphestSubpriorityController.php | 18 +----- .../ManiphestTaskEditController.php | 38 ++++++++++-- .../maniphest/view/ManiphestTaskListView.php | 12 ++++ .../rsrc/externals/javelin/lib/Workflow.js | 14 ++++- .../maniphest/behavior-list-edit.js | 29 +++++++++ 7 files changed, 148 insertions(+), 47 deletions(-) create mode 100644 webroot/rsrc/js/application/maniphest/behavior-list-edit.js diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index e37e474226..00569443a6 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1780,6 +1780,21 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/maniphest/behavior-task-preview.js', ), + 'javelin-behavior-maniphest-list-editor' => + array( + 'uri' => '/res/170f8457/rsrc/js/application/maniphest/behavior-list-edit.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + 2 => 'javelin-stratcom', + 3 => 'javelin-workflow', + 4 => 'javelin-fx', + 5 => 'javelin-util', + ), + 'disk' => '/rsrc/js/application/maniphest/behavior-list-edit.js', + ), 'javelin-behavior-maniphest-subpriority-editor' => array( 'uri' => '/res/21b73c2a/rsrc/js/application/maniphest/behavior-subpriorityeditor.js', @@ -2737,7 +2752,7 @@ celerity_register_resource_map(array( ), 'javelin-workflow' => array( - 'uri' => '/res/8274d65f/rsrc/externals/javelin/lib/Workflow.js', + 'uri' => '/res/7626494b/rsrc/externals/javelin/lib/Workflow.js', 'type' => 'js', 'requires' => array( @@ -4156,7 +4171,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/96909266/diffusion.pkg.js', 'type' => 'js', ), - 'c1359b5d' => + 'a9f14d76' => array( 'name' => 'javelin.pkg.js', 'symbols' => @@ -4182,7 +4197,7 @@ celerity_register_resource_map(array( 18 => 'javelin-tokenizer', 19 => 'javelin-history', ), - 'uri' => '/res/pkg/c1359b5d/javelin.pkg.js', + 'uri' => '/res/pkg/a9f14d76/javelin.pkg.js', 'type' => 'js', ), '6b1fccc6' => @@ -4242,7 +4257,7 @@ celerity_register_resource_map(array( 'global-drag-and-drop-css' => '1b14560c', 'inline-comment-summary-css' => 'dd27a69b', 'javelin-aphlict' => '98f60e3f', - 'javelin-behavior' => 'c1359b5d', + 'javelin-behavior' => 'a9f14d76', 'javelin-behavior-aphlict-dropdown' => '98f60e3f', 'javelin-behavior-aphlict-listen' => '98f60e3f', 'javelin-behavior-aphront-basic-tokenizer' => '98f60e3f', @@ -4294,25 +4309,25 @@ celerity_register_resource_map(array( 'javelin-behavior-repository-crossreference' => '9488bb69', 'javelin-behavior-toggle-class' => '98f60e3f', 'javelin-behavior-workflow' => '98f60e3f', - 'javelin-dom' => 'c1359b5d', - 'javelin-event' => 'c1359b5d', - 'javelin-history' => 'c1359b5d', - 'javelin-install' => 'c1359b5d', - 'javelin-json' => 'c1359b5d', - 'javelin-mask' => 'c1359b5d', - 'javelin-request' => 'c1359b5d', - 'javelin-resource' => 'c1359b5d', - 'javelin-stratcom' => 'c1359b5d', - 'javelin-tokenizer' => 'c1359b5d', - 'javelin-typeahead' => 'c1359b5d', - 'javelin-typeahead-normalizer' => 'c1359b5d', - 'javelin-typeahead-ondemand-source' => 'c1359b5d', - 'javelin-typeahead-preloaded-source' => 'c1359b5d', - 'javelin-typeahead-source' => 'c1359b5d', - 'javelin-uri' => 'c1359b5d', - 'javelin-util' => 'c1359b5d', - 'javelin-vector' => 'c1359b5d', - 'javelin-workflow' => 'c1359b5d', + 'javelin-dom' => 'a9f14d76', + 'javelin-event' => 'a9f14d76', + 'javelin-history' => 'a9f14d76', + 'javelin-install' => 'a9f14d76', + 'javelin-json' => 'a9f14d76', + 'javelin-mask' => 'a9f14d76', + 'javelin-request' => 'a9f14d76', + 'javelin-resource' => 'a9f14d76', + 'javelin-stratcom' => 'a9f14d76', + 'javelin-tokenizer' => 'a9f14d76', + 'javelin-typeahead' => 'a9f14d76', + 'javelin-typeahead-normalizer' => 'a9f14d76', + 'javelin-typeahead-ondemand-source' => 'a9f14d76', + 'javelin-typeahead-preloaded-source' => 'a9f14d76', + 'javelin-typeahead-source' => 'a9f14d76', + 'javelin-uri' => 'a9f14d76', + 'javelin-util' => 'a9f14d76', + 'javelin-vector' => 'a9f14d76', + 'javelin-workflow' => 'a9f14d76', 'lightbox-attachment-css' => '1b14560c', 'maniphest-task-summary-css' => '6b1fccc6', 'maniphest-transaction-detail-css' => '6b1fccc6', diff --git a/src/applications/maniphest/controller/ManiphestController.php b/src/applications/maniphest/controller/ManiphestController.php index c93a248c89..4bca8bd852 100644 --- a/src/applications/maniphest/controller/ManiphestController.php +++ b/src/applications/maniphest/controller/ManiphestController.php @@ -84,4 +84,27 @@ abstract class ManiphestController extends PhabricatorController { return $this->defaultQuery; } + protected function renderSingleTask(ManiphestTask $task) { + $user = $this->getRequest()->getUser(); + + $phids = $task->getProjectPHIDs(); + if ($task->getOwnerPHID()) { + $phids[] = $task->getOwnerPHID(); + } + + $handles = id(new PhabricatorObjectHandleData($phids)) + ->setViewer($user) + ->loadHandles(); + + $view = id(new ManiphestTaskListView()) + ->setUser($user) + ->setShowSubpriorityControls(true) + ->setShowBatchControls(true) + ->setHandles($handles) + ->setTasks(array($task)); + + return $view; + } + + } diff --git a/src/applications/maniphest/controller/ManiphestSubpriorityController.php b/src/applications/maniphest/controller/ManiphestSubpriorityController.php index 4cbeb661c6..4208dacce8 100644 --- a/src/applications/maniphest/controller/ManiphestSubpriorityController.php +++ b/src/applications/maniphest/controller/ManiphestSubpriorityController.php @@ -51,25 +51,9 @@ final class ManiphestSubpriorityController extends ManiphestController { $task->setSubpriority($new_sub); $task->save(); - $phids = $task->getProjectPHIDs(); - if ($task->getOwnerPHID()) { - $phids[] = $task->getOwnerPHID(); - } - - $handles = id(new PhabricatorObjectHandleData($phids)) - ->setViewer($user) - ->loadHandles(); - - $view = id(new ManiphestTaskListView()) - ->setUser($user) - ->setShowSubpriorityControls(true) - ->setShowBatchControls(true) - ->setHandles($handles) - ->setTasks(array($task)); - return id(new AphrontAjaxResponse())->setContent( array( - 'tasks' => $view, + 'tasks' => $this->renderSingleTask($task), )); } diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index 4290abdb1a..b42c5ca961 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -242,6 +242,13 @@ final class ManiphestTaskEditController extends ManiphestController { $workflow = $parent_task->getID(); } + if ($request->isAjax()) { + return id(new AphrontAjaxResponse())->setContent( + array( + 'tasks' => $this->renderSingleTask($task), + )); + } + $redirect_uri = '/T'.$task->getID(); if ($workflow) { @@ -354,12 +361,15 @@ final class ManiphestTaskEditController extends ManiphestController { $project_tokenizer_id = celerity_generate_unique_node_id(); - $form = new AphrontFormView(); - $form->setFlexible(true); - $form - ->setUser($user) - ->setAction($request->getRequestURI()->getPath()) - ->addHiddenInput('template', $template_id); + if ($request->isAjax()) { + $form = new AphrontFormLayoutView(); + } else { + $form = new AphrontFormView(); + $form->setFlexible(true); + $form + ->setUser($user) + ->addHiddenInput('template', $template_id); + } if ($parent_task) { $form @@ -485,6 +495,22 @@ final class ManiphestTaskEditController extends ManiphestController { $form ->appendChild($description_control); + + if ($request->isAjax()) { + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->setWidth(AphrontDialogView::WIDTH_FULL) + ->setTitle($header_name) + ->appendChild( + array( + $error_view, + $form, + )) + ->addCancelButton($cancel_uri) + ->addSubmitButton($button_name); + return id(new AphrontDialogResponse())->setDialog($dialog); + } + $form ->appendChild( id(new AphrontFormSubmitControl()) diff --git a/src/applications/maniphest/view/ManiphestTaskListView.php b/src/applications/maniphest/view/ManiphestTaskListView.php index c6348c76a8..0446f4c5ee 100644 --- a/src/applications/maniphest/view/ManiphestTaskListView.php +++ b/src/applications/maniphest/view/ManiphestTaskListView.php @@ -49,6 +49,10 @@ final class ManiphestTaskListView extends ManiphestView { ManiphestTaskPriority::PRIORITY_WISH => 'sky', ); + if ($this->showBatchControls) { + Javelin::initBehavior('maniphest-list-editor'); + } + foreach ($this->tasks as $task) { $item = new PhabricatorObjectItemView(); $item->setObjectName('T'.$task->getID()); @@ -97,6 +101,14 @@ final class ManiphestTaskListView extends ManiphestView { 'taskID' => $task->getID(), )); + if ($this->showBatchControls) { + $item->addAction( + id(new PhabricatorMenuItemView()) + ->setIcon('edit') + ->addSigil('maniphest-edit-task') + ->setHref('/maniphest/task/edit/'.$task->getID().'/')); + } + $list->addItem($item); } diff --git a/webroot/rsrc/externals/javelin/lib/Workflow.js b/webroot/rsrc/externals/javelin/lib/Workflow.js index 7e594e3944..1d1c03ed5c 100644 --- a/webroot/rsrc/externals/javelin/lib/Workflow.js +++ b/webroot/rsrc/externals/javelin/lib/Workflow.js @@ -144,7 +144,13 @@ JX.install('Workflow', { var d = JX.Vector.getDim(this._root); var v = JX.Vector.getViewport(); var s = JX.Vector.getScroll(); - JX.$V((v.x - d.x) / 2, s.y + 100).setPos(this._root); + + // Normally, we position dialogs 100px from the top of the screen. + // Use more space if the dialog is large (at least roughly the size + // of the viewport). + var offset = Math.min(Math.max(20, (v.y - d.y) / 2), 100); + JX.$V((v.x - d.x) / 2, s.y + offset).setPos(this._root); + try { JX.DOM.focus(JX.DOM.find(this._root, 'button', '__default__')); var inputs = JX.DOM.scry(this._root, 'input') @@ -163,6 +169,12 @@ JX.install('Workflow', { } target && JX.DOM.focus(target); } catch (_ignored) {} + + // The `focus()` call may have scrolled the window. Scroll it back to + // where it was before -- we want to focus the control, but not adjust + // the scroll position. + window.scrollTo(s.x, s.y); + } else if (this.getHandler()) { this.getHandler()(r); this._pop(); diff --git a/webroot/rsrc/js/application/maniphest/behavior-list-edit.js b/webroot/rsrc/js/application/maniphest/behavior-list-edit.js new file mode 100644 index 0000000000..873b9d8aee --- /dev/null +++ b/webroot/rsrc/js/application/maniphest/behavior-list-edit.js @@ -0,0 +1,29 @@ +/** + * @provides javelin-behavior-maniphest-list-editor + * @requires javelin-behavior + * javelin-dom + * javelin-stratcom + * javelin-workflow + * javelin-fx + * javelin-util + */ + +JX.behavior('maniphest-list-editor', function(config) { + + var onedit = function(task, r) { + var nodes = JX.$H(r.tasks).getFragment().firstChild; + var new_task = JX.DOM.find(nodes, 'li', 'maniphest-task'); + JX.DOM.replace(task, new_task); + + new JX.FX(new_task).setDuration(500).start({opacity: [0, 1]}); + }; + + JX.Stratcom.listen('click', 'maniphest-edit-task', function(e) { + e.kill(); + var task = e.getNode('maniphest-task'); + JX.Workflow.newFromLink(e.getNode('maniphest-edit-task')) + .setHandler(JX.bind(null, onedit, task)) + .start(); + }); + +});