fix comments in diff mode

This commit is contained in:
swithek 2026-09-19 18:05:37 +02:00
parent 556ca8fa01
commit db3dbc37de
7 changed files with 1390 additions and 4 deletions

View File

@ -199,7 +199,15 @@ cheaper tier can:
is rendered inside the paragraph, so a plain text read splices the other
user's name into the content.
- **The title and the body are two editors** (`titleEditor()`,
`contentEditor()`); Enter in the title moves into the body.
`contentEditor()`); Enter in the title moves into the body. With "show
changes" on, the body is a third, `diffEditor()`, and everything an
editor renders outside its ProseMirror element (handle, bubble menu,
comment popover) is reached through its pane, `contentPane()` or
`diffPane()`.
- **Block handles come through `openBlockMenu()`**, which hovers the block
until the handle shows: the handle stays away while the pointer is level
with any `drag-handle-ignore` element in the editor, such as a parameter
separator in the other column of a split block.
### Independence & concurrency
@ -228,10 +236,27 @@ cheaper tier can:
- **`documentPersisted()` is the one permitted `waitForTimeout`**:
hocuspocus stores two seconds after the last change and nothing reports
it.
- **The editor's selection lags the browser's caret.** A click or an arrow
key moves the caret at once; tiptap learns of it from the queued
`selectionchange` event. A key tiptap handles itself (Enter, Backspace)
or a click handler that reads the selection, sent straight after, acts
on the old one. Place and extend selections through `placeCaret()` and
`selectText()`, and open a text comment through `openTextComment()`,
which all wait for the editor to catch up. Never `Home` or `End`: macOS
chromium binds them to the document, not the line.
- Three waits are raised above the default (post-signup redirect,
post-login redirect, cold document load), all cross-service chains. Any
other raised timeout carries a comment saying what is slow.
### A test that fails after a code change
- **Never edit a test just to make it pass again.** First decide whether
the code or the test is wrong. A failure caused by anything other than an
intended change is a bug the test caught; fix the code.
- Change a test only where the behaviour it asserts changed on purpose,
and change only those assertions. Name that intended change when
reporting the edit.
### Failure artifacts
`trace: "on-first-retry"`, `screenshot: "only-on-failure"`; `test-results/`

View File

@ -1,5 +1,5 @@
import { expect, type Locator, type Page } from "@playwright/test"
import { contentEditor } from "./editor"
import { contentEditor, openBlockMenu, selectionSettled } from "./editor"
import { t } from "./i18n"
// commentPopover is the floating comment-thread panel. It is rendered
@ -9,6 +9,13 @@ export function commentPopover(page: Page): Locator {
return page.locator(".content-editor div.z-popover")
}
// paneCommentPopover is the same panel found through the pane it belongs
// to, for a test that drives the draft's diff view, where a second
// renderer owns the popover.
export function paneCommentPopover(pane: Locator): Locator {
return pane.locator("div.z-popover")
}
// commentComposer is the tiptap instance inside the popover that new
// comment text is typed into. Saved comment bodies are read-only tiptap
// instances in the same popover, so the editable one is the composer.
@ -46,3 +53,116 @@ export async function addComment(page: Page, text: string): Promise<void> {
),
).toBeVisible()
}
// commentOnSelection files a comment on the text already selected in the
// pane's editor and returns the id the server gave it. The popover is
// left open.
export async function commentOnSelection(
page: Page,
pane: Locator,
text: string,
): Promise<string> {
await pane
.locator("[data-visible]")
.getByRole("button", { name: t("editor.bubble-menu.comment-label") })
.click()
await submitNewComment(page, pane, text)
return savedCommentId(pane, "[data-comment-id]")
}
// commentOnBlock files a comment on a whole block through its block
// handle and returns the id the server gave it. The popover is left open.
export async function commentOnBlock(
page: Page,
pane: Locator,
block: Locator,
text: string,
): Promise<string> {
await openBlockMenu(page, pane, block)
await page
.getByRole("menuitem", {
name: t("editor.drag-handle.options.add-node-comment"),
})
.click()
await submitNewComment(page, pane, text)
// the commented node and the highlight drawn over it both carry the
// id, so the highlight alone is read
return savedCommentId(pane, ".node-comment-overlay[data-node-comment-id]")
}
// openTextComment clicks a text comment's highlight to open its thread.
// The thread opens on click and reads the editor's selection, which the
// editor takes from the browser's caret only once the selectionchange
// event has fired. A human click is slow enough for that; a synthetic
// one is not, so the button is held down until the editor has caught up.
// A highlight over changed text is several spans, so the one to click
// is named by the text it covers.
export async function openTextComment(
page: Page,
pane: Locator,
id: string,
text: string,
): Promise<void> {
const span = pane.locator(`[data-comment-id="${id}"]`, { hasText: text })
await span.hover()
await page.mouse.down()
await selectionSettled(span)
await page.mouse.up()
}
// commentedText is what a text comment highlights in the pane's editor,
// read across every span the highlight is split into. A highlight over
// changed text is split wherever the diff marks change.
export function commentedText(pane: Locator, id: string): Promise<string> {
return pane
.locator(`[data-comment-id="${id}"]`)
.evaluateAll((spans) => spans.map((span) => span.textContent).join(""))
}
async function submitNewComment(
page: Page,
pane: Locator,
text: string,
): Promise<void> {
await paneCommentPopover(pane)
.locator('.ProseMirror[contenteditable="true"]')
.click()
await page.keyboard.type(text)
await paneCommentPopover(pane)
.getByRole("button", {
name: t("editor.comment-thread.comment-button"),
exact: true,
})
.click()
}
// savedCommentId waits until the pending highlight has been traded for
// the server's id — the proof the comment was saved — and returns that
// id. A highlight can be several elements; they all carry the one id.
async function savedCommentId(
pane: Locator,
selector: string,
): Promise<string> {
await expect(pane.locator('[data-comment-id^="pending-"]')).toHaveCount(0)
await expect(pane.locator('[data-node-comment-id^="pending-"]')).toHaveCount(
0,
)
const ids = await pane
.locator(selector)
.evaluateAll((elements) => [
...new Set(
elements.map(
(el) =>
el.getAttribute("data-comment-id") ??
el.getAttribute("data-node-comment-id"),
),
),
])
expect(ids).toHaveLength(1)
return ids[0] ?? ""
}

View File

@ -13,6 +13,170 @@ export function contentEditor(page: Page): Locator {
return page.locator(".content-editor .ProseMirror")
}
// diffEditor is the merged read-only editor a draft shows while "show
// changes" is on. It is the one editor that hides its caret, which is
// also what tells it apart from the comment composer mounted in the same
// pane.
export function diffEditor(page: Page): Locator {
return page.locator(".diff-editor .ProseMirror.caret-transparent")
}
// the panes wrapping each editor together with its block handle, bubble
// menu and comment popover. Anything the editor owns but renders outside
// its ProseMirror element is found through the pane.
export function contentPane(page: Page): Locator {
return page.locator(".content-editor")
}
export function diffPane(page: Page): Locator {
return page.locator(".diff-editor")
}
// placeCaret clicks just inside the left or right edge of a block, which
// maps to the first or last position of its line. The block is a
// full-width element, so its edge sits well past its text, and the click
// is retried until focus sticks: a late re-render of a freshly switched
// branch can swallow the first one. Home and End are not used because on
// macOS chromium binds them to the document, not the line.
export async function placeCaret(
block: Locator,
edge: "start" | "end",
): Promise<void> {
await expect(async () => {
const box = await block.boundingBox()
expect(box).not.toBeNull()
await block.click({
position: {
x: edge === "start" ? 1 : (box?.width ?? 2) - 1,
y: (box?.height ?? 2) / 2,
},
})
await expect(
block.locator("xpath=ancestor::*[contains(@class, 'ProseMirror')]"),
).toBeFocused()
}).toPass()
await selectionSettled(block)
}
// selectText selects one run of a block's text with the keyboard, walking
// to it from the block's start. A mouse drag would depend on where each
// glyph is drawn.
export async function selectText(
page: Page,
block: Locator,
text: string,
): Promise<void> {
const content = await editorText(block)
const start = content.indexOf(text)
if (start === -1) {
throw new Error(`"${text}" is not in the block reading "${content}"`)
}
await placeCaret(block, "start")
await pressTimes(page, "ArrowRight", start)
await pressTimes(page, "Shift+ArrowRight", text.length)
await selectionSettled(block)
}
// the slice of tiptap's editor, reachable from its element, that
// selectionSettled reads
interface EditorHandle {
state: { selection: { from: number; to: number } }
view: { posAtDOM(node: Node, offset: number): number }
}
// selectionSettled waits until the editor's own selection matches the
// browser's. A click or an arrow key moves the browser's caret at once,
// but the editor only learns of it from the selectionchange event, which
// queues behind the input that caused it. A key the editor handles
// itself, such as Enter or Backspace, sent before then acts on the old
// selection.
export async function selectionSettled(block: Locator): Promise<void> {
await expect
.poll(() =>
block.evaluate((el) => {
const root = el.closest<Element & { editor?: EditorHandle }>(
".ProseMirror",
)
const editor = root?.editor
const selection = document.getSelection()
if (!editor || !selection?.anchorNode || !selection.focusNode) {
return false
}
try {
const anchor = editor.view.posAtDOM(
selection.anchorNode,
selection.anchorOffset,
)
const focus = editor.view.posAtDOM(
selection.focusNode,
selection.focusOffset,
)
const { from, to } = editor.state.selection
return (
Math.min(anchor, focus) === from && Math.max(anchor, focus) === to
)
} catch {
return false
}
}),
)
.toBe(true)
}
async function pressTimes(
page: Page,
key: string,
times: number,
): Promise<void> {
for (let pressed = 0; pressed < times; pressed += 1) {
await page.keyboard.press(key)
}
}
// the heights, as fractions of a block, at which openBlockMenu hovers it
const HOVER_HEIGHTS = [0.5, 0.25, 0.75, 0.1, 0.9]
// openBlockMenu opens the block handle's menu for a block. The handle
// slides in beside whichever block the pointer is over, so the block is
// hovered first, at more than one height if need be: the handle stays
// away while the pointer is level with an element that opts out of it,
// such as a parameter separator in the other column of a split block.
// On an editable branch the handle carries two triggers, hooks and block
// actions, told apart by the icon each one shows.
export async function openBlockMenu(
page: Page,
pane: Locator,
block: Locator,
): Promise<Locator> {
const trigger = pane.locator(
'.z-drag-handle [data-slot="dropdown-menu-trigger"]',
{ has: page.locator('[class*="dots-line"]') },
)
let attempt = 0
await expect(async () => {
const box = await block.boundingBox()
expect(box).not.toBeNull()
const height = HOVER_HEIGHTS[attempt % HOVER_HEIGHTS.length] ?? 0.5
attempt += 1
await block.hover({
position: { x: (box?.width ?? 2) / 2, y: (box?.height ?? 2) * height },
})
await expect(trigger).toBeVisible({ timeout: 1_000 })
}).toPass()
await trigger.click()
const menu = page.getByRole("menu")
await expect(menu).toBeVisible()
return menu
}
// readModeToggle is the navbar button switching the page between read and
// edit mode. Its accessible name is the mode the page is in.
export function readModeToggle(page: Page): Locator {

View File

@ -1,5 +1,5 @@
import { expect, type Locator, type Page } from "@playwright/test"
import { contentEditor, waitForEditor } from "./editor"
import { contentEditor, diffEditor, waitForEditor } from "./editor"
import { t } from "./i18n"
// the two branches a reviewable page carries. They are the branch names
@ -69,6 +69,22 @@ export async function switchToBranch(
}
}
// showChanges turns the draft's "show changes" switch on and waits for the
// merged diff editor it mounts over the draft. The diff needs the main
// branch synced as well, so on a fresh page the editor takes a moment.
export async function showChanges(page: Page): Promise<void> {
await page.locator("#show-diff").click()
await expect(diffEditor(page)).toBeVisible({ timeout: 15_000 })
}
// hideChanges turns the switch off again and waits for the draft editor
// to be back in view.
export async function hideChanges(page: Page): Promise<void> {
await page.locator("#show-diff").click()
await expect(diffEditor(page)).toHaveCount(0)
await expect(contentEditor(page)).toBeVisible()
}
// reviewActions is the split button carrying the draft's approve and
// merge actions. It only renders while the draft branch is open.
export function reviewActions(page: Page): Locator {

View File

@ -0,0 +1,919 @@
import {
expect,
test,
type APIRequestContext,
type Locator,
type Page,
} from "@playwright/test"
import {
commentedText,
commentOnBlock,
commentOnSelection,
openTextComment,
paneCommentPopover,
} from "../helpers/comments"
import { joinAsSecondUser } from "../helpers/collaboration"
import {
contentEditor,
contentPane,
createDocument,
diffEditor,
diffPane,
documentPersisted,
editorText,
openBlockMenu,
openSlashMenu,
placeCaret,
selectText,
waitForEditor,
} from "../helpers/editor"
import { t } from "../helpers/i18n"
import { visit } from "../helpers/page"
import {
hideChanges,
makeReviewable,
showChanges,
switchToBranch,
} from "../helpers/review"
import { signUpWithWorkspace } from "../helpers/workspace"
test.describe("comments on draft changes", () => {
// every test builds a page, makes it reviewable, edits the draft and
// waits for the edits to be stored before the first assertion. slow()
// triples the budget for the whole flow.
test.beforeEach(() => {
test.slow()
})
test("files a comment on an added paragraph", async ({ page, request }) => {
await draftWithParagraphChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedParagraph(page),
"Is this paragraph needed?",
)
await expect(
paneCommentPopover(diffPane(page)).getByText("Is this paragraph needed?"),
).toBeVisible()
await expect(
diffEditor(page).locator(`p[data-node-comment-id="${id}"]`),
).toHaveText("Added paragraph")
})
test("shows the added paragraph's comment on the draft once changes are hidden", async ({
page,
request,
}) => {
await draftWithParagraphChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedParagraph(page),
"Reads well",
)
await hideChanges(page)
await expect(
contentEditor(page).locator(`p[data-node-comment-id="${id}"]`),
).toHaveText("Added paragraph")
await showChanges(page)
await expect(
diffEditor(page).locator(`p[data-node-comment-id="${id}"]`),
).toHaveText("Added paragraph")
})
test("keeps the added paragraph's comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithParagraphChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedParagraph(page),
"Still here later",
)
await documentPersisted(page)
await reopenDraftChanges(page, url)
await diffEditor(page).locator(`p[data-node-comment-id="${id}"]`).click()
await expect(
paneCommentPopover(diffPane(page)).getByText("Still here later"),
).toBeVisible()
})
test("files a comment on a removed paragraph", async ({ page, request }) => {
await draftWithParagraphChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
removedParagraph(page),
"Why was this dropped?",
)
await expect(
paneCommentPopover(diffPane(page)).getByText("Why was this dropped?"),
).toBeVisible()
await expect(
diffEditor(page).locator(`p[data-node-comment-id="${id}"]`),
).toHaveText("Doomed paragraph")
})
test("keeps the removed paragraph's comment when changes are hidden and shown again", async ({
page,
request,
}) => {
await draftWithParagraphChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
removedParagraph(page),
"Bring it back",
)
await hideChanges(page)
await showChanges(page)
await diffEditor(page).locator(`p[data-node-comment-id="${id}"]`).click()
await expect(
paneCommentPopover(diffPane(page)).getByText("Bring it back"),
).toBeVisible()
})
test("keeps the removed paragraph's comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithParagraphChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
removedParagraph(page),
"Gone but discussed",
)
await reopenDraftChanges(page, url)
await diffEditor(page).locator(`p[data-node-comment-id="${id}"]`).click()
await expect(
paneCommentPopover(diffPane(page)).getByText("Gone but discussed"),
).toBeVisible()
})
test("files a comment on added text", async ({ page, request }) => {
await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "delta")
const id = await commentOnSelection(page, diffPane(page), "Prefer gamma")
await expect(
paneCommentPopover(diffPane(page)).getByText("Prefer gamma"),
).toBeVisible()
await expect.poll(() => commentedText(diffPane(page), id)).toBe("delta")
})
test("shows the added text's comment on the draft once changes are hidden", async ({
page,
request,
}) => {
await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "delta")
const id = await commentOnSelection(page, diffPane(page), "Typo?")
await hideChanges(page)
await expect.poll(() => commentedText(contentPane(page), id)).toBe("delta")
await showChanges(page)
await expect.poll(() => commentedText(diffPane(page), id)).toBe("delta")
})
test("keeps the added text's comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "delta")
const id = await commentOnSelection(page, diffPane(page), "Keep delta")
await documentPersisted(page)
await reopenDraftChanges(page, url)
await expect.poll(() => commentedText(diffPane(page), id)).toBe("delta")
await openTextComment(page, diffPane(page), id, "delta")
await expect(
paneCommentPopover(diffPane(page)).getByText("Keep delta"),
).toBeVisible()
})
test("files a comment on removed text", async ({ page, request }) => {
await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "beta")
const id = await commentOnSelection(page, diffPane(page), "Beta was fine")
await expect(
paneCommentPopover(diffPane(page)).getByText("Beta was fine"),
).toBeVisible()
await expect.poll(() => commentedText(diffPane(page), id)).toBe("beta")
})
test("keeps the removed text's comment when changes are hidden and shown again", async ({
page,
request,
}) => {
await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "beta")
const id = await commentOnSelection(page, diffPane(page), "Restore beta")
await hideChanges(page)
await showChanges(page)
await expect.poll(() => commentedText(diffPane(page), id)).toBe("beta")
})
test("keeps the removed text's comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "beta")
const id = await commentOnSelection(page, diffPane(page), "Beta again")
await reopenDraftChanges(page, url)
await expect.poll(() => commentedText(diffPane(page), id)).toBe("beta")
await openTextComment(page, diffPane(page), id, "beta")
await expect(
paneCommentPopover(diffPane(page)).getByText("Beta again"),
).toBeVisible()
})
test("files a comment spanning kept, removed and added text", async ({
page,
request,
}) => {
await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "alpha betadelta gamma")
const id = await commentOnSelection(page, diffPane(page), "Whole line")
await expect
.poll(() => commentedText(diffPane(page), id))
.toBe("alpha betadelta gamma")
})
test("keeps the kept and added part of a spanning comment on the draft", async ({
page,
request,
}) => {
await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "alpha betadelta gamma")
const id = await commentOnSelection(page, diffPane(page), "Whole line")
await hideChanges(page)
await expect
.poll(() => commentedText(contentPane(page), id))
.toBe("alpha delta gamma")
// the removed part is drawn back in once the changes are shown again
await showChanges(page)
await expect
.poll(() => commentedText(diffPane(page), id))
.toBe("alpha betadelta gamma")
})
test("keeps a spanning comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithParagraphChanges(page, request)
await selectText(page, modifiedParagraph(page), "alpha betadelta gamma")
const id = await commentOnSelection(page, diffPane(page), "Whole line")
await documentPersisted(page)
await reopenDraftChanges(page, url)
await expect
.poll(() => commentedText(diffPane(page), id))
.toBe("alpha betadelta gamma")
})
test("files a comment on text inside a removed paragraph", async ({
page,
request,
}) => {
await draftWithParagraphChanges(page, request)
await selectText(page, removedParagraph(page), "Doomed")
const id = await commentOnSelection(page, diffPane(page), "Harsh word")
await expect(
paneCommentPopover(diffPane(page)).getByText("Harsh word"),
).toBeVisible()
await expect.poll(() => commentedText(diffPane(page), id)).toBe("Doomed")
})
test("keeps a comment on text inside a removed paragraph across a reload", async ({
page,
request,
}) => {
const url = await draftWithParagraphChanges(page, request)
await selectText(page, removedParagraph(page), "Doomed")
const id = await commentOnSelection(page, diffPane(page), "Harsh word")
await reopenDraftChanges(page, url)
await expect.poll(() => commentedText(diffPane(page), id)).toBe("Doomed")
await openTextComment(page, diffPane(page), id, "Doomed")
await expect(
paneCommentPopover(diffPane(page)).getByText("Harsh word"),
).toBeVisible()
})
test("files a comment on a parameter added to a split block", async ({
page,
request,
}) => {
await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedParameter(page),
"Document the default",
)
await expect(
paneCommentPopover(diffPane(page)).getByText("Document the default"),
).toBeVisible()
await expect(commentedParameter(diffEditor(page), id)).toContainText(
"limit",
)
})
test("shows the added parameter's comment on the draft once changes are hidden", async ({
page,
request,
}) => {
await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedParameter(page),
"Reads well",
)
await hideChanges(page)
await expect(commentedParameter(contentEditor(page), id)).toContainText(
"limit",
)
await showChanges(page)
await expect(commentedParameter(diffEditor(page), id)).toContainText(
"limit",
)
})
test("keeps the added parameter's comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedParameter(page),
"Still here later",
)
await documentPersisted(page)
await reopenDraftChanges(page, url)
await commentedParameter(diffEditor(page), id).click()
await expect(
paneCommentPopover(diffPane(page)).getByText("Still here later"),
).toBeVisible()
})
test("files a comment on a parameter removed from a split block", async ({
page,
request,
}) => {
await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
removedParameter(page),
"Callers still send this",
)
await expect(
paneCommentPopover(diffPane(page)).getByText("Callers still send this"),
).toBeVisible()
await expect(commentedParameter(diffEditor(page), id)).toContainText(
"Identifier",
)
})
test("keeps the removed parameter's comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
removedParameter(page),
"Gone but discussed",
)
await reopenDraftChanges(page, url)
await commentedParameter(diffEditor(page), id).click()
await expect(
paneCommentPopover(diffPane(page)).getByText("Gone but discussed"),
).toBeVisible()
})
test("files a comment on a code block added to a split block", async ({
page,
request,
}) => {
await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedCodeBlock(page),
"Add a usage example",
)
await expect(
paneCommentPopover(diffPane(page)).getByText("Add a usage example"),
).toBeVisible()
await expect(commentedCodeBlock(diffEditor(page), id)).toContainText(
"Gamma",
)
})
test("shows the added code block's comment on the draft once changes are hidden", async ({
page,
request,
}) => {
await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedCodeBlock(page),
"Looks right",
)
await hideChanges(page)
await expect(commentedCodeBlock(contentEditor(page), id)).toContainText(
"Gamma",
)
await showChanges(page)
await expect(commentedCodeBlock(diffEditor(page), id)).toContainText(
"Gamma",
)
})
test("keeps the added code block's comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
addedCodeBlock(page),
"Still here later",
)
await documentPersisted(page)
await reopenDraftChanges(page, url)
await commentedCodeBlock(diffEditor(page), id).click()
await expect(
paneCommentPopover(diffPane(page)).getByText("Still here later"),
).toBeVisible()
})
test("files a comment on a code block removed from a split block", async ({
page,
request,
}) => {
await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
removedCodeBlock(page),
"This one was useful",
)
await expect(
paneCommentPopover(diffPane(page)).getByText("This one was useful"),
).toBeVisible()
await expect(commentedCodeBlock(diffEditor(page), id)).toContainText("Beta")
})
test("keeps the removed code block's comment across a reload", async ({
page,
request,
}) => {
const url = await draftWithSplitChanges(page, request)
const id = await commentOnBlock(
page,
diffPane(page),
removedCodeBlock(page),
"Gone but discussed",
)
await reopenDraftChanges(page, url)
await commentedCodeBlock(diffEditor(page), id).click()
await expect(
paneCommentPopover(diffPane(page)).getByText("Gone but discussed"),
).toBeVisible()
})
test("files a comment on the changed description of a split block", async ({
page,
request,
}) => {
await draftWithSplitChanges(page, request)
await selectText(page, modifiedParagraph(page), "thingsstuff")
const id = await commentOnSelection(page, diffPane(page), "Pick one word")
await expect
.poll(() => commentedText(diffPane(page), id))
.toBe("thingsstuff")
await hideChanges(page)
await expect.poll(() => commentedText(contentPane(page), id)).toBe("stuff")
await showChanges(page)
await expect
.poll(() => commentedText(diffPane(page), id))
.toBe("thingsstuff")
})
test("shows a comment on added text to a teammate viewing the changes", async ({
page,
request,
browser,
}) => {
await draftWithParagraphChanges(page, request)
const other = await joinAsSecondUser(browser, page, request)
await switchToBranch(other.page, "draft")
await showChanges(other.page)
await selectText(page, modifiedParagraph(page), "delta")
const id = await commentOnSelection(page, diffPane(page), "Seen by both")
await expect
.poll(() => commentedText(diffPane(other.page), id))
.toBe("delta")
await openTextComment(other.page, diffPane(other.page), id, "delta")
await expect(
paneCommentPopover(diffPane(other.page)).getByText("Seen by both"),
).toBeVisible()
await other.context.close()
})
test("shows a comment on a removed paragraph to a teammate viewing the changes", async ({
page,
request,
browser,
}) => {
await draftWithParagraphChanges(page, request)
const other = await joinAsSecondUser(browser, page, request)
await switchToBranch(other.page, "draft")
await showChanges(other.page)
const id = await commentOnBlock(
page,
diffPane(page),
removedParagraph(page),
"Missed by the draft",
)
const commented = diffEditor(other.page).locator(
`p[data-node-comment-id="${id}"]`,
)
await expect(commented).toHaveText("Doomed paragraph")
await commented.click()
await expect(
paneCommentPopover(diffPane(other.page)).getByText("Missed by the draft"),
).toBeVisible()
await other.context.close()
})
test("shows a teammate's comment on removed text to the owner", async ({
page,
request,
browser,
}) => {
await draftWithParagraphChanges(page, request)
const other = await joinAsSecondUser(browser, page, request)
await switchToBranch(other.page, "draft")
await showChanges(other.page)
await selectText(other.page, modifiedParagraph(other.page), "beta")
const id = await commentOnSelection(
other.page,
diffPane(other.page),
"Keep beta please",
)
await expect.poll(() => commentedText(diffPane(page), id)).toBe("beta")
await openTextComment(page, diffPane(page), id, "beta")
await expect(
paneCommentPopover(diffPane(page)).getByText("Keep beta please"),
).toBeVisible()
await other.context.close()
})
})
// the paragraphs a draft changes, as the diff shows them. The merged
// document marks each block with the change it carries.
function addedParagraph(page: Page): Locator {
return diffEditor(page).locator('p[data-diff-status="added"]')
}
function removedParagraph(page: Page): Locator {
return diffEditor(page).locator('p[data-diff-status="removed"]')
}
function modifiedParagraph(page: Page): Locator {
return diffEditor(page).locator('p[data-diff-status="modified"]')
}
// draftWithParagraphChanges builds a page of three paragraphs, makes it
// reviewable and gives the draft one change of each kind: a paragraph
// added after the first, "beta" replaced by "delta" in the second and the
// third removed. It leaves the draft open with the changes shown and
// returns the page's url.
async function draftWithParagraphChanges(
page: Page,
request: APIRequestContext,
): Promise<string> {
await signUpWithWorkspace(page, request)
await createDocument(page)
const url = page.url()
await contentEditor(page).click()
await page.keyboard.type("First paragraph")
await page.keyboard.press("Enter")
await page.keyboard.type("alpha beta gamma")
await page.keyboard.press("Enter")
await page.keyboard.type("Doomed paragraph")
await documentPersisted(page)
await makeReviewable(page)
await switchToBranch(page, "draft")
await expect
.poll(() => editorText(contentEditor(page)))
.toContain("Doomed paragraph")
await placeCaret(draftText(page, "First paragraph"), "end")
await page.keyboard.press("Enter")
await page.keyboard.type("Added paragraph")
await expect
.poll(() => editorText(contentEditor(page)))
.toContain("Added paragraph")
await selectText(page, draftText(page, "alpha beta gamma"), "beta")
await page.keyboard.type("delta")
await expect
.poll(() => editorText(contentEditor(page)))
.toContain("alpha delta gamma")
// the text goes first and the empty paragraph joins the one before it,
// which removes the block without touching its neighbour
await selectText(
page,
draftText(page, "Doomed paragraph"),
"Doomed paragraph",
)
await page.keyboard.press("Backspace")
await page.keyboard.press("Backspace")
await expect
.poll(() => editorText(contentEditor(page)))
.not.toContain("Doomed")
await documentPersisted(page)
await showChanges(page)
await expect(addedParagraph(page)).toHaveText("Added paragraph")
await expect(removedParagraph(page)).toHaveText("Doomed paragraph")
await expect(modifiedParagraph(page)).toHaveText("alpha betadelta gamma")
return url
}
// reopenDraftChanges reloads the page, which lands on the main branch,
// and opens the draft with its changes shown again.
async function reopenDraftChanges(page: Page, url: string): Promise<void> {
await visit(page, url)
await waitForEditor(page)
await switchToBranch(page, "draft")
await showChanges(page)
}
// the parts of the split documentation block the draft changes, as the
// diff shows them. A parameter and a code block are found by the text
// they hold, since the change is marked on the block or on its children.
function addedParameter(page: Page): Locator {
return diffEditor(page).locator(
'[data-type="split-documentation-parameter-list-item"]',
{ hasText: "Page size" },
)
}
function removedParameter(page: Page): Locator {
return diffEditor(page).locator(
'[data-type="split-documentation-parameter-list-item"]',
{ hasText: "Identifier" },
)
}
function addedCodeBlock(page: Page): Locator {
return diffEditor(page).locator('[data-type="titled-code-block"]', {
hasText: "Gamma",
})
}
function removedCodeBlock(page: Page): Locator {
return diffEditor(page).locator('[data-type="titled-code-block"]', {
hasText: "Beta",
})
}
function commentedParameter(editor: Locator, id: string): Locator {
return editor.locator(
`[data-type="split-documentation-parameter-list-item"][data-node-comment-id="${id}"]`,
)
}
function commentedCodeBlock(editor: Locator, id: string): Locator {
return editor.locator(
`[data-type="titled-code-block"][data-node-comment-id="${id}"]`,
)
}
// draftWithSplitChanges builds a page holding one split documentation
// block — a description, two parameters and two code blocks — makes it
// reviewable and changes every part on the draft: a parameter and a code
// block are added, a parameter and a code block removed, and a word of
// the description replaced. It leaves the draft open with the changes
// shown and returns the page's url.
async function draftWithSplitChanges(
page: Page,
request: APIRequestContext,
): Promise<string> {
await signUpWithWorkspace(page, request)
await createDocument(page)
const url = page.url()
await contentEditor(page).click()
const menu = await openSlashMenu(page)
await page.keyboard.type("Split")
await expect(menu.getByRole("button")).toHaveCount(1)
await page.keyboard.press("Enter")
// the block opens with the caret in its heading, and its description
// is the line below
await page.keyboard.type("Topic")
await page.keyboard.press("ArrowDown")
await page.keyboard.type("Describe things")
await bottomAction(
page,
t("editor.split-documentation.left-side-bottom-action-button"),
).click()
await page.keyboard.type("Options")
await page.keyboard.press("ArrowDown")
await typeParameter(page, "id", "string", "Identifier")
await page.keyboard.press("Enter")
await typeParameter(page, "name", "string", "Display name")
await bottomAction(
page,
t("editor.split-documentation.right-side-bottom-action-buttons.add-code"),
).click()
await page.keyboard.type("Beta")
await expect.poll(() => editorText(contentEditor(page))).toContain("Beta")
await documentPersisted(page)
await makeReviewable(page)
await switchToBranch(page, "draft")
await expect
.poll(() => editorText(contentEditor(page)))
.toContain("Display name")
// Enter at the end of a parameter's description starts a new parameter
await placeCaret(draftText(page, "Display name"), "end")
await page.keyboard.press("Enter")
await typeParameter(page, "limit", "int", "Page size")
await expect
.poll(() => editorText(contentEditor(page)))
.toContain("Page size")
// Backspace at the start of a parameter's name removes the parameter
await placeCaret(draftText(page, "id"), "start")
await page.keyboard.press("Backspace")
await expect
.poll(() => editorText(contentEditor(page)))
.not.toContain("Identifier")
await selectText(page, draftText(page, "Describe things"), "things")
await page.keyboard.type("stuff")
await expect
.poll(() => editorText(contentEditor(page)))
.toContain("Describe stuff")
await bottomAction(
page,
t("editor.split-documentation.right-side-bottom-action-buttons.add-code"),
).click()
await page.keyboard.type("Gamma")
await expect.poll(() => editorText(contentEditor(page))).toContain("Gamma")
await openBlockMenu(page, contentPane(page), draftText(page, "Beta"))
await page
.getByRole("menuitem", {
name: t("editor.drag-handle.options.delete-block"),
})
.click()
await expect.poll(() => editorText(contentEditor(page))).not.toContain("Beta")
await documentPersisted(page)
await showChanges(page)
await expect(
diffEditor(page).locator('[data-diff-status="added"]', {
hasText: "Page size",
}),
).not.toHaveCount(0)
await expect(
diffEditor(page).locator('[data-diff-status="removed"]', {
hasText: "Identifier",
}),
).not.toHaveCount(0)
await expect(
diffEditor(page).locator('[data-diff-status="added"]', {
hasText: "Gamma",
}),
).not.toHaveCount(0)
await expect(
diffEditor(page).locator('[data-diff-status="removed"]', {
hasText: "Beta",
}),
).not.toHaveCount(0)
await expect(modifiedParagraph(page)).toHaveText("Describe thingsstuff")
return url
}
// the split block's bottom actions, which add a parameter list on the
// left and a code block on the right
function bottomAction(page: Page, name: string): Locator {
return contentPane(page).getByRole("button", { name: name })
}
// typeParameter fills the parameter the caret has just been placed in:
// its name, then its type to the right, then its description below
async function typeParameter(
page: Page,
name: string,
type: string,
description: string,
): Promise<void> {
await page.keyboard.type(name)
await page.keyboard.press("ArrowRight")
await page.keyboard.type(type)
await page.keyboard.press("ArrowDown")
await page.keyboard.type(description)
}
function draftText(page: Page, text: string): Locator {
return contentEditor(page).getByText(text, { exact: true })
}

View File

@ -9,11 +9,12 @@ import {
import { setResponseStatus } from "h3"
import { afterEach, beforeEach, describe, it, vi } from "vitest"
import CommentRenderer from "./CommentRenderer.vue"
import { CommentMark } from "./comment-mark"
import { CommentMark, findCommentMarkById } from "./comment-mark"
import { NodeComment } from "./node-comment-extension"
import { CommentExtensions } from "./utils"
import UniqueID from "../tiptap-utils/unique-id"
import { stubThemeColorContext } from "../test-helpers/theme"
import { DiffStatus, type PositionMap } from "../diff/position-map"
import {
clearQueryCache,
disposeMockEndpoints,
@ -65,8 +66,10 @@ const DOCUMENT_ID = makeXid("doc")
const BRANCH_ID = makeXid("branch")
const ME = makeXid("usme")
const COMMENT_ID = makeXid("cmt")
const BLOCK_UID = "block-1"
let contentEditor: Editor | null = null
let diffEditor: Editor | null = null
// the renderer anchors comments in a live document, so the suite drives a
// real editor holding the same mark and node comment extensions the app
@ -115,6 +118,83 @@ function serverComment(overrides: Record<string, unknown> = {}) {
}
}
// a diff editor over the same content. Both editors carry the block under
// one uid, the way the diff view pairs merged blocks with their source.
function diffDocOf(source: Editor): Editor {
const element = document.createElement("div")
document.body.appendChild(element)
diffEditor = new TiptapEditor({
element: element,
extensions: [
...CommentExtensions,
UniqueID.configure({ types: ["paragraph"], attributeName: "uid" }),
CommentMark,
NodeComment.configure({ types: ["paragraph"] }),
],
content: source.getJSON(),
})
for (const editor of [source, diffEditor]) {
const block = editor.state.doc.firstChild
editor.view.dispatch(
editor.state.tr.setNodeMarkup(0, undefined, {
...block?.attrs,
uid: BLOCK_UID,
}),
)
}
return diffEditor
}
// the first block, reported as added on the branch
function addedBlockMap(editor: Editor): PositionMap {
return [
{
source: "modified",
blockIndex: 0,
startPos: 0,
nodeSize: editor.state.doc.firstChild?.nodeSize ?? 0,
diffStatus: DiffStatus.Added,
uid: BLOCK_UID,
},
]
}
function mountDiffRenderer(content: Editor, diff: Editor) {
return mountSuspended(CommentRenderer, {
props: {
contentEditor: content,
container: null,
diffContext: {
diffEditor: diff,
positionMap: addedBlockMap(diff),
suppressNextRecompute: () => undefined,
},
},
})
}
// types a draft into the open thread and saves it against the stubbed
// server, which answers with COMMENT_ID
async function saveDraft(wrapper: VueWrapper) {
mockEndpoint("POST", `/api/documents/${DOCUMENT_ID}/comments`, () =>
serverComment(),
)
// saving refetches the thread list
mockEndpoint("GET", `/api/documents/${DOCUMENT_ID}/comments`, () => [
serverComment(),
])
commentEditor(wrapper).commands.setContent(commentBody("looks good"))
await flushPromises()
await popoverButton(
wrapper,
t("editor.comment-thread.comment-button"),
).trigger("click")
}
function mountRenderer(editor: Editor) {
return mountSuspended(CommentRenderer, {
props: { contentEditor: editor, container: null },
@ -198,6 +278,14 @@ describe("<CommentRenderer>", { concurrent: false }, () => {
element?.remove()
contentEditor = null
}
if (diffEditor) {
const element = diffEditor.view.dom.parentElement
diffEditor.destroy()
element?.remove()
diffEditor = null
}
})
it("stays out of the way until a comment is opened", async ({ expect }) => {
@ -700,4 +788,51 @@ describe("<CommentRenderer>", { concurrent: false }, () => {
expect(calls.length).toBeGreaterThan(0)
}, WAIT_FOR_OPTIONS)
})
// the content editor keeps its own renderer mounted, hidden, next to
// the diff one
describe("when the diff view is on", () => {
beforeEach(() => {
useEditorStore().setReviewableDiffActive(true)
})
it("keeps a block comment on added content in the branch", async ({
expect,
}) => {
const content = textDoc("hello world")
const diff = diffDocOf(content)
await mountRenderer(content)
const wrapper = await mountDiffRenderer(content, diff)
await api(wrapper).addNewComment(0)
await nextTick()
await saveDraft(wrapper)
await vi.waitFor(() => {
expect(diff.state.doc.firstChild?.attrs.nodeCommentId).toBe(COMMENT_ID)
}, WAIT_FOR_OPTIONS)
expect(content.state.doc.firstChild?.attrs.nodeCommentId).toBe(COMMENT_ID)
})
it("keeps a text comment on added content in the branch", async ({
expect,
}) => {
const content = textDoc("hello world")
const diff = diffDocOf(content)
diff.commands.setTextSelection({ from: 1, to: 6 })
await mountRenderer(content)
const wrapper = await mountDiffRenderer(content, diff)
await api(wrapper).addNewComment("text-selection")
await nextTick()
await saveDraft(wrapper)
await vi.waitFor(() => {
expect(findCommentMarkById(diff.state, COMMENT_ID)).not.toBeNull()
}, WAIT_FOR_OPTIONS)
expect(findCommentMarkById(content.state, COMMENT_ID)).toMatchObject({
from: 1,
to: 6,
})
})
})
})

View File

@ -352,6 +352,13 @@ watch(
)
watchDeep([fetchComments.state, fetchComments.isPending], () => {
// the content editor's renderer stays mounted, hidden, while the diff
// view is on. The diff renderer then owns the drafts in the content
// editor, and removing them here loses comments on added content.
if (!isDiffMode.value && editorStore.reviewableDiffActive) {
return
}
if (isDiffMode.value && props.diffContext?.diffEditor) {
deletePendingCommentData(
props.diffContext.diffEditor,