Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/editor/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { EditorState, StateField } from "@codemirror/state";
import { EditorView } from "@codemirror/view";
import { commentField } from "./state";
import { commentConfig } from "./config";
import { hasMarginAnchor } from "../format/parse";

// Defined ABOVE editorLayoutField: StateField.define evaluates `provide` EAGERLY
// at module load, and the provider arrow it builds references this helper. A
Expand All @@ -24,7 +25,9 @@ const editorLayoutClasses = (state: EditorState): string => {
// in it once every comment was resolved (issue #30). Mirror that visibility here.
// The reading-view margin applies the same guard (src/reading/margin.ts).
const hasColumn =
showInline && !!fv && fv.comments.some((c) => c.body && (cfg.showResolved() || c.status !== "resolved"));
showInline &&
!!fv &&
fv.comments.some((c) => hasMarginAnchor(c) && (cfg.showResolved() || c.status !== "resolved"));

const classes: string[] = [];
if (hasColumn) classes.push("dc-has");
Expand Down
23 changes: 7 additions & 16 deletions src/editor/margin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Notice, setIcon } from "obsidian";
import { Result } from "better-result";
import { EditorView, PluginValue, ViewPlugin } from "@codemirror/view";
import { ParsedComment } from "../format/types";
import { anchorRange, isAnchored } from "../format/parse";
import { anchorRange, hasMarginAnchor } from "../format/parse";
import { commentField } from "./state";
import { commentConfig } from "./config";
import { clearDraft, draftField } from "./draft";
Expand All @@ -25,8 +25,6 @@ const CARD_GAP = 8;
const notifyErr = (result: Result<unknown, string>): void => {
if (result.isErr()) new Notice(`Couldn't save the comment: ${result.error}`);
};
const ORPHAN_TOP = 8;

/**
* Renders the floating right-margin column: one card per comment, vertically
* aligned to its anchor line, with a stacking pass so cards never overlap.
Expand Down Expand Up @@ -145,7 +143,7 @@ class MarginView implements PluginValue {
if (!fv) return [];
// Resolved cards stay in the DOM and are hidden via a container class, so
// toggling visibility never requires rebuilding the editor.
return fv.comments.filter((c) => c.body);
return fv.comments.filter(hasMarginAnchor);
}

private reconcile(): void {
Expand Down Expand Up @@ -189,7 +187,6 @@ class MarginView implements PluginValue {

const editorTop = this.view.dom.getBoundingClientRect().top;
const placements: Array<{ el: HTMLElement; top: number }> = [];
let orphanCursor = ORPHAN_TOP;

const place = (el: HTMLElement, pos: number) => {
const coords = this.view.coordsAtPos(pos);
Expand All @@ -205,24 +202,18 @@ class MarginView implements PluginValue {
for (const c of this.comments()) {
const card = this.cards.get(c.id);
if (!card) continue;
if (!isAnchored(c)) {
card.el.removeClass("dc-offscreen");
if (card.el.offsetHeight === 0) continue;
card.el.setCssStyles({ top: `${orphanCursor}px` });
orphanCursor += card.el.offsetHeight + CARD_GAP;
continue;
}
place(card.el, anchorRange(c)!.from);
const range = anchorRange(c);
if (range) place(card.el, range.from);
}

if (draft && this.draftEl) place(this.draftEl, draft.from);

// Stack the cards: honor anchor order, push each down past the previous one so
// they never overlap. The first card's floor is -Infinity (unless orphans pin
// the top), so a card whose anchor has scrolled above the viewport keeps a
// they never overlap. The first card's floor is -Infinity, so a card whose
// anchor has scrolled above the viewport keeps a
// negative top and slides off the top edge instead of sticking there in view.
placements.sort((a, b) => a.top - b.top);
let cursor = orphanCursor > ORPHAN_TOP ? orphanCursor : Number.NEGATIVE_INFINITY;
let cursor = Number.NEGATIVE_INFINITY;
for (const p of placements) {
const y = Math.max(p.top, cursor);
p.el.setCssStyles({ top: `${y}px` });
Expand Down
6 changes: 6 additions & 0 deletions src/format/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ export const isAnchored = (c: ParsedComment): boolean => {
return !!c.open && !!c.close && c.open.to <= c.close.from;
};

/** A floating margin card needs both its thread body and a valid text anchor.
* Orphaned threads remain available in the sidebar, where no anchor is needed. */
export const hasMarginAnchor = (c: ParsedComment): boolean => {
return !!c.body && isAnchored(c);
};

/** The highlighted text range (between the markers), or null if not anchored. */
export const anchorRange = (c: ParsedComment): TextRange | null => {
return isAnchored(c) ? { from: c.open!.to, to: c.close!.from } : null;
Expand Down
4 changes: 2 additions & 2 deletions src/reading/margin.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { App, MarkdownView, Notice, setIcon } from "obsidian";
import { Result } from "better-result";
import { ParsedComment } from "../format/types";
import { existingIds, parseComments } from "../format/parse";
import { existingIds, hasMarginAnchor, parseComments } from "../format/parse";
import { generateId } from "../format/ids";
import { Card, CardCallbacks, cardSignature } from "../ui/card";
import {
Expand Down Expand Up @@ -98,7 +98,7 @@ class ReadingMargin {
} catch {
return; // file vanished or unreadable — keep the last render
}
const all = parseComments(data).filter((c) => c.body);
const all = parseComments(data).filter(hasMarginAnchor);
// Sidebar open → inline cards step aside (the panel lists them instead).
this.comments = this.deps.showComments() && !this.deps.sidebarOpen() ? all : [];
this.reconcileCards();
Expand Down
6 changes: 6 additions & 0 deletions test/editor-view.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ describe("editor extensions open every note without crashing", () => {
);
expect(withComment).toContain("dc-has"); // a comment reserves the column
expect(withComment).toContain("dc-highlights");

const orphanOnly = open(
'Text without an anchor.\n<!--co:orphan status:open quote:"old text"\nme: dangling\n-->',
);
expect(orphanOnly).not.toContain("dc-has");
expect(orphanOnly).toContain("dc-highlights");
});

// Regression for issue #30: once every comment is resolved and resolved
Expand Down
10 changes: 9 additions & 1 deletion test/format.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from "vitest";
import { parseComments, anchorRange, isAnchored, isOrphan, existingIds } from "../src/format/parse";
import { parseComments, anchorRange, isAnchored, isOrphan, existingIds, hasMarginAnchor } from "../src/format/parse";
import { serializeBody, openMarker, closeMarker } from "../src/format/serialize";
import { generateId } from "../src/format/ids";
import { CommentData } from "../src/format/types";
Expand Down Expand Up @@ -50,6 +50,14 @@ describe("parseComments", () => {
const c = parseComments(doc)[0];
expect(isAnchored(c)).toBe(false);
expect(isOrphan(c)).toBe(true);
expect(hasMarginAnchor(c)).toBe(false);
});

it("only treats a complete anchored thread as a margin card", () => {
const anchored = parseComments(CANONICAL)[0];
const markerOnly = parseComments(openMarker("missing") + "text" + closeMarker("missing"))[0];
expect(hasMarginAnchor(anchored)).toBe(true);
expect(hasMarginAnchor(markerOnly)).toBe(false);
});

it("handles multiple and overlapping comments", () => {
Expand Down