Fix lit based diff to match legacy diff exactly (Step 2)
The legacy diff builder was adjusted to always rendern `thread-group`
<div>s with a slot instead of just doing that when a change of thread
DOM nodes was observed. The DOM observation has always been a bit of a
headache, and there is no indication that this optimization is required.
We may bring back some kind of optimization later, but for the migration
to Lit diff and its transition period let's just always render the
thread group divs and their slots.
Release-Notes: skip
Google-Bug-Id: b/237393560
Change-Id: I763bc9917f923fa1c0718d61ce12b5792cfb0817
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
index 8176e14..2c9f210 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-legacy.ts
@@ -396,6 +396,17 @@
} else if (line.beforeNumber === 'FILE') td.classList.add('file');
else if (line.beforeNumber === 'LOST') td.classList.add('lost');
+ if (side && line.lineNumber(side)) {
+ const lineNumber = line.lineNumber(side);
+ const threadGroupEl = document.createElement('div');
+ threadGroupEl.className = 'thread-group';
+ threadGroupEl.setAttribute('data-side', side);
+ const slot = document.createElement('slot');
+ slot.name = `${side}-${lineNumber}`;
+ threadGroupEl.appendChild(slot);
+ td.appendChild(threadGroupEl);
+ }
+
return td;
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index 6029e65..1b9cfcd 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -1410,23 +1410,6 @@
);
}
- /**
- * Gets or creates a comment thread group for a specific line and side on a
- * diff.
- * Private but used in tests.
- */
- getOrCreateThreadGroup(contentEl: Element, commentSide: Side) {
- // Check if thread group exists.
- let threadGroupEl = contentEl.querySelector('.thread-group');
- if (!threadGroupEl) {
- threadGroupEl = document.createElement('div');
- threadGroupEl.className = 'thread-group';
- threadGroupEl.setAttribute('data-side', commentSide);
- contentEl.appendChild(threadGroupEl);
- }
- return threadGroupEl;
- }
-
private getCommentSideByLineAndContent(
lineEl: Element,
contentEl: Element
@@ -1696,7 +1679,6 @@
if (lineNum === 'LOST') {
this.insertPortedCommentsWithoutRangeMessage(contentEl);
}
- const threadGroupEl = this.getOrCreateThreadGroup(contentEl, commentSide);
const slotAtt = threadEl.getAttribute('slot');
if (range && isLongCommentRange(range) && slotAtt) {
@@ -1709,16 +1691,6 @@
this.insertBefore(longRangeCommentHint, threadEl);
this.redispatchHoverEvents(longRangeCommentHint, threadEl);
}
-
- // Create a slot for the thread and attach it to the thread group.
- // The Polyfill has some bugs and this only works if the slot is
- // attached to the group after the group is attached to the DOM.
- // The thread group may already have a slot with the right name, but
- // that is okay because the first matching slot is used and the rest
- // are ignored.
- const slot = document.createElement('slot');
- if (slotAtt) slot.name = slotAtt;
- threadGroupEl.appendChild(slot);
}
for (const threadEl of removedThreadEls) {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
index fa15dcf..e969435 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -108,14 +108,14 @@
<td class="blame gr-diff" data-line-number="LOST"></td>
<td class="gr-diff left lineNum" data-value="LOST"></td>
<td class="gr-diff left no-intraline-info sign"></td>
- <td
- class="both content gr-diff left lost no-intraline-info"
- ></td>
+ <td class="both content gr-diff left lost no-intraline-info">
+ <div class="thread-group" data-side="left"></div>
+ </td>
<td class="gr-diff lineNum right" data-value="LOST"></td>
<td class="gr-diff no-intraline-info right sign"></td>
- <td
- class="both content gr-diff lost no-intraline-info right"
- ></td>
+ <td class="both content gr-diff lost no-intraline-info right">
+ <div class="thread-group" data-side="right"></div>
+ </td>
</tr>
</tbody>
<tbody class="both gr-diff section">
@@ -138,9 +138,9 @@
</button>
</td>
<td class="gr-diff left no-intraline-info sign"></td>
- <td
- class="both content file gr-diff left no-intraline-info"
- ></td>
+ <td class="both content file gr-diff left no-intraline-info">
+ <div class="thread-group" data-side="left"></div>
+ </td>
<td class="gr-diff lineNum right" data-value="FILE">
<button
aria-label="Add file comment"
@@ -153,9 +153,9 @@
</button>
</td>
<td class="gr-diff no-intraline-info right sign"></td>
- <td
- class="both content file gr-diff no-intraline-info right"
- ></td>
+ <td class="both content file gr-diff no-intraline-info right">
+ <div class="thread-group" data-side="right"></div>
+ </td>
</tr>
</tbody>
<tbody class="both gr-diff section">
@@ -185,6 +185,7 @@
data-side="left"
id="left-content-1"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="1">
<button
@@ -204,6 +205,7 @@
data-side="right"
id="right-content-1"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -232,6 +234,7 @@
data-side="left"
id="left-content-2"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="2">
<button
@@ -251,6 +254,7 @@
data-side="right"
id="right-content-2"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -279,6 +283,7 @@
data-side="left"
id="left-content-3"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="3">
<button
@@ -298,6 +303,7 @@
data-side="right"
id="right-content-3"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -326,6 +332,7 @@
data-side="left"
id="left-content-4"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="4">
<button
@@ -345,6 +352,7 @@
data-side="right"
id="right-content-4"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -384,6 +392,7 @@
data-side="right"
id="right-content-5"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -421,6 +430,7 @@
data-side="right"
id="right-content-6"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -458,6 +468,7 @@
data-side="right"
id="right-content-7"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -488,6 +499,7 @@
data-side="left"
id="left-content-5"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="8">
<button
@@ -507,6 +519,7 @@
data-side="right"
id="right-content-8"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -535,6 +548,7 @@
data-side="left"
id="left-content-6"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="9">
<button
@@ -554,6 +568,7 @@
data-side="right"
id="right-content-9"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -582,6 +597,7 @@
data-side="left"
id="left-content-7"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="10">
<button
@@ -601,6 +617,7 @@
data-side="right"
id="right-content-10"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -629,6 +646,7 @@
data-side="left"
id="left-content-8"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="11">
<button
@@ -648,6 +666,7 @@
data-side="right"
id="right-content-11"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -676,6 +695,7 @@
data-side="left"
id="left-content-9"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="12">
<button
@@ -695,6 +715,7 @@
data-side="right"
id="right-content-12"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -725,6 +746,7 @@
data-side="left"
id="left-content-10"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
@@ -762,6 +784,7 @@
data-side="left"
id="left-content-11"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
@@ -799,6 +822,7 @@
data-side="left"
id="left-content-12"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
@@ -836,6 +860,7 @@
data-side="left"
id="left-content-13"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="blankLineNum gr-diff right"></td>
<td class="blank gr-diff no-intraline-info right sign"></td>
@@ -875,6 +900,7 @@
data-side="left"
id="left-content-14"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="13">
<button
@@ -894,6 +920,7 @@
data-side="right"
id="right-content-13"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -922,6 +949,7 @@
data-side="left"
id="left-content-15"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="14">
<button
@@ -941,6 +969,7 @@
data-side="right"
id="right-content-14"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -971,6 +1000,7 @@
data-side="left"
id="left-content-16"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="15">
<button
@@ -990,6 +1020,7 @@
data-side="right"
id="right-content-15"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1020,6 +1051,7 @@
data-side="left"
id="left-content-17"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="16">
<button
@@ -1039,6 +1071,7 @@
data-side="right"
id="right-content-16"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1067,6 +1100,7 @@
data-side="left"
id="left-content-18"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="17">
<button
@@ -1086,6 +1120,7 @@
data-side="right"
id="right-content-17"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1114,6 +1149,7 @@
data-side="left"
id="left-content-19"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="18">
<button
@@ -1133,6 +1169,7 @@
data-side="right"
id="right-content-18"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1201,6 +1238,7 @@
data-side="left"
id="left-content-38"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="37">
<button
@@ -1220,6 +1258,7 @@
data-side="right"
id="right-content-37"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1248,6 +1287,7 @@
data-side="left"
id="left-content-39"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="38">
<button
@@ -1267,6 +1307,7 @@
data-side="right"
id="right-content-38"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1295,6 +1336,7 @@
data-side="left"
id="left-content-40"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="39">
<button
@@ -1314,6 +1356,7 @@
data-side="right"
id="right-content-39"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1353,6 +1396,7 @@
data-side="right"
id="right-content-40"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1390,6 +1434,7 @@
data-side="right"
id="right-content-41"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1427,6 +1472,7 @@
data-side="right"
id="right-content-42"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1464,6 +1510,7 @@
data-side="right"
id="right-content-43"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1494,6 +1541,7 @@
data-side="left"
id="left-content-41"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="44">
<button
@@ -1513,6 +1561,7 @@
data-side="right"
id="right-content-44"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1541,6 +1590,7 @@
data-side="left"
id="left-content-42"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="45">
<button
@@ -1560,6 +1610,7 @@
data-side="right"
id="right-content-45"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1588,6 +1639,7 @@
data-side="left"
id="left-content-43"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="46">
<button
@@ -1607,6 +1659,7 @@
data-side="right"
id="right-content-46"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1635,6 +1688,7 @@
data-side="left"
id="left-content-44"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="47">
<button
@@ -1654,6 +1708,7 @@
data-side="right"
id="right-content-47"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
<tr
@@ -1682,6 +1737,7 @@
data-side="left"
id="left-content-45"
></div>
+ <div class="thread-group" data-side="left"></div>
</td>
<td class="gr-diff lineNum right" data-value="48">
<button
@@ -1701,6 +1757,7 @@
data-side="right"
id="right-content-48"
></div>
+ <div class="thread-group" data-side="right"></div>
</td>
</tr>
</tbody>
@@ -1708,7 +1765,7 @@
</div>
`,
{
- ignoreTags: ['gr-legacy-text'],
+ ignoreTags: ['gr-legacy-text', 'slot'],
}
);
};
@@ -1856,25 +1913,6 @@
assert.isTrue(container.classList.contains('displayLine'));
});
- test('thread groups', () => {
- const contentEl = document.createElement('div');
-
- element.path = 'file.txt';
-
- // No thread groups.
- assert.equal(contentEl.querySelectorAll('.thread-group').length, 0);
-
- // A thread group gets created.
- const threadGroupEl = element.getOrCreateThreadGroup(
- contentEl,
- Side.LEFT
- );
- assert.isOk(threadGroupEl);
-
- // The new thread group can be fetched.
- assert.equal(contentEl.querySelectorAll('.thread-group').length, 1);
- });
-
suite('image diffs', () => {
let mockFile1: ImageInfo;
let mockFile2: ImageInfo;
