mirror of
https://we.phorge.it/source/phorge.git
synced 2024-09-20 09:18:48 +02:00
Use subtler highlighting for copied and moved code
Summary: The highlighting is distracting according to Nick Shrock and others. Real designer, Lee Byron, helped me with this. It also gives us unagressive target for jumping to the source line in future. Another feature I will probably implement is highlighting also the source of copies/moves. I will use the right side of the left column for it. Test Plan: Hover copied notifier. Hover coverage notifier. I've also checked that this doesn't break our super-flaky old/new code JavaScript detector. Reviewers: epriestley Reviewed By: epriestley CC: aran, Koolvin, leebyron, schrockn Differential Revision: https://secure.phabricator.com/D2403
This commit is contained in:
vrana
parent
1c5412ebe6
commit
cd63d9b2ce
4 changed files with 87 additions and 56 deletions
|
|
@ -538,7 +538,7 @@ celerity_register_resource_map(array(
|
|||
),
|
||||
'differential-changeset-view-css' =>
|
||||
array(
|
||||
'uri' => '/res/4ce438cd/rsrc/css/application/differential/changeset-view.css',
|
||||
'uri' => '/res/8e2ace51/rsrc/css/application/differential/changeset-view.css',
|
||||
'type' => 'css',
|
||||
'requires' =>
|
||||
array(
|
||||
|
|
@ -969,7 +969,7 @@ celerity_register_resource_map(array(
|
|||
),
|
||||
'javelin-behavior-differential-populate' =>
|
||||
array(
|
||||
'uri' => '/res/c0979571/rsrc/js/application/differential/behavior-populate.js',
|
||||
'uri' => '/res/a955bf2c/rsrc/js/application/differential/behavior-populate.js',
|
||||
'type' => 'js',
|
||||
'requires' =>
|
||||
array(
|
||||
|
|
@ -2510,7 +2510,7 @@ celerity_register_resource_map(array(
|
|||
'uri' => '/res/pkg/0c96375e/core.pkg.js',
|
||||
'type' => 'js',
|
||||
),
|
||||
'2debe0e0' =>
|
||||
'd9299c35' =>
|
||||
array(
|
||||
'name' => 'differential.pkg.css',
|
||||
'symbols' =>
|
||||
|
|
@ -2529,10 +2529,10 @@ celerity_register_resource_map(array(
|
|||
11 => 'differential-local-commits-view-css',
|
||||
12 => 'inline-comment-summary-css',
|
||||
),
|
||||
'uri' => '/res/pkg/2debe0e0/differential.pkg.css',
|
||||
'uri' => '/res/pkg/d9299c35/differential.pkg.css',
|
||||
'type' => 'css',
|
||||
),
|
||||
'5b7b36d7' =>
|
||||
'dc7ca445' =>
|
||||
array(
|
||||
'name' => 'differential.pkg.js',
|
||||
'symbols' =>
|
||||
|
|
@ -2556,7 +2556,7 @@ celerity_register_resource_map(array(
|
|||
16 => 'javelin-behavior-differential-dropdown-menus',
|
||||
17 => 'javelin-behavior-buoyant',
|
||||
),
|
||||
'uri' => '/res/pkg/5b7b36d7/differential.pkg.js',
|
||||
'uri' => '/res/pkg/dc7ca445/differential.pkg.js',
|
||||
'type' => 'js',
|
||||
),
|
||||
'c8ce2d88' =>
|
||||
|
|
@ -2652,7 +2652,7 @@ celerity_register_resource_map(array(
|
|||
'aphront-dialog-view-css' => '2b054c5c',
|
||||
'aphront-error-view-css' => '2b054c5c',
|
||||
'aphront-form-view-css' => '2b054c5c',
|
||||
'aphront-headsup-action-list-view-css' => '2debe0e0',
|
||||
'aphront-headsup-action-list-view-css' => 'd9299c35',
|
||||
'aphront-headsup-view-css' => '2b054c5c',
|
||||
'aphront-list-filter-view-css' => '2b054c5c',
|
||||
'aphront-pager-view-css' => '2b054c5c',
|
||||
|
|
@ -2662,36 +2662,36 @@ celerity_register_resource_map(array(
|
|||
'aphront-tokenizer-control-css' => '2b054c5c',
|
||||
'aphront-tooltip-css' => '2b054c5c',
|
||||
'aphront-typeahead-control-css' => '2b054c5c',
|
||||
'differential-changeset-view-css' => '2debe0e0',
|
||||
'differential-core-view-css' => '2debe0e0',
|
||||
'differential-inline-comment-editor' => '5b7b36d7',
|
||||
'differential-local-commits-view-css' => '2debe0e0',
|
||||
'differential-results-table-css' => '2debe0e0',
|
||||
'differential-revision-add-comment-css' => '2debe0e0',
|
||||
'differential-revision-comment-css' => '2debe0e0',
|
||||
'differential-revision-comment-list-css' => '2debe0e0',
|
||||
'differential-revision-history-css' => '2debe0e0',
|
||||
'differential-table-of-contents-css' => '2debe0e0',
|
||||
'differential-changeset-view-css' => 'd9299c35',
|
||||
'differential-core-view-css' => 'd9299c35',
|
||||
'differential-inline-comment-editor' => 'dc7ca445',
|
||||
'differential-local-commits-view-css' => 'd9299c35',
|
||||
'differential-results-table-css' => 'd9299c35',
|
||||
'differential-revision-add-comment-css' => 'd9299c35',
|
||||
'differential-revision-comment-css' => 'd9299c35',
|
||||
'differential-revision-comment-list-css' => 'd9299c35',
|
||||
'differential-revision-history-css' => 'd9299c35',
|
||||
'differential-table-of-contents-css' => 'd9299c35',
|
||||
'diffusion-commit-view-css' => 'c8ce2d88',
|
||||
'diffusion-icons-css' => 'c8ce2d88',
|
||||
'inline-comment-summary-css' => '2debe0e0',
|
||||
'inline-comment-summary-css' => 'd9299c35',
|
||||
'javelin-behavior' => '8a5de8a3',
|
||||
'javelin-behavior-aphront-basic-tokenizer' => '97f65640',
|
||||
'javelin-behavior-aphront-drag-and-drop' => '5b7b36d7',
|
||||
'javelin-behavior-aphront-drag-and-drop-textarea' => '5b7b36d7',
|
||||
'javelin-behavior-aphront-drag-and-drop' => 'dc7ca445',
|
||||
'javelin-behavior-aphront-drag-and-drop-textarea' => 'dc7ca445',
|
||||
'javelin-behavior-aphront-form-disable-on-submit' => '0c96375e',
|
||||
'javelin-behavior-audit-preview' => '5e68be89',
|
||||
'javelin-behavior-buoyant' => '5b7b36d7',
|
||||
'javelin-behavior-differential-accept-with-errors' => '5b7b36d7',
|
||||
'javelin-behavior-differential-add-reviewers-and-ccs' => '5b7b36d7',
|
||||
'javelin-behavior-differential-comment-jump' => '5b7b36d7',
|
||||
'javelin-behavior-differential-diff-radios' => '5b7b36d7',
|
||||
'javelin-behavior-differential-dropdown-menus' => '5b7b36d7',
|
||||
'javelin-behavior-differential-edit-inline-comments' => '5b7b36d7',
|
||||
'javelin-behavior-differential-feedback-preview' => '5b7b36d7',
|
||||
'javelin-behavior-differential-keyboard-navigation' => '5b7b36d7',
|
||||
'javelin-behavior-differential-populate' => '5b7b36d7',
|
||||
'javelin-behavior-differential-show-more' => '5b7b36d7',
|
||||
'javelin-behavior-buoyant' => 'dc7ca445',
|
||||
'javelin-behavior-differential-accept-with-errors' => 'dc7ca445',
|
||||
'javelin-behavior-differential-add-reviewers-and-ccs' => 'dc7ca445',
|
||||
'javelin-behavior-differential-comment-jump' => 'dc7ca445',
|
||||
'javelin-behavior-differential-diff-radios' => 'dc7ca445',
|
||||
'javelin-behavior-differential-dropdown-menus' => 'dc7ca445',
|
||||
'javelin-behavior-differential-edit-inline-comments' => 'dc7ca445',
|
||||
'javelin-behavior-differential-feedback-preview' => 'dc7ca445',
|
||||
'javelin-behavior-differential-keyboard-navigation' => 'dc7ca445',
|
||||
'javelin-behavior-differential-populate' => 'dc7ca445',
|
||||
'javelin-behavior-differential-show-more' => 'dc7ca445',
|
||||
'javelin-behavior-diffusion-commit-graph' => '5e68be89',
|
||||
'javelin-behavior-diffusion-pull-lastmodified' => '5e68be89',
|
||||
'javelin-behavior-maniphest-batch-selector' => '7707de41',
|
||||
|
|
@ -2701,12 +2701,12 @@ celerity_register_resource_map(array(
|
|||
'javelin-behavior-maniphest-transaction-preview' => '7707de41',
|
||||
'javelin-behavior-phabricator-autofocus' => '0c96375e',
|
||||
'javelin-behavior-phabricator-keyboard-shortcuts' => '0c96375e',
|
||||
'javelin-behavior-phabricator-object-selector' => '5b7b36d7',
|
||||
'javelin-behavior-phabricator-object-selector' => 'dc7ca445',
|
||||
'javelin-behavior-phabricator-oncopy' => '0c96375e',
|
||||
'javelin-behavior-phabricator-tooltips' => '0c96375e',
|
||||
'javelin-behavior-phabricator-watch-anchor' => '0c96375e',
|
||||
'javelin-behavior-refresh-csrf' => '0c96375e',
|
||||
'javelin-behavior-repository-crossreference' => '5b7b36d7',
|
||||
'javelin-behavior-repository-crossreference' => 'dc7ca445',
|
||||
'javelin-behavior-workflow' => '0c96375e',
|
||||
'javelin-dom' => '8a5de8a3',
|
||||
'javelin-event' => '8a5de8a3',
|
||||
|
|
@ -2728,23 +2728,23 @@ celerity_register_resource_map(array(
|
|||
'maniphest-task-summary-css' => '7839ae2d',
|
||||
'maniphest-transaction-detail-css' => '7839ae2d',
|
||||
'phabricator-app-buttons-css' => '2b054c5c',
|
||||
'phabricator-content-source-view-css' => '2debe0e0',
|
||||
'phabricator-content-source-view-css' => 'd9299c35',
|
||||
'phabricator-core-buttons-css' => '2b054c5c',
|
||||
'phabricator-core-css' => '2b054c5c',
|
||||
'phabricator-directory-css' => '2b054c5c',
|
||||
'phabricator-drag-and-drop-file-upload' => '5b7b36d7',
|
||||
'phabricator-drag-and-drop-file-upload' => 'dc7ca445',
|
||||
'phabricator-dropdown-menu' => '0c96375e',
|
||||
'phabricator-flag-css' => '2b054c5c',
|
||||
'phabricator-jump-nav' => '2b054c5c',
|
||||
'phabricator-keyboard-shortcut' => '0c96375e',
|
||||
'phabricator-keyboard-shortcut-manager' => '0c96375e',
|
||||
'phabricator-menu-item' => '0c96375e',
|
||||
'phabricator-object-selector-css' => '2debe0e0',
|
||||
'phabricator-object-selector-css' => 'd9299c35',
|
||||
'phabricator-paste-file-upload' => '0c96375e',
|
||||
'phabricator-prefab' => '0c96375e',
|
||||
'phabricator-project-tag-css' => '7839ae2d',
|
||||
'phabricator-remarkup-css' => '2b054c5c',
|
||||
'phabricator-shaped-request' => '5b7b36d7',
|
||||
'phabricator-shaped-request' => 'dc7ca445',
|
||||
'phabricator-standard-page-view' => '2b054c5c',
|
||||
'phabricator-tooltip' => '0c96375e',
|
||||
'phabricator-transaction-view-css' => '2b054c5c',
|
||||
|
|
|
|||
|
|
@ -1396,6 +1396,7 @@ final class DifferentialChangesetParser {
|
|||
$o_attr = null;
|
||||
}
|
||||
|
||||
$n_copy = '<td class="copy"></td>';
|
||||
|
||||
if (isset($this->new[$ii])) {
|
||||
$n_num = $this->new[$ii]['line'];
|
||||
|
|
@ -1417,8 +1418,17 @@ final class DifferentialChangesetParser {
|
|||
if ($this->new[$ii]['type']) {
|
||||
if ($this->new[$ii]['type'] == '\\') {
|
||||
$n_text = $this->new[$ii]['text'];
|
||||
$n_attr = ' class="comment"';
|
||||
} else if (isset($copy_lines[$n_num])) {
|
||||
$n_class = 'comment';
|
||||
} else if (empty($this->old[$ii])) {
|
||||
$n_class = 'new new-full';
|
||||
} else {
|
||||
$n_class = 'new';
|
||||
}
|
||||
$n_attr = ' class="'.$n_class.'"';
|
||||
|
||||
if ($this->new[$ii]['type'] == '\\' || !isset($copy_lines[$n_num])) {
|
||||
$n_copy = '<td class="copy '.$n_class.'"></td>';
|
||||
} else {
|
||||
list($orig_file, $orig_line, $orig_type) = $copy_lines[$n_num];
|
||||
$title = ($orig_type == '-' ? 'Moved' : 'Copied').' from ';
|
||||
if ($orig_file == '') {
|
||||
|
|
@ -1430,12 +1440,15 @@ final class DifferentialChangesetParser {
|
|||
dirname('/'.$orig_file);
|
||||
}
|
||||
$class = ($orig_type == '-' ? 'new-move' : 'new-copy');
|
||||
$n_attr =
|
||||
' class="'.$class.'" title="'.phutil_escape_html($title).'"';
|
||||
} else if (empty($this->old[$ii])) {
|
||||
$n_attr = ' class="new new-full"';
|
||||
} else {
|
||||
$n_attr = ' class="new"';
|
||||
$n_copy = javelin_render_tag(
|
||||
'td',
|
||||
array(
|
||||
'meta' => array(
|
||||
'msg' => $title,
|
||||
),
|
||||
'class' => 'copy '.$class,
|
||||
),
|
||||
'');
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
|
@ -1470,6 +1483,7 @@ final class DifferentialChangesetParser {
|
|||
'<th'.$o_id.'>'.$o_num.'</th>'.
|
||||
'<td'.$o_attr.'>'.$o_text.'</td>'.
|
||||
'<th'.$n_id.'>'.$n_num.'</th>'.
|
||||
$n_copy.
|
||||
// NOTE: This is a unicode zero-width space, which we use as a hint
|
||||
// when intercepting 'copy' events to make sure sensible text ends
|
||||
// up on the clipboard. See the 'phabricator-oncopy' behavior.
|
||||
|
|
|
|||
|
|
@ -80,19 +80,26 @@
|
|||
background: #ffaaaa;
|
||||
}
|
||||
|
||||
.differential-diff td.new-copy {
|
||||
background: #ffffaa;
|
||||
}
|
||||
|
||||
.differential-diff td.new-move {
|
||||
background: #ffddaa;
|
||||
}
|
||||
|
||||
.differential-diff td.new-full,
|
||||
.differential-diff td.new span.bright {
|
||||
background: #aaffaa;
|
||||
}
|
||||
|
||||
.differential-diff td.copy {
|
||||
width: 6px;
|
||||
padding: 0;
|
||||
}
|
||||
|
||||
.differential-diff td.new-copy,
|
||||
.differential-diff td.new-copy span.bright {
|
||||
background: #ffffaa;
|
||||
}
|
||||
|
||||
.differential-diff td.new-move,
|
||||
.differential-diff td.new-move span.bright {
|
||||
background: #eeee44;
|
||||
}
|
||||
|
||||
.differential-diff td.comment {
|
||||
background: #dddddd;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -65,7 +65,7 @@ JX.behavior('differential-populate', function(config) {
|
|||
|
||||
// NOTE: Using className is not best practice, but the diff UI is perf
|
||||
// sensitive.
|
||||
if (!t.className.match(/cov/)) {
|
||||
if (!t.className.match(/cov|copy/)) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -78,6 +78,8 @@ JX.behavior('differential-populate', function(config) {
|
|||
} else {
|
||||
highlight_class = null;
|
||||
var msg;
|
||||
var align = 'E';
|
||||
var sibling = 'previousSibling';
|
||||
if (t.className.match(/cov-C/)) {
|
||||
msg = 'Covered';
|
||||
highlight_class = 'source-cov-C';
|
||||
|
|
@ -87,14 +89,22 @@ JX.behavior('differential-populate', function(config) {
|
|||
} else if (t.className.match(/cov-N/)) {
|
||||
msg = 'Not Executable';
|
||||
highlight_class = 'source-cov-N';
|
||||
} else {
|
||||
var match = /new-copy|new-move/.exec(t.className);
|
||||
if (match) {
|
||||
align = 'N'; // TODO: 'W'
|
||||
sibling = 'nextSibling';
|
||||
msg = JX.Stratcom.getData(t).msg;
|
||||
highlight_class = match[0];
|
||||
}
|
||||
}
|
||||
|
||||
if (msg) {
|
||||
JX.Tooltip.show(t, 120, 'E', msg);
|
||||
JX.Tooltip.show(t, 120, align, msg);
|
||||
}
|
||||
|
||||
if (highlight_class) {
|
||||
highlighted = t.previousSibling;
|
||||
highlighted = t[sibling];
|
||||
JX.DOM.alterClass(highlighted, highlight_class, true);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue