diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 852fe7f057..99b7e68be0 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -145,7 +145,7 @@ celerity_register_resource_map(array( ), 'differential-changeset-view-css' => array( - 'uri' => '/res/4cb57f5f/rsrc/css/application/differential/changeset-view.css', + 'uri' => '/res/313e96e9/rsrc/css/application/differential/changeset-view.css', 'type' => 'css', 'requires' => array( @@ -161,6 +161,20 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/css/application/differential/core.css', ), + 'differential-inline-comment-editor' => + array( + 'uri' => '/res/0ee4fd79/rsrc/js/application/differential/DifferentialInlineCommentEditor.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-dom', + 1 => 'javelin-workflow', + 2 => 'javelin-util', + 3 => 'javelin-stratcom', + 4 => 'javelin-install', + ), + 'disk' => '/rsrc/js/application/differential/DifferentialInlineCommentEditor.js', + ), 'differential-revision-add-comment-css' => array( 'uri' => '/res/849748d3/rsrc/css/application/differential/add-comment.css', @@ -197,16 +211,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/css/application/differential/revision-detail.css', ), - 0 => - array( - 'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js', - 'type' => 'js', - 'requires' => - array( - 0 => 'javelin-install', - ), - 'disk' => '/rsrc/js/javelin/docs/Base.js', - ), 'differential-revision-history-css' => array( 'uri' => '/res/0d7d515d/rsrc/css/application/differential/revision-history.css', @@ -289,6 +293,16 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/javelin/lib/behavior.js', ), + 0 => + array( + 'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-install', + ), + 'disk' => '/rsrc/js/javelin/docs/Base.js', + ), 'javelin-behavior-aphront-basic-tokenizer' => array( 'uri' => '/res/bce3961b/rsrc/js/application/core/behavior-tokenizer.js', @@ -383,15 +397,16 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-edit-inline-comments' => array( - 'uri' => '/res/682d1a9c/rsrc/js/application/differential/behavior-edit-inline-comments.js', + 'uri' => '/res/e21bb634/rsrc/js/application/differential/behavior-edit-inline-comments.js', 'type' => 'js', 'requires' => array( 0 => 'javelin-behavior', 1 => 'javelin-stratcom', 2 => 'javelin-dom', - 3 => 'javelin-workflow', + 3 => 'javelin-util', 4 => 'javelin-vector', + 5 => 'differential-inline-comment-editor', ), 'disk' => '/rsrc/js/application/differential/behavior-edit-inline-comments.js', ), @@ -437,7 +452,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-show-more' => array( - 'uri' => '/res/9cbf1c9c/rsrc/js/application/differential/behavior-show-more.js', + 'uri' => '/res/a766c717/rsrc/js/application/differential/behavior-show-more.js', 'type' => 'js', 'requires' => array( @@ -1042,6 +1057,23 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/03ef179e/diffusion.pkg.css', 'type' => 'css', ), + '234a821b' => + array ( + 'name' => 'differential.pkg.css', + 'symbols' => + array ( + 0 => 'differential-core-view-css', + 1 => 'differential-changeset-view-css', + 2 => 'differential-revision-detail-css', + 3 => 'differential-revision-history-css', + 4 => 'differential-table-of-contents-css', + 5 => 'differential-revision-comment-css', + 6 => 'differential-revision-add-comment-css', + 7 => 'differential-revision-comment-list-css', + ), + 'uri' => '/res/pkg/234a821b/differential.pkg.css', + 'type' => 'css', + ), '33f413ef' => array ( 'name' => 'typeahead.pkg.js', @@ -1082,23 +1114,6 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/64383b02/core.pkg.css', 'type' => 'css', ), - 'b1200c80' => - array ( - 'name' => 'differential.pkg.css', - 'symbols' => - array ( - 0 => 'differential-core-view-css', - 1 => 'differential-changeset-view-css', - 2 => 'differential-revision-detail-css', - 3 => 'differential-revision-history-css', - 4 => 'differential-table-of-contents-css', - 5 => 'differential-revision-comment-css', - 6 => 'differential-revision-add-comment-css', - 7 => 'differential-revision-comment-list-css', - ), - 'uri' => '/res/pkg/b1200c80/differential.pkg.css', - 'type' => 'css', - ), 'db95a6d0' => array ( 'name' => 'javelin.pkg.js', @@ -1134,7 +1149,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/e26c5e06/workflow.pkg.js', 'type' => 'js', ), - 'ed383f69' => + 'f292b274' => array ( 'name' => 'differential.pkg.js', 'symbols' => @@ -1145,7 +1160,7 @@ celerity_register_resource_map(array( 3 => 'javelin-behavior-differential-show-more', 4 => 'javelin-behavior-differential-diff-radios', ), - 'uri' => '/res/pkg/ed383f69/differential.pkg.js', + 'uri' => '/res/pkg/f292b274/differential.pkg.js', 'type' => 'js', ), ), @@ -1160,23 +1175,23 @@ celerity_register_resource_map(array( 'aphront-table-view-css' => '64383b02', 'aphront-tokenizer-control-css' => '64383b02', 'aphront-typeahead-control-css' => '64383b02', - 'differential-changeset-view-css' => 'b1200c80', - 'differential-core-view-css' => 'b1200c80', - 'differential-revision-add-comment-css' => 'b1200c80', - 'differential-revision-comment-css' => 'b1200c80', - 'differential-revision-comment-list-css' => 'b1200c80', - 'differential-revision-detail-css' => 'b1200c80', - 'differential-revision-history-css' => 'b1200c80', - 'differential-table-of-contents-css' => 'b1200c80', + 'differential-changeset-view-css' => '234a821b', + 'differential-core-view-css' => '234a821b', + 'differential-revision-add-comment-css' => '234a821b', + 'differential-revision-comment-css' => '234a821b', + 'differential-revision-comment-list-css' => '234a821b', + 'differential-revision-detail-css' => '234a821b', + 'differential-revision-history-css' => '234a821b', + 'differential-table-of-contents-css' => '234a821b', 'diffusion-commit-view-css' => '03ef179e', 'javelin-behavior' => 'db95a6d0', 'javelin-behavior-aphront-basic-tokenizer' => '33f413ef', 'javelin-behavior-aphront-form-disable-on-submit' => 'e26c5e06', - 'javelin-behavior-differential-diff-radios' => 'ed383f69', - 'javelin-behavior-differential-edit-inline-comments' => 'ed383f69', - 'javelin-behavior-differential-feedback-preview' => 'ed383f69', - 'javelin-behavior-differential-populate' => 'ed383f69', - 'javelin-behavior-differential-show-more' => 'ed383f69', + 'javelin-behavior-differential-diff-radios' => 'f292b274', + 'javelin-behavior-differential-edit-inline-comments' => 'f292b274', + 'javelin-behavior-differential-feedback-preview' => 'f292b274', + 'javelin-behavior-differential-populate' => 'f292b274', + 'javelin-behavior-differential-show-more' => 'f292b274', 'javelin-behavior-phabricator-keyboard-shortcuts' => 'e26c5e06', 'javelin-behavior-workflow' => 'e26c5e06', 'javelin-dom' => 'db95a6d0', diff --git a/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php index 7dc1731190..8bd919e124 100644 --- a/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php +++ b/src/applications/differential/controller/inlinecommentedit/DifferentialInlineCommentEditController.php @@ -49,24 +49,9 @@ class DifferentialInlineCommentEditController extends DifferentialController { $edit_dialog->addSubmitButton(); $edit_dialog->addCancelButton('#'); - $inline = null; - if ($inline_id) { - $inline = id(new DifferentialInlineComment()) - ->load($inline_id); - - if (!$inline || - $inline->getAuthorPHID() != $user->getPHID() || - $inline->getCommentID() || - $inline->getRevisionID() != $this->revisionID) { - throw new Exception("That comment is not editable!"); - } - } - switch ($op) { case 'delete': - if (!$inline) { - return new Aphront400Response(); - } + $inline = $this->loadInlineCommentForEditing($inline_id); if ($request->isFormPost()) { $inline->delete(); @@ -81,9 +66,7 @@ class DifferentialInlineCommentEditController extends DifferentialController { return id(new AphrontDialogResponse())->setDialog($edit_dialog); case 'edit': - if (!$inline) { - return new Aphront400Response(); - } + $inline = $this->loadInlineCommentForEditing($inline_id); if ($request->isFormPost()) { if (strlen($text)) { @@ -106,7 +89,7 @@ class DifferentialInlineCommentEditController extends DifferentialController { $edit_dialog->appendChild( $this->renderTextArea( - $inline->getContent())); + nonempty($text, $inline->getContent()))); return id(new AphrontDialogResponse())->setDialog($edit_dialog); case 'create': @@ -134,8 +117,21 @@ class DifferentialInlineCommentEditController extends DifferentialController { ->save(); return $this->buildRenderedCommentResponse($inline, $on_right); + + case 'reply': default: - $edit_dialog->setTitle('New Inline Comment'); + + if ($op == 'reply') { + $inline = $this->loadInlineComment($inline_id); + // Override defaults. + $changeset = $inline->getChangesetID(); + $is_new = $inline->getIsNewFile(); + $number = $inline->getLineNumber(); + $length = $inline->getLineLength(); + $edit_dialog->setTitle('Reply to Inline Comment'); + } else { + $edit_dialog->setTitle('New Inline Comment'); + } $edit_dialog->addHiddenInput('op', 'create'); $edit_dialog->addHiddenInput('changeset', $changeset); @@ -143,7 +139,7 @@ class DifferentialInlineCommentEditController extends DifferentialController { $edit_dialog->addHiddenInput('number', $number); $edit_dialog->addHiddenInput('length', $length); - $edit_dialog->appendChild($this->renderTextArea('')); + $edit_dialog->appendChild($this->renderTextArea($text)); return id(new AphrontDialogResponse())->setDialog($edit_dialog); } @@ -189,13 +185,61 @@ class DifferentialInlineCommentEditController extends DifferentialController { } private function renderTextArea($text) { - return phutil_render_tag( + return javelin_render_tag( 'textarea', array( 'class' => 'differential-inline-comment-edit-textarea', + 'sigil' => 'differential-inline-comment-edit-textarea', 'name' => 'text', ), phutil_escape_html($text)); } + private function loadInlineComment($id) { + $inline = null; + + if ($id) { + $inline = id(new DifferentialInlineComment())->load($id); + } + + if (!$inline) { + throw new Exception("No such inline comment!"); + } + + return $inline; + } + + private function loadInlineCommentForEditing($id) { + $inline = $this->loadInlineComment($id); + $user = $this->getRequest()->getUser(); + + if (!$this->canEditInlineComment($user, $inline, $this->revisionID)) { + throw new Exception("That comment is not editable!"); + } + return $inline; + } + + private function canEditInlineComment( + PhabricatorUser $user, + DifferentialInlineComment $inline, + $revision_id) { + + // Only the author may edit a comment. + if ($inline->getAuthorPHID() != $user->getPHID()) { + return false; + } + + // Saved comments may not be edited. + if ($inline->getCommentID()) { + return false; + } + + // Inline must be attached to the active revision. + if ($inline->getRevisionID() != $revision_id) { + return false; + } + + return true; + } + } diff --git a/src/applications/differential/controller/inlinecommentedit/__init__.php b/src/applications/differential/controller/inlinecommentedit/__init__.php index 8df121645d..8cb908d5a2 100644 --- a/src/applications/differential/controller/inlinecommentedit/__init__.php +++ b/src/applications/differential/controller/inlinecommentedit/__init__.php @@ -6,7 +6,6 @@ -phutil_require_module('phabricator', 'aphront/response/400'); phutil_require_module('phabricator', 'aphront/response/ajax'); phutil_require_module('phabricator', 'aphront/response/dialog'); phutil_require_module('phabricator', 'applications/differential/controller/base'); @@ -16,6 +15,7 @@ phutil_require_module('phabricator', 'applications/differential/storage/inlineco phutil_require_module('phabricator', 'applications/differential/storage/revision'); phutil_require_module('phabricator', 'applications/differential/view/inlinecomment'); phutil_require_module('phabricator', 'applications/phid/handle/data'); +phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/dialog'); phutil_require_module('phutil', 'markup'); diff --git a/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php b/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php index 61135cc71e..4dd6f8be2e 100644 --- a/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php @@ -83,13 +83,6 @@ class DifferentialChangesetDetailView extends AphrontView { '
'. $this->renderChildren()); - if ($edit) { - Javelin::initBehavior( - 'differential-edit-inline-comments', array( - 'uri' => '/differential/comment/inline/edit/'.$this->revisionID.'/', - )); - } - return $output; } diff --git a/src/applications/differential/view/changesetdetailview/__init__.php b/src/applications/differential/view/changesetdetailview/__init__.php index 783a68af76..4e50ea0f06 100644 --- a/src/applications/differential/view/changesetdetailview/__init__.php +++ b/src/applications/differential/view/changesetdetailview/__init__.php @@ -7,7 +7,6 @@ phutil_require_module('phabricator', 'infrastructure/celerity/api'); -phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/base'); diff --git a/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php b/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php index 4acc578a49..3dfef7f781 100644 --- a/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php +++ b/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php @@ -126,9 +126,13 @@ class DifferentialChangesetListView extends AphrontView { Javelin::initBehavior('differential-comment-jump', array()); if ($this->editable) { + + $undo_templates = $this->renderUndoTemplates(); + $revision = $this->revision; Javelin::initBehavior('differential-edit-inline-comments', array( 'uri' => '/differential/comment/inline/edit/'.$revision->getID().'/', + 'undo_templates' => $undo_templates, )); } @@ -138,4 +142,35 @@ class DifferentialChangesetListView extends AphrontView { ''; } + /** + * Render the "Undo" markup for the inline comment undo feature. + */ + private function renderUndoTemplates() { + $link = javelin_render_tag( + 'a', + array( + 'href' => '#', + 'sigil' => 'differential-inline-comment-undo', + ), + 'Undo'); + + $div = phutil_render_tag( + 'div', + array( + 'class' => 'differential-inline-undo', + ), + 'Changes discarded. '.$link); + + $content = ''.$div.''; + $empty = ''; + + $left = array($content, $empty); + $right = array($empty, $content); + + return array( + 'l' => ''.implode('', $left).'
', + 'r' => ''.implode('', $right).'
', + ); + } + } diff --git a/src/applications/differential/view/changesetlistview/__init__.php b/src/applications/differential/view/changesetlistview/__init__.php index 3077c4f454..b8ca61df16 100644 --- a/src/applications/differential/view/changesetlistview/__init__.php +++ b/src/applications/differential/view/changesetlistview/__init__.php @@ -9,6 +9,7 @@ phutil_require_module('phabricator', 'applications/differential/view/changesetdetailview'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); +phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/base'); phutil_require_module('phutil', 'markup'); diff --git a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php index 8d92b79c3b..0a5ce04855 100644 --- a/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php +++ b/src/applications/differential/view/inlinecomment/DifferentialInlineCommentView.php @@ -73,6 +73,7 @@ final class DifferentialInlineCommentView extends AphrontView { 'number' => $inline->getLineNumber(), 'length' => $inline->getLineLength(), 'on_right' => $this->onRight, + 'original' => $inline->getContent(), ); $sigil = 'differential-inline-comment'; @@ -106,13 +107,6 @@ final class DifferentialInlineCommentView extends AphrontView { 'href' => '#', 'mustcapture' => true, 'sigil' => 'differential-inline-reply', - 'meta' => array( - 'is_new' => true, - 'changeset' => $inline->getChangesetID(), - 'number' => $inline->getLineNumber(), - 'length' => $inline->getLineLength(), - 'on_right' => $this->onRight, - ) ), 'Reply'); diff --git a/src/infrastructure/lint/linter/javelin/PhabricatorJavelinLinter.php b/src/infrastructure/lint/linter/javelin/PhabricatorJavelinLinter.php index 31cc60b589..4af3069773 100644 --- a/src/infrastructure/lint/linter/javelin/PhabricatorJavelinLinter.php +++ b/src/infrastructure/lint/linter/javelin/PhabricatorJavelinLinter.php @@ -78,7 +78,7 @@ class PhabricatorJavelinLinter extends ArcanistLinter { // TODO: Write build documentation for the Javelin binaries and point // the user at it. $this->raiseLintAtLine( - 0, + 1, 0, self::LINT_MISSING_BINARY, "The 'javelinsymbols' binary in the Javelin project has not been ". diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 9599b1f7a5..56aec58c7c 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -181,7 +181,6 @@ margin-left: 8px; } - .differential-property-table { width: auto; margin: .75em auto; @@ -229,3 +228,16 @@ .differential-property-table tr.property-table-header td.nval { background: #aaffaa; } + +.differential-inline-undo { + padding: 4px; + text-align: center; + background: #ffeeaa; + margin: 3px 0 1px; + font: 11px "Verdana"; + color: 444444; +} + +.differential-inline-undo a { + font-weight: bold; +} diff --git a/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js b/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js new file mode 100644 index 0000000000..6ad3370793 --- /dev/null +++ b/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js @@ -0,0 +1,197 @@ +/** + * @provides differential-inline-comment-editor + * @requires javelin-dom + * javelin-workflow + * javelin-util + * javelin-stratcom + * javelin-install + */ + +JX.install('DifferentialInlineCommentEditor', { + + construct : function(uri) { + this._uri = uri; + }, + + events : ['done'], + + members : { + _uri : null, + _undoText : null, + _skipOverInlineCommentRows : function(node) { + // TODO: Move this semantic information out of class names. + while (node && node.className.indexOf('inline') !== -1) { + node = node.nextSibling; + } + return node; + }, + _buildRequestData : function() { + return { + op : this.getOperation(), + on_right : this.getOnRight(), + id : this.getID(), + number : this.getLineNumber(), + is_new : this.getIsNew(), + length : this.getLength(), + changeset : this.getChangeset(), + text : this.getText() || '' + }; + }, + _draw : function(content, exact_row) { + var row = this.getRow(); + var table = row.parentNode; + var target = exact_row ? row : this._skipOverInlineCommentRows(row); + + return copyRows(table, content, target); + }, + _removeUndoLink : function() { + var rows = JX.DifferentialInlineCommentEditor._undoRows; + if (rows) { + for (var ii = 0; ii < rows.length; ii++) { + JX.DOM.remove(rows[ii]); + } + } + }, + _undo : function() { + this._removeUndoLink(); + + this.setText(this._undoText); + this.start(); + }, + _registerUndoListener : function() { + if (!JX.DifferentialInlineCommentEditor._activeEditor) { + JX.Stratcom.listen( + 'click', + 'differential-inline-comment-undo', + function(e) { + JX.DifferentialInlineCommentEditor._activeEditor._undo(); + e.kill(); + }); + } + JX.DifferentialInlineCommentEditor._activeEditor = this; + }, + _didCompleteWorkflow : function(response) { + var op = this.getOperation(); + + // We don't get any markup back if the user deletes a comment, or saves + // an empty comment (which effects a delete). + if (response.markup) { + this._draw(JX.$N('div', JX.$H(response.markup))); + } + + // These operations remove the old row (edit adds a new row first). + var remove_old = (op == 'edit' || op == 'delete'); + if (remove_old) { + JX.DOM.remove(this.getRow()); + } + + // Once the user saves something, get rid of the 'undo' option. A + // particular case where we need this is saving a delete, when we might + // otherwise leave around an 'undo' for an earlier edit to the same + // comment. + this._removeUndoLink(); + + JX.Stratcom.invoke('differential-inline-comment-update'); + this.invoke('done'); + }, + _didCancelWorkflow : function() { + this.invoke('done'); + + var op = this.getOperation(); + if (op == 'delete') { + // No undo for delete, we prompt the user explicitly. + return; + } + + try { + var textarea = JX.DOM.find( + document.body, // TODO: use getDialogRootNode() when available + 'textarea', + 'differential-inline-comment-edit-textarea'); + } catch (ex) { + if (ex !== JX.$.NotFound) { + throw ex; + } + // The close handler is called whenever the dialog closes, even if the + // user closed it by completing the workflow with "Save". The + // JX.Workflow API should probably be refined to allow programmatic + // distinction of close caused by 'cancel' vs 'submit'. Testing for + // presence of the textarea serves as a proxy for detecting a 'cancel'. + return; + } + + var text = textarea.value; + + // If the user hasn't edited the text (i.e., no change from original for + // 'edit' or no text for 'new' or 'reply'), don't offer them an undo. + if (text == (this.getOriginalText() || '')) { + return; + } + + // Save the text so we can 'undo' back to it. + this._undoText = text; + + var template = this.getOnRight() + ? this.getTemplates().r + : this.getTemplates().l; + template = JX.$N('div', JX.$H(template)); + + // NOTE: Operation order matters here; we can't remove anything until + // after we draw the new rows because _draw uses the old rows to figure + // out where to place the comment. + + // We use 'exact_row' to put the "undo" text directly above the affected + // comment. + var exact_row = true; + var rows = this._draw(template, exact_row); + + this._removeUndoLink(); + + JX.DifferentialInlineCommentEditor._undoRows = rows; + }, + + start : function() { + this._registerUndoListener(); + + var data = this._buildRequestData(); + var handler = JX.bind(this, this._didCompleteWorkflow); + var close_handler = JX.bind(this, this._didCancelWorkflow); + + new JX.Workflow(this._uri, data) + .setHandler(handler) + .setCloseHandler(close_handler) + .start(); + + return this; + } + }, + + statics : { + /** + * Global refernece to the 'undo' rows currently rendered in the document. + */ + _undoRows : null, + + /** + * Global listener for the 'undo' click associated with the currently + * displayed 'undo' link. When an editor is start()ed, it becomes the active + * editor. + */ + _activeEditor : null + }, + + properties : { + operation : null, + row : null, + onRight : null, + ID : null, + lineNumber : null, + changeset : null, + length : null, + isNew : null, + text : null, + templates : null, + originalText : null + } + +}); diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js index ecbd876d5e..067755e7d7 100644 --- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js +++ b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js @@ -3,8 +3,9 @@ * @requires javelin-behavior * javelin-stratcom * javelin-dom - * javelin-workflow + * javelin-util * javelin-vector + * differential-inline-comment-editor */ JX.behavior('differential-edit-inline-comments', function(config) { @@ -18,8 +19,8 @@ JX.behavior('differential-edit-inline-comments', function(config) { var target = null; var root = null; var changeset = null; - var workflow = false; - var is_new = false; + + var editor = null; function updateReticle() { var top = origin; @@ -45,16 +46,11 @@ JX.behavior('differential-edit-inline-comments', function(config) { JX.DOM.hide(reticle); } - function finishSelect() { + JX.DifferentialInlineCommentEditor.listen('done', function() { selecting = false; - workflow = false; + editor = false; hideReticle(); - } - - function drawInlineComment(table, anchor, r) { - copyRows(table, JX.$N('div', JX.$H(r.markup)), anchor); - finishSelect(); - } + }); function isOnRight(node) { return node.parentNode.firstChild != node; @@ -73,25 +69,11 @@ JX.behavior('differential-edit-inline-comments', function(config) { } } - function isInlineCommentNode(target) { - return target && - (!JX.DOM.isType(target, 'tr') - || target.className.indexOf('inline') !== -1); - - } - - function findInlineCommentTarget(target) { - while (isInlineCommentNode(target)) { - target = target.nextSibling; - } - return target; - } - JX.Stratcom.listen( 'mousedown', ['differential-changeset', 'tag:th'], function(e) { - if (workflow || + if (editor || selecting || getRowNumber(e.getTarget()) === undefined) { return; @@ -119,7 +101,7 @@ JX.behavior('differential-edit-inline-comments', function(config) { ['differential-changeset', 'tag:th'], function(e) { if (!selecting || - workflow || + editor || (getRowNumber(e.getTarget()) === undefined) || (isOnRight(e.getTarget()) != isOnRight(origin)) || (e.getNode('differential-changeset') !== root)) { @@ -135,7 +117,7 @@ JX.behavior('differential-edit-inline-comments', function(config) { 'mouseup', null, function(e) { - if (workflow || !selecting) { + if (editor || !selecting) { return; } @@ -153,38 +135,16 @@ JX.behavior('differential-edit-inline-comments', function(config) { insert = target.parentNode; } - var data = { - op: 'new', - changeset: changeset, - number: o, - length: len, - is_new: isNewFile(target) ? 1 : 0, - on_right: isOnRight(target) ? 1 : 0 - }; - - workflow = true; - - var w = new JX.Workflow(config.uri, data) - .setHandler(function(r) { - // Skip over any rows which contain inline feedback. Don't mimic this! - // We're shipping around raw HTML here for performance reasons, but - // normally you should use sigils to encode this kind of data on - // the document. - var target = findInlineCommentTarget(insert.nextSibling); - drawInlineComment(insert.parentNode, target, r); - finishSelect(); - JX.Stratcom.invoke('differential-inline-comment-update'); - }) - .setCloseHandler(finishSelect); - - - w.listen('error', function(e) { - // TODO: uh, tell the user I guess - finishSelect(); - JX.Stratcom.context().stop(); - }); - - w.start(); + editor = new JX.DifferentialInlineCommentEditor(config.uri) + .setTemplates(config.undo_templates) + .setOperation('new') + .setChangeset(changeset) + .setLineNumber(o) + .setLength(len) + .setIsNew(isNewFile(target) ? 1 : 0) + .setOnRight(isOnRight(target) ? 1 : 0) + .setRow(insert.nextSibling) + .start(); e.kill(); }); @@ -193,7 +153,7 @@ JX.behavior('differential-edit-inline-comments', function(config) { ['mouseover', 'mouseout'], 'differential-inline-comment', function(e) { - if (selecting || workflow) { + if (selecting || editor) { return; } @@ -220,50 +180,28 @@ JX.behavior('differential-edit-inline-comments', function(config) { } }); - JX.Stratcom.listen( - 'click', - [['differential-inline-comment', 'differential-inline-reply']], - function(e) { - new JX.Workflow(config.uri, e.getNodeData('differential-inline-reply')) - .setHandler(function(r) { - var base_row = - findInlineCommentTarget( - e.getNode('differential-inline-comment') - .parentNode - .parentNode - ); - drawInlineComment(base_row.parentNode, base_row, r); - JX.Stratcom.invoke('differential-inline-comment-update'); - }) - .start(); + var action_handler = function(op, e) { + var data = e.getNodeData('differential-inline-comment'); + var node = e.getNode('differential-inline-comment'); - e.kill(); - } - ); + editor = new JX.DifferentialInlineCommentEditor(config.uri) + .setTemplates(config.undo_templates) + .setOperation(op) + .setID(data.id) + .setOnRight(data.on_right) + .setOriginalText(data.original) + .setRow(node.parentNode.parentNode) + .start(); - JX.Stratcom.listen( - 'click', - [['differential-inline-comment', 'differential-inline-delete'], - ['differential-inline-comment', 'differential-inline-edit']], - function(e) { - var data = { - op: e.getNode('differential-inline-edit') ? 'edit' : 'delete', - id: e.getNodeData('differential-inline-comment').id, - on_right: e.getNodeData('differential-inline-comment').on_right - }; - new JX.Workflow(config.uri, data) - .setHandler(function(r) { - var base_row = e.getNode('differential-inline-comment') - .parentNode - .parentNode; - if (data.op == 'edit' && r.markup) { - drawInlineComment(base_row.parentNode, base_row, r); - } - JX.DOM.remove(base_row); - JX.Stratcom.invoke('differential-inline-comment-update'); - }) - .start(); - e.kill(); - }); + e.kill(); + } + + for (var op in {'edit' : 1, 'delete' : 1, 'reply' : 1}) { + JX.Stratcom.listen( + 'click', + ['differential-inline-comment', 'differential-inline-' + op], + JX.bind(null, action_handler, op)); + } }); + diff --git a/webroot/rsrc/js/application/differential/behavior-show-more.js b/webroot/rsrc/js/application/differential/behavior-show-more.js index 135276bb11..591c95e198 100644 --- a/webroot/rsrc/js/application/differential/behavior-show-more.js +++ b/webroot/rsrc/js/application/differential/behavior-show-more.js @@ -44,4 +44,5 @@ function copyRows(dst, src, before) { dst.appendChild(rows[ii]); } } + return rows; }