Commit b804e8cf authored by epriestley's avatar epriestley
Browse files

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
parent 24ba66f1
......@@ -13,7 +13,7 @@ return array(
'core.pkg.js' => '1e667bcb',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => 'd71d4531',
'differential.pkg.js' => 'cf4f3263',
'differential.pkg.js' => '30307170',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => 'a98c0bf7',
'maniphest.pkg.css' => '35995d6d',
......@@ -380,11 +380,10 @@ return array(
'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' => '700bf848',
'rsrc/js/application/diff/DiffChangesetList.js' => '6992b85c',
'rsrc/js/application/diff/DiffChangesetList.js' => '6e668c5b',
'rsrc/js/application/diff/DiffInline.js' => 'db754a7b',
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17',
'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
'rsrc/js/application/differential/behavior-populate.js' => 'b86ef6c2',
'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => '94243d89',
......@@ -612,7 +611,6 @@ return array(
'javelin-behavior-desktop-notifications-control' => '070679fe',
'javelin-behavior-detect-timezone' => '78bc5d94',
'javelin-behavior-device' => '0cf79f45',
'javelin-behavior-diff-preview-link' => 'f51e9c17',
'javelin-behavior-differential-diff-radios' => '925fe8cd',
'javelin-behavior-differential-populate' => 'b86ef6c2',
'javelin-behavior-diffusion-commit-branches' => '4b671572',
......@@ -777,7 +775,7 @@ return array(
'phabricator-darkmessage' => '26cd4b73',
'phabricator-dashboard-css' => '5a205b9d',
'phabricator-diff-changeset' => '700bf848',
'phabricator-diff-changeset-list' => '6992b85c',
'phabricator-diff-changeset-list' => '6e668c5b',
'phabricator-diff-inline' => 'db754a7b',
'phabricator-diff-path-view' => '8207abf9',
'phabricator-diff-tree-view' => '5d83623b',
......@@ -1511,11 +1509,6 @@ return array(
'javelin-install',
'javelin-dom',
),
'6992b85c' => array(
'javelin-install',
'phuix-button-view',
'phabricator-diff-tree-view',
),
'6a1583a8' => array(
'javelin-behavior',
'javelin-history',
......@@ -1552,6 +1545,11 @@ return array(
'javelin-install',
'javelin-util',
),
'6e668c5b' => array(
'javelin-install',
'phuix-button-view',
'phabricator-diff-tree-view',
),
'700bf848' => array(
'javelin-dom',
'javelin-util',
......@@ -2174,11 +2172,6 @@ return array(
'javelin-dom',
'javelin-vector',
),
'f51e9c17' => array(
'javelin-behavior',
'javelin-stratcom',
'javelin-dom',
),
'f84bcbf4' => array(
'javelin-behavior',
'javelin-stratcom',
......@@ -2429,7 +2422,6 @@ return array(
'phuix-formation-view',
'phuix-formation-column-view',
'phuix-formation-flank-view',
'javelin-behavior-diff-preview-link',
),
'diffusion.pkg.css' => array(
'diffusion-icons-css',
......
......@@ -220,7 +220,6 @@ return array(
'phuix-formation-view',
'phuix-formation-column-view',
'phuix-formation-flank-view',
'javelin-behavior-diff-preview-link',
),
'diffusion.pkg.css' => array(
'diffusion-icons-css',
......
......@@ -212,10 +212,10 @@ final class PHUIDiffInlineCommentDetailView
'a',
array(
'class' => 'inline-button-divider pml msl',
'meta' => array(
'anchor' => $anchor_name,
'meta' => array(
'inlineCommentID' => $inline->getID(),
),
'sigil' => 'differential-inline-preview-jump',
'sigil' => 'differential-inline-preview-jump',
),
pht('View'));
......
......@@ -28,14 +28,6 @@ final class PHUIDiffInlineCommentPreviewListView
public function render() {
$viewer = $this->getViewer();
$config = array(
'pht' => array(
'view' => pht('View'),
),
);
Javelin::initBehavior('diff-preview-link', $config);
$inlines = $this->getInlineComments();
foreach ($inlines as $key => $inline) {
$inlines[$key] = $inline->newInlineCommentObject();
......
......@@ -1115,17 +1115,19 @@ JX.install('DiffChangesetList', {
this.selectInline(inline);
},
selectInline: function(inline) {
selectInline: function(inline, force, scroll) {
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, false);
return;
if (!force) {
// 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, false);
return;
}
}
}
......@@ -1136,9 +1138,10 @@ JX.install('DiffChangesetList', {
for (var ii = 0; ii < items.length; ii++) {
item = items[ii];
if (item.target === inline) {
this._setSelectionState(item, false);
this._setSelectionState(item, scroll);
}
}
},
redrawPreview: function() {
......@@ -2117,6 +2120,31 @@ JX.install('DiffChangesetList', {
['differential-inline-comment', 'tag:textarea'],
ondraft);
var on_preview_view = JX.bind(this, this._onPreviewEvent, 'view');
JX.Stratcom.listen(
'click',
'differential-inline-preview-jump',
on_preview_view);
},
_onPreviewEvent: function(action, e) {
if (this.isAsleep()) {
return;
}
var data = e.getNodeData('differential-inline-preview-jump');
var inline = this.getInlineByID(data.inlineCommentID);
if (!inline) {
return;
}
e.kill();
switch (action) {
case 'view':
this.selectInline(inline, true, true);
break;
}
},
_onInlineEvent: function(action, e) {
......
/**
* @provides javelin-behavior-diff-preview-link
* @requires javelin-behavior
* javelin-stratcom
* javelin-dom
*/
JX.behavior('diff-preview-link', function(config, statics) {
if (statics.initialized) {
return;
}
statics.initialized = true;
var pht = JX.phtize(config.pht);
// After inline comment previews are rendered, hook up the links to the
// comments that are visible on the current page.
function link_inline_preview(e) {
var root = e.getData().rootNode;
var links = JX.DOM.scry(root, 'a', 'differential-inline-preview-jump');
for (var ii = 0; ii < links.length; ii++) {
var data = JX.Stratcom.getData(links[ii]);
try {
JX.$(data.anchor);
links[ii].href = '#' + data.anchor;
JX.DOM.setContent(links[ii], pht('view'));
} catch (ignored) {
// This inline comment isn't visible, e.g. on some other diff.
}
}
}
JX.Stratcom.listen('EditEngine.didCommentPreview', null, link_inline_preview);
});
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment