fix: Improve patch merging of links in table cells (#12614)

* fix: Improve merging of links in table cells through patch

* PR feedback
This commit is contained in:
Tom Moor
2026-06-07 12:09:44 -04:00
committed by GitHub
parent bc63aba1d1
commit 0d198294eb
2 changed files with 90 additions and 2 deletions
+78
View File
@@ -3,6 +3,7 @@ import * as Y from "yjs";
import { TextEditMode } from "@shared/types";
import { APIUpdateExtension } from "@server/collaboration/APIUpdateExtension";
import { Event } from "@server/models";
import { parser } from "@server/editor";
import { DocumentHelper } from "@server/models/helpers/DocumentHelper";
import { ProsemirrorHelper } from "@server/models/helpers/ProsemirrorHelper";
import { buildDocument, buildUser } from "@server/test/factories";
@@ -1481,5 +1482,82 @@ describe("documentUpdater", () => {
"Second item"
);
});
it("should apply a link when wrapping existing table cell text", async () => {
const user = await buildUser();
const document = await buildDocument({ teamId: user.teamId });
document.content = parser
.parse("| Name | Notes |\n|------|-------|\n| Alpha | see |\n")
.toJSON();
await document.save();
const result = DocumentHelper.applyMarkdownToDocument(
document,
"[see](https://example.com/docs)",
TextEditMode.Patch,
"see"
);
// The cell text is unchanged but should now carry a link mark — it must
// not be silently dropped during the merge.
const cellText =
result.content!.content![0].content![1].content![1].content![0]
.content![0];
expect(cellText.text).toEqual("see");
expect(cellText.marks).toEqual([
expect.objectContaining({
type: "link",
attrs: expect.objectContaining({ href: "https://example.com/docs" }),
}),
]);
});
it("should preserve other table cells when adding a link to one cell", async () => {
const user = await buildUser();
const document = await buildDocument({ teamId: user.teamId });
document.content = parser
.parse(
"| Name | Notes |\n|------|-------|\n| Alpha | see |\n| Beta | other |\n"
)
.toJSON();
await document.save();
// Capture the untouched (Beta) row BEFORE patching so we can assert its
// structure and attrs are preserved exactly, not just its text.
const beforeDoc = DocumentHelper.toProsemirror(document).toJSON();
const untouchedRow = beforeDoc.content[0].content[2];
const result = DocumentHelper.applyMarkdownToDocument(
document,
"see [docs](https://example.com/d)",
TextEditMode.Patch,
"see"
);
// The patched cell gained the link
expect(result.text).toContain("[docs](https://example.com/d)");
// The untouched row node must remain identical
expect(result.content!.content![0].content![2]).toEqual(untouchedRow);
});
it("should apply a mark when wrapping existing list item text", async () => {
const user = await buildUser();
const document = await buildDocument({ teamId: user.teamId });
document.content = parser.parse("- clickme\n- other\n").toJSON();
await document.save();
const result = DocumentHelper.applyMarkdownToDocument(
document,
"[clickme](https://example.com)",
TextEditMode.Patch,
"clickme"
);
expect(result.text).toContain("[clickme](https://example.com)");
expect(result.text).toContain("other");
});
});
});
+12 -2
View File
@@ -817,8 +817,18 @@ export class DocumentHelper {
const textSame = oldChild.textContent === newChild.textContent;
if (textSame && oldChild.sameMarkup(newChild)) {
// Fully unchanged — keep original with its rich content
merged.push(oldChild);
// Compare against the round-tripped baseline: when the
// updated child is identical to a plain round-trip of the original,
// the patch did not touch it
if (!rtChild || newChild.eq(rtChild)) {
merged.push(oldChild);
} else if (!oldChild.isTextblock && !oldChild.isLeaf) {
// Container child changed deeper down — recurse to preserve rich
// content in the parts that did not change.
merged.push(DocumentHelper.mergeNodes(oldChild, newChild, rtChild));
} else {
merged.push(newChild);
}
} else if (textSame) {
// Attrs changed (e.g. checked state) but content same — merge attrs
// so that non-markdown-representable values (colwidth, highlight