Skip to content

Commit b804e8c

Browse files
author
epriestley
committed
Make "View" from inline comment previews correctly jump to "isEditing" inlines
Summary: Ref T13513. Currently, clicking "View" from the inline comment preview (below the "add comment" area at the bottom of the page) only works if the inline isn't being edited. Update this behavior so it works on inlines in either "Viewing" or "Editing" states. Test Plan: - Clicked "View" on a normal inline, got jumped/selected. - Clicked "View" on an editing inline, got jumped/selected. Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21237
1 parent 24ba66f commit b804e8c

File tree

6 files changed

+48
-73
lines changed

6 files changed

+48
-73
lines changed

resources/celerity/map.php

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
'core.pkg.js' => '1e667bcb',
1414
'dark-console.pkg.js' => '187792c2',
1515
'differential.pkg.css' => 'd71d4531',
16-
'differential.pkg.js' => 'cf4f3263',
16+
'differential.pkg.js' => '30307170',
1717
'diffusion.pkg.css' => '42c75c37',
1818
'diffusion.pkg.js' => 'a98c0bf7',
1919
'maniphest.pkg.css' => '35995d6d',
@@ -380,11 +380,10 @@
380380
'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
381381
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
382382
'rsrc/js/application/diff/DiffChangeset.js' => '700bf848',
383-
'rsrc/js/application/diff/DiffChangesetList.js' => '6992b85c',
383+
'rsrc/js/application/diff/DiffChangesetList.js' => '6e668c5b',
384384
'rsrc/js/application/diff/DiffInline.js' => 'db754a7b',
385385
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
386386
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
387-
'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17',
388387
'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
389388
'rsrc/js/application/differential/behavior-populate.js' => 'b86ef6c2',
390389
'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => '94243d89',
@@ -612,7 +611,6 @@
612611
'javelin-behavior-desktop-notifications-control' => '070679fe',
613612
'javelin-behavior-detect-timezone' => '78bc5d94',
614613
'javelin-behavior-device' => '0cf79f45',
615-
'javelin-behavior-diff-preview-link' => 'f51e9c17',
616614
'javelin-behavior-differential-diff-radios' => '925fe8cd',
617615
'javelin-behavior-differential-populate' => 'b86ef6c2',
618616
'javelin-behavior-diffusion-commit-branches' => '4b671572',
@@ -777,7 +775,7 @@
777775
'phabricator-darkmessage' => '26cd4b73',
778776
'phabricator-dashboard-css' => '5a205b9d',
779777
'phabricator-diff-changeset' => '700bf848',
780-
'phabricator-diff-changeset-list' => '6992b85c',
778+
'phabricator-diff-changeset-list' => '6e668c5b',
781779
'phabricator-diff-inline' => 'db754a7b',
782780
'phabricator-diff-path-view' => '8207abf9',
783781
'phabricator-diff-tree-view' => '5d83623b',
@@ -1511,11 +1509,6 @@
15111509
'javelin-install',
15121510
'javelin-dom',
15131511
),
1514-
'6992b85c' => array(
1515-
'javelin-install',
1516-
'phuix-button-view',
1517-
'phabricator-diff-tree-view',
1518-
),
15191512
'6a1583a8' => array(
15201513
'javelin-behavior',
15211514
'javelin-history',
@@ -1552,6 +1545,11 @@
15521545
'javelin-install',
15531546
'javelin-util',
15541547
),
1548+
'6e668c5b' => array(
1549+
'javelin-install',
1550+
'phuix-button-view',
1551+
'phabricator-diff-tree-view',
1552+
),
15551553
'700bf848' => array(
15561554
'javelin-dom',
15571555
'javelin-util',
@@ -2174,11 +2172,6 @@
21742172
'javelin-dom',
21752173
'javelin-vector',
21762174
),
2177-
'f51e9c17' => array(
2178-
'javelin-behavior',
2179-
'javelin-stratcom',
2180-
'javelin-dom',
2181-
),
21822175
'f84bcbf4' => array(
21832176
'javelin-behavior',
21842177
'javelin-stratcom',
@@ -2429,7 +2422,6 @@
24292422
'phuix-formation-view',
24302423
'phuix-formation-column-view',
24312424
'phuix-formation-flank-view',
2432-
'javelin-behavior-diff-preview-link',
24332425
),
24342426
'diffusion.pkg.css' => array(
24352427
'diffusion-icons-css',

resources/celerity/packages.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@
220220
'phuix-formation-view',
221221
'phuix-formation-column-view',
222222
'phuix-formation-flank-view',
223-
'javelin-behavior-diff-preview-link',
224223
),
225224
'diffusion.pkg.css' => array(
226225
'diffusion-icons-css',

src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,10 @@ public function render() {
212212
'a',
213213
array(
214214
'class' => 'inline-button-divider pml msl',
215-
'meta' => array(
216-
'anchor' => $anchor_name,
215+
'meta' => array(
216+
'inlineCommentID' => $inline->getID(),
217217
),
218-
'sigil' => 'differential-inline-preview-jump',
218+
'sigil' => 'differential-inline-preview-jump',
219219
),
220220
pht('View'));
221221

src/infrastructure/diff/view/PHUIDiffInlineCommentPreviewListView.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,6 @@ public function getOwnerPHID() {
2828
public function render() {
2929
$viewer = $this->getViewer();
3030

31-
$config = array(
32-
'pht' => array(
33-
'view' => pht('View'),
34-
),
35-
);
36-
37-
Javelin::initBehavior('diff-preview-link', $config);
38-
3931
$inlines = $this->getInlineComments();
4032
foreach ($inlines as $key => $inline) {
4133
$inlines[$key] = $inline->newInlineCommentObject();

webroot/rsrc/js/application/diff/DiffChangesetList.js

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,17 +1115,19 @@ JX.install('DiffChangesetList', {
11151115
this.selectInline(inline);
11161116
},
11171117

1118-
selectInline: function(inline) {
1118+
selectInline: function(inline, force, scroll) {
11191119
var selection = this._getSelectionState();
11201120
var item;
11211121

1122-
// If the comment the user clicked is currently selected, deselect it.
1123-
// This makes it easy to undo things if you clicked by mistake.
1124-
if (selection.cursor !== null) {
1125-
item = selection.items[selection.cursor];
1126-
if (item.target === inline) {
1127-
this._setSelectionState(null, false);
1128-
return;
1122+
if (!force) {
1123+
// If the comment the user clicked is currently selected, deselect it.
1124+
// This makes it easy to undo things if you clicked by mistake.
1125+
if (selection.cursor !== null) {
1126+
item = selection.items[selection.cursor];
1127+
if (item.target === inline) {
1128+
this._setSelectionState(null, false);
1129+
return;
1130+
}
11291131
}
11301132
}
11311133

@@ -1136,9 +1138,10 @@ JX.install('DiffChangesetList', {
11361138
for (var ii = 0; ii < items.length; ii++) {
11371139
item = items[ii];
11381140
if (item.target === inline) {
1139-
this._setSelectionState(item, false);
1141+
this._setSelectionState(item, scroll);
11401142
}
11411143
}
1144+
11421145
},
11431146

11441147
redrawPreview: function() {
@@ -2117,6 +2120,31 @@ JX.install('DiffChangesetList', {
21172120
['differential-inline-comment', 'tag:textarea'],
21182121
ondraft);
21192122

2123+
var on_preview_view = JX.bind(this, this._onPreviewEvent, 'view');
2124+
JX.Stratcom.listen(
2125+
'click',
2126+
'differential-inline-preview-jump',
2127+
on_preview_view);
2128+
},
2129+
2130+
_onPreviewEvent: function(action, e) {
2131+
if (this.isAsleep()) {
2132+
return;
2133+
}
2134+
2135+
var data = e.getNodeData('differential-inline-preview-jump');
2136+
var inline = this.getInlineByID(data.inlineCommentID);
2137+
if (!inline) {
2138+
return;
2139+
}
2140+
2141+
e.kill();
2142+
2143+
switch (action) {
2144+
case 'view':
2145+
this.selectInline(inline, true, true);
2146+
break;
2147+
}
21202148
},
21212149

21222150
_onInlineEvent: function(action, e) {

webroot/rsrc/js/application/diff/behavior-preview-link.js

Lines changed: 0 additions & 36 deletions
This file was deleted.

0 commit comments

Comments
 (0)