Page MenuHomePhabricator

D21244.diff
No OneTemporary

D21244.diff

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();
+ }
}
}

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 16, 6:58 PM (2 w, 17 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7342409
Default Alt Text
D21244.diff (15 KB)

Event Timeline