Skip to content

Commit 5efe7fb

Browse files
author
epriestley
committedMar 29, 2021
On inline comments, track an explicit "committed" content state
Summary: Ref T13559. To allow the client to make correct decisions about what buttons mean, track an explicit "Committed" content state. This is the last version of the comment that has been saved on the server, and does not exist if the comment has never been saved. Test Plan: Created comments, etc. See followups. Maniphest Tasks: T13559 Differential Revision: https://secure.phabricator.com/D21651
1 parent 428fff2 commit 5efe7fb

File tree

3 files changed

+42
-24
lines changed

3 files changed

+42
-24
lines changed
 

‎resources/celerity/map.php

+15-15
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010
'conpherence.pkg.css' => '0e3cf785',
1111
'conpherence.pkg.js' => '020aebcf',
1212
'core.pkg.css' => '0ae696de',
13-
'core.pkg.js' => 'ab3502fe',
13+
'core.pkg.js' => '68f29322',
1414
'dark-console.pkg.js' => '187792c2',
1515
'differential.pkg.css' => 'ffb69e3d',
16-
'differential.pkg.js' => 'fdec5f60',
16+
'differential.pkg.js' => 'bbf6d742',
1717
'diffusion.pkg.css' => '42c75c37',
1818
'diffusion.pkg.js' => '78c9885d',
1919
'maniphest.pkg.css' => '35995d6d',
@@ -253,7 +253,7 @@
253253
'rsrc/externals/javelin/lib/Mask.js' => '7c4d8998',
254254
'rsrc/externals/javelin/lib/Quicksand.js' => 'd3799cb4',
255255
'rsrc/externals/javelin/lib/Request.js' => '84e6891f',
256-
'rsrc/externals/javelin/lib/Resource.js' => '740956e1',
256+
'rsrc/externals/javelin/lib/Resource.js' => '20514cc2',
257257
'rsrc/externals/javelin/lib/Routable.js' => '6a18c42e',
258258
'rsrc/externals/javelin/lib/Router.js' => '32755edb',
259259
'rsrc/externals/javelin/lib/Scrollbar.js' => 'a43ae2ae',
@@ -385,7 +385,7 @@
385385
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
386386
'rsrc/js/application/diff/DiffChangeset.js' => 'd7d3ba75',
387387
'rsrc/js/application/diff/DiffChangesetList.js' => 'cc2c5de5',
388-
'rsrc/js/application/diff/DiffInline.js' => '2a6fac17',
388+
'rsrc/js/application/diff/DiffInline.js' => 'c794b624',
389389
'rsrc/js/application/diff/DiffInlineContentState.js' => '68e6339d',
390390
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
391391
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
@@ -734,7 +734,7 @@
734734
'javelin-reactor-node-calmer' => '225bbb98',
735735
'javelin-reactornode' => '72960bc1',
736736
'javelin-request' => '84e6891f',
737-
'javelin-resource' => '740956e1',
737+
'javelin-resource' => '20514cc2',
738738
'javelin-routable' => '6a18c42e',
739739
'javelin-router' => '32755edb',
740740
'javelin-scrollbar' => 'a43ae2ae',
@@ -788,7 +788,7 @@
788788
'phabricator-dashboard-css' => '5a205b9d',
789789
'phabricator-diff-changeset' => 'd7d3ba75',
790790
'phabricator-diff-changeset-list' => 'cc2c5de5',
791-
'phabricator-diff-inline' => '2a6fac17',
791+
'phabricator-diff-inline' => 'c794b624',
792792
'phabricator-diff-inline-content-state' => '68e6339d',
793793
'phabricator-diff-path-view' => '8207abf9',
794794
'phabricator-diff-tree-view' => '5d83623b',
@@ -1106,6 +1106,11 @@
11061106
'javelin-behavior',
11071107
'javelin-request',
11081108
),
1109+
'20514cc2' => array(
1110+
'javelin-util',
1111+
'javelin-uri',
1112+
'javelin-install',
1113+
),
11091114
'225bbb98' => array(
11101115
'javelin-install',
11111116
'javelin-reactor',
@@ -1166,10 +1171,6 @@
11661171
'javelin-stratcom',
11671172
'javelin-behavior',
11681173
),
1169-
'2a6fac17' => array(
1170-
'javelin-dom',
1171-
'phabricator-diff-inline-content-state',
1172-
),
11731174
'2a8b62d9' => array(
11741175
'multirow-row-manager',
11751176
'javelin-install',
@@ -1597,11 +1598,6 @@
15971598
'javelin-stratcom',
15981599
'phabricator-tooltip',
15991600
),
1600-
'740956e1' => array(
1601-
'javelin-util',
1602-
'javelin-uri',
1603-
'javelin-install',
1604-
),
16051601
74446546 => array(
16061602
'javelin-behavior',
16071603
'javelin-dom',
@@ -2092,6 +2088,10 @@
20922088
'javelin-workflow',
20932089
'javelin-json',
20942090
),
2091+
'c794b624' => array(
2092+
'javelin-dom',
2093+
'phabricator-diff-inline-content-state',
2094+
),
20952095
'cc2c5de5' => array(
20962096
'javelin-install',
20972097
'phuix-button-view',

‎webroot/rsrc/externals/javelin/lib/Resource.js

+4
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ JX.install('Resource', {
149149
}
150150
}
151151

152+
for (var jj = 0; jj < errors.length; jj++) {
153+
JX.log(errors[jj]);
154+
}
155+
152156
if (errors.length) {
153157
throw errors[0];
154158
}

‎webroot/rsrc/js/application/diff/DiffInline.js

+23-9
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ JX.install('DiffInline', {
99

1010
construct : function() {
1111
this._activeContentState = new JX.DiffInlineContentState();
12+
this._committedContentState = new JX.DiffInlineContentState();
1213
},
1314

1415
members: {
@@ -21,7 +22,6 @@ JX.install('DiffInline', {
2122
_displaySide: null,
2223
_isNewFile: null,
2324
_replyToCommentPHID: null,
24-
_originalState: null,
2525
_snippet: null,
2626
_menuItems: null,
2727
_documentEngineKey: null,
@@ -56,6 +56,7 @@ JX.install('DiffInline', {
5656
_isSelected: false,
5757
_canSuggestEdit: false,
5858

59+
_committedContentState: null,
5960
_activeContentState: null,
6061

6162
bindToRow: function(row) {
@@ -336,8 +337,6 @@ JX.install('DiffInline', {
336337
this._phid = null;
337338
this._isCollapsed = false;
338339

339-
this._originalState = null;
340-
341340
return row;
342341
},
343342

@@ -602,7 +601,10 @@ JX.install('DiffInline', {
602601

603602
_readInlineState: function(state) {
604603
this._id = state.id;
605-
this._originalState = state.contentState;
604+
605+
// TODO: This is not the correct content state after a reload: it is
606+
// the draft state.
607+
this._getCommittedContentState().readWireFormat(state.contentState);
606608

607609
this._getActiveContentState().readWireFormat(state.contentState);
608610

@@ -673,7 +675,10 @@ JX.install('DiffInline', {
673675
this._editRow = this._drawRows(rows, null, 'edit');
674676

675677
this._drawSuggestionState(this._editRow);
676-
this.setHasSuggestion(this._originalState.hasSuggestion);
678+
679+
// TODO: We're just doing this for the rendering side effect of drawing
680+
// the button text.
681+
this.setHasSuggestion(this.getHasSuggestion());
677682
},
678683

679684
_drawRows: function(rows, cursor, type) {
@@ -800,6 +805,10 @@ JX.install('DiffInline', {
800805
return state;
801806
},
802807

808+
_getCommittedContentState: function() {
809+
return this._committedContentState;
810+
},
811+
803812
setHasSuggestion: function(has_suggestion) {
804813
var state = this._getActiveContentState();
805814
state.setHasSuggestion(has_suggestion);
@@ -853,12 +862,13 @@ JX.install('DiffInline', {
853862
},
854863

855864
_shouldUndoOnCancel: function() {
856-
var state = this._getActiveContentState().getWireFormat();
865+
var new_state = this._getActiveContentState().getWireFormat();
866+
var old_state = this._getCommittedContentState().getWireFormat();
857867

858868
// TODO: This is also simplified.
859869

860-
var is_empty = this._isVoidContentState(state);
861-
var is_same = this._isSameContentState(state, this._originalState);
870+
var is_empty = this._isVoidContentState(new_state);
871+
var is_same = this._isSameContentState(new_state, old_state);
862872

863873
if (!is_empty && !is_same) {
864874
return true;
@@ -956,7 +966,8 @@ JX.install('DiffInline', {
956966
this.setEditing(false);
957967
this.setInvisible(false);
958968

959-
this._applyCancel(this._originalState);
969+
var old_state = this._getCommittedContentState();
970+
this._applyCancel(old_state.getWireFormat());
960971

961972
this._didUpdate(true);
962973
},
@@ -1184,6 +1195,9 @@ JX.install('DiffInline', {
11841195
},
11851196

11861197
_isVoidContentState: function(state) {
1198+
if (!state.text) {
1199+
return true;
1200+
}
11871201
return (!state.text.length && !state.suggestionText.length);
11881202
},
11891203

0 commit comments

Comments
 (0)
Failed to load comments.