Page MenuHomePhabricator
Files F14399928

D21653.diff
No OneTemporary
Actions

D21653.diff
View Options

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) {

File Metadata

Mime Type
text/plain
Expires
Mon, Dec 23, 6:13 PM (18 h, 23 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6920076
Default Alt Text
D21653.diff (13 KB)

Event Timeline

Phabricator · Built by Phacility