diff --git a/resources/celerity/map.php b/resources/celerity/map.php index c7ac86ab3c..309e698272 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,10 +10,10 @@ return array( 'conpherence.pkg.css' => '0e3cf785', 'conpherence.pkg.js' => '020aebcf', 'core.pkg.css' => '0ae696de', - 'core.pkg.js' => 'ab3502fe', + 'core.pkg.js' => '68f29322', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => 'ffb69e3d', - 'differential.pkg.js' => '5986f349', + 'differential.pkg.js' => '8deec4cd', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => '78c9885d', 'maniphest.pkg.css' => '35995d6d', @@ -253,7 +253,7 @@ return array( 'rsrc/externals/javelin/lib/Mask.js' => '7c4d8998', 'rsrc/externals/javelin/lib/Quicksand.js' => 'd3799cb4', 'rsrc/externals/javelin/lib/Request.js' => '84e6891f', - 'rsrc/externals/javelin/lib/Resource.js' => '740956e1', + 'rsrc/externals/javelin/lib/Resource.js' => '20514cc2', 'rsrc/externals/javelin/lib/Routable.js' => '6a18c42e', 'rsrc/externals/javelin/lib/Router.js' => '32755edb', 'rsrc/externals/javelin/lib/Scrollbar.js' => 'a43ae2ae', @@ -383,9 +383,10 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', - 'rsrc/js/application/diff/DiffChangeset.js' => '3b6e1fde', + 'rsrc/js/application/diff/DiffChangeset.js' => 'd7d3ba75', 'rsrc/js/application/diff/DiffChangesetList.js' => 'cc2c5de5', - 'rsrc/js/application/diff/DiffInline.js' => '511a1315', + 'rsrc/js/application/diff/DiffInline.js' => '9c775532', + 'rsrc/js/application/diff/DiffInlineContentState.js' => 'aa51efb4', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', @@ -733,7 +734,7 @@ return array( 'javelin-reactor-node-calmer' => '225bbb98', 'javelin-reactornode' => '72960bc1', 'javelin-request' => '84e6891f', - 'javelin-resource' => '740956e1', + 'javelin-resource' => '20514cc2', 'javelin-routable' => '6a18c42e', 'javelin-router' => '32755edb', 'javelin-scrollbar' => 'a43ae2ae', @@ -785,9 +786,10 @@ return array( 'phabricator-darklog' => '3b869402', 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', - 'phabricator-diff-changeset' => '3b6e1fde', + 'phabricator-diff-changeset' => 'd7d3ba75', 'phabricator-diff-changeset-list' => 'cc2c5de5', - 'phabricator-diff-inline' => '511a1315', + 'phabricator-diff-inline' => '9c775532', + 'phabricator-diff-inline-content-state' => 'aa51efb4', 'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-tree-view' => '5d83623b', 'phabricator-drag-and-drop-file-upload' => '4370900d', @@ -1104,6 +1106,11 @@ return array( 'javelin-behavior', 'javelin-request', ), + '20514cc2' => array( + 'javelin-util', + 'javelin-uri', + 'javelin-install', + ), '225bbb98' => array( 'javelin-install', 'javelin-reactor', @@ -1248,20 +1255,6 @@ return array( 'javelin-behavior', 'phabricator-prefab', ), - '3b6e1fde' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - 'phabricator-diff-path-view', - 'phuix-button-view', - 'javelin-external-editor-link-engine', - ), '3dc5ad43' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1413,9 +1406,6 @@ return array( 'javelin-stratcom', 'javelin-dom', ), - '511a1315' => array( - 'javelin-dom', - ), '5202e831' => array( 'javelin-install', 'javelin-dom', @@ -1605,11 +1595,6 @@ return array( 'javelin-stratcom', 'phabricator-tooltip', ), - '740956e1' => array( - 'javelin-util', - 'javelin-uri', - 'javelin-install', - ), 74446546 => array( 'javelin-behavior', 'javelin-dom', @@ -1831,6 +1816,10 @@ return array( 'javelin-dom', 'javelin-workflow', ), + '9c775532' => array( + 'javelin-dom', + 'phabricator-diff-inline-content-state', + ), '9cec214e' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1921,6 +1910,9 @@ return array( 'javelin-typeahead-ondemand-source', 'javelin-dom', ), + 'aa51efb4' => array( + 'javelin-dom', + ), 'aa6d2308' => array( 'javelin-behavior', 'javelin-dom', @@ -2129,6 +2121,20 @@ return array( 'd4cc2d2a' => array( 'javelin-install', ), + 'd7d3ba75' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + 'phabricator-diff-path-view', + 'phuix-button-view', + 'javelin-external-editor-link-engine', + ), 'd8a86cfb' => array( 'javelin-behavior', 'javelin-dom', @@ -2441,6 +2447,7 @@ return array( 'javelin-behavior-phabricator-object-selector', 'javelin-behavior-repository-crossreference', 'javelin-behavior-aphront-more', + 'phabricator-diff-inline-content-state', 'phabricator-diff-inline', 'phabricator-diff-changeset', 'phabricator-diff-changeset-list', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index 47e13932de..fc5a4f11b6 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -212,6 +212,7 @@ return array( 'javelin-behavior-aphront-more', + 'phabricator-diff-inline-content-state', 'phabricator-diff-inline', 'phabricator-diff-changeset', 'phabricator-diff-changeset-list', diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index dcae3b979b..711d7d574b 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -718,6 +718,9 @@ abstract class DifferentialChangesetRenderer extends Phobject { foreach ($views as $key => $view) { $scaffold = $this->getRowScaffoldForInline($view); + + $scaffold->setIsUndoTemplate(true); + $views[$key] = id(new PHUIDiffInlineCommentTableScaffold()) ->addRowScaffold($scaffold); } diff --git a/src/applications/phid/PhabricatorObjectHandle.php b/src/applications/phid/PhabricatorObjectHandle.php index 95c40ffa2b..577fd05692 100644 --- a/src/applications/phid/PhabricatorObjectHandle.php +++ b/src/applications/phid/PhabricatorObjectHandle.php @@ -402,6 +402,10 @@ final class PhabricatorObjectHandle } public function hasCapabilities() { + if (!$this->isComplete()) { + return false; + } + return ($this->getType() === PhabricatorPeopleUserPHIDType::TYPECONST); } diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php index 693f66066f..6f1d3b66c0 100644 --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -172,7 +172,9 @@ abstract class PhabricatorInlineCommentController $inline = $this->loadCommentByIDForEdit($this->getCommentID()); if ($is_delete) { - $inline->setIsDeleted(1); + $inline + ->setIsEditing(false) + ->setIsDeleted(1); } else { $inline->setIsDeleted(0); } @@ -180,64 +182,60 @@ abstract class PhabricatorInlineCommentController $this->saveComment($inline); return $this->buildEmptyResponse(); - case 'edit': case 'save': $inline = $this->loadCommentByIDForEdit($this->getCommentID()); - if ($op === 'save') { + + $this->updateCommentContentState($inline); + + $inline + ->setIsEditing(false) + ->setIsDeleted(0); + + // Since we're saving the comment, update the committed state. + $active_state = $inline->getContentState(); + $inline->setCommittedContentState($active_state); + + $this->saveComment($inline); + + return $this->buildRenderedCommentResponse( + $inline, + $this->getIsOnRight()); + case 'edit': + $inline = $this->loadCommentByIDForEdit($this->getCommentID()); + + // NOTE: At time of writing, the "editing" state of inlines is + // preserved by simulating a click on "Edit" when the inline loads. + + // In this case, we don't want to "saveComment()", because it + // recalculates object drafts and purges versioned drafts. + + // The recalculation is merely unnecessary (state doesn't change) + // but purging drafts means that loading a page and then closing it + // discards your drafts. + + // To avoid the purge, only invoke "saveComment()" if we actually + // have changes to apply. + + $is_dirty = false; + if (!$inline->getIsEditing()) { + $inline + ->setIsDeleted(0) + ->setIsEditing(true); + + $is_dirty = true; + } + + if ($this->hasContentState()) { $this->updateCommentContentState($inline); - - $inline->setIsEditing(false); - - if (!$inline->isVoidComment($viewer)) { - $inline->setIsDeleted(0); - - $this->saveComment($inline); - - return $this->buildRenderedCommentResponse( - $inline, - $this->getIsOnRight()); - } else { - $inline->setIsDeleted(1); - - $this->saveComment($inline); - - return $this->buildEmptyResponse(); - } + $is_dirty = true; } else { - // NOTE: At time of writing, the "editing" state of inlines is - // preserved by simulating a click on "Edit" when the inline loads. + PhabricatorInlineComment::loadAndAttachVersionedDrafts( + $viewer, + array($inline)); + } - // In this case, we don't want to "saveComment()", because it - // recalculates object drafts and purges versioned drafts. - - // The recalculation is merely unnecessary (state doesn't change) - // but purging drafts means that loading a page and then closing it - // discards your drafts. - - // To avoid the purge, only invoke "saveComment()" if we actually - // have changes to apply. - - $is_dirty = false; - if (!$inline->getIsEditing()) { - $inline - ->setIsDeleted(0) - ->setIsEditing(true); - - $is_dirty = true; - } - - if ($this->hasContentState()) { - $this->updateCommentContentState($inline); - $is_dirty = true; - } else { - PhabricatorInlineComment::loadAndAttachVersionedDrafts( - $viewer, - array($inline)); - } - - if ($is_dirty) { - $this->saveComment($inline); - } + if ($is_dirty) { + $this->saveComment($inline); } $edit_dialog = $this->buildEditDialog($inline) @@ -256,10 +254,6 @@ abstract class PhabricatorInlineCommentController // to set the stored state back to "A". $this->updateCommentContentState($inline); - if ($inline->isVoidComment($viewer)) { - $inline->setIsDeleted(1); - } - $this->saveComment($inline); return $this->buildEmptyResponse(); @@ -328,11 +322,31 @@ abstract class PhabricatorInlineCommentController $this->updateCommentContentState($inline); } + // NOTE: We're writing the comment as "deleted", then reloading to + // pick up context and undeleting it. This is silly -- we just want + // to load and attach context -- but just loading context is currently + // complicated (for example, context relies on cache keys that expect + // the inline to have an ID). + + $inline->setIsDeleted(1); + $this->saveComment($inline); // Reload the inline to attach context. $inline = $this->loadCommentByIDForEdit($inline->getID()); + // Now, we can read the source file and set the initial state. + $state = $inline->getContentState(); + $default_suggestion = $inline->getDefaultSuggestionText(); + $state->setContentSuggestionText($default_suggestion); + + $inline->setInitialContentState($state); + $inline->setContentState($state); + + $inline->setIsDeleted(0); + + $this->saveComment($inline); + $edit_dialog = $this->buildEditDialog($inline); if ($this->getOperation() == 'reply') { @@ -456,6 +470,7 @@ abstract class PhabricatorInlineCommentController PhabricatorInlineComment $inline, $view, $is_edit) { + $viewer = $this->getViewer(); if ($inline->getReplyToCommentPHID()) { $can_suggest = false; @@ -464,18 +479,15 @@ abstract class PhabricatorInlineCommentController } if ($is_edit) { - $viewer = $this->getViewer(); - $content_state = $inline->getContentStateForEdit($viewer); + $state = $inline->getContentStateMapForEdit($viewer); } else { - $content_state = $inline->getContentState(); + $state = $inline->getContentStateMap(); } - $state_map = $content_state->newStorageMap(); - $response = array( 'inline' => array( 'id' => $inline->getID(), - 'contentState' => $state_map, + 'state' => $state, 'canSuggestEdit' => $can_suggest, ), 'view' => hsprintf('%s', $view), diff --git a/src/infrastructure/diff/interface/PhabricatorInlineComment.php b/src/infrastructure/diff/interface/PhabricatorInlineComment.php index 76976b5929..613dfb08aa 100644 --- a/src/infrastructure/diff/interface/PhabricatorInlineComment.php +++ b/src/infrastructure/diff/interface/PhabricatorInlineComment.php @@ -336,13 +336,29 @@ abstract class PhabricatorInlineComment return $this->newContentState()->readFromRequest($request); } - public function getContentState() { - $state = $this->newContentState(); + public function getInitialContentState() { + return $this->getNamedContentState('inline.state.initial'); + } - $storage = $this->getStorageObject(); - $storage_map = $storage->getAttribute('inline.state'); - if (is_array($storage_map)) { - $state->readStorageMap($storage_map); + public function setInitialContentState( + PhabricatorInlineCommentContentState $state) { + return $this->setNamedContentState('inline.state.initial', $state); + } + + public function getCommittedContentState() { + return $this->getNamedContentState('inline.state.committed'); + } + + public function setCommittedContentState( + PhabricatorInlineCommentContentState $state) { + return $this->setNamedContentState('inline.state.committed', $state); + } + + public function getContentState() { + $state = $this->getNamedContentState('inline.state'); + + if (!$state) { + $state = $this->newContentState(); } $state->setContentText($this->getContent()); @@ -351,11 +367,31 @@ abstract class PhabricatorInlineComment } public function setContentState(PhabricatorInlineCommentContentState $state) { + $this->setContent($state->getContentText()); + + return $this->setNamedContentState('inline.state', $state); + } + + private function getNamedContentState($key) { + $storage = $this->getStorageObject(); + + $storage_map = $storage->getAttribute($key); + if (!is_array($storage_map)) { + return null; + } + + $state = $this->newContentState(); + $state->readStorageMap($storage_map); + return $state; + } + + private function setNamedContentState( + $key, + PhabricatorInlineCommentContentState $state) { + $storage = $this->getStorageObject(); $storage_map = $state->newStorageMap(); - $storage->setAttribute('inline.state', $storage_map); - - $this->setContent($state->getContentText()); + $storage->setAttribute($key, $storage_map); return $this; } @@ -364,6 +400,55 @@ abstract class PhabricatorInlineComment return $this->getStorageObject()->getInlineContext(); } + public function getContentStateMapForEdit(PhabricatorUser $viewer) { + return $this->getWireContentStateMap(true, $viewer); + } + + public function getContentStateMap() { + return $this->getWireContentStateMap(false, null); + } + + private function getWireContentStateMap( + $is_edit, + PhabricatorUser $viewer = null) { + + $initial_state = $this->getInitialContentState(); + $committed_state = $this->getCommittedContentState(); + + if ($is_edit) { + $active_state = $this->getContentStateForEdit($viewer); + } else { + $active_state = $this->getContentState(); + } + + return array( + 'initial' => $this->getWireContentState($initial_state), + 'committed' => $this->getWireContentState($committed_state), + 'active' => $this->getWireContentState($active_state), + ); + } + + private function getWireContentState($content_state) { + if ($content_state === null) { + return null; + } + + return $content_state->newStorageMap(); + } + + public function getDefaultSuggestionText() { + $context = $this->getInlineContext(); + + if (!$context) { + return null; + } + + $default = $context->getBodyLines(); + $default = implode('', $default); + + return $default; + } + /* -( PhabricatorMarkupInterface Implementation )-------------------------- */ diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php index 5c29c1666c..72e5adbe1f 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php @@ -114,9 +114,6 @@ final class PHUIDiffInlineCommentEditView $main = $state->getContentSuggestionText(); $main_count = count(phutil_split_lines($main)); - $default = $context->getBodyLines(); - $default = implode('', $default); - // Browsers ignore one leading newline in text areas. Add one so that // any actual leading newlines in the content are preserved. $main = "\n".$main; @@ -127,9 +124,6 @@ final class PHUIDiffInlineCommentEditView 'class' => 'inline-suggestion-input PhabricatorMonospaced', 'rows' => max(3, $main_count + 1), 'sigil' => 'inline-content-suggestion', - 'meta' => array( - 'defaultText' => $default, - ), ), $main); diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php index 7c6b1c54b5..9bb68d63ba 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php @@ -10,6 +10,16 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView { private $views = array(); + private $isUndoTemplate; + + final public function setIsUndoTemplate($is_undo_template) { + $this->isUndoTemplate = $is_undo_template; + return $this; + } + + final public function getIsUndoTemplate() { + return $this->isUndoTemplate; + } public function getInlineViews() { return $this->views; @@ -21,11 +31,28 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView { } protected function getRowAttributes() { + $is_undo_template = $this->getIsUndoTemplate(); + $is_hidden = false; - foreach ($this->getInlineViews() as $view) { - if ($view->isHidden()) { - $is_hidden = true; + if ($is_undo_template) { + + // NOTE: When this scaffold is turned into an "undo" template, it is + // important it not have any metadata: the metadata reference will be + // copied to each instance of the row. This is a complicated mess; for + // now, just sneak by without generating metadata when rendering undo + // templates. + + $metadata = null; + } else { + foreach ($this->getInlineViews() as $view) { + if ($view->isHidden()) { + $is_hidden = true; + } } + + $metadata = array( + 'hidden' => $is_hidden, + ); } $classes = array(); @@ -37,9 +64,7 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView { $result = array( 'class' => implode(' ', $classes), 'sigil' => 'inline-row', - 'meta' => array( - 'hidden' => $is_hidden, - ), + 'meta' => $metadata, ); return $result; diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php index 4abdb00e0b..a5fd7036d9 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php @@ -27,7 +27,7 @@ final class PHUIDiffInlineCommentUndoView array( 'class' => 'differential-inline-undo', ), - array(pht('Changes discarded. '), $link)); + array(pht('Changes discarded.'), ' ', $link)); } } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php index 11be8d0ec1..448a7f2f50 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php @@ -93,7 +93,7 @@ abstract class PHUIDiffInlineCommentView extends AphrontView { 'startOffset' => $inline->getStartOffset(), 'endOffset' => $inline->getEndOffset(), 'on_right' => $this->getIsOnRight(), - 'contentState' => $inline->getContentState()->newStorageMap(), + 'state' => $inline->getContentStateMap(), ); } diff --git a/webroot/rsrc/externals/javelin/lib/Resource.js b/webroot/rsrc/externals/javelin/lib/Resource.js index 4e7d528740..68219e0389 100644 --- a/webroot/rsrc/externals/javelin/lib/Resource.js +++ b/webroot/rsrc/externals/javelin/lib/Resource.js @@ -149,6 +149,10 @@ JX.install('Resource', { } } + for (var jj = 0; jj < errors.length; jj++) { + JX.log(errors[jj]); + } + if (errors.length) { throw errors[0]; } diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js index 668634cb30..af25b59c6b 100644 --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -671,9 +671,6 @@ JX.install('DiffChangeset', { // Code shared by autoload and context responses. this._loadChangesetState(response); - - JX.Stratcom.invoke('differential-inline-comment-refresh'); - this._rebuildAllInlines(); JX.Stratcom.invoke('resize'); @@ -846,9 +843,7 @@ JX.install('DiffChangeset', { }, _rebuildAllInlines: function() { - if (this._inlines === null) { - this._inlines = []; - } + this._inlines = []; var rows = JX.DOM.scry(this._node, 'tr'); var ii; diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js index 705d95e70a..4a0db5b520 100644 --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -1,12 +1,14 @@ /** * @provides phabricator-diff-inline * @requires javelin-dom + * phabricator-diff-inline-content-state * @javelin */ JX.install('DiffInline', { construct : function() { + this._state = {}; }, members: { @@ -19,7 +21,6 @@ JX.install('DiffInline', { _displaySide: null, _isNewFile: null, _replyToCommentPHID: null, - _originalState: null, _snippet: null, _menuItems: null, _documentEngineKey: null, @@ -53,6 +54,8 @@ JX.install('DiffInline', { _isSelected: false, _canSuggestEdit: false, + _state: null, + bindToRow: function(row) { this._row = row; @@ -323,8 +326,6 @@ JX.install('DiffInline', { this._phid = null; this._isCollapsed = false; - this._originalState = null; - return row; }, @@ -412,25 +413,12 @@ JX.install('DiffInline', { .send(); }, - _getContentState: function() { - var state; - - if (this._editRow) { - state = this._readFormState(this._editRow); - } else { - state = this._originalState; - } - - return state; - }, - reply: function(with_quote) { this._closeMenu(); var content_state = this._newContentState(); if (with_quote) { - var text = this._getContentState().text; - text = '> ' + text.replace(/\n/g, '\n> ') + '\n\n'; + var text = this._getActiveContentState().getTextForQuote(); content_state.text = text; } @@ -455,21 +443,12 @@ JX.install('DiffInline', { this._undoText = null; } - var uri = this._getInlineURI(); - var handler = JX.bind(this, this._oneditresponse); - - var data = this._newRequestData('edit', content_state); - - this.setLoading(true); - - new JX.Request(uri, handler) - .setData(data) - .send(); + this._applyEdit(content_state); }, delete: function(is_ref) { var uri = this._getInlineURI(); - var handler = JX.bind(this, this._ondeleteresponse); + var handler = JX.bind(this, this._ondeleteresponse, false); // NOTE: This may be a direct delete (the user clicked on the inline // itself) or a "refdelete" (the user clicked somewhere else, like the @@ -589,7 +568,6 @@ JX.install('DiffInline', { this._readInlineState(response.inline); this._drawEditRows(rows); - this.setLoading(false); this.setInvisible(true); }, @@ -602,28 +580,44 @@ JX.install('DiffInline', { _readInlineState: function(state) { this._id = state.id; - this._originalState = state.contentState; + + this._state = { + initial: this._newContentStateFromWireFormat(state.state.initial), + committed: this._newContentStateFromWireFormat(state.state.committed), + active: this._newContentStateFromWireFormat(state.state.active) + }; + this._canSuggestEdit = state.canSuggestEdit; }, - _ondeleteresponse: function() { - // If there's an existing "unedit" undo element, remove it. - if (this._undoRow) { - JX.DOM.remove(this._undoRow); - this._undoRow = null; + _newContentStateFromWireFormat: function(map) { + if (map === null) { + return null; } - // If there's an existing editor, remove it. This happens when you - // delete a comment from the comment preview area. In this case, we - // read and preserve the text so "Undo" restores it. - var state = null; - if (this._editRow) { - state = this._readFormState(this._editRow); - JX.DOM.remove(this._editRow); - this._editRow = null; - } + return new JX.DiffInlineContentState().readWireFormat(map); + }, - this._drawUndeleteRows(state); + _ondeleteresponse: function(prevent_undo) { + if (!prevent_undo) { + // If there's an existing "unedit" undo element, remove it. + if (this._undoRow) { + JX.DOM.remove(this._undoRow); + this._undoRow = null; + } + + // If there's an existing editor, remove it. This happens when you + // delete a comment from the comment preview area. In this case, we + // read and preserve the text so "Undo" restores it. + var state = null; + if (this._editRow) { + state = this._getActiveContentState().getWireFormat(); + JX.DOM.remove(this._editRow); + this._editRow = null; + } + + this._drawUndeleteRows(state); + } this.setLoading(false); this.setDeleted(true); @@ -668,7 +662,10 @@ JX.install('DiffInline', { this._editRow = this._drawRows(rows, null, 'edit'); this._drawSuggestionState(this._editRow); - this.setHasSuggestion(this._originalState.hasSuggestion); + + // TODO: We're just doing this for the rendering side effect of drawing + // the button text. + this.setHasSuggestion(this.getHasSuggestion()); }, _drawRows: function(rows, cursor, type) { @@ -677,7 +674,6 @@ JX.install('DiffInline', { var anchor = cursor || this._row; cursor = cursor || this._row.nextSibling; - var result_row; var next_row; while (row) { @@ -762,21 +758,12 @@ JX.install('DiffInline', { this.setHasSuggestion(!this.getHasSuggestion()); - // The first time the user actually clicks the button and enables - // suggestions for a given editor state, fill the input with the - // underlying text if there isn't any text yet. + // Resize the suggestion input for size of the text. if (this.getHasSuggestion()) { if (this._editRow) { var node = this._getSuggestionNode(this._editRow); if (node) { - if (!node.value.length) { - var data = JX.Stratcom.getData(node); - if (!data.hasSetDefault) { - data.hasSetDefault = true; - node.value = data.defaultText; - node.rows = Math.max(3, node.value.split('\n').length); - } - } + node.rows = Math.max(3, node.value.split('\n').length); } } } @@ -785,8 +772,27 @@ JX.install('DiffInline', { this.triggerDraft(); }, + _getActiveContentState: function() { + var state = this._state.active; + + if (this._editRow) { + state.readForm(this._editRow); + } + + return state; + }, + + _getCommittedContentState: function() { + return this._state.committed; + }, + + _getInitialContentState: function() { + return this._state.initial; + }, + setHasSuggestion: function(has_suggestion) { - this._hasSuggestion = has_suggestion; + var state = this._getActiveContentState(); + state.setHasSuggestion(has_suggestion); var button = this._getSuggestionButton(); var pht = this.getChangeset().getChangesetList().getTranslations(); @@ -806,20 +812,121 @@ JX.install('DiffInline', { }, getHasSuggestion: function() { - return this._hasSuggestion; + return this._getActiveContentState().getHasSuggestion(); }, save: function() { - var handler = JX.bind(this, this._onsubmitresponse); + if (this._shouldDeleteOnSave()) { + JX.DOM.remove(this._editRow); + this._editRow = null; + + this._applyDelete(true); + return; + } + + this._applySave(); + }, + + _shouldDeleteOnSave: function() { + var active = this._getActiveContentState(); + var initial = this._getInitialContentState(); + + // When a user clicks "Save", it counts as a "delete" if the content + // of the comment is functionally empty. + + // This isn't a delete if there's any text. Even if the text is a + // quote (so the state is the same as the initial state), we preserve + // it when the user clicks "Save". + if (!active.isTextEmpty()) { + return false; + } + + // This isn't a delete if there's a suggestion and that suggestion is + // different from the initial state. (This means that an inline which + // purely suggests a block of code should be deleted is non-empty.) + if (active.getHasSuggestion()) { + if (!active.isSuggestionSimilar(initial)) { + return false; + } + } + + // Otherwise, this comment is functionally empty, so we can just treat + // a "Save" as a "delete". + return true; + }, + + _shouldUndoOnCancel: function() { + var committed = this._getCommittedContentState(); + var active = this._getActiveContentState(); + var initial = this._getInitialContentState(); + + // When a user clicks "Cancel", we only offer to let them "Undo" the + // action if the undo would be substantive. + + // The undo is substantive if the text is nonempty, and not similar to + // the last state. + var versus = committed || initial; + if (!active.isTextEmpty() && !active.isTextSimilar(versus)) { + return true; + } + + // The undo is substantive if there's a suggestion, and the suggestion + // is not similar to the last state. + if (active.getHasSuggestion()) { + if (!active.isSuggestionSimilar(versus)) { + return true; + } + } + + return false; + }, + + _applySave: function() { + var handler = JX.bind(this, this._onsaveresponse); + + var state = this._getActiveContentState(); + var data = this._newRequestData('save', state.getWireFormat()); + + this._applyCall(handler, data); + }, + + _applyDelete: function(prevent_undo) { + var handler = JX.bind(this, this._ondeleteresponse, prevent_undo); + + var data = this._newRequestData('delete'); + + this._applyCall(handler, data); + }, + + _applyCancel: function(state) { + var handler = JX.bind(this, this._onCancelResponse); + + var data = this._newRequestData('cancel', state); + + this._applyCall(handler, data); + }, + + _applyEdit: function(state) { + var handler = JX.bind(this, this._oneditresponse); + + var data = this._newRequestData('edit', state); + + this._applyCall(handler, data); + }, + + _applyCall: function(handler, data) { + var uri = this._getInlineURI(); + + var callback = JX.bind(this, function() { + this.setLoading(false); + handler.apply(null, arguments); + }); this.setLoading(true); - var uri = this._getInlineURI(); - var data = this._newRequestData('save', this._getContentState()); - - new JX.Request(uri, handler) - .setData(data) - .send(); + new JX.Workflow(uri, data) + .setHandler(callback) + .start(); }, undo: function() { @@ -850,79 +957,45 @@ JX.install('DiffInline', { }, cancel: function() { - var state = this._readFormState(this._editRow); + // NOTE: Read the state before we remove the editor. Otherwise, we might + // miss text the user has entered into the textarea. + var state = this._getActiveContentState().getWireFormat(); JX.DOM.remove(this._editRow); this._editRow = null; - var is_empty = this._isVoidContentState(state); - var is_same = this._isSameContentState(state, this._originalState); - if (!is_empty && !is_same) { - this._drawUneditRows(state); - } + // When a user clicks "Cancel", we delete the comment if it has never + // been saved: we don't have a non-empty display state to revert to. + var is_delete = (this._getCommittedContentState() === null); - // If this was an empty box and we typed some text and then hit cancel, - // don't show the empty concrete inline. - if (this._isVoidContentState(this._originalState)) { - this.setInvisible(true); - } else { - this.setInvisible(false); - } + var is_undo = this._shouldUndoOnCancel(); // If you "undo" to restore text ("AB") and then "Cancel", we put you // back in the original text state ("A"). We also send the original // text ("A") to the server as the current persistent state. - var uri = this._getInlineURI(); - var data = this._newRequestData('cancel', this._originalState); - var handler = JX.bind(this, this._onCancelResponse); + if (is_undo) { + this._drawUneditRows(state); + } - this.setLoading(true); + if (is_delete) { + // NOTE: We're always suppressing the undo from "delete". We want to + // use the "undo" we just added above instead, which will get us + // back to the ephemeral, client-side editor state. + this._applyDelete(true); + } else { + this.setEditing(false); + this.setInvisible(false); - new JX.Request(uri, handler) - .setData(data) - .send(); + var old_state = this._getCommittedContentState(); + this._applyCancel(old_state.getWireFormat()); - this._didUpdate(true); + this._didUpdate(true); + } }, _onCancelResponse: function(response) { - this.setEditing(false); - this.setLoading(false); - - // If the comment was empty when we started editing it (there's no - // original text) and empty when we finished editing it (there's no - // undo row), just delete the comment. - if (this._isVoidContentState(this._originalState) && !this.isUndo()) { - this.setDeleted(true); - - JX.DOM.remove(this._row); - this._row = null; - - this._didUpdate(); - } - }, - - _readFormState: function(row) { - var state = this._newContentState(); - - var node; - - try { - node = JX.DOM.find(row, 'textarea', 'inline-content-text'); - state.text = node.value; - } catch (ex) { - // Ignore. - } - - node = this._getSuggestionNode(row); - if (node) { - state.suggestionText = node.value; - } - - state.hasSuggestion = this.getHasSuggestion(); - - return state; + // Nothing to do. }, _getSuggestionNode: function(row) { @@ -933,40 +1006,18 @@ JX.install('DiffInline', { } }, - _onsubmitresponse: function(response) { + _onsaveresponse: function(response) { if (this._editRow) { JX.DOM.remove(this._editRow); this._editRow = null; } - this.setLoading(false); - this.setInvisible(false); this.setEditing(false); + this.setInvisible(false); - this._onupdate(response); - }, - - _onupdate: function(response) { - var new_row; - if (response.view) { - new_row = this._drawContentRows(JX.$H(response.view).getNode()); - } - - // TODO: Save the old row so the action it's undo-able if it was a - // delete. - var remove_old = true; - if (remove_old) { - JX.DOM.remove(this._row); - } - - // If you delete the content on a comment and save it, it acts like a - // delete: the server does not return a new row. - if (new_row) { - this.bindToRow(new_row); - } else { - this.setDeleted(true); - this._row = null; - } + var new_row = this._drawContentRows(JX.$H(response.view).getNode()); + JX.DOM.remove(this._row); + this.bindToRow(new_row); this._didUpdate(); }, @@ -1037,8 +1088,8 @@ JX.install('DiffInline', { return null; } - var state = this._readFormState(this._editRow); - if (this._isVoidContentState(state)) { + var state = this._getActiveContentState(); + if (state.isStateEmpty()) { return null; } @@ -1047,7 +1098,7 @@ JX.install('DiffInline', { id: this.getID(), }; - JX.copy(draft_data, state); + JX.copy(draft_data, state.getWireFormat()); return draft_data; }, @@ -1163,19 +1214,8 @@ JX.install('DiffInline', { suggestionText: '', hasSuggestion: false }; - }, - - _isVoidContentState: function(state) { - return (!state.text.length && !state.suggestionText.length); - }, - - _isSameContentState: function(u, v) { - return ( - ((u === null) === (v === null)) && - (u.text === v.text) && - (u.suggestionText === v.suggestionText) && - (u.hasSuggestion === v.hasSuggestion)); } + } }); diff --git a/webroot/rsrc/js/application/diff/DiffInlineContentState.js b/webroot/rsrc/js/application/diff/DiffInlineContentState.js new file mode 100644 index 0000000000..0ac8628e74 --- /dev/null +++ b/webroot/rsrc/js/application/diff/DiffInlineContentState.js @@ -0,0 +1,137 @@ +/** + * @provides phabricator-diff-inline-content-state + * @requires javelin-dom + * @javelin + */ + +JX.install('DiffInlineContentState', { + + construct : function() { + + }, + + properties: { + text: null, + suggestionText: null, + hasSuggestion: false + }, + + members: { + readForm: function(row) { + var node; + + try { + node = JX.DOM.find(row, 'textarea', 'inline-content-text'); + this.setText(node.value); + } catch (ex) { + this.setText(null); + } + + node = this._getSuggestionNode(row); + if (node) { + this.setSuggestionText(node.value); + } else { + this.setSuggestionText(null); + } + + return this; + }, + + getWireFormat: function() { + return { + text: this.getText(), + suggestionText: this.getSuggestionText(), + hasSuggestion: this.getHasSuggestion() + }; + }, + + readWireFormat: function(map) { + this.setText(map.text || null); + this.setSuggestionText(map.suggestionText || null); + this.setHasSuggestion(!!map.hasSuggestion); + + return this; + }, + + getTextForQuote: function() { + var text = this.getText(); + text = '> ' + text.replace(/\n/g, '\n> ') + '\n\n'; + return text; + }, + + isStateEmpty: function() { + return (this.isTextEmpty() && this.isSuggestionEmpty()); + }, + + isTextEmpty: function() { + var text = this.getText(); + if (text === null) { + return true; + } + + if (this._isStringSimilar(text, '')) { + return true; + } + + return false; + }, + + isSuggestionEmpty: function() { + if (!this.getHasSuggestion()) { + return true; + } + + var suggestion = this.getSuggestionText(); + if (suggestion === null) { + return true; + } + + if (this._isStringSimilar(suggestion, '')) { + return true; + } + + return false; + }, + + isTextSimilar: function(v) { + if (!v) { + return false; + } + + var us = this.getText(); + var vs = v.getText(); + + return this._isStringSimilar(us, vs); + }, + + isSuggestionSimilar: function(v) { + // If we don't have a comparison state, treat them as dissimilar. This + // is expected to occur in old inline comments that did not save an + // initial state. + + if (!v) { + return false; + } + + var us = this.getSuggestionText(); + var vs = v.getSuggestionText(); + + return this._isStringSimilar(us, vs); + }, + + _isStringSimilar: function(u, v) { + u = u || ''; + v = v || ''; + return (u === v); + }, + + _getSuggestionNode: function(row) { + try { + return JX.DOM.find(row, 'textarea', 'inline-content-suggestion'); + } catch (ex) { + return null; + } + } + } + +});