From 093a6256984cb14cc89f2cea64adf15197b3887a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Sep 2015 11:36:38 -0700 Subject: [PATCH] Show users what's wrong when they try to edit an inline with an editor already open Summary: Fixes T8572. Ideally we would probably just permit this, but clean up the behavior until the day arrives when inline code is actually rewritten. Test Plan: - Tried to launch editors in Differential and Diffusion while comments were already open. - Verified that "Jump to inline" works in both cases. {F788008} {F788009} Reviewers: chad Reviewed By: chad Maniphest Tasks: T8572 Differential Revision: https://secure.phabricator.com/D14094 --- resources/celerity/map.php | 42 +++++++++---------- .../PhabricatorInlineCommentController.php | 13 ++++++ .../DifferentialInlineCommentEditor.js | 24 +++++++++-- .../behavior-edit-inline-comments.js | 16 +++++-- 4 files changed, 68 insertions(+), 27 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 9e02f6bb8b..e3e81484cc 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -11,7 +11,7 @@ return array( 'core.pkg.js' => '47dc9ebb', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '2de124c9', - 'differential.pkg.js' => '52d725be', + 'differential.pkg.js' => '6223dd9d', 'diffusion.pkg.css' => '385e85b3', 'diffusion.pkg.js' => '0115b37c', 'maniphest.pkg.css' => '4845691a', @@ -357,13 +357,13 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', 'rsrc/js/application/differential/ChangesetViewManager.js' => '58562350', - 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'd4c87bf4', + 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '64a5550f', 'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => 'e10f8e18', 'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d', 'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', 'rsrc/js/application/differential/behavior-dropdown-menus.js' => '2035b9cb', - 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '037b59eb', + 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '65ef6074', 'rsrc/js/application/differential/behavior-keyboard-nav.js' => '2c426492', 'rsrc/js/application/differential/behavior-populate.js' => '8694b1df', 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', @@ -517,7 +517,7 @@ return array( 'conpherence-widget-pane-css' => '775eaaba', 'differential-changeset-view-css' => 'b6b0d1bb', 'differential-core-view-css' => '7ac3cabc', - 'differential-inline-comment-editor' => 'd4c87bf4', + 'differential-inline-comment-editor' => '64a5550f', 'differential-revision-add-comment-css' => 'c47f8c40', 'differential-revision-comment-css' => '14b8565a', 'differential-revision-history-css' => '0e8eb855', @@ -568,7 +568,7 @@ return array( 'javelin-behavior-differential-comment-jump' => '4fdb476d', 'javelin-behavior-differential-diff-radios' => 'e1ff79b1', 'javelin-behavior-differential-dropdown-menus' => '2035b9cb', - 'javelin-behavior-differential-edit-inline-comments' => '037b59eb', + 'javelin-behavior-differential-edit-inline-comments' => '65ef6074', 'javelin-behavior-differential-feedback-preview' => 'b064af76', 'javelin-behavior-differential-keyboard-navigation' => '2c426492', 'javelin-behavior-differential-populate' => '8694b1df', @@ -853,14 +853,6 @@ return array( 'javelin-behavior-device', 'phabricator-title', ), - '037b59eb' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-util', - 'javelin-vector', - 'differential-inline-comment-editor', - ), '048330fa' => array( 'javelin-behavior', 'javelin-typeahead-ondemand-source', @@ -1289,6 +1281,22 @@ return array( 'javelin-workflow', 'javelin-dom', ), + '64a5550f' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-request', + 'javelin-workflow', + ), + '65ef6074' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-util', + 'javelin-vector', + 'differential-inline-comment-editor', + ), '665cf6ac' => array( 'javelin-behavior', 'javelin-util', @@ -1817,14 +1825,6 @@ return array( 'javelin-dom', 'javelin-view', ), - 'd4c87bf4' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-request', - 'javelin-workflow', - ), 'd4eecc63' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php index fd847b5aca..aa37d29c72 100644 --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -94,6 +94,19 @@ abstract class PhabricatorInlineCommentController $op = $this->getOperation(); switch ($op) { + case 'busy': + if ($request->isFormPost()) { + return new AphrontAjaxResponse(); + } + + return $this->newDialog() + ->setTitle(pht('Already Editing')) + ->appendParagraph( + pht( + 'You are already editing an inline comment. Finish editing '. + 'your current comment before adding new comments.')) + ->addCancelButton('/') + ->addSubmitButton(pht('Jump to Inline')); case 'hide': case 'show': if (!$request->validateCSRF()) { diff --git a/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js b/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js index 9842da11f5..8defff554d 100644 --- a/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js +++ b/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js @@ -250,12 +250,30 @@ JX.install('DifferentialInlineCommentEditor', { JX.DifferentialInlineCommentEditor._undoRows = rows; }, - start : function() { - this._registerUndoListener(); + _onBusyWorkflow: function() { + // If the user clicks the "Jump to Inline" button, scroll to the row + // being edited. + JX.DOM.scrollTo(this.getRow()); + }, - var data = this._buildRequestData(); + start : function() { var op = this.getOperation(); + // The user is already editing a comment, we're going to give them an + // error message. + if (op == 'busy') { + var onbusy = JX.bind(this, this._onBusyWorkflow); + + new JX.Workflow(this._uri, {op: op}) + .setHandler(onbusy) + .start(); + + return this; + } + + this._registerUndoListener(); + var data = this._buildRequestData(); + if (op == 'delete' || op == 'refdelete' || op == 'undelete') { this._setRowState('loading'); diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js index 821d279f50..aac66d66b7 100644 --- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js +++ b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js @@ -138,13 +138,23 @@ JX.behavior('differential-edit-inline-comments', function(config) { 'mousedown', ['differential-changeset', 'tag:th'], function(e) { - if (editor || - selecting || - e.isRightButton() || + if (e.isRightButton() || getRowNumber(e.getTarget()) === undefined) { return; } + if (editor) { + new JX.DifferentialInlineCommentEditor(config.uri) + .setOperation('busy') + .setRow(editor.getRow().previousSibling) + .start(); + return; + } + + if (selecting) { + return; + } + selecting = true; root = e.getNode('differential-changeset');