diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ 'core.pkg.js' => '68f29322', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => 'ffb69e3d', - 'differential.pkg.js' => 'fbde899f', + 'differential.pkg.js' => '59453886', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => '78c9885d', 'maniphest.pkg.css' => '35995d6d', @@ -385,7 +385,7 @@ 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', 'rsrc/js/application/diff/DiffChangeset.js' => 'd7d3ba75', 'rsrc/js/application/diff/DiffChangesetList.js' => 'cc2c5de5', - 'rsrc/js/application/diff/DiffInline.js' => '62fff8eb', + 'rsrc/js/application/diff/DiffInline.js' => '26664c24', 'rsrc/js/application/diff/DiffInlineContentState.js' => '68e6339d', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', @@ -788,7 +788,7 @@ 'phabricator-dashboard-css' => '5a205b9d', 'phabricator-diff-changeset' => 'd7d3ba75', 'phabricator-diff-changeset-list' => 'cc2c5de5', - 'phabricator-diff-inline' => '62fff8eb', + 'phabricator-diff-inline' => '26664c24', 'phabricator-diff-inline-content-state' => '68e6339d', 'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-tree-view' => '5d83623b', @@ -1162,6 +1162,10 @@ 'javelin-json', 'phabricator-prefab', ), + '26664c24' => array( + 'javelin-dom', + 'phabricator-diff-inline-content-state', + ), '289bf236' => array( 'javelin-install', 'javelin-util', @@ -1532,10 +1536,6 @@ 'javelin-request', 'javelin-uri', ), - '62fff8eb' => array( - 'javelin-dom', - 'phabricator-diff-inline-content-state', - ), '65bb0011' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -180,56 +180,60 @@ $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); + $this->updateCommentContentState($inline); - $this->saveComment($inline); + $inline + ->setIsEditing(false) + ->setIsDeleted(0); - return $this->buildRenderedCommentResponse( - $inline, - $this->getIsOnRight()); - } else { - // NOTE: At time of writing, the "editing" state of inlines is - // preserved by simulating a click on "Edit" when the inline loads. + // Since we're saving the comment, update the committed state. + $active_state = $inline->getContentState(); + $inline->setCommittedContentState($active_state); - // In this case, we don't want to "saveComment()", because it - // recalculates object drafts and purges versioned drafts. + $this->saveComment($inline); - // The recalculation is merely unnecessary (state doesn't change) - // but purging drafts means that loading a page and then closing it - // discards your drafts. + return $this->buildRenderedCommentResponse( + $inline, + $this->getIsOnRight()); + case 'edit': + $inline = $this->loadCommentByIDForEdit($this->getCommentID()); - // To avoid the purge, only invoke "saveComment()" if we actually - // have changes to apply. + // NOTE: At time of writing, the "editing" state of inlines is + // preserved by simulating a click on "Edit" when the inline loads. - $is_dirty = false; - if (!$inline->getIsEditing()) { - $inline - ->setIsDeleted(0) - ->setIsEditing(true); + // In this case, we don't want to "saveComment()", because it + // recalculates object drafts and purges versioned drafts. - $is_dirty = true; - } + // The recalculation is merely unnecessary (state doesn't change) + // but purging drafts means that loading a page and then closing it + // discards your drafts. - if ($this->hasContentState()) { - $this->updateCommentContentState($inline); - $is_dirty = true; - } else { - PhabricatorInlineComment::loadAndAttachVersionedDrafts( - $viewer, - array($inline)); - } + // To avoid the purge, only invoke "saveComment()" if we actually + // have changes to apply. - if ($is_dirty) { - $this->saveComment($inline); - } + $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); } $edit_dialog = $this->buildEditDialog($inline) @@ -333,7 +337,10 @@ $state = $inline->getContentState(); $default_suggestion = $inline->getDefaultSuggestionText(); $state->setContentSuggestionText($default_suggestion); + + $inline->setInitialContentState($state); $inline->setContentState($state); + $inline->setIsDeleted(0); $this->saveComment($inline); @@ -461,6 +468,7 @@ PhabricatorInlineComment $inline, $view, $is_edit) { + $viewer = $this->getViewer(); if ($inline->getReplyToCommentPHID()) { $can_suggest = false; @@ -469,18 +477,15 @@ } 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 --- a/src/infrastructure/diff/interface/PhabricatorInlineComment.php +++ b/src/infrastructure/diff/interface/PhabricatorInlineComment.php @@ -336,13 +336,29 @@ return $this->newContentState()->readFromRequest($request); } + public function getInitialContentState() { + return $this->getNamedContentState('inline.state.initial'); + } + + 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->newContentState(); + $state = $this->getNamedContentState('inline.state'); - $storage = $this->getStorageObject(); - $storage_map = $storage->getAttribute('inline.state'); - if (is_array($storage_map)) { - $state->readStorageMap($storage_map); + if (!$state) { + $state = $this->newContentState(); } $state->setContentText($this->getContent()); @@ -351,11 +367,31 @@ } public function setContentState(PhabricatorInlineCommentContentState $state) { + $this->setContent($state->getContentText()); + + return $this->setNamedContentState('inline.state', $state); + } + + private function getNamedContentState($key) { $storage = $this->getStorageObject(); - $storage_map = $state->newStorageMap(); - $storage->setAttribute('inline.state', $storage_map); - $this->setContent($state->getContentText()); + $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($key, $storage_map); return $this; } @@ -364,6 +400,42 @@ 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(); diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php @@ -93,7 +93,7 @@ 'startOffset' => $inline->getStartOffset(), 'endOffset' => $inline->getEndOffset(), 'on_right' => $this->getIsOnRight(), - 'contentState' => $inline->getContentState()->newStorageMap(), + 'state' => $inline->getContentStateMap(), ); } diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -8,8 +8,7 @@ JX.install('DiffInline', { construct : function() { - this._activeContentState = new JX.DiffInlineContentState(); - this._committedContentState = new JX.DiffInlineContentState(); + this._state = {}; }, members: { @@ -56,8 +55,7 @@ _isSelected: false, _canSuggestEdit: false, - _committedContentState: null, - _activeContentState: null, + _state: null, bindToRow: function(row) { this._row = row; @@ -602,15 +600,23 @@ _readInlineState: function(state) { this._id = state.id; - // TODO: This is not the correct content state after a reload: it is - // the draft state. - this._getCommittedContentState().readWireFormat(state.contentState); - - this._getActiveContentState().readWireFormat(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; }, + _newContentStateFromWireFormat: function(map) { + if (map === null) { + return null; + } + + return new JX.DiffInlineContentState().readWireFormat(map); + }, + _ondeleteresponse: function() { // If there's an existing "unedit" undo element, remove it. if (this._undoRow) { @@ -787,7 +793,7 @@ }, _getActiveContentState: function() { - var state = this._activeContentState; + var state = this._state.active; if (this._editRow) { state.readForm(this._editRow); @@ -797,7 +803,11 @@ }, _getCommittedContentState: function() { - return this._committedContentState; + return this._state.committed; + }, + + _getInitialContentState: function() { + return this._state.initial; }, setHasSuggestion: function(has_suggestion) {