Commit e959f934 authored by epriestley's avatar epriestley
Browse files

Use a more consistent inline highlighting style with fewer redraws

Summary:
Ref T13513. The on-hover-inline reticle has switched over to have cell-based behavior. Switch the on-hover-line-number reticle to use the same behavior.

Also, clean up the dirty/redraw loop slightly: we no longer need to dirty on resize, and we don't need to redraw if the range isn't actually dirty.

Test Plan: Highlighted lines and line ranges. Hovered over inlines.

Maniphest Tasks: T13513

Differential Revision: https://secure.phabricator.com/D21262
parent c666cb9f
......@@ -9,11 +9,11 @@ return array(
'names' => array(
'conpherence.pkg.css' => '0e3cf785',
'conpherence.pkg.js' => '020aebcf',
'core.pkg.css' => 'a560707d',
'core.pkg.css' => 'ba768cdb',
'core.pkg.js' => '845355f4',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => '319dca29',
'differential.pkg.js' => 'bb2a17fc',
'differential.pkg.css' => '42a2334f',
'differential.pkg.js' => '623b4801',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => 'a98c0bf7',
'maniphest.pkg.css' => '35995d6d',
......@@ -63,7 +63,7 @@ return array(
'rsrc/css/application/diff/diff-tree-view.css' => 'e2d3e222',
'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d',
'rsrc/css/application/differential/add-comment.css' => '7e5900d9',
'rsrc/css/application/differential/changeset-view.css' => 'df3afa61',
'rsrc/css/application/differential/changeset-view.css' => '60c3d405',
'rsrc/css/application/differential/core.css' => '7300a73e',
'rsrc/css/application/differential/phui-inline-comment.css' => 'd5749acc',
'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d',
......@@ -116,7 +116,7 @@ return array(
'rsrc/css/core/core.css' => '1b29ed61',
'rsrc/css/core/remarkup.css' => 'c286eaef',
'rsrc/css/core/syntax.css' => '548567f6',
'rsrc/css/core/z-index.css' => '612e9522',
'rsrc/css/core/z-index.css' => 'ac3bfcd4',
'rsrc/css/diviner/diviner-shared.css' => '4bd263b0',
'rsrc/css/font/font-awesome.css' => '3883938a',
'rsrc/css/font/font-lato.css' => '23631304',
......@@ -379,8 +379,8 @@ return array(
'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be',
'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' => 'e02670b5',
'rsrc/js/application/diff/DiffChangesetList.js' => 'b1b8500b',
'rsrc/js/application/diff/DiffChangeset.js' => '68d963eb',
'rsrc/js/application/diff/DiffChangesetList.js' => 'ac403c32',
'rsrc/js/application/diff/DiffInline.js' => 'b00168c1',
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
......@@ -559,7 +559,7 @@ return array(
'conpherence-transaction-css' => '3a3f5e7e',
'd3' => '9d068042',
'diff-tree-view-css' => 'e2d3e222',
'differential-changeset-view-css' => 'df3afa61',
'differential-changeset-view-css' => '60c3d405',
'differential-core-view-css' => '7300a73e',
'differential-revision-add-comment-css' => '7e5900d9',
'differential-revision-comment-css' => '7dbc8d1d',
......@@ -774,8 +774,8 @@ return array(
'phabricator-darklog' => '3b869402',
'phabricator-darkmessage' => '26cd4b73',
'phabricator-dashboard-css' => '5a205b9d',
'phabricator-diff-changeset' => 'e02670b5',
'phabricator-diff-changeset-list' => 'b1b8500b',
'phabricator-diff-changeset' => '68d963eb',
'phabricator-diff-changeset-list' => 'ac403c32',
'phabricator-diff-inline' => 'b00168c1',
'phabricator-diff-path-view' => '8207abf9',
'phabricator-diff-tree-view' => '5d83623b',
......@@ -806,7 +806,7 @@ return array(
'phabricator-title' => '43bc9360',
'phabricator-tooltip' => '83754533',
'phabricator-ui-example-css' => 'b4795059',
'phabricator-zindex-css' => '612e9522',
'phabricator-zindex-css' => 'ac3bfcd4',
'phame-css' => 'bb442327',
'pholio-css' => '88ef5ef1',
'pholio-edit-css' => '4df55b3b',
......@@ -1487,6 +1487,9 @@ return array(
'5faf27b9' => array(
'phuix-form-control-view',
),
'60c3d405' => array(
'phui-inline-comment-view-css',
),
'60cd9241' => array(
'javelin-behavior',
),
......@@ -1509,6 +1512,19 @@ return array(
'javelin-install',
'javelin-dom',
),
'68d963eb' => array(
'javelin-dom',
'javelin-util',
'javelin-stratcom',
'javelin-install',
'javelin-workflow',
'javelin-router',
'javelin-behavior-device',
'javelin-vector',
'phabricator-diff-inline',
'phabricator-diff-path-view',
'phuix-button-view',
),
'6a1583a8' => array(
'javelin-behavior',
'javelin-history',
......@@ -1895,6 +1911,11 @@ return array(
'javelin-dom',
'phabricator-notification',
),
'ac403c32' => array(
'javelin-install',
'phuix-button-view',
'phabricator-diff-tree-view',
),
'ad258e28' => array(
'javelin-behavior',
'javelin-dom',
......@@ -1927,11 +1948,6 @@ return array(
'javelin-stratcom',
'javelin-dom',
),
'b1b8500b' => array(
'javelin-install',
'phuix-button-view',
'phabricator-diff-tree-view',
),
'b26a41e4' => array(
'javelin-behavior',
'javelin-stratcom',
......@@ -2108,22 +2124,6 @@ return array(
'javelin-uri',
'phabricator-notification',
),
'df3afa61' => array(
'phui-inline-comment-view-css',
),
'e02670b5' => array(
'javelin-dom',
'javelin-util',
'javelin-stratcom',
'javelin-install',
'javelin-workflow',
'javelin-router',
'javelin-behavior-device',
'javelin-vector',
'phabricator-diff-inline',
'phabricator-diff-path-view',
'phuix-button-view',
),
'e150bd50' => array(
'javelin-behavior',
'javelin-stratcom',
......
......@@ -329,17 +329,6 @@ td.cov-I {
margin-right: 8px;
}
.differential-reticle {
background-color: {$sh-yellowbackground};
border: 1px solid {$sh-yellowborder};
position: absolute;
opacity: 0.5;
top: 0;
left: 0;
box-sizing: border-box;
pointer-events: none;
}
.differential-loading {
border-top: 1px solid {$gentle.highlight.border};
border-bottom: 1px solid {$gentle.highlight.border};
......@@ -496,6 +485,10 @@ unselectable. */
background: {$highlight.bright};
}
.differential-diff tr td.n.inline-hover {
background: {$yellow};
}
.inline-hover-container {
position: absolute;
color: {$lightgreytext};
......@@ -511,3 +504,7 @@ unselectable. */
color: {$blacktext};
background: {$highlight.bright};
}
.differential-diff td.inline-hover span.bright {
background: transparent;
}
......@@ -16,10 +16,6 @@
z-index: 2;
}
.differential-reticle {
z-index: 2;
}
.differential-changeset {
z-index: 2;
}
......
......@@ -44,7 +44,7 @@ JX.install('DiffChangeset', {
this._isOwned = data.isOwned;
this._isLoading = true;
this._inlines = [];
this._inlines = null;
if (data.changesetState) {
this._loadChangesetState(data.changesetState);
......
......@@ -87,7 +87,6 @@ JX.install('DiffChangesetList', {
_focusStart: null,
_focusEnd: null,
_hoverNode: null,
_hoverInline: null,
_hoverOrigin: null,
_hoverTarget: null,
......@@ -1127,7 +1126,6 @@ JX.install('DiffChangesetList', {
_onresize: function() {
this._redrawFocus();
this._redrawSelection();
this._redrawHover();
// Force a banner redraw after a resize event. Particularly, this makes
// sure the inline state updates immediately after an inline edit
......@@ -1280,11 +1278,8 @@ JX.install('DiffChangesetList', {
},
_setHoverInline: function(inline) {
if (inline && (this._hoverInline === inline)) {
return;
}
this._hoverInline = inline;
var origin = null;
var target = null;
if (inline) {
var changeset = inline.getChangeset();
......@@ -1310,11 +1305,8 @@ JX.install('DiffChangesetList', {
var length = inline.getLineLength();
try {
var origin = JX.$(prefix + number);
var target = JX.$(prefix + (number + length));
this._hoverOrigin = origin;
this._hoverTarget = target;
origin = JX.$(prefix + number);
target = JX.$(prefix + (number + length));
} catch (error) {
// There may not be any nodes present in the document. A case where
// this occurs is when you reply to a ghost inline which was made
......@@ -1322,52 +1314,46 @@ JX.install('DiffChangesetList', {
// the file was later shortened so those lines no longer exist. For
// more details, see T11662.
this._hoverOrigin = null;
this._hoverTarget = null;
origin = null;
target = null;
}
} else {
this._hoverOrigin = null;
this._hoverTarget = null;
}
this._redrawHover();
this._setHoverRange(origin, target, inline);
},
_setHoverRange: function(origin, target) {
this._hoverOrigin = origin;
this._hoverTarget = target;
_setHoverRange: function(origin, target, inline) {
inline = inline || null;
var origin_dirty = (origin !== this._hoverOrigin);
var target_dirty = (target !== this._hoverTarget);
var inline_dirty = (inline !== this._hoverInline);
this._redrawHover();
var any_dirty = (origin_dirty || target_dirty || inline_dirty);
if (any_dirty) {
this._hoverOrigin = origin;
this._hoverTarget = target;
this._hoverInline = inline;
this._redrawHover();
}
},
resetHover: function() {
this._setHoverInline(null);
this._hoverOrigin = null;
this._hoverTarget = null;
this._setHoverRange(null, null, null);
},
_redrawHover: function() {
var ii;
var map = this._hoverMap;
if (map) {
for (ii = 0; ii < map.length; ii++) {
JX.DOM.alterClass(map[ii].cellNode, 'inline-hover', false);
if (map[ii].bright) {
JX.DOM.alterClass(map[ii].cellNode, 'inline-hover-bright', false);
}
if (map[ii].hoverNode) {
JX.DOM.remove(map[ii].hoverNode);
}
}
this._hoverMap = null;
this._applyHoverHighlight(map, false);
}
var reticle = this._getHoverNode();
JX.DOM.remove(reticle);
var rows = this._hoverRows;
if (rows) {
this._hoverRows = null;
this._applyHoverHighlight(rows, false);
}
if (!this._hoverOrigin || this.isAsleep()) {
return;
......@@ -1394,52 +1380,47 @@ JX.install('DiffChangesetList', {
return;
}
rows = this._findContentCells(top, bot, content_cell);
var inline = this._hoverInline;
if (!inline) {
var pos = JX.$V(content_cell)
.add(JX.Vector.getAggregateScrollForNode(content_cell));
var dim = JX.$V(content_cell)
.add(JX.Vector.getAggregateScrollForNode(content_cell))
.add(-pos.x, -pos.y)
.add(JX.Vector.getDim(content_cell));
var bpos = JX.$V(bot)
.add(JX.Vector.getAggregateScrollForNode(bot));
dim.y = (bpos.y - pos.y) + JX.Vector.getDim(bot).y;
pos.setPos(reticle);
dim.setDim(reticle);
JX.DOM.getContentFrame().appendChild(reticle);
JX.DOM.show(reticle);
this._hoverRows = rows;
this._applyHoverHighlight(this._hoverRows, true);
return;
}
if (!inline.hoverMap) {
inline.hoverMap = this._newHoverMap(top, bot, content_cell, inline);
inline.hoverMap = this._newHoverMap(rows, inline);
}
map = inline.hoverMap;
for (ii = 0; ii < map.length; ii++) {
JX.DOM.alterClass(map[ii].cellNode, 'inline-hover', true);
if (map[ii].bright) {
JX.DOM.alterClass(map[ii].cellNode, 'inline-hover-bright', true);
this._hoverMap = inline.hoverMap;
this._applyHoverHighlight(this._hoverMap, true);
},
_applyHoverHighlight: function(items, on) {
for (var ii = 0; ii < items.length; ii++) {
var item = items[ii];
JX.DOM.alterClass(item.lineNode, 'inline-hover', on);
JX.DOM.alterClass(item.cellNode, 'inline-hover', on);
if (item.bright) {
JX.DOM.alterClass(item.cellNode, 'inline-hover-bright', on);
}
if (map[ii].hoverNode) {
map[ii].cellNode.insertBefore(
map[ii].hoverNode,
map[ii].cellNode.firstChild);
if (item.hoverNode) {
if (on) {
item.cellNode.insertBefore(
item.hoverNode,
item.cellNode.firstChild);
} else {
JX.DOM.remove(item.hoverNode);
}
}
}
this._hoverMap = map;
},
_newHoverMap: function(top, bot, content_cell, inline) {
var start = inline.getStartOffset();
var end = inline.getEndOffset();
_findContentCells: function(top, bot, content_cell) {
var head_row = JX.DOM.findAbove(top, 'tr');
var last_row = JX.DOM.findAbove(bot, 'tr');
......@@ -1447,25 +1428,35 @@ JX.install('DiffChangesetList', {
var rows = [];
var idx = null;
var ii;
var line_cell = null;
do {
line_cell = null;
for (ii = 0; ii < cursor.childNodes.length; ii++) {
var child = cursor.childNodes[ii];
if (!JX.DOM.isType(child, 'td')) {
continue;
}
if (child.getAttribute('data-n')) {
line_cell = child;
}
if (child === content_cell) {
idx = ii;
}
if (ii === idx) {
if (!this._isContentCell(child)) {
break;
}
if (ii !== idx) {
continue;
}
if (this._isContentCell(child)) {
rows.push({
lineNode: line_cell,
cellNode: child
});
}
break;
}
if (cursor === last_row) {
......@@ -1475,6 +1466,13 @@ JX.install('DiffChangesetList', {
cursor = cursor.nextSibling;
} while (cursor);
return rows;
},
_newHoverMap: function(rows, inline) {
var start = inline.getStartOffset();
var end = inline.getEndOffset();
var info;
var content;
for (ii = 0; ii < rows.length; ii++) {
......@@ -1574,17 +1572,6 @@ JX.install('DiffChangesetList', {
return rows;
},
_getHoverNode: function() {
if (!this._hoverNode) {
var attributes = {
className: 'differential-reticle'
};
this._hoverNode = JX.$N('div', attributes);
}
return this._hoverNode;
},
_deleteInlineByID: function(id) {
var uri = this.getInlineURI();
var data = {
......
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