1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 05:50:55 +01: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 2012-05-04 17:41:06 -07:00
parent 1c5412ebe6
commit cd63d9b2ce
4 changed files with 87 additions and 56 deletions

View file

@ -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',

View file

@ -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.

View file

@ -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;
}

View file

@ -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);
}
}