feat: For changes in long tables do not print the entire table in the emailed diff (#4800)

This commit is contained in:
Tom Moor
2023-01-29 11:48:37 -08:00
committed by GitHub
parent 5473b698a4
commit 80a8f5b7e2
3 changed files with 139 additions and 63 deletions

View File

@@ -2,7 +2,8 @@ import Revision from "@server/models/Revision";
import DocumentHelper from "./DocumentHelper";
describe("DocumentHelper", () => {
test("toEmailDiff", async () => {
describe("toEmailDiff", () => {
it("should render a compact diff", async () => {
const before = new Revision({
title: "Title",
text: `
@@ -81,7 +82,40 @@ same on both sides`,
expect(html).not.toContain("this is a highlight");
});
test("toPlainText", async () => {
it("should trim table rows to show minimal diff including header", async () => {
const before = new Revision({
title: "Title",
text: `
| Syntax | Description |
| ----------- | ----------- |
| Header | Title |
| Paragraph | Text |
| Content | Another |
| More | Content |
| Long | Table |`,
});
const after = new Revision({
title: "Title",
text: `
| Syntax | Description |
| ----------- | ----------- |
| Header | Title |
| Paragraph | Text |
| Content | Changed |
| More | Content |
| Long | Table |`,
});
const html = await DocumentHelper.toEmailDiff(before, after);
expect(html).toContain("Changed");
expect(html).not.toContain("Long");
});
});
describe("toPlainText", () => {
it("should return only plain text", async () => {
const revision = new Revision({
title: "Title",
text: `
@@ -149,3 +183,4 @@ In a cell
`);
});
});
});

View File

@@ -124,7 +124,7 @@ export default class DocumentHelper {
{options?.includeTitle !== false && (
<h1 dir={rtl ? "rtl" : "ltr"}>{document.title}</h1>
)}
{options?.includeStyles ? (
{options?.includeStyles !== false ? (
<EditorContainer dir={rtl ? "rtl" : "ltr"} rtl={rtl}>
{content}
</EditorContainer>
@@ -278,9 +278,54 @@ export default class DocumentHelper {
previousNodeRemoved = false;
previousDiffClipped = true;
// Special case for largetables, as this block can get very large we
// want to clip it to only the changed rows and surrounding context.
if (childNode.classList.contains("table-wrapper")) {
const rows = childNode.querySelectorAll("tr");
if (rows.length < 3) {
continue;
}
let previousRowRemoved = false;
let previousRowDiffClipped = false;
for (const row of rows) {
if (containsDiffElement(row)) {
const cells = row.querySelectorAll("td");
if (previousRowRemoved && previousRowDiffClipped) {
const tr = doc.createElement("tr");
const br = doc.createElement("td");
br.colSpan = cells.length;
br.innerHTML = "…";
br.className = "diff-context-break";
tr.appendChild(br);
childNode.parentElement?.insertBefore(tr, childNode);
}
previousRowRemoved = false;
previousRowDiffClipped = true;
continue;
}
if (containsDiffElement(row.nextElementSibling)) {
previousRowRemoved = false;
continue;
}
if (containsDiffElement(row.previousElementSibling)) {
previousRowRemoved = false;
continue;
}
previousRowRemoved = true;
row.remove();
}
}
continue;
}
// If the block node does not contain a diff tag and the previous
// block node did not contain a diff tag then remove the previous.
} else {
if (
childNode.nodeName === "P" &&
childNode.textContent &&
@@ -288,10 +333,7 @@ export default class DocumentHelper {
containsDiffElement(childNode.nextElementSibling)
) {
if (previousDiffClipped) {
childNode.parentElement?.insertBefore(
br.cloneNode(true),
childNode
);
childNode.parentElement?.insertBefore(br.cloneNode(true), childNode);
}
previousNodeRemoved = false;
continue;
@@ -308,7 +350,6 @@ export default class DocumentHelper {
previousNodeRemoved = true;
childNode.remove();
}
}
const head = doc.querySelector("head");
const body = doc.querySelector("body");

View File

@@ -43,7 +43,7 @@ export default class Table extends Node {
toDOM() {
return [
"div",
{ class: "scrollable-wrapper" },
{ class: "scrollable-wrapper table-wrapper" },
[
"div",
{ class: "scrollable" },