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
This commit is contained in:
Ole Rehmsen
2020-04-24 15:24:29 +02:00
parent 58ad4b2a42
commit 56c2f1da4d
4 changed files with 24 additions and 19 deletions

View File

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

View File

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

View File

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

View File

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