summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOle Rehmsen <oler@google.com>2020-04-24 15:24:29 +0200
committerOle Rehmsen <oler@google.com>2020-04-24 16:36:07 +0200
commit56c2f1da4d6601bcff97eebe27df9c202c966fc0 (patch)
tree4a29902bf238263ce194cfe37a218e717d8fd8e8
parent58ad4b2a426e6e6938773fd129916640d3558867 (diff)
Fix line number padding and size
Separate the CSS classes for the line number td and button. Move all styling to the button and make it take the full size of the td, so that the entire line number area is clickable and a button. Change-Id: Icd7078afa0d71c3066c665c6e3d9b7f442b916fe
-rw-r--r--polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html4
-rw-r--r--polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js3
-rw-r--r--polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js3
-rw-r--r--polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.js33
4 files changed, 24 insertions, 19 deletions
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html
index f4a8073262..dabf884d1c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.html
@@ -916,7 +916,7 @@ suite('gr-diff-builder tests', () => {
test('aria-labels on added line numbers', () => {
const deltaLineNumberButton = element.diffElement.querySelectorAll(
- 'button.lineNum.right')[5];
+ '.lineNumButton.right')[5];
assert.isOk(deltaLineNumberButton);
assert.equal(deltaLineNumberButton.getAttribute('aria-label'), '5 added');
@@ -924,7 +924,7 @@ suite('gr-diff-builder tests', () => {
test('aria-labels on removed line numbers', () => {
const deltaLineNumberButton = element.diffElement.querySelectorAll(
- 'button.lineNum.left')[10];
+ '.lineNumButton.left')[10];
assert.isOk(deltaLineNumberButton);
assert.equal(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 7b7ea53c20..4b910004d0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -308,7 +308,9 @@ GrDiffBuilder.prototype._createLineEl = function(
// Both td and button need a number of classes/attributes for various
// selectors to work.
this._decorateLineEl(td, number, side);
+ td.classList.add('lineNum');
this._decorateLineEl(button, number, side);
+ button.classList.add('lineNumButton');
button.textContent = number === 'FILE' ? 'File' : number;
@@ -330,7 +332,6 @@ GrDiffBuilder.prototype._createLineEl = function(
GrDiffBuilder.prototype._decorateLineEl = function(el, number, side) {
el.classList.add(side);
- el.classList.add('lineNum');
el.dataset.value = number;
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 8da366a33f..a5eb233f3a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -492,7 +492,8 @@ class GrDiff extends mixinBehaviors( [
composed: true, bubbles: true,
}));
this.$.diffBuilder.showContext(e.detail.groups, e.detail.section);
- } else if (el.classList.contains('lineNum')) {
+ } else if (el.classList.contains('lineNum') ||
+ el.classList.contains('lineNumButton')) {
this.addDraftAtLine(el);
} else if (el.tagName === 'HL' ||
el.classList.contains('content') ||
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.js
index 7d6af0b704..1ec8175cc9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.js
@@ -49,14 +49,17 @@ export const htmlTemplate = html`
border-right: 1px solid var(--border-color);
table-layout: fixed;
}
- .lineNum {
+ .lineNumButton {
+ display: block;
+ width: 100%;
+ height: 100%;
background-color: var(--diff-blank-background-color);
}
/*
The only way to focus this (clicking) will apply our own focus styling,
so this default styling is not needed and distracting.
*/
- button.lineNum:focus {
+ .lineNumButton:focus {
outline: none;
}
.image-diff .gr-diff {
@@ -66,7 +69,7 @@ export const htmlTemplate = html`
box-shadow: var(--elevation-level-1);
max-width: 50em;
}
- .image-diff .right.lineNum {
+ .image-diff .right.lineNumButton {
border-left: 1px solid var(--border-color);
}
.image-diff label,
@@ -78,9 +81,9 @@ export const htmlTemplate = html`
outline: none;
user-select: none;
}
- .diff-row.target-row.target-side-left .lineNum.left,
- .diff-row.target-row.target-side-right .lineNum.right,
- .diff-row.target-row.unified .lineNum {
+ .diff-row.target-row.target-side-left .lineNumButton.left,
+ .diff-row.target-row.target-side-right .lineNumButton.right,
+ .diff-row.target-row.unified .lineNumButton {
background-color: var(--diff-selection-background-color);
color: var(--primary-text-color);
}
@@ -103,13 +106,13 @@ export const htmlTemplate = html`
white-space: pre-wrap;
word-wrap: break-word;
}
- .lineNum,
+ .lineNumButton,
.content {
vertical-align: top;
white-space: pre;
}
.contextLineNum,
- .lineNum {
+ .lineNumButton {
-webkit-user-select: none;
-moz-user-select: none;
-ms-user-select: none;
@@ -119,7 +122,7 @@ export const htmlTemplate = html`
padding: 0 var(--spacing-m);
text-align: right;
}
- .canComment .lineNum {
+ .canComment .lineNumButton {
cursor: pointer;
}
.content {
@@ -198,7 +201,7 @@ export const htmlTemplate = html`
width: var(--line-height-mono, 18px);
height: var(--line-height-mono, 18px);
}
- .contextControl td:not(.lineNum) {
+ .contextControl td:not(.lineNumButton) {
text-align: center;
}
.displayLine .diff-row.target-row td {
@@ -295,13 +298,13 @@ export const htmlTemplate = html`
.newlineWarning.hidden {
display: none;
}
- .lineNum.COVERED {
+ .lineNumButton.COVERED {
background-color: var(--coverage-covered, #e0f2f1);
}
- .lineNum.NOT_COVERED {
+ .lineNumButton.NOT_COVERED {
background-color: var(--coverage-not-covered, #ffd1a4);
}
- .lineNum.PARTIALLY_COVERED {
+ .lineNumButton.PARTIALLY_COVERED {
background: linear-gradient(to right bottom, var(--coverage-not-covered, #ffd1a4) 0%,
var(--coverage-not-covered, #ffd1a4) 50%,
var(--coverage-covered, #e0f2f1) 50%,
@@ -321,8 +324,8 @@ export const htmlTemplate = html`
.selected-left:not(.selected-comment) .side-by-side .left + .content .contentText,
.selected-right:not(.selected-comment) .side-by-side .right + .content .contentText,
- .selected-left:not(.selected-comment) .unified .left.lineNum ~ .content:not(.both) .contentText,
- .selected-right:not(.selected-comment) .unified .right.lineNum ~ .content .contentText,
+ .selected-left:not(.selected-comment) .unified .left.lineNumButton ~ .content:not(.both) .contentText,
+ .selected-right:not(.selected-comment) .unified .right.lineNumButton ~ .content .contentText,
.selected-left.selected-comment .side-by-side .left + .content .message,
.selected-right.selected-comment .side-by-side .right + .content .message :not(.collapsedContent),
.selected-comment .unified .message :not(.collapsedContent),