diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -13,7 +13,7 @@ 'core.pkg.js' => '2ff7879f', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '58712637', - 'differential.pkg.js' => 'e01579c4', + 'differential.pkg.js' => '271a1e1e', 'diffusion.pkg.css' => 'b93d9b8c', 'diffusion.pkg.js' => '84c8f8fd', 'favicon.ico' => '30672e08', @@ -390,15 +390,15 @@ 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '408bf173', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', - 'rsrc/js/application/diff/DiffChangeset.js' => '80ac3298', - 'rsrc/js/application/diff/DiffChangesetList.js' => '12575699', - 'rsrc/js/application/diff/DiffInline.js' => 'bd1b3258', + 'rsrc/js/application/diff/DiffChangeset.js' => '57b29223', + 'rsrc/js/application/diff/DiffChangesetList.js' => '50bc5b50', + 'rsrc/js/application/diff/DiffInline.js' => '64dfc791', 'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832', 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '9ed8d2b6', 'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d', 'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', - 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '97f363fc', + 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '9d9dbc38', 'rsrc/js/application/differential/behavior-keyboard-nav.js' => '92904457', 'rsrc/js/application/differential/behavior-populate.js' => '5e41c819', 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', @@ -624,7 +624,7 @@ 'javelin-behavior-diff-preview-link' => '051c7832', 'javelin-behavior-differential-comment-jump' => '4fdb476d', 'javelin-behavior-differential-diff-radios' => 'e1ff79b1', - 'javelin-behavior-differential-edit-inline-comments' => '97f363fc', + 'javelin-behavior-differential-edit-inline-comments' => '9d9dbc38', 'javelin-behavior-differential-feedback-preview' => 'b064af76', 'javelin-behavior-differential-keyboard-navigation' => '92904457', 'javelin-behavior-differential-populate' => '5e41c819', @@ -785,9 +785,9 @@ 'phabricator-darklog' => 'c8e1ffe3', 'phabricator-darkmessage' => 'c48cccdd', 'phabricator-dashboard-css' => 'fe5b1869', - 'phabricator-diff-changeset' => '80ac3298', - 'phabricator-diff-changeset-list' => '12575699', - 'phabricator-diff-inline' => 'bd1b3258', + 'phabricator-diff-changeset' => '57b29223', + 'phabricator-diff-changeset-list' => '50bc5b50', + 'phabricator-diff-inline' => '64dfc791', 'phabricator-drag-and-drop-file-upload' => '58dea2fa', 'phabricator-draggable-list' => 'bea6e7f4', 'phabricator-fatal-config-template-css' => '8f18fa41', @@ -1000,9 +1000,6 @@ 'javelin-dom', 'javelin-typeahead-normalizer', ), - 12575699 => array( - 'javelin-install', - ), '1499a8cb' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1301,6 +1298,9 @@ 'javelin-typeahead-source', 'javelin-util', ), + '50bc5b50' => array( + 'javelin-install', + ), '519705ea' => array( 'javelin-install', 'javelin-dom', @@ -1342,6 +1342,17 @@ 'javelin-vector', 'javelin-dom', ), + '57b29223' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + ), '58dea2fa' => array( 'javelin-install', 'javelin-util', @@ -1411,6 +1422,9 @@ 'javelin-workflow', 'javelin-dom', ), + '64dfc791' => array( + 'javelin-dom', + ), '680ea2c8' => array( 'javelin-install', 'javelin-dom', @@ -1530,17 +1544,6 @@ 'javelin-vector', 'javelin-stratcom', ), - '80ac3298' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - ), '834a1173' => array( 'javelin-behavior', 'javelin-scrollbar', @@ -1667,14 +1670,6 @@ 'javelin-mask', 'phabricator-drag-and-drop-file-upload', ), - '97f363fc' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-util', - 'javelin-vector', - 'differential-inline-comment-editor', - ), '988040b4' => array( 'javelin-install', 'javelin-dom', @@ -1703,6 +1698,14 @@ '9d9685d6' => array( 'phui-oi-list-view-css', ), + '9d9dbc38' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-util', + 'javelin-vector', + 'differential-inline-comment-editor', + ), '9ed8d2b6' => array( 'javelin-dom', 'javelin-util', @@ -1910,9 +1913,6 @@ 'javelin-vector', 'phui-hovercard', ), - 'bd1b3258' => array( - 'javelin-dom', - ), 'bdaf4d04' => array( 'javelin-behavior', 'javelin-dom', diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -322,7 +322,6 @@ return JX.Stratcom.getData(this._node); }, - _onresponse: function(sequence, response) { if (sequence != this._sequence) { // If this isn't the most recent request, ignore it. This normally @@ -420,8 +419,25 @@ } return data.inline; + }, + + getInlineByID: function(id) { + // TODO: Currently, this will only find inlines which the user has + // already interacted with! Inlines are built lazily as events arrive. + // This can not yet find inlines which are passively present in the + // document. + + for (var ii = 0; ii < this._inlines.length; ii++) { + var inline = this._inlines[ii]; + if (inline.getID() == id) { + return inline; + } + } + + return null; } + }, statics: { diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -35,6 +35,12 @@ 'click', ['differential-inline-comment', 'differential-inline-done'], ondone); + + var ondelete = JX.bind(this, this._ifawake, this._onaction, 'delete'); + JX.Stratcom.listen( + 'click', + ['differential-inline-comment', 'differential-inline-delete'], + ondelete); }, properties: { @@ -71,6 +77,19 @@ return JX.DiffChangeset.getForNode(node); }, + getInlineByID: function(id) { + var inline = null; + + for (var ii = 0; ii < this._changesets.length; ii++) { + inline = this._changesets[ii].getInlineByID(id); + if (inline) { + break; + } + } + + return inline; + }, + _ifawake: function(f) { // This function takes another function and only calls it if the // changeset list is awake, so we basically just ignore events when we @@ -351,6 +370,33 @@ e.prevent(); var inline = this._getInlineForEvent(e); + var is_ref = false; + + // If we don't have a natural inline object, the user may have clicked + // an action (like "Delete") inside a preview element at the bottom of + // the page. + + // If they did, try to find an associated normal inline to act on, and + // pretend they clicked that instead. This makes the overall state of + // the page more consistent. + + // However, there may be no normal inline (for example, because it is + // on a version of the diff which is not visible). In this case, we + // act by reference. + + if (inline === null) { + var data = e.getNodeData('differential-inline-comment'); + inline = this.getInlineByID(data.id); + if (inline) { + is_ref = true; + } else { + switch (action) { + case 'delete': + this._deleteInlineByID(data.id); + return; + } + } + } // TODO: For normal operations, highlight the inline range here. @@ -361,11 +407,41 @@ case 'done': inline.toggleDone(); break; + case 'delete': + inline.delete(is_ref); + break; + } + }, + + redrawPreview: function() { + // TODO: This isn't the cleanest way to find the preview form, but + // rendering no longer has direct access to it. + var forms = JX.DOM.scry(document.body, 'form', 'transaction-append'); + if (forms.length) { + JX.DOM.invoke(forms[0], 'shouldRefresh'); } }, + _deleteInlineByID: function(id) { + var uri = this.getInlineURI(); + var data = { + op: 'refdelete', + id: id + }; + + var handler = JX.bind(this, this.redrawPreview); + + new JX.Workflow(uri, data) + .setHandler(handler) + .start(); + }, + _getInlineForEvent: function(e) { var node = e.getNode('differential-changeset'); + if (!node) { + return null; + } + var changeset = this.getChangesetForNode(node); var inline_row = e.getNode('inline-row'); diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js --- a/webroot/rsrc/js/application/diff/DiffInline.js +++ b/webroot/rsrc/js/application/diff/DiffInline.js @@ -47,6 +47,11 @@ _length: null, _displaySide: null, _isNewFile: null, + _undoRow: null, + + _isDeleted: false, + _isInvisible: false, + _isLoading: false, setHidden: function(hidden) { this._hidden = hidden; @@ -106,17 +111,47 @@ }, edit: function() { - var handler = JX.bind(this, this._oneditresponse); var uri = this._getInlineURI(); - var data = this._newRequestData(); + var handler = JX.bind(this, this._oneditresponse); + var data = this._newRequestData('edit'); - // TODO: Set state to "loading". + 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'; + } + + var data = this._newRequestData(op); + + this.setLoading(true); + + new JX.Workflow(uri, data) + .setHandler(handler) + .start(); + }, + getDisplaySide: function() { return this._displaySide; }, @@ -133,9 +168,31 @@ return this._isNewFile; }, - _newRequestData: function() { + getID: function() { + return this._id; + }, + + 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) { return { - op: 'edit', + op: operation, id: this._id, on_right: ((this.getDisplaySide() == 'right') ? 1 : 0), renderer: this.getChangeset().getRenderer(), @@ -151,42 +208,89 @@ this._drawEditRows(rows); - // TODO: Set the row state to "hidden". + this.setLoading(false); + this.setInvisible(true); + }, + + _ondeleteresponse: function() { + this._drawUndoRows(); + + this.setLoading(false); + this.setDeleted(true); + + this._didUpdate(); + }, + + _drawUndoRows: function() { + 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, this._row, 'undo'); }, _drawEditRows: function(rows) { + return this._drawRows(rows, null, 'edit'); + }, + + _drawRows: function(rows, cursor, type) { var first_row = JX.DOM.scry(rows, 'tr')[0]; + var first_meta; var row = first_row; - var cursor = this._row; + cursor = cursor || this._row.nextSibling; + 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; + cursor.parentNode.insertBefore(row, cursor.nextSibling); cursor = row; var row_meta = { node: row, - type: 'edit', + type: type, listeners: [] }; - row_meta.listeners.push( - JX.DOM.listen( - row, - ['submit', 'didSyntheticSubmit'], - 'inline-edit-form', - JX.bind(this, this._onsubmit, row_meta))); - - row_meta.listeners.push( - JX.DOM.listen( - row, - 'click', - 'inline-edit-cancel', - JX.bind(this, this._oncancel, row_meta))); - - row = row.nextSibling; + if (!first_meta) { + first_meta = row_meta; + } + + if (type == 'edit') { + row_meta.listeners.push( + JX.DOM.listen( + row, + ['submit', 'didSyntheticSubmit'], + 'inline-edit-form', + JX.bind(this, this._onsubmit, row_meta))); + + row_meta.listeners.push( + JX.DOM.listen( + row, + 'click', + 'inline-edit-cancel', + JX.bind(this, this._oncancel, row_meta))); + } else { + row_meta.listeners.push( + JX.DOM.listen( + row, + 'click', + 'differential-inline-comment-undo', + JX.bind(this, this._onundo, row_meta))); + } + + row = next_row; } - return first_row; + return first_meta; }, _onsubmit: function(row, e) { @@ -194,11 +298,33 @@ var handler = JX.bind(this, this._onsubmitresponse, row); + this.setLoading(true); + JX.Workflow.newFromForm(e.getTarget()) .setHandler(handler) .start(); + }, + + _onundo: function(row, e) { + e.kill(); + + this._removeRow(row); - // TODO: Set state to "loading". + 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(); + }, + + _onundelete: function() { + this.setLoading(false); + this._didUpdate(); }, _oncancel: function(row, e) { @@ -206,18 +332,15 @@ // TODO: Capture edited text and offer "undo". - JX.DOM.remove(row.node); - this._removeListeners(row.listeners); + this._removeRow(row); - // TODO: Restore state to "normal". + this.setInvisible(false); }, _onsubmitresponse: function(row, response) { + this._removeRow(row); - JX.DOM.remove(row.node); - this._removeListeners(row.listeners); - - // TODO: Restore state to "normal". + this.setInvisible(false); this._onupdate(response); }, @@ -225,7 +348,7 @@ _onupdate: function(response) { var new_row; if (response.markup) { - new_row = this._drawEditRows(JX.$H(response.markup).getNode()); + new_row = this._drawEditRows(JX.$H(response.markup).getNode()).node; } // TODO: Save the old row so the action it's undo-able if it was a @@ -243,18 +366,22 @@ _didUpdate: function() { // After making changes to inline comments, refresh the transaction // preview at the bottom of the page. + this.getChangeset().getChangesetList().redrawPreview(); + }, - // TODO: This isn't the cleanest way to find the preview form, but - // rendering no longer has direct access to it. - var forms = JX.DOM.scry(document.body, 'form', 'transaction-append'); - if (forms.length) { - JX.DOM.invoke(forms[0], 'shouldRefresh'); - } + _redraw: function() { + var is_invisible = (this._isInvisible || this._isDeleted); + var is_loading = (this._isLoading); + + var row = this._row; + JX.DOM.alterClass(row, 'differential-inline-hidden', is_invisible); + JX.DOM.alterClass(row, 'differential-inline-loading', is_loading); }, - _removeListeners: function(listeners) { - for (var ii = 0; ii < listeners.length; ii++) { - listeners[ii].remove(); + _removeRow: function(row) { + JX.DOM.remove(row.node); + for (var ii = 0; ii < row.listeners.length; ii++) { + row.listeners[ii].remove(); } }, diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js --- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js +++ b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js @@ -331,46 +331,6 @@ var handle_inline_action = function(node, op) { var data = JX.Stratcom.getData(node); - // If you click an action in the preview at the bottom of the page, we - // find the corresponding node and simulate clicking that, if it's - // present on the page. This gives the editor a more consistent view - // of the document. - if (JX.Stratcom.hasSigil(node, 'differential-inline-comment-preview')) { - var nodes = JX.DOM.scry( - JX.DOM.getContentFrame(), - 'div', - 'differential-inline-comment'); - - var found = false; - var node_data; - for (var ii = 0; ii < nodes.length; ++ii) { - if (nodes[ii] == node) { - // Don't match the preview itself. - continue; - } - node_data = JX.Stratcom.getData(nodes[ii]); - if (node_data.id == data.id) { - node = nodes[ii]; - data = node_data; - found = true; - break; - } - } - - if (!found) { - switch (op) { - case 'delete': - new JX.DifferentialInlineCommentEditor(config.uri) - .deleteByID(data.id); - return; - } - } - - if (op == 'delete') { - op = 'refdelete'; - } - } - var original = data.original; var reply_phid = null; if (op == 'reply') { @@ -387,30 +347,25 @@ 'differential-changeset'); var view = JX.DiffChangeset.getForNode(changeset_root); - if (op == 'edit') { - // This is now handled by DiffChangesetList. - editor = true; - } else { - editor = new JX.DifferentialInlineCommentEditor(config.uri) - .setTemplates(view.getUndoTemplates()) - .setOperation(op) - .setID(data.id) - .setChangesetID(data.changesetID) - .setLineNumber(data.number) - .setLength(data.length) - .setOnRight(data.on_right) - .setOriginalText(original) - .setRow(row) - .setTable(row.parentNode) - .setReplyToCommentPHID(reply_phid) - .setRenderer(view.getRenderer()) - .start(); - } + editor = new JX.DifferentialInlineCommentEditor(config.uri) + .setTemplates(view.getUndoTemplates()) + .setOperation(op) + .setID(data.id) + .setChangesetID(data.changesetID) + .setLineNumber(data.number) + .setLength(data.length) + .setOnRight(data.on_right) + .setOriginalText(original) + .setRow(row) + .setTable(row.parentNode) + .setReplyToCommentPHID(reply_phid) + .setRenderer(view.getRenderer()) + .start(); set_link_state(true); }; - for (var op in {'edit': 1, 'delete': 1, 'reply': 1}) { + for (var op in {'reply': 1}) { JX.Stratcom.listen( 'click', ['differential-inline-comment', 'differential-inline-' + op],