Page MenuHomePhabricator
Files F15392990

D17908.id43080.diff
No OneTemporary
Actions

D17908.id43080.diff
View Options

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' => '8c5f913d',
'darkconsole.pkg.js' => '1f9a31bc',
'differential.pkg.css' => '58712637',
- 'differential.pkg.js' => 'e486afd0',
+ 'differential.pkg.js' => 'bd321b6e',
'diffusion.pkg.css' => 'b93d9b8c',
'diffusion.pkg.js' => '84c8f8fd',
'favicon.ico' => '30672e08',
@@ -391,10 +391,9 @@
'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' => '145c34e2',
- 'rsrc/js/application/diff/DiffChangesetList.js' => 'd93e34c2',
+ 'rsrc/js/application/diff/DiffChangesetList.js' => 'f1101e6e',
'rsrc/js/application/diff/DiffInline.js' => 'bdf6b568',
'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
- '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' => '89d11432',
@@ -619,7 +618,6 @@
'javelin-behavior-detect-timezone' => '4c193c96',
'javelin-behavior-device' => 'bb1dd507',
'javelin-behavior-diff-preview-link' => '051c7832',
- 'javelin-behavior-differential-comment-jump' => '4fdb476d',
'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
'javelin-behavior-differential-edit-inline-comments' => '89d11432',
'javelin-behavior-differential-feedback-preview' => 'b064af76',
@@ -782,7 +780,7 @@
'phabricator-darkmessage' => 'c48cccdd',
'phabricator-dashboard-css' => 'fe5b1869',
'phabricator-diff-changeset' => '145c34e2',
- 'phabricator-diff-changeset-list' => 'd93e34c2',
+ 'phabricator-diff-changeset-list' => 'f1101e6e',
'phabricator-diff-inline' => 'bdf6b568',
'phabricator-drag-and-drop-file-upload' => '58dea2fa',
'phabricator-draggable-list' => 'bea6e7f4',
@@ -1291,11 +1289,6 @@
'javelin-stratcom',
'javelin-dom',
),
- '4fdb476d' => array(
- 'javelin-behavior',
- 'javelin-stratcom',
- 'javelin-dom',
- ),
'503e17fd' => array(
'javelin-install',
'javelin-typeahead-source',
@@ -2079,9 +2072,6 @@
'javelin-util',
'phabricator-shaped-request',
),
- 'd93e34c2' => array(
- 'javelin-install',
- ),
'de2e896f' => array(
'javelin-behavior',
'javelin-dom',
@@ -2184,6 +2174,9 @@
'javelin-workflow',
'javelin-json',
),
+ 'f1101e6e' => array(
+ 'javelin-install',
+ ),
'f50152ad' => array(
'phui-timeline-view-css',
),
@@ -2431,7 +2424,6 @@
'javelin-behavior-differential-edit-inline-comments',
'javelin-behavior-differential-populate',
'javelin-behavior-differential-diff-radios',
- 'javelin-behavior-differential-comment-jump',
'javelin-behavior-aphront-drag-and-drop-textarea',
'javelin-behavior-phabricator-object-selector',
'javelin-behavior-repository-crossreference',
diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php
--- a/resources/celerity/packages.php
+++ b/resources/celerity/packages.php
@@ -196,7 +196,6 @@
'javelin-behavior-differential-edit-inline-comments',
'javelin-behavior-differential-populate',
'javelin-behavior-differential-diff-radios',
- 'javelin-behavior-differential-comment-jump',
'javelin-behavior-aphront-drag-and-drop-textarea',
'javelin-behavior-phabricator-object-selector',
'javelin-behavior-repository-crossreference',
diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php
--- a/src/applications/differential/view/DifferentialChangesetListView.php
+++ b/src/applications/differential/view/DifferentialChangesetListView.php
@@ -270,8 +270,6 @@
),
));
- $this->initBehavior('differential-comment-jump', array());
-
if ($this->inlineURI) {
Javelin::initBehavior('differential-edit-inline-comments', array(
'uri' => $this->inlineURI,
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
@@ -190,61 +190,31 @@
}
}
- $nextprev = null;
- if (!$this->preview) {
- $nextprev = new PHUIButtonBarView();
- $nextprev->setBorderless(true);
- $nextprev->addClass('inline-button-divider');
-
+ $anchor_name = $this->getAnchorName();
- $up = id(new PHUIButtonView())
- ->setTag('a')
- ->setTooltip(pht('Previous'))
- ->setIcon('fa-chevron-up')
- ->addSigil('differential-inline-prev')
- ->setMustCapture(true);
+ $action_buttons = array();
- $down = id(new PHUIButtonView())
- ->setTag('a')
- ->setTooltip(pht('Next'))
- ->setIcon('fa-chevron-down')
- ->addSigil('differential-inline-next')
- ->setMustCapture(true);
+ $can_reply =
+ (!$this->editable) &&
+ (!$this->preview) &&
+ ($this->allowReply) &&
- if ($this->canHide()) {
- $hide = id(new PHUIButtonView())
- ->setTag('a')
- ->setTooltip(pht('Hide Comment'))
- ->setIcon('fa-times')
- ->addSigil('hide-inline')
- ->setMustCapture(true);
+ // 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);
- $nextprev->addButton($hide);
- }
- $nextprev->addButton($up);
- $nextprev->addButton($down);
-
- $action_buttons = array();
- if ($this->allowReply) {
- if (!$is_synthetic) {
- // NOTE: No product reason why you can't reply to these, 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.
-
- $action_buttons[] = id(new PHUIButtonView())
- ->setTag('a')
- ->setIcon('fa-reply')
- ->setTooltip(pht('Reply'))
- ->addSigil('differential-inline-reply')
- ->setMustCapture(true);
- }
- }
+ if ($can_reply) {
+ $action_buttons[] = id(new PHUIButtonView())
+ ->setTag('a')
+ ->setIcon('fa-reply')
+ ->setTooltip(pht('Reply'))
+ ->addSigil('differential-inline-reply')
+ ->setMustCapture(true);
}
- $anchor_name = $this->getAnchorName();
-
if ($this->editable && !$this->preview) {
$action_buttons[] = id(new PHUIButtonView())
->setTag('a')
@@ -280,6 +250,15 @@
->setMustCapture(true);
}
+ if (!$this->preview && $this->canHide()) {
+ $action_buttons[] = id(new PHUIButtonView())
+ ->setTag('a')
+ ->setTooltip(pht('Hide Comment'))
+ ->setIcon('fa-times')
+ ->addSigil('hide-inline')
+ ->setMustCapture(true);
+ }
+
$done_button = null;
if (!$is_synthetic) {
@@ -430,7 +409,6 @@
$done_button,
$links,
$actions,
- $nextprev,
));
$markup = javelin_tag(
@@ -441,10 +419,16 @@
'meta' => $metadata,
),
array(
- phutil_tag_div('differential-inline-comment-head grouped', array(
- $group_left,
- $group_right,
- )),
+ javelin_tag(
+ 'div',
+ array(
+ 'class' => 'differential-inline-comment-head grouped',
+ 'sigil' => 'differential-inline-header',
+ ),
+ array(
+ $group_left,
+ $group_right,
+ )),
phutil_tag_div(
'differential-inline-comment-content',
phutil_tag_div('phabricator-remarkup', $content)),
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
@@ -50,6 +50,12 @@
var onresize = JX.bind(this, this._ifawake, this._onresize);
JX.Stratcom.listen('resize', null, onresize);
+
+ var onselect = JX.bind(this, this._ifawake, this._onselect);
+ JX.Stratcom.listen(
+ 'mousedown',
+ ['differential-inline-comment', 'differential-inline-header'],
+ onselect);
},
properties: {
@@ -665,6 +671,47 @@
this._redrawFocus();
},
+ _onselect: function(e) {
+ // If the user clicked some element inside the header, like an action
+ // icon, ignore the event. They have to click the header element itself.
+ if (e.getTarget() !== e.getNode('differential-inline-header')) {
+ return;
+ }
+
+ var inline = this._getInlineForEvent(e);
+ if (!inline) {
+ return;
+ }
+
+ // The user definitely clicked an inline, so we're going to handle the
+ // event.
+ e.kill();
+
+ var selection = this._getSelectionState();
+ var item;
+
+ // If the comment the user clicked is currently selected, deselect it.
+ // This makes it easy to undo things if you clicked by mistake.
+ if (selection.cursor !== null) {
+ item = selection.items[selection.cursor];
+ if (item.target === inline) {
+ this._setSelectionState(null);
+ return;
+ }
+ }
+
+ // Otherwise, select the item that the user clicked. This makes it
+ // easier to resume keyboard operations after using the mouse to do
+ // something else.
+ var items = selection.items;
+ for (var ii = 0; ii < items.length; ii++) {
+ item = items[ii];
+ if (item.target === inline) {
+ this._setSelectionState(item);
+ }
+ }
+ },
+
_onaction: function(action, e) {
e.kill();
diff --git a/webroot/rsrc/js/application/differential/behavior-comment-jump.js b/webroot/rsrc/js/application/differential/behavior-comment-jump.js
deleted file mode 100644
--- a/webroot/rsrc/js/application/differential/behavior-comment-jump.js
+++ /dev/null
@@ -1,32 +0,0 @@
-/**
- * @provides javelin-behavior-differential-comment-jump
- * @requires javelin-behavior
- * javelin-stratcom
- * javelin-dom
- */
-
-JX.behavior('differential-comment-jump', function() {
- function handle_jump(offset) {
- return function(e) {
- var parent = JX.$('differential-review-stage');
- var clicked = e.getNode('differential-inline-comment');
- var inlines = JX.DOM.scry(parent, 'div', 'differential-inline-comment');
- var jumpto = null;
-
- for (var ii = 0; ii < inlines.length; ii++) {
- if (inlines[ii] == clicked) {
- jumpto = inlines[(ii + offset + inlines.length) % inlines.length];
- break;
- }
- }
- JX.Stratcom.invoke('differential-toggle-file-request', null, {
- element: jumpto
- });
- JX.DOM.scrollTo(jumpto);
- e.kill();
- };
- }
-
- JX.Stratcom.listen('click', 'differential-inline-prev', handle_jump(-1));
- JX.Stratcom.listen('click', 'differential-inline-next', handle_jump(+1));
-});

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 16, 6:45 PM (1 w, 12 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7223711
Default Alt Text
D17908.id43080.diff (11 KB)

Event Timeline

Phabricator · Built by Phacility