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' => '845355f4', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => '42a2334f', - 'differential.pkg.js' => 'ff8ca085', + 'differential.pkg.js' => 'd0ddfb19', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -381,7 +381,7 @@ 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', 'rsrc/js/application/diff/DiffChangeset.js' => '6e5e03d2', 'rsrc/js/application/diff/DiffChangesetList.js' => 'b51ba93a', - 'rsrc/js/application/diff/DiffInline.js' => '28e53a2c', + 'rsrc/js/application/diff/DiffInline.js' => '6fa445ef', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', @@ -776,7 +776,7 @@ 'phabricator-dashboard-css' => '5a205b9d', 'phabricator-diff-changeset' => '6e5e03d2', 'phabricator-diff-changeset-list' => 'b51ba93a', - 'phabricator-diff-inline' => '28e53a2c', + 'phabricator-diff-inline' => '6fa445ef', 'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-tree-view' => '5d83623b', 'phabricator-drag-and-drop-file-upload' => '4370900d', @@ -1137,9 +1137,6 @@ 'javelin-install', 'javelin-util', ), - '28e53a2c' => array( - 'javelin-dom', - ), '29819b75' => array( 'phabricator-notification', 'javelin-stratcom', @@ -1564,6 +1561,9 @@ 'phabricator-diff-path-view', 'phuix-button-view', ), + '6fa445ef' => array( + 'javelin-dom', + ), 70245195 => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3160,6 +3160,7 @@ 'PhabricatorDestructionEngineExtensionModule' => 'applications/system/engine/PhabricatorDestructionEngineExtensionModule.php', 'PhabricatorDeveloperConfigOptions' => 'applications/config/option/PhabricatorDeveloperConfigOptions.php', 'PhabricatorDeveloperPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDeveloperPreferencesSettingsPanel.php', + 'PhabricatorDiffInlineCommentContentState' => 'infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php', 'PhabricatorDiffInlineCommentQuery' => 'infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php', 'PhabricatorDiffPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php', 'PhabricatorDiffScopeEngine' => 'infrastructure/diff/PhabricatorDiffScopeEngine.php', @@ -3592,6 +3593,7 @@ 'PhabricatorInfrastructureTestCase' => '__tests__/PhabricatorInfrastructureTestCase.php', 'PhabricatorInlineComment' => 'infrastructure/diff/interface/PhabricatorInlineComment.php', 'PhabricatorInlineCommentAdjustmentEngine' => 'infrastructure/diff/engine/PhabricatorInlineCommentAdjustmentEngine.php', + 'PhabricatorInlineCommentContentState' => 'infrastructure/diff/inline/PhabricatorInlineCommentContentState.php', 'PhabricatorInlineCommentController' => 'infrastructure/diff/PhabricatorInlineCommentController.php', 'PhabricatorInlineCommentInterface' => 'applications/transactions/interface/PhabricatorInlineCommentInterface.php', 'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php', @@ -9627,6 +9629,7 @@ 'PhabricatorDestructionEngineExtensionModule' => 'PhabricatorConfigModule', 'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorDeveloperPreferencesSettingsPanel' => 'PhabricatorEditEngineSettingsPanel', + 'PhabricatorDiffInlineCommentContentState' => 'PhabricatorInlineCommentContentState', 'PhabricatorDiffInlineCommentQuery' => 'PhabricatorApplicationTransactionCommentQuery', 'PhabricatorDiffPreferencesSettingsPanel' => 'PhabricatorEditEngineSettingsPanel', 'PhabricatorDiffScopeEngine' => 'Phobject', @@ -10118,6 +10121,7 @@ 'PhabricatorMarkupInterface', ), 'PhabricatorInlineCommentAdjustmentEngine' => 'Phobject', + 'PhabricatorInlineCommentContentState' => 'Phobject', 'PhabricatorInlineCommentController' => 'PhabricatorController', 'PhabricatorInlineSummaryView' => 'AphrontView', 'PhabricatorInstructionsEditField' => 'PhabricatorEditField', 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 @@ -52,10 +52,6 @@ return $this->operation; } - public function getCommentText() { - return $this->commentText; - } - public function getLineLength() { return $this->lineLength; } @@ -270,10 +266,8 @@ $viewer->getPHID(), $inline->getID()); - $map = $this->getContentState(); - foreach ($map as $key => $value) { - $versioned_draft->setProperty($key, $value); - } + $map = $this->newRequestContentState($inline)->newStorageMap(); + $versioned_draft->setProperty('inline.state', $map); $versioned_draft->save(); // We have to synchronize the draft engine after saving a versioned @@ -524,14 +518,9 @@ return (bool)$request->getBool('hasContentState'); } - private function getContentState() { + private function newRequestContentState($inline) { $request = $this->getRequest(); - - $comment_text = $request->getStr('text'); - - return array( - 'inline.text' => (string)$comment_text, - ); + return $inline->newContentStateFromRequest($request); } private function updateCommentContentState(PhabricatorInlineComment $inline) { @@ -542,11 +531,8 @@ 'content state.')); } - $state = $this->getContentState(); - - $text = $state['inline.text']; - - $inline->setContent($text); + $state = $this->newRequestContentState($inline); + $inline->setContentState($state); } private function saveComment(PhabricatorInlineComment $inline) { diff --git a/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php b/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php new file mode 100644 --- /dev/null +++ b/src/infrastructure/diff/inline/PhabricatorDiffInlineCommentContentState.php @@ -0,0 +1,69 @@ +getContentHasSuggestion()) { + if (strlen($this->getSuggestionText())) { + return false; + } + } + + return true; + } + + public function setContentSuggestionText($suggestion_text) { + $this->suggestionText = $suggestion_text; + return $this; + } + + public function getContentSuggestionText() { + return $this->suggestionText; + } + + public function setContentHasSuggestion($has_suggestion) { + $this->hasSuggestion = $has_suggestion; + return $this; + } + + public function getContentHasSuggestion() { + return $this->hasSuggestion; + } + + public function newStorageMap() { + return parent::writeStorageMap() + array( + 'hasSuggestion' => $this->getContentHasSuggestion(), + 'suggestionText' => $this->getContentSuggestionText(), + ); + } + + public function readStorageMap(array $map) { + $result = parent::readStorageMap($map); + + $has_suggestion = (bool)idx($map, 'hasSuggestion'); + $this->setContentHasSuggestion($has_suggestion); + + $suggestion_text = (string)idx($map, 'suggestionText'); + $this->setContentSuggestionText($suggestion_text); + + return $result; + } + + protected function newStorageMapFromRequest(AphrontRequest $request) { + $map = parent::newStorageMapFromRequest($request); + + $map['hasSuggestion'] = (bool)$request->getBool('hasSuggestion'); + $map['suggestionText'] = (string)$request->getStr('suggestionText'); + + return $map; + } + +} diff --git a/src/infrastructure/diff/inline/PhabricatorInlineCommentContentState.php b/src/infrastructure/diff/inline/PhabricatorInlineCommentContentState.php new file mode 100644 --- /dev/null +++ b/src/infrastructure/diff/inline/PhabricatorInlineCommentContentState.php @@ -0,0 +1,47 @@ +contentText = $content_text; + return $this; + } + + public function getContentText() { + return $this->contentText; + } + + public function isEmptyContentState() { + return !strlen($this->getContentText()); + } + + public function writeStorageMap() { + return array( + 'text' => $this->getContentText(), + ); + } + + public function readStorageMap(array $map) { + $text = (string)idx($map, 'text'); + $this->setContentText($text); + + return $this; + } + + final public function readFromRequest(AphrontRequest $request) { + $map = $this->newStorageMapFromRequest($request); + return $this->readStorageMap($map); + } + + protected function newStorageMapFromRequest(AphrontRequest $request) { + $map = array(); + + $map['text'] = (string)$request->getStr('text'); + + return $map; + } + +} 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 @@ -299,35 +299,58 @@ } public function isVoidComment(PhabricatorUser $viewer) { - return !strlen($this->getContentForEdit($viewer)); + return $this->getContentStateForEdit($viewer)->isEmptyContentState(); } - public function getContentForEdit(PhabricatorUser $viewer) { - $content = $this->getContent(); + public function getContentStateForEdit(PhabricatorUser $viewer) { + $state = $this->getContentState(); - if (!$this->hasVersionedDraftForViewer($viewer)) { - return $content; + if ($this->hasVersionedDraftForViewer($viewer)) { + $versioned_draft = $this->getVersionedDraftForViewer($viewer); + if ($versioned_draft) { + $storage_map = $versioned_draft->getProperty('inline.state'); + if (is_array($storage_map)) { + $state->readStorageMap($storage_map); + } + } } - $versioned_draft = $this->getVersionedDraftForViewer($viewer); - if (!$versioned_draft) { - return $content; - } + return $state; + } - $draft_text = $versioned_draft->getProperty('inline.text'); - if ($draft_text === null) { - return $content; - } + protected function newContentState() { + return new PhabricatorDiffInlineCommentContentState(); + } - return $draft_text; + public function newContentStateFromRequest(AphrontRequest $request) { + return $this->newContentState()->readFromRequest($request); } public function getContentState() { - return array( - 'text' => $this->getContent(), - ); + $state = $this->newContentState(); + + $storage = $this->getStorageObject(); + $storage_map = $storage->getAttribute('inline.state'); + if (is_array($storage_map)) { + $state->readStorageMap($storage_map); + } + + $state->setContentText($this->getContent()); + + return $state; } + public function setContentState(PhabricatorInlineCommentContentState $state) { + $storage = $this->getStorageObject(); + $storage_map = $state->newStorageMap(); + $storage->setAttribute('inline.state', $storage_map); + + $this->setContent($state->getContentText()); + + return $this; + } + + /* -( PhabricatorMarkupInterface Implementation )-------------------------- */ diff --git a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php --- a/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php +++ b/src/infrastructure/diff/query/PhabricatorDiffInlineCommentQuery.php @@ -218,9 +218,9 @@ // as it is currently shown to the user, not as it was stored the last // time they clicked "Save". - $draft_content = $inline->getContentForEdit($viewer); - if (strlen($draft_content)) { - $inline->setContent($draft_content); + $draft_state = $inline->getContentStateForEdit($viewer); + if (!$draft_state->isEmptyContentState()) { + $inline->setContentState($draft_state); } } } diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php @@ -101,7 +101,6 @@ $classes[] = 'inline-comment-element'; } - $content = $inline->getContent(); $handles = $this->handles; $links = array(); diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php @@ -82,13 +82,12 @@ $viewer = $this->getViewer(); $inline = $this->getInlineComment(); - $text = $inline->getContentForEdit($viewer); + $state = $inline->getContentStateForEdit($viewer); return id(new PhabricatorRemarkupControl()) ->setViewer($viewer) - ->setSigil('differential-inline-comment-edit-textarea') - ->setName('text') - ->setValue($text) + ->setSigil('inline-content-text') + ->setValue($state->getContentText()) ->setDisableFullScreen(true); } 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 @@ -701,7 +701,7 @@ var textareas = JX.DOM.scry( row, 'textarea', - 'differential-inline-comment-edit-textarea'); + 'inline-content-text'); if (textareas.length) { var area = textareas[0]; area.focus(); @@ -814,19 +814,25 @@ }, _readFormState: function(row) { - var textarea; + var state = this._newContentState(); + + var node; + try { - textarea = JX.DOM.find( - row, - 'textarea', - 'differential-inline-comment-edit-textarea'); + node = JX.DOM.find(row, 'textarea', 'inline-content-text'); + state.text = node.value; } catch (ex) { - return null; + // Ignore. } - return { - text: textarea.value - }; + try { + node = JX.DOM.find(row, 'textarea', 'inline-content-suggestion'); + state.suggestionText = node.value; + } catch (ex) { + // Ignore. + } + + return state; }, _onsubmitresponse: function(response) { @@ -1053,20 +1059,24 @@ } }, - _isVoidContentState: function(state) { - return !state.text.length; + _newContentState: function() { + return { + text: '', + suggestionText: '', + hasSuggestion: true + }; }, - _isSameContentState: function(u, v) { - return (u.text === v.text); + _isVoidContentState: function(state) { + return (!state.text.length && !state.suggestionText.length); }, - _newContentState: function() { - return { - text: '' - }; + _isSameContentState: function(u, v) { + return ( + (u.text === v.text) && + (u.suggestionText === v.suggestionText) && + (u.hasSuggestion === v.hasSuggestion)); } - } });