1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-10-23 17:18:51 +02:00
phorge-phorge/webroot/rsrc/js/application/diff/DiffInline.js

756 lines
18 KiB
JavaScript
Raw Normal View History

/**
* @provides phabricator-diff-inline
* @requires javelin-dom
* @javelin
*/
JX.install('DiffInline', {
construct : function() {
},
members: {
_id: null,
_phid: null,
_changesetID: null,
_row: null,
_number: null,
_length: null,
_displaySide: null,
_isNewFile: null,
_replyToCommentPHID: null,
_originalText: null,
_snippet: null,
_isDeleted: false,
_isInvisible: false,
_isLoading: false,
_changeset: null,
_isCollapsed: false,
_isDraft: null,
_isDraftDone: null,
_isFixed: null,
_isEditing: false,
_isNew: false,
_isSynthetic: false,
_isHidden: false,
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
_editRow: null,
_undoRow: null,
_undoType: null,
_undoText: null,
bindToRow: function(row) {
this._row = row;
var row_data = JX.Stratcom.getData(row);
row_data.inline = this;
this._isCollapsed = row_data.hidden || false;
// TODO: Get smarter about this once we do more editing, this is pretty
// hacky.
var comment = JX.DOM.find(row, 'div', 'differential-inline-comment');
var data = JX.Stratcom.getData(comment);
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
this._readInlineState(data);
this._phid = data.phid;
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
if (data.on_right) {
this._displaySide = 'right';
} else {
this._displaySide = 'left';
}
this._number = parseInt(data.number, 10);
this._length = parseInt(data.length, 10);
this._originalText = data.original;
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
this._isNewFile = data.isNewFile;
this._replyToCommentPHID = data.replyToCommentPHID;
this._isDraft = data.isDraft;
this._isFixed = data.isFixed;
this._isGhost = data.isGhost;
this._isSynthetic = data.isSynthetic;
this._isDraftDone = data.isDraftDone;
this._changesetID = data.changesetID;
this._isNew = false;
this._snippet = data.snippet;
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
this._isEditing = data.isEditing;
if (this._isEditing) {
this.edit();
} else {
this.setInvisible(false);
}
return this;
},
isDraft: function() {
return this._isDraft;
},
isDone: function() {
return this._isFixed;
},
isEditing: function() {
return this._isEditing;
},
isDeleted: function() {
return this._isDeleted;
},
isSynthetic: function() {
return this._isSynthetic;
},
isDraftDone: function() {
return this._isDraftDone;
},
isHidden: function() {
return this._isHidden;
},
isGhost: function() {
return this._isGhost;
},
bindToRange: function(data) {
this._displaySide = data.displaySide;
this._number = parseInt(data.number, 10);
this._length = parseInt(data.length, 10);
this._isNewFile = data.isNewFile;
this._changesetID = data.changesetID;
this._isNew = true;
// Insert the comment after any other comments which already appear on
// the same row.
var parent_row = JX.DOM.findAbove(data.target, 'tr');
var target_row = parent_row.nextSibling;
while (target_row && JX.Stratcom.hasSigil(target_row, 'inline-row')) {
target_row = target_row.nextSibling;
}
var row = this._newRow();
parent_row.parentNode.insertBefore(row, target_row);
this.setInvisible(true);
return this;
},
bindToReply: function(inline) {
this._displaySide = inline._displaySide;
this._number = inline._number;
this._length = inline._length;
this._isNewFile = inline._isNewFile;
this._changesetID = inline._changesetID;
this._isNew = true;
this._replyToCommentPHID = inline._phid;
var changeset = this.getChangeset();
// We're going to figure out where in the document to position the new
// inline. Normally, it goes after any existing inline rows (so if
// several inlines reply to the same line, they appear in chronological
// order).
// However: if inlines are threaded, we want to put the new inline in
// the right place in the thread. This might be somewhere in the middle,
// so we need to do a bit more work to figure it out.
// To find the right place in the thread, we're going to look for any
// inline which is at or above the level of the comment we're replying
// to. This means we've reached a new fork of the thread, and should
// put our new inline before the comment we found.
var ancestor_map = {};
var ancestor = inline;
var reply_phid;
while (ancestor) {
reply_phid = ancestor.getReplyToCommentPHID();
if (!reply_phid) {
break;
}
ancestor_map[reply_phid] = true;
ancestor = changeset.getInlineByPHID(reply_phid);
}
var parent_row = inline._row;
var target_row = parent_row.nextSibling;
while (target_row && JX.Stratcom.hasSigil(target_row, 'inline-row')) {
var target = changeset.getInlineForRow(target_row);
reply_phid = target.getReplyToCommentPHID();
// If we found an inline which is replying directly to some ancestor
// of this new comment, this is where the new rows go.
if (ancestor_map.hasOwnProperty(reply_phid)) {
break;
}
target_row = target_row.nextSibling;
}
var row = this._newRow();
parent_row.parentNode.insertBefore(row, target_row);
this.setInvisible(true);
return this;
},
setChangeset: function(changeset) {
this._changeset = changeset;
return this;
},
getChangeset: function() {
return this._changeset;
},
setEditing: function(editing) {
this._isEditing = editing;
return this;
},
setHidden: function(hidden) {
this._isHidden = hidden;
this._redraw();
return this;
},
canReply: function() {
if (!this._hasAction('reply')) {
return false;
}
return true;
},
canEdit: function() {
if (!this._hasAction('edit')) {
return false;
}
return true;
},
canDone: function() {
if (!JX.DOM.scry(this._row, 'input', 'differential-inline-done').length) {
return false;
}
return true;
},
canCollapse: function() {
if (!JX.DOM.scry(this._row, 'a', 'hide-inline').length) {
return false;
}
return true;
},
getRawText: function() {
return this._originalText;
},
_hasAction: function(action) {
var nodes = JX.DOM.scry(this._row, 'a', 'differential-inline-' + action);
return (nodes.length > 0);
},
_newRow: function() {
var attributes = {
sigil: 'inline-row'
};
var row = JX.$N('tr', attributes);
JX.Stratcom.getData(row).inline = this;
this._row = row;
this._id = null;
this._phid = null;
this._isCollapsed = false;
this._originalText = null;
return row;
},
setCollapsed: function(collapsed) {
this._isCollapsed = collapsed;
var op;
if (collapsed) {
op = 'hide';
} else {
op = 'show';
}
var inline_uri = this._getInlineURI();
var comment_id = this._id;
new JX.Workflow(inline_uri, {op: op, ids: comment_id})
.setHandler(JX.bag)
.start();
this._redraw();
this._didUpdate(true);
},
isCollapsed: function() {
return this._isCollapsed;
},
toggleDone: function() {
var uri = this._getInlineURI();
var data = {
op: 'done',
id: this._id
};
var ondone = JX.bind(this, this._ondone);
new JX.Workflow(uri, data)
.setHandler(ondone)
.start();
},
_ondone: function(response) {
var checkbox = JX.DOM.find(
this._row,
'input',
'differential-inline-done');
checkbox.checked = (response.isChecked ? 'checked' : null);
var comment = JX.DOM.findAbove(
checkbox,
'div',
'differential-inline-comment');
JX.DOM.alterClass(comment, 'inline-is-done', response.isChecked);
// NOTE: This is marking the inline as having an unsubmitted checkmark,
// as opposed to a submitted checkmark. This is different from the
// top-level "draft" state of unsubmitted comments.
JX.DOM.alterClass(comment, 'inline-state-is-draft', response.draftState);
this._isFixed = response.isChecked;
this._isDraftDone = !!response.draftState;
this._didUpdate();
},
create: function(text) {
var uri = this._getInlineURI();
var handler = JX.bind(this, this._oncreateresponse);
var data = this._newRequestData('new', text);
this.setLoading(true);
new JX.Request(uri, handler)
.setData(data)
.send();
},
reply: function(text) {
var changeset = this.getChangeset();
return changeset.newInlineReply(this, text);
},
edit: function(text) {
var uri = this._getInlineURI();
var handler = JX.bind(this, this._oneditresponse);
var data = this._newRequestData('edit', text || null);
this.setLoading(true);
new JX.Request(uri, handler)
.setData(data)
.send();
},
delete: function(is_ref) {
var uri = this._getInlineURI();
var handler = JX.bind(this, this._ondeleteresponse);
// NOTE: This may be a direct delete (the user clicked on the inline
// itself) or a "refdelete" (the user clicked somewhere else, like the
// preview, but the inline is present on the page).
// For a "refdelete", we prompt the user to confirm that they want to
// delete the comment, because they can not undo deletions from the
// preview. We could jump the user to the inline instead, but this would
// be somewhat disruptive and make deleting several comments more
// difficult.
var op;
if (is_ref) {
op = 'refdelete';
} else {
op = 'delete';
}
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
// If there's an existing "unedit" undo element, remove it.
if (this._undoRow) {
JX.DOM.remove(this._undoRow);
this._undoRow = null;
}
var data = this._newRequestData(op);
this.setLoading(true);
new JX.Workflow(uri, data)
.setHandler(handler)
.start();
},
getDisplaySide: function() {
return this._displaySide;
},
getLineNumber: function() {
return this._number;
},
getLineLength: function() {
return this._length;
},
isNewFile: function() {
return this._isNewFile;
},
getID: function() {
return this._id;
},
getPHID: function() {
return this._phid;
},
getChangesetID: function() {
return this._changesetID;
},
getReplyToCommentPHID: function() {
return this._replyToCommentPHID;
},
setDeleted: function(deleted) {
this._isDeleted = deleted;
this._redraw();
return this;
},
setInvisible: function(invisible) {
this._isInvisible = invisible;
this._redraw();
return this;
},
setLoading: function(loading) {
this._isLoading = loading;
this._redraw();
return this;
},
_newRequestData: function(operation, text) {
return {
op: operation,
id: this._id,
on_right: ((this.getDisplaySide() == 'right') ? 1 : 0),
renderer: this.getChangeset().getRendererKey(),
number: this.getLineNumber(),
length: this.getLineLength(),
is_new: this.isNewFile(),
changesetID: this.getChangesetID(),
replyToCommentPHID: this.getReplyToCommentPHID() || '',
text: text || ''
};
},
_oneditresponse: function(response) {
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
var rows = JX.$H(response.view).getNode();
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
this._readInlineState(response.inline);
this._drawEditRows(rows);
this.setLoading(false);
this.setInvisible(true);
},
_oncreateresponse: function(response) {
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
var rows = JX.$H(response.view).getNode();
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
this._readInlineState(response.inline);
this._drawEditRows(rows);
},
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
_readInlineState: function(state) {
this._id = state.id;
},
_ondeleteresponse: function() {
this._drawUndeleteRows();
this.setLoading(false);
this.setDeleted(true);
this._didUpdate();
},
_drawUndeleteRows: function() {
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
this._undoType = 'undelete';
this._undoText = null;
return this._drawUndoRows('undelete', this._row);
},
_drawUneditRows: function(text) {
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
this._undoType = 'unedit';
this._undoText = text;
return this._drawUndoRows('unedit', null, text);
},
_drawUndoRows: function(mode, cursor, text) {
var templates = this.getChangeset().getUndoTemplates();
var template;
if (this.getDisplaySide() == 'right') {
template = templates.r;
} else {
template = templates.l;
}
template = JX.$H(template).getNode();
this._undoRow = this._drawRows(template, cursor, mode, text);
},
_drawContentRows: function(rows) {
return this._drawRows(rows, null, 'content');
},
_drawEditRows: function(rows) {
this.setEditing(true);
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
this._editRow = this._drawRows(rows, null, 'edit');
},
_drawRows: function(rows, cursor, type, text) {
var first_row = JX.DOM.scry(rows, 'tr')[0];
var row = first_row;
var anchor = cursor || this._row;
cursor = cursor || this._row.nextSibling;
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
var result_row;
var next_row;
while (row) {
// Grab this first, since it's going to change once we insert the row
// into the document.
next_row = row.nextSibling;
// Bind edit and undo rows to this DiffInline object so that
// interactions like hovering work properly.
JX.Stratcom.getData(row).inline = this;
anchor.parentNode.insertBefore(row, cursor);
cursor = row;
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
if (!result_row) {
result_row = row;
}
// If the row has a textarea, focus it. This allows the user to start
// typing a comment immediately after a "new", "edit", or "reply"
// action.
var textareas = JX.DOM.scry(
row,
'textarea',
'differential-inline-comment-edit-textarea');
if (textareas.length) {
var area = textareas[0];
area.focus();
var length = area.value.length;
JX.TextAreaUtils.setSelectionRange(area, length, length);
}
row = next_row;
}
JX.Stratcom.invoke('resize');
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
return result_row;
},
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
save: function(form) {
var handler = JX.bind(this, this._onsubmitresponse);
this.setLoading(true);
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
JX.Workflow.newFromForm(form)
.setHandler(handler)
.start();
},
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
undo: function() {
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
JX.DOM.remove(this._undoRow);
this._undoRow = null;
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
if (this._undoType === 'undelete') {
var uri = this._getInlineURI();
var data = this._newRequestData('undelete');
var handler = JX.bind(this, this._onundelete);
this.setDeleted(false);
this.setLoading(true);
new JX.Request(uri, handler)
.setData(data)
.send();
}
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
if (this._undoType === 'unedit') {
this.edit(this._undoText);
}
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
},
_onundelete: function() {
this.setLoading(false);
this._didUpdate();
},
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
cancel: function() {
var text = this._readText(this._editRow);
JX.DOM.remove(this._editRow);
this._editRow = null;
if (text && text.length && (text != this._originalText)) {
this._drawUneditRows(text);
}
this.setEditing(false);
this.setInvisible(false);
var uri = this._getInlineURI();
var data = this._newRequestData('cancel');
var handler = JX.bind(this, this._onCancelResponse);
this.setLoading(true);
new JX.Request(uri, handler)
.setData(data)
.send();
this._didUpdate(true);
},
_onCancelResponse: function(response) {
this.setLoading(false);
},
_readText: function(row) {
var textarea;
try {
textarea = JX.DOM.find(
row,
'textarea',
'differential-inline-comment-edit-textarea');
} catch (ex) {
return null;
}
return textarea.value;
},
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
_onsubmitresponse: function(response) {
if (this._editRow) {
JX.DOM.remove(this._editRow);
this._editRow = null;
}
this.setLoading(false);
this.setInvisible(false);
this.setEditing(false);
this._onupdate(response);
},
_onupdate: function(response) {
var new_row;
Make "editing" state persistent for inline comments Summary: Ref T13513. This is mostly an infrastructure cleanup change. In a perfect world, this would be a series of several changes, but they're tightly interconnected and don't have an obvious clean, nontrivial partition (or, at least, I don't see one). Followup changes will exercise this code repeatedly and all of these individual mutations are "obviously good", so I'm not too worried about the breadth of this change. --- Inline comments are stored as transaction comments in the `PhabricatorAuditTransactionComment` and `DifferentialTransactionComment` classes. On top of these two storage classes sit `PhabricatorAuditInlineComment` and `DifferentialInlineComment`. Historically, these were an indirection layer over significantly different storage classes, but nowadays both storage classes look pretty similar and most of the logic is actually the same. Prior to this change, these two classes were about 80% copy/pastes of one another. Part of the reason they're so copy/pastey is that they implement a parent `Interface`. They are the only classes which implement this interface, and the interface does not provide any correctness guarantees (the storage objects are not actually constrained by it). To simplify this: - Make `PhabricatorInlineCommentInterface` an abstract base class instead. - Lift as much code out of the `Audit` and `Differential` subclasses as possible. - Delete methods which no longer have callers, or have only trivial callers. --- Inline comments have two `View` rendering classes, `DetailView` and `EditView`. They share very little code. Partly, this is because `EditView` does not take an `$inline` object. Historically, it needed to be able to operate on inlines that did not have an ID yet, and even further back in history this was probably just an outgrowth of a simple `<form />`. These classes can be significantly simplified by passing an `$inline` to the `EditView`, instead of individually setting all the properties on the `View` itself. This allows the `DetailView` and `EditView` classes to share a lot of code. The `EditView` can not fully render its content. Move the content rendering code into the view. --- Prior to this change, some operations need to work on inlines that don't have an inline ID yet (we assign an ID the first time you "Save" a comment). Since "editing" comments will now be saved, we can instead create a row immediately. This means that all the inline code can always rely on having a valid ID to work with, even if that ID corresponds to an empty, draft, "isEditing" comment. This simplifies more code in `EditView` and allows the "create" and "reply" code to be merged in `PhabricatorInlineCommentController`. --- Client-side inline events are currently handled through a mixture of `ChangesetList` listeners (good) and ad-hoc row-level listeners (less good). In particular, the "save", "cancel", and "undo" events are row-level. All other events are list-level. Move all events to list-level. This is supported by all inlines now having an ID at all stages of their lifecycle. This allows some of the client behavior to be simplified. It currently depends on binding complex ad-hoc dictionaries into event handlers in `_drawRows()`, but it seems like almost all of this code can be removed. In fact, no more than one row ever seems to be drawn, so this code can probably be simplified further. --- Finally, save an "isEditing" state. When we rebuild a revision on the client, click the "edit" button if it's in this state. This is a little hacky, but simpler to get into a stable state, since the row layout of an inline depends on a "view row" followed by an "edit row". Test Plan: - Created comments on either side of a diff. - Edited a comment, reloaded, saw edit stick. - Saved comments, reloaded, saw save stick. - Edited a comment, typed text, cancelled, "unedited" to get state back. - Created a comment, typed text, cancelled, "unedited" to get state back. - Deleted a comment, "undeleted" to get state back. Weirdness / known issues: - Drafts don't autosave yet. - Fixed in D21187: - When you create an empty comment then reload, you get an empty editor. This is a bit silly. - "Cancel" does not save state, but should, once drafts autosave. - Mostly fixed in D21188: - "Editing" comments aren't handled specially by the overall submission flow. - "Editing" comments submitted in that state try to edit themselves again on load, which doesn't work. Subscribers: jmeador Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21186
2020-04-28 23:45:54 +02:00
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;
}
this._didUpdate();
},
_didUpdate: function(local_only) {
// After making changes to inline comments, refresh the transaction
// preview at the bottom of the page.
if (!local_only) {
this.getChangeset().getChangesetList().redrawPreview();
}
this.getChangeset().getChangesetList().redrawCursor();
this.getChangeset().getChangesetList().resetHover();
// Emit a resize event so that UI elements like the keyboard focus
// reticle can redraw properly.
JX.Stratcom.invoke('resize');
},
_redraw: function() {
var is_invisible =
(this._isInvisible || this._isDeleted || this._isHidden);
var is_loading = this._isLoading;
var is_collapsed = (this._isCollapsed && !this._isHidden);
var row = this._row;
JX.DOM.alterClass(row, 'differential-inline-hidden', is_invisible);
JX.DOM.alterClass(row, 'differential-inline-loading', is_loading);
JX.DOM.alterClass(row, 'inline-hidden', is_collapsed);
},
_getInlineURI: function() {
var changeset = this.getChangeset();
var list = changeset.getChangesetList();
return list.getInlineURI();
}
}
});