From 56c2f1da4d6601bcff97eebe27df9c202c966fc0 Mon Sep 17 00:00:00 2001 From: Ole Rehmsen Date: Fri, 24 Apr 2020 15:24:29 +0200 Subject: [PATCH] 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 --- .../gr-diff-builder-element_test.html | 4 +-- .../diff/gr-diff-builder/gr-diff-builder.js | 3 +- .../app/elements/diff/gr-diff/gr-diff.js | 3 +- .../app/elements/diff/gr-diff/gr-diff_html.js | 33 ++++++++++--------- 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),