opendev/gerrit
Code Issues Proposed changes

Fix selection on unified view and coverage style

Browse Source 
This was broken by change 264294
as lineNumButton is inside of lineNum, the `user-select` should
still on lineNum (the `td` element) instead of the button.

Same happens to coverage, lineNum is the lineElement, coverage
class was added to it instead of the button inside.

Added a regression test on `user-select` for diff line.

Change-Id: Ie40fa3ce9451fe58c3c4c2b5b14bfe13d61bceab
This commit is contained in:
Tao Zhou
2020-04-29 10:48:43 +02:00
parent beb7449273
commit 96b5fa47c4
2 changed files with 45 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.js
View File

@@ -298,13 +298,13 @@ export const htmlTemplate = html`
.newlineWarning.hidden {
display: none;
}
.lineNumButton.COVERED {
.lineNum.COVERED .lineNumButton {
background-color: var(--coverage-covered, #e0f2f1);
}
.lineNumButton.NOT_COVERED {
.lineNum.NOT_COVERED .lineNumButton {
background-color: var(--coverage-not-covered, #ffd1a4);
}
.lineNumButton.PARTIALLY_COVERED {
.lineNum.PARTIALLY_COVERED .lineNumButton {
background: linear-gradient(
to right bottom,
var(--coverage-not-covered, #ffd1a4) 0%,
@@ -337,12 +337,12 @@ export const htmlTemplate = html`
.contentText,
.selected-left:not(.selected-comment)
.unified
.left.lineNumButton
.left.lineNum
~ .content:not(.both)
.contentText,
.selected-right:not(.selected-comment)
.unified
.right.lineNumButton
.right.lineNum
~ .content
.contentText,
.selected-left.selected-comment .side-by-side .left + .content .message,

40
polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
View File

@@ -1013,6 +1013,46 @@ suite('gr-diff tests', () => {
assertDiffTableWithContent();
});
suite('selection test', () => {
test('user-select set correctly on side-by-side view', () => {
const content = [{
a: ['all work and no play make andybons a dull boy'],
b: ['elgoog elgoog elgoog'],
}, {
ab: [
'Non eram nescius, Brute, cum, quae summis ingeniis ',
'exquisitaque doctrina philosophi Graeco sermone tractavissent',
],
}];
setupSampleDiff({content});
flushAsynchronousOperations();
const diffLine = element.shadowRoot.querySelectorAll('.contentText')[2];
assert.equal(getComputedStyle(diffLine).userSelect, 'none');
// click to mark it as selected
MockInteractions.tap(diffLine);
assert.equal(getComputedStyle(diffLine).userSelect, 'text');
});
test('user-select set correctly on unified view', () => {
const content = [{
a: ['all work and no play make andybons a dull boy'],
b: ['elgoog elgoog elgoog'],
}, {
ab: [
'Non eram nescius, Brute, cum, quae summis ingeniis ',
'exquisitaque doctrina philosophi Graeco sermone tractavissent',
],
}];
setupSampleDiff({content});
element.viewMode = 'UNIFIED_DIFF';
flushAsynchronousOperations();
const diffLine = element.shadowRoot.querySelectorAll('.contentText')[2];
assert.equal(getComputedStyle(diffLine).userSelect, 'none');
MockInteractions.tap(diffLine);
assert.equal(getComputedStyle(diffLine).userSelect, 'text');
});
});
suite('whitespace changes only message', () => {
test('show the message if ignore_whitespace is criteria matches', () => {
setupSampleDiff({content: [{skip: 100}]});