archive/phabricator
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Make line selection in source code views less fragile and more consistent

Browse Source 
Summary:
Depends on D19347. Ref T13105. See PHI565. The "highlight lines" behavior is interacting poorly with the new blame element in Diffusion.

Make the behavior a little simpler and hopefully more robust.

Test Plan:
  - Clicked commit/revision links in Diffusion, saw the links get followed instead of the lines highlighted.
  - Highlighted lines in Diffusion, saw just the line/code highlight instead of the whole thing.
  - Highlighted lines in Paste and new-style Harbormaster build logs, saw consistent behavior.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19348
This commit is contained in:
epriestley
2018-04-11 14:12:40 -07:00
parent ac570fd4bc
commit c5c53e277a
3 changed files with 25 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
webroot/rsrc/js/core/behavior-line-linker.js
View File

@@ -19,24 +19,7 @@ JX.behavior('phabricator-line-linker', function() {
// Ignore.
}
function getRowNumber(tr) {
// Starting from the left, find the rightmost "<th />" tag among all
// "<th />" tags at the start of the row. Our goal here is to skip over
// blame information in Diffusion. This could probably be significantly
// more graceful.
var th = null;
for (var ii = 0; ii < tr.childNodes.length; ii++) {
if (JX.DOM.isType(tr.childNodes[ii], 'th')) {
th = tr.childNodes[ii];
continue;
}
break;
}
if (!th) {
return null;
}
function getRowNumber(th) {
// If the "<th />" tag contains an "<a />" with "data-n" that we're using
// to prevent copy/paste of line numbers, use that.
if (th.firstChild) {
@@ -51,7 +34,7 @@ JX.behavior('phabricator-line-linker', function() {
JX.Stratcom.listen(
['click', 'mousedown'],
['phabricator-source', 'tag:tr', 'tag:th', 'tag:a'],
['phabricator-source', 'tag:th', 'tag:a'],
function(e) {
if (!e.isNormalMouseEvent()) {
return;
@@ -62,13 +45,13 @@ JX.behavior('phabricator-line-linker', function() {
// table. The row's immediate ancestor table needs to be the table with
// the "phabricator-source" sigil.
var row = e.getNode('tag:tr');
var cell = e.getNode('tag:th');
var table = e.getNode('phabricator-source');
if (JX.DOM.findAbove(row, 'table') !== table) {
if (JX.DOM.findAbove(cell, 'table') !== table) {
return;
}
var number = getRowNumber(row);
var number = getRowNumber(cell);
if (!number) {
return;
}
@@ -81,7 +64,7 @@ JX.behavior('phabricator-line-linker', function() {
return;
}
origin = row;
origin = cell;
target = origin;
root = table;
@@ -95,7 +78,7 @@ JX.behavior('phabricator-line-linker', function() {
if (e.getNode('phabricator-source') !== root) {
return;
}
target = e.getNode('tag:tr');
target = e.getNode('tag:th');
var min;
var max;
@@ -130,6 +113,9 @@ JX.behavior('phabricator-line-linker', function() {
highlighted = [];
// Highlight the newly selected rows.
min = JX.DOM.findAbove(min, 'tr');
max = JX.DOM.findAbove(max, 'tr');
var cursor = min;
while (true) {
JX.DOM.alterClass(cursor, 'phabricator-source-highlight', true);