Fix comments on Markdown tables

CHANGELOG.md

@@ -5,6 +5,11 @@ matching the pushed tag as that GitHub release's notes, so add an entry here bef
 
 ## Unreleased
 
+- Comment cards on a table now sit beside the row they belong to in Live Preview instead of stacking at the top of the table, and the new-comment composer opens beside its row too ([#79](https://github.com/kylemcd/obsidian-document-comments/issues/79)).
+- Commenting on a whole table row, a whole table, a selection that crosses a cell border, or a selection that runs in from the text around a table no longer breaks the table. Comment markers are kept inside the cell borders, off the blank line a table needs above it, and on both sides of an escaped pipe (`\|`), where they can't stop the block rendering as a table; a selection holding nothing but borders is now refused with an explanation.
+- Hovering a table comment's card highlights its text, and hovering the text raises its card — both of which previously did nothing inside a Live Preview table.
+- Added a repair for tables an older comment already broke. The comment's card, which sits beside the damage, says so and offers a one-click **Repair**; the **Repair table comments in this note** command fixes every one at once, which is the way out when several anchors broke the same table. Both land as a single undo step, and nothing is rewritten unless you ask.
+
 ## 0.1.15
 
 - Fixed emoji reactions added to a reply being attached to the first comment in the thread. Reply reactions now remain with the thread entry where they were added, while existing reaction data remains compatible.

src/editor/commands.ts

@@ -16,6 +16,7 @@ import {
 	computeToggleReaction,
 	findHighlightAtSelection,
 } from "./edits";
+import { computeRepairTableAnchors } from "./table-repair";
 
 /** Create a comment/highlight, or update an exact matching highlight. Ok carries
  *  the affected id, or an empty string when a disabled blank submit writes nothing. */
@@ -46,6 +47,14 @@ export const addComment = (
 	});
 };
 
+/** Move broken table anchors back inside their cells. `only` limits it to one
+ *  comment (the card's Repair action); without it, the whole note. */
+export const repairTableAnchors = (view: EditorView, only?: ReadonlySet<string>): Result<void, string> => {
+	return computeRepairTableAnchors(view.state.doc.toString(), only).map((changes) => {
+		if (changes.length > 0) view.dispatch({ changes });
+	});
+};
+
 export const appendReply = (view: EditorView, id: string, text: string, author: string): Result<void, string> => {
 	return computeAppendReply(view.state.doc.toString(), id, { createdAt: now(), author, text }).map((changes) => {
 		view.dispatch({ changes });

src/editor/edits.ts

@@ -1,8 +1,9 @@
 import { Result } from "better-result";
-import { CommentData, ParsedComment, Reaction, ReactionTarget } from "../format/types";
+import { CommentData, ParsedComment, Reaction, ReactionTarget, TextRange } from "../format/types";
 import { anchorRange, isAnchored, isHighlight, isInFencedCode, parseComments } from "../format/parse";
 import { codeSelectionTarget, isCodeComment, resolveCodeAnchor } from "../format/code-anchor";
 import { closeMarker, openMarker, serializeBody } from "../format/serialize";
+import { clampToTableCells } from "../format/table";
 
 /** A document edit in original coordinates (matches CodeMirror's ChangeSpec shape). */
 export type Change = {
@@ -80,7 +81,8 @@ export const computeAddComment = (
 	if (isInFencedCode(doc, from) || isInFencedCode(doc, to - 1)) {
 		return computeAddCodeComment(doc, from, to, input);
 	}
-	({ from, to } = expandInlineCodeSelection(doc, from, to));
+	({ from, to } = anchorSelection(doc, from, to));
+	if (to === from) return Result.err("Select the text inside a table cell, not its borders.");
 
 	const quote = doc.slice(from, to);
 	const data: CommentData = {
@@ -117,7 +119,7 @@ export const findHighlightAtSelection = (doc: string, from: number, to: number):
 		);
 	}
 
-	({ from, to } = expandInlineCodeSelection(doc, from, to));
+	({ from, to } = anchorSelection(doc, from, to));
 	return (
 		comments.find((comment) => {
 			if (isCodeComment(comment)) return false;
@@ -156,6 +158,14 @@ const computeAddCodeComment = (
 	]);
 };
 
+/** Normalize a raw selection to the range we actually wrap in markers. Both the
+ *  write and the "is this already a highlight?" lookup have to agree, or running
+ *  Add comment twice on the same text stops finding the comment it just made. */
+const anchorSelection = (doc: string, from: number, to: number): TextRange => {
+	const cell = clampToTableCells(doc, from, to);
+	return expandInlineCodeSelection(doc, cell.from, cell.to);
+};
+
 /** HTML comments inside a Markdown code span render as literal code. When a
  * selection is within one inline-code token, anchor the whole token so the
  * comment markers remain invisible outside its backtick delimiters. */

src/editor/margin.ts

@@ -1,10 +1,13 @@
 import { Notice, editorInfoField } from "obsidian";
 import { Result } from "better-result";
+import type { Text } from "@codemirror/state";
 import { EditorView, PluginValue, ViewPlugin } from "@codemirror/view";
-import { ParsedComment } from "../format/types";
+import { ParsedComment, TextRange } from "../format/types";
 import { anchorRange, hasMarginAnchor } from "../format/parse";
 import { isCodeComment, resolveCodeAnchor } from "../format/code-anchor";
 import { commentField } from "./state";
+import { setActiveTableComment, tableCellsForRanges, tableCommentAtPoint } from "./table-highlights";
+import { brokenTableAnchors } from "./table-repair";
 import { commentConfig } from "./config";
 import { Draft, clearDraft, draftField } from "./draft";
 import { Card, CardCallbacks, CardView } from "../ui/card";
@@ -15,6 +18,7 @@ import {
 	deleteComment,
 	deleteEntry,
 	editEntry,
+	repairTableAnchors,
 	setResolved,
 	toggleReaction,
 } from "./commands";
@@ -24,6 +28,9 @@ import { CARD_GAP, FLASH_MS } from "../ui/constants";
 import { buildDraftComposer } from "../ui/draft-composer";
 import { EmptySubmitAction } from "../ui/draft-behavior";
 
+/** Key for the draft composer's anchor in the table-cell lookup. */
+const DRAFT_ANCHOR = "draft:composer";
+
 /** Editor-margin writes go through a live CodeMirror view (no I/O), so the only
  *  failure is a compute error — surface it as a notice rather than swallowing it. */
 const notifyErr = <T>(result: Result<T, string>): Result<T, string> => {
@@ -48,6 +55,9 @@ class MarginView implements PluginValue {
 	private animFrames = 0;
 	private animatingLoop = false;
 	private destroyed = false;
+	/** Broken table anchors and the document they were found in. Only an edit can
+	 *  change them, while reconcile runs on every cursor move and scroll too. */
+	private brokenAnchors: { doc: Text; ids: Set<string> } | null = null;
 
 	constructor(private view: EditorView) {
 		this.container = view.dom.createDiv("doc-comment-margin");
@@ -85,6 +95,7 @@ class MarginView implements PluginValue {
 			toggleReaction: ({ id, entry, emoji }) =>
 				notifyErr(toggleReaction({ view, id, entry, emoji, author: this.cb.getAuthor() })),
 			openInSidebar: (id) => view.state.facet(commentConfig).openInSidebar?.(id),
+			repairTableAnchor: (id) => notifyErr(repairTableAnchors(view, new Set([id]))),
 		};
 
 		view.scrollDOM.addEventListener("scroll", this.scrollHandler, { passive: true });
@@ -93,6 +104,7 @@ class MarginView implements PluginValue {
 		view.contentDOM.addEventListener("mousedown", this.onContentMouseDown);
 		view.contentDOM.addEventListener("mouseover", this.onContentMouseOver);
 		view.contentDOM.addEventListener("mouseout", this.onContentMouseOut);
+		view.contentDOM.addEventListener("mousemove", this.onContentMouseMove);
 
 		this.reconcile();
 		this.requestReposition();
@@ -137,6 +149,7 @@ class MarginView implements PluginValue {
 		this.view.contentDOM.removeEventListener("mousedown", this.onContentMouseDown);
 		this.view.contentDOM.removeEventListener("mouseover", this.onContentMouseOver);
 		this.view.contentDOM.removeEventListener("mouseout", this.onContentMouseOut);
+		this.view.contentDOM.removeEventListener("mousemove", this.onContentMouseMove);
 		this.removeDraftOutside();
 		for (const card of this.cards.values()) card.destroy();
 		this.cards.clear();
@@ -170,19 +183,30 @@ class MarginView implements PluginValue {
 		}
 
 		const cardView = this.cardView();
+		const broken = comments.length > 0 ? this.brokenTableAnchorIds() : new Set<string>();
 		for (const c of comments) {
 			const existing = this.cards.get(c.id);
 			if (!existing) {
 				const card = new Card(c, this.cb, cardView);
 				this.cards.set(c.id, card);
 				this.container.appendChild(card.el);
+				card.setTableAnchorBroken(broken.has(c.id));
 			} else {
 				if (existing.signature !== cardSignature(c)) existing.update(c);
 				existing.refreshAuthorColors();
+				existing.setTableAnchorBroken(broken.has(c.id));
 			}
 		}
 	}
 
+	private brokenTableAnchorIds(): Set<string> {
+		const doc = this.view.state.doc;
+		if (this.brokenAnchors?.doc === doc) return this.brokenAnchors.ids;
+		const comments = this.view.state.field(commentField, false)?.comments ?? [];
+		this.brokenAnchors = { doc, ids: brokenTableAnchors(doc.toString(), comments) };
+		return this.brokenAnchors.ids;
+	}
+
 	private cardView(): CardView {
 		const cfg = this.view.state.facet(commentConfig);
 		// Resolve links/embeds in comment text against THIS editor's file, not the
@@ -209,28 +233,44 @@ class MarginView implements PluginValue {
 		// absolutely positioned, so a top write can't change any height.
 		const placements: Array<{ el: HTMLElement; top: number; height: number }> = [];
 
-		const place = (el: HTMLElement, pos: number) => {
-			const coords = this.view.coordsAtPos(pos);
-			if (!coords) {
+		// Live Preview renders a whole table as ONE block widget, and coordsAtPos
+		// reports that widget's rect for every position inside it — so measuring a
+		// table anchor that way piles every card in the table onto its top edge
+		// (issue #79). Where the anchor resolves to a rendered cell, measure the
+		// cell; everywhere else coordsAtPos is still the right answer.
+		const place = (el: HTMLElement, pos: number, cell: HTMLElement | undefined) => {
+			const rect = cell?.getBoundingClientRect() ?? this.view.coordsAtPos(pos);
+			if (!rect) {
 				el.addClass("dc-offscreen");
 				return;
 			}
 			el.removeClass("dc-offscreen");
 			if (el.offsetHeight === 0) return; // hidden (e.g. resolved)
-			placements.push({ el, top: coords.top - editorTop, height: el.offsetHeight });
+			placements.push({ el, top: rect.top - editorTop, height: el.offsetHeight });
 		};
 
 		const doc = this.view.state.doc.toString();
-		for (const c of this.comments()) {
+		const comments = this.comments();
+		const anchors = new Map(
+			comments.flatMap((c): Array<[string, TextRange]> => {
+				// A code comment's card aligns to its target line, not the block top.
+				const range = isCodeComment(c) ? resolveCodeAnchor(doc, c) : anchorRange(c);
+				return range ? [[c.id, range]] : [];
+			}),
+		);
+		// Comment ids are alphanumeric, so this key can never collide with one.
+		if (draft) anchors.set(DRAFT_ANCHOR, { from: draft.from, to: draft.to });
+		const cells = tableCellsForRanges(this.view, doc, anchors);
+
+		for (const c of comments) {
 			const card = this.cards.get(c.id);
 			if (!card) continue;
-			// A code comment's card aligns to its target line, not the block top.
-			const range = isCodeComment(c) ? resolveCodeAnchor(doc, c) : anchorRange(c);
-			if (range) place(card.el, range.from);
+			const range = anchors.get(c.id);
+			if (range) place(card.el, range.from, cells.get(c.id));
 			else card.el.addClass("dc-offscreen"); // orphaned (e.g. the commented code changed)
 		}
 
-		if (draft && this.draftEl) place(this.draftEl, draft.from);
+		if (draft && this.draftEl) place(this.draftEl, draft.from, cells.get(DRAFT_ANCHOR));
 
 		const tops = stackTops(placements, CARD_GAP);
 		placements.forEach((p, i) => p.el.setCssStyles({ top: `${tops[i]}px` }));
@@ -318,6 +358,10 @@ class MarginView implements PluginValue {
 			this.cards.get(id)?.setActive(true);
 			this.markHighlight(id, true);
 		}
+		// Text inside a Live-Preview table is painted with the CSS Custom Highlight
+		// API, which has no element to carry `is-active` — the painter re-registers
+		// the range under its active name instead.
+		setActiveTableComment(this.view, id);
 	}
 
 	private markHighlight(id: string, active: boolean): void {
@@ -357,7 +401,7 @@ class MarginView implements PluginValue {
 	}
 
 	private onContentMouseDown = (e: MouseEvent): void => {
-		const id = closestSpanId(e.target);
+		const id = closestSpanId(e.target) ?? this.tableIdAt(e);
 		if (id) this.setActive(id);
 	};
 
@@ -366,12 +410,27 @@ class MarginView implements PluginValue {
 		if (id) this.setActive(id);
 	};
 
+	/** A table cell is usually one text node, so `mouseover` never fires as the
+	 *  pointer crosses into the commented words inside it. Track the pointer while
+	 *  it is over a table and hit-test the painted ranges directly. */
+	private onContentMouseMove = (e: MouseEvent): void => {
+		if (!(e.target instanceof Element) || !e.target.closest(".cm-table-widget")) return;
+		this.setActive(this.tableIdAt(e));
+	};
+
+	private tableIdAt(e: MouseEvent): string | null {
+		if (!(e.target instanceof Element) || !e.target.closest(".cm-table-widget")) return null;
+		return tableCommentAtPoint(this.view, e.clientX, e.clientY);
+	}
+
 	private onContentMouseOut = (e: MouseEvent): void => {
-		const span = e.target instanceof Element ? e.target.closest(".doc-comment-span") : null;
-		if (!span) return;
+		// A table widget counts as one hover region: its highlights are painted
+		// ranges rather than elements, so leaving the table is what ends the hover.
+		const region = e.target instanceof Element ? e.target.closest(".doc-comment-span, .cm-table-widget") : null;
+		if (!region) return;
 		// Ignore moves that stay within the same highlight element (avoids flicker).
 		const to = e.relatedTarget;
-		if (to instanceof Node && span.contains(to)) return;
+		if (to instanceof Node && region.contains(to)) return;
 		this.setActive(null);
 	};
 }