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
			
			
This commit is contained in:
		| @@ -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', | ||||
|   | ||||
| @@ -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()) { | ||||
|   | ||||
| @@ -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'); | ||||
|  | ||||
|   | ||||
| @@ -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'); | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley