Fix colspan of context controls
Submitting change 390315 revealed that counting the colspan for the
context control section was not accurate at all. It did not take the
`hide_left_side` render preference into account. So showing context
control rows messed up the column layout, see referenced issue.
Release-Notes: skip
Google-Bug-Id: b/307980964
Change-Id: Iab6001b8625b0b2aaa153f39dce93003d553012b
diff --git a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section.ts b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section.ts
index 394d09b..6231cc0 100644
--- a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section.ts
+++ b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section.ts
@@ -101,18 +101,44 @@
return this.viewMode !== DiffViewMode.UNIFIED;
}
+ /**
+ * The context control table cell should span all the columns, but not the blame column.
+ * The tricky bit is to figure out, which of the 6 other table columns are actually shown or not.
+ */
+ private computeColspan() {
+ const hideLeft = !!this.renderPrefs?.hide_left_side;
+ const showSign = !!this.renderPrefs?.show_sign_col;
+ const unified = this.viewMode === DiffViewMode.UNIFIED;
+
+ // Hiding the left side in unified diff mode does not make a lot of sense and is not supported.
+ const showLeftNumberCol = !hideLeft || unified;
+ const showLeftSignCol = !hideLeft && showSign && !unified;
+ const showLeftContentCol = !hideLeft && !unified;
+ const showRightNumberCol = true;
+ const showRightSignCol = showSign && !unified;
+ const showRightContentCol = true;
+
+ const shownCols = [
+ showLeftNumberCol,
+ showLeftSignCol,
+ showLeftContentCol,
+ showRightNumberCol,
+ showRightSignCol,
+ showRightContentCol,
+ ];
+ const colspan = shownCols.filter(show => show).length;
+ return colspan;
+ }
+
private createContextControlRow() {
- // Note that <td> table cells that have `display: none` don't count!
- const colspan = this.renderPrefs?.show_sign_col ? '5' : '3';
const showConfig = getShowConfig(this.showAbove, this.showBelow);
return html`
<tr class=${diffClasses('dividerRow', `show-${showConfig}`)}>
<td class=${diffClasses('blame')} data-line-number="0"></td>
- ${when(
- this.isSideBySide(),
- () => html`<td class=${diffClasses()}></td>`
- )}
- <td class=${diffClasses('dividerCell')} colspan=${colspan}>
+ <td
+ class=${diffClasses('dividerCell')}
+ colspan=${this.computeColspan()}
+ >
<gr-context-controls
class=${diffClasses()}
.diff=${this.diff}
diff --git a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
index 0962de7..6d2b72c 100644
--- a/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-context-controls/gr-context-controls-section_test.ts
@@ -56,8 +56,7 @@
</tr>
<tr class="dividerRow gr-diff show-both">
<td class="blame gr-diff" data-line-number="0"></td>
- <td class="gr-diff"></td>
- <td class="dividerCell gr-diff" colspan="3">
+ <td class="dividerCell gr-diff" colspan="4">
<gr-context-controls class="gr-diff" showconfig="both">
</gr-context-controls>
</td>
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-element_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-element_test.ts
index c6194c5..732e7f5 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-element_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-element_test.ts
@@ -2358,8 +2358,7 @@
</tr>
<tr class="dividerRow gr-diff show-both">
<td class="blame gr-diff" data-line-number="0"></td>
- <td class="gr-diff"></td>
- <td class="dividerCell gr-diff" colspan="3">
+ <td class="dividerCell gr-diff" colspan="4">
<gr-context-controls
class="gr-diff"
showconfig="both"
