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' => '1e667bcb', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => 'd71d4531', - 'differential.pkg.js' => '39781f05', + 'differential.pkg.js' => '21616a78', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -380,8 +380,8 @@ 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', 'rsrc/js/application/diff/DiffChangeset.js' => '20715b98', - 'rsrc/js/application/diff/DiffChangesetList.js' => '564cbd20', - 'rsrc/js/application/diff/DiffInline.js' => 'a0ef0b54', + 'rsrc/js/application/diff/DiffChangesetList.js' => '40d6c41c', + 'rsrc/js/application/diff/DiffInline.js' => '15de2478', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', @@ -775,8 +775,8 @@ 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', 'phabricator-diff-changeset' => '20715b98', - 'phabricator-diff-changeset-list' => '564cbd20', - 'phabricator-diff-inline' => 'a0ef0b54', + 'phabricator-diff-changeset-list' => '40d6c41c', + 'phabricator-diff-inline' => '15de2478', 'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-tree-view' => '5d83623b', 'phabricator-drag-and-drop-file-upload' => '4370900d', @@ -1034,6 +1034,9 @@ 'javelin-stratcom', 'javelin-util', ), + '15de2478' => array( + 'javelin-dom', + ), '1a844c06' => array( 'javelin-install', 'javelin-util', @@ -1259,6 +1262,11 @@ 'javelin-behavior', 'javelin-uri', ), + '40d6c41c' => array( + 'javelin-install', + 'phuix-button-view', + 'phabricator-diff-tree-view', + ), '42c44e8b' => array( 'javelin-behavior', 'javelin-workflow', @@ -1418,11 +1426,6 @@ 'javelin-stratcom', 'javelin-dom', ), - '564cbd20' => array( - 'javelin-install', - 'phuix-button-view', - 'phabricator-diff-tree-view', - ), '5793d835' => array( 'javelin-install', 'javelin-util', @@ -1833,9 +1836,6 @@ 'javelin-util', 'phabricator-keyboard-shortcut', ), - 'a0ef0b54' => array( - 'javelin-dom', - ), 'a17b84f1' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php @@ -88,10 +88,12 @@ $is_synthetic = true; } + $is_preview = $this->preview; + $metadata = $this->getInlineCommentMetadata(); $sigil = 'differential-inline-comment'; - if ($this->preview) { + if ($is_preview) { $sigil = $sigil.' differential-inline-comment-preview'; } @@ -146,11 +148,6 @@ $classes[] = 'inline-comment-ghost'; } - // I think this is unused - if ($inline->getHasReplies()) { - $classes[] = 'inline-comment-has-reply'; - } - if ($inline->getReplyToCommentPHID()) { $classes[] = 'inline-comment-is-reply'; } @@ -167,47 +164,22 @@ $anchor_name = $this->getAnchorName(); $action_buttons = array(); - - $can_reply = - (!$this->editable) && - (!$this->preview) && - ($this->allowReply) && - - // NOTE: No product reason why you can't reply to synthetic comments, - // but the reply mechanism currently sends the inline comment ID to the - // server, not file/line information, and synthetic comments don't have - // an inline comment ID. - (!$is_synthetic); - - - if ($can_reply) { - $action_buttons[] = id(new PHUIButtonView()) - ->setTag('a') - ->setIcon('fa-reply') - ->setTooltip(pht('Reply')) - ->addSigil('differential-inline-reply') - ->setMustCapture(true) - ->setAuralLabel(pht('Reply')); - } - - if ($this->editable && !$this->preview) { - $action_buttons[] = id(new PHUIButtonView()) - ->setTag('a') - ->setIcon('fa-pencil') - ->setTooltip(pht('Edit')) - ->addSigil('differential-inline-edit') - ->setMustCapture(true) - ->setAuralLabel(pht('Edit')); - - $action_buttons[] = id(new PHUIButtonView()) - ->setTag('a') - ->setIcon('fa-trash-o') - ->setTooltip(pht('Delete')) - ->addSigil('differential-inline-delete') - ->setMustCapture(true) - ->setAuralLabel(pht('Delete')); - - } else if ($this->preview) { + $menu_items = array(); + + if ($this->editable && !$is_preview) { + $menu_items[] = array( + 'label' => pht('Edit Comment'), + 'icon' => 'fa-pencil', + 'action' => 'edit', + 'key' => 'e', + ); + + $menu_items[] = array( + 'label' => pht('Delete Comment'), + 'icon' => 'fa-trash-o', + 'action' => 'delete', + ); + } else if ($is_preview) { $links[] = javelin_tag( 'a', array( @@ -228,14 +200,40 @@ ->setAuralLabel(pht('Delete')); } - if (!$this->preview && $this->canHide()) { - $action_buttons[] = id(new PHUIButtonView()) - ->setTag('a') - ->setTooltip(pht('Collapse')) - ->setIcon('fa-times') - ->addSigil('hide-inline') - ->setMustCapture(true) - ->setAuralLabel(pht('Collapse')); + if (!$is_preview && $this->canHide()) { + $menu_items[] = array( + 'label' => pht('Collapse'), + 'icon' => 'fa-times', + 'action' => 'collapse', + 'key' => 'q', + ); + } + + $can_reply = + (!$this->editable) && + (!$is_preview) && + ($this->allowReply) && + + // NOTE: No product reason why you can't reply to synthetic comments, + // but the reply mechanism currently sends the inline comment ID to the + // server, not file/line information, and synthetic comments don't have + // an inline comment ID. + (!$is_synthetic); + + if ($can_reply) { + $menu_items[] = array( + 'label' => pht('Reply to Comment'), + 'icon' => 'fa-reply', + 'action' => 'reply', + 'key' => 'r', + ); + + $menu_items[] = array( + 'label' => pht('Quote Comment'), + 'icon' => 'fa-quote-left', + 'action' => 'quote', + 'key' => 'R', + ); } $done_button = null; @@ -283,7 +281,7 @@ $classes[] = 'inline-state-is-draft'; } - if ($mark_done && !$this->preview) { + if ($mark_done && !$is_preview) { $done_input = javelin_tag( 'input', array( @@ -327,7 +325,7 @@ $inline, PhabricatorInlineComment::MARKUP_FIELD_BODY); - if ($this->preview) { + if ($is_preview) { $anchor = null; } else { $anchor = phutil_tag( @@ -364,13 +362,24 @@ } $actions = null; - if ($action_buttons) { + if ($action_buttons || $menu_items) { $actions = new PHUIButtonBarView(); $actions->setBorderless(true); $actions->addClass('inline-button-divider'); foreach ($action_buttons as $button) { $actions->addButton($button); } + + if (!$is_preview) { + $menu_button = id(new PHUIButtonView()) + ->setTag('a') + ->setColor(PHUIButtonView::GREY) + ->setDropdown(true) + ->setAuralLabel(pht('Inline Actions')) + ->addSigil('inline-action-dropdown'); + + $actions->addButton($menu_button); + } } $group_left = phutil_tag( @@ -401,6 +410,8 @@ ->truncateString($inline->getContent()); $metadata['snippet'] = pht('%s: %s', $author, $snippet); + $metadata['menuItems'] = $menu_items; + $markup = javelin_tag( 'div', array( 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 @@ -20,9 +20,6 @@ var onmenu = JX.bind(this, this._ifawake, this._onmenu); JX.Stratcom.listen('click', 'differential-view-options', onmenu); - var oncollapse = JX.bind(this, this._ifawake, this._oncollapse, true); - JX.Stratcom.listen('click', 'hide-inline', oncollapse); - var onexpand = JX.bind(this, this._ifawake, this._oncollapse, false); JX.Stratcom.listen('click', 'reveal-inline', onexpand); @@ -336,16 +333,7 @@ var inline = cursor.target; if (inline.canReply()) { this.setFocus(null); - - var text; - if (is_quote) { - text = inline.getRawText(); - text = '> ' + text.replace(/\n/g, '\n> ') + '\n\n'; - } else { - text = ''; - } - - inline.reply(text); + inline.reply(true); return; } } @@ -2094,12 +2082,6 @@ 'differential-inline-comment-undo', onundo); - var onedit = JX.bind(this, this._onInlineEvent, 'edit'); - JX.Stratcom.listen( - 'click', - ['differential-inline-comment', 'differential-inline-edit'], - onedit); - var ondone = JX.bind(this, this._onInlineEvent, 'done'); JX.Stratcom.listen( 'click', @@ -2112,11 +2094,11 @@ ['differential-inline-comment', 'differential-inline-delete'], ondelete); - var onreply = JX.bind(this, this._onInlineEvent, 'reply'); + var onmenu = JX.bind(this, this._onInlineEvent, 'menu'); JX.Stratcom.listen( 'click', - ['differential-inline-comment', 'differential-inline-reply'], - onreply); + ['differential-inline-comment', 'inline-action-dropdown'], + onmenu); var ondraft = JX.bind(this, this._onInlineEvent, 'draft'); JX.Stratcom.listen( @@ -2156,7 +2138,7 @@ return; } - if (action !== 'draft') { + if (action !== 'draft' && action !== 'menu') { e.kill(); } @@ -2201,21 +2183,19 @@ case 'undo': inline.undo(); break; - case 'edit': - inline.edit(); - break; case 'done': inline.toggleDone(); break; case 'delete': inline.delete(is_ref); break; - case 'reply': - inline.reply(); - break; case 'draft': inline.triggerDraft(); break; + case 'menu': + var node = e.getNode('inline-action-dropdown'); + inline.activateMenu(node, e); + break; } } 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 @@ -21,6 +21,7 @@ _replyToCommentPHID: null, _originalText: null, _snippet: null, + _menuItems: null, _documentEngineKey: null, _isDeleted: false, @@ -45,6 +46,7 @@ _draftRequest: null, _skipFocus: false, + _menu: null, bindToRow: function(row) { this._row = row; @@ -89,6 +91,7 @@ this._changesetID = data.changesetID; this._isNew = false; this._snippet = data.snippet; + this._menuItems = data.menuItems; this._documentEngineKey = data.documentEngineKey; this._isEditing = data.isEditing; @@ -252,19 +255,11 @@ }, canReply: function() { - if (!this._hasAction('reply')) { - return false; - } - - return true; + return this._hasMenuAction('reply'); }, canEdit: function() { - if (!this._hasAction('edit')) { - return false; - } - - return true; + return this._hasMenuAction('edit'); }, canDone: function() { @@ -276,22 +271,13 @@ }, canCollapse: function() { - if (!JX.DOM.scry(this._row, 'a', 'hide-inline').length) { - return false; - } - - return true; + return this._hasMenuAction('collapse'); }, 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' @@ -312,6 +298,8 @@ }, setCollapsed: function(collapsed) { + this._closeMenu(); + this._isCollapsed = collapsed; var op; @@ -393,12 +381,24 @@ .send(); }, - reply: function(text) { + reply: function(with_quote) { + this._closeMenu(); + + var text; + if (with_quote) { + text = this.getRawText(); + text = '> ' + text.replace(/\n/g, '\n> ') + '\n\n'; + } else { + text = ''; + } + var changeset = this.getChangeset(); return changeset.newInlineReply(this, text); }, edit: function(text, skip_focus) { + this._closeMenu(); + this._skipFocus = !!skip_focus; // If you edit an inline ("A"), modify the text ("AB"), cancel, and then @@ -881,6 +881,101 @@ if (this._draftRequest) { this._draftRequest.trigger(); } + }, + + activateMenu: function(button, e) { + // If we already have a menu for this button, let the menu handle the + // event. + var data = JX.Stratcom.getData(button); + if (data.menu) { + return; + } + + e.prevent(); + + var menu = new JX.PHUIXDropdownMenu(button) + .setWidth(240); + + var list = new JX.PHUIXActionListView(); + var items = this._newMenuItems(menu); + for (var ii = 0; ii < items.length; ii++) { + list.addItem(items[ii]); + } + + menu.setContent(list.getNode()); + + data.menu = menu; + this._menu = menu; + + menu.listen('open', JX.bind(this, function() { + var changeset_list = this.getChangeset().getChangesetList(); + changeset_list.selectInline(this, true); + })); + + menu.open(); + }, + + _newMenuItems: function(menu) { + var items = []; + + for (var ii = 0; ii < this._menuItems.length; ii++) { + var spec = this._menuItems[ii]; + + var onmenu = JX.bind(this, this._onMenuItem, menu, spec.action); + + var item = new JX.PHUIXActionView() + .setIcon(spec.icon) + .setName(spec.label) + .setHandler(onmenu); + + if (spec.key) { + item.setKeyCommand(spec.key); + } + + items.push(item); + } + + return items; + }, + + _onMenuItem: function(menu, action, e) { + e.prevent(); + menu.close(); + + switch (action) { + case 'reply': + this.reply(); + break; + case 'quote': + this.reply(true); + break; + case 'collapse': + this.setCollapsed(true); + break; + case 'delete': + this.delete(); + break; + case 'edit': + this.edit(); + break; + } + + }, + + _hasMenuAction: function(action) { + for (var ii = 0; ii < this._menuItems.length; ii++) { + var spec = this._menuItems[ii]; + if (spec.action === action) { + return true; + } + } + return false; + }, + + _closeMenu: function() { + if (this._menu) { + this._menu.close(); + } } }