fix: robust wiki-link round-trip and navigable popover (BUG-586 follow-ups)

Follow-up to the BUG-586 fix that surfaced several edge cases under real-world use. Covers three related improvements to the wiki-link experience in the editor. Reference-based wiki-link storage Previously `[Title](/url)` round-tripped to `[[Title]]`. That form broke for titles containing `[`, `]`, `/`, or `|`. Storage now uses the item's opaque ref (e.g. `[[BUG-586]]`, or `[[BUG-586|Custom]]` when the visible text differs from the item's current title). `wikiLinksToMarkdown` accepts three forms in preference order: ref-only, ref-with-display-override, and legacy title lookup. Titles can now contain any characters and links survive renames. Escape-aware parsing Both `markdownToWikiLinks` and `wikiLinksToMarkdown` now recognize `\.` escape sequences inside their capture groups. tiptap-markdown emits `\[`, `\]`, `\\` in link text when the text contains literal brackets, so the prior regexes (`[^\]]+`) terminated prematurely and missed valid links. Helper functions escape/unescape the markdown link-text layer and the wiki-link body layer separately so `]`, `|`, and `\` can appear in display-override text. Leave unresolved [[X]] untouched The `[[…]]` regex is greedy and can match spans that were never intended as wiki-links — notably `[[` sequences inside another markdown link's text. On miss, the function now returns the original match verbatim instead of emitting `[…](broken)`, which previously hijacked surrounding content and accumulated corruption on each save cycle. Broken items heal themselves on the next auto-save. Picker: show ref + align URL with the route The `[[` picker now lists the ref badge next to the title and keys `{#each}` by `doc.id` so duplicate titles don't collide. `execLink` now reads `page.params.username`/`page.params.workspace` from the live route (previously `workspaceStore.current`, which could be empty), so the inserted `href` matches the URL shape that the round-trip expects. Clickable link popover The popover's URL label is now a real `<a href="…">`. Plain click → `goto()` for internal paths, full navigation for external. Ctrl / Cmd / middle-click pass through to the browser so "new tab" and "copy link" work naturally. `onmousedown.stopPropagation` keeps the outer popover's focus-trap from swallowing the click.
2026-09-21 18:13:26 +00:00 · 2026-04-17 03:44:41 +00:00
parent e328844a1b
commit d1a5ea3975
3 changed files with 220 additions and 37 deletions
@@ -1,5 +1,6 @@
 <script lang="ts">
 	import { onMount, onDestroy, untrack } from 'svelte';
+	import { page } from '$app/state';
 	import { Editor, mergeAttributes } from '@tiptap/core';
 	import { Plugin } from '@tiptap/pm/state';
 	import StarterKit from '@tiptap/starter-kit';
@@ -205,8 +206,9 @@
 	});
 	import { Markdown } from 'tiptap-markdown';
 	import { unescapeDocLinks } from '$lib/utils/markdown';
-	import { formatItemRef } from '$lib/types';
+	import { formatItemRef, itemUrlId, type Item } from '$lib/types';
 	import { collectionStore } from '$lib/stores/collections.svelte';
+	import { workspaceStore } from '$lib/stores/workspace.svelte';
 	import { BlockDragHandle } from './block-drag-handle';
 	import { SLASH_ITEMS } from './block-types';

@@ -299,11 +301,41 @@
 			.slice(0, 10);
 	}

-	function execLink(title: string) {
+	function execLink(doc: Item) {
 		if (!editor) return;
+		// Build the URL in the same shape wikiLinksToMarkdown produces so the
+		// save round-trip (markdownToWikiLinks) reliably converts it back to
+		// [[Title]]. We read from the live route params (not workspaceStore)
+		// because that's what the slug page uses when converting wiki-links —
+		// keeping the two in sync is what lets the round-trip work.
+		const routeUsername = page.params.username ?? '';
+		const routeWorkspace = page.params.workspace ?? workspaceStore.current?.slug ?? '';
+		const collSlug = doc.collection_slug ?? '';
+		const idSeg = itemUrlId(doc);
+		const prefix = routeUsername ? `/${routeUsername}/${routeWorkspace}` : `/${routeWorkspace}`;
+		const href = collSlug && idSeg && routeWorkspace ? `${prefix}/${collSlug}/${idSeg}` : '';
+
 		// Delete the [[ and any query text typed so far
 		const to = editor.state.selection.from;
-		editor.chain().focus().deleteRange({ from: linkStartPos, to }).insertContent(`[[${title}]]`).run();
+		const chain = editor.chain().focus().deleteRange({ from: linkStartPos, to });
+
+		if (href) {
+			// Insert the title as a real Tiptap link mark so it's clickable
+			// immediately (no reload needed). On save, tiptap-markdown emits
+			// [Title](href), which markdownToWikiLinks converts back to [[Title]].
+			chain.insertContent([
+				{
+					type: 'text',
+					text: doc.title,
+					marks: [{ type: 'link', attrs: { href } }],
+				},
+				// Trailing space drops the link mark so subsequent typing is plain text.
+				{ type: 'text', text: ' ' },
+			]).run();
+		} else {
+			// Fall back to [[Title]] text if we can't resolve a URL.
+			chain.insertContent(`[[${doc.title}]]`).run();
+		}
 		closeLink();
 	}

@@ -418,7 +450,7 @@
 						const items = getFilteredLinks();
 						if (event.key === 'ArrowDown') { event.preventDefault(); linkIdx = (linkIdx + 1) % Math.max(items.length, 1); return true; }
 						if (event.key === 'ArrowUp') { event.preventDefault(); linkIdx = (linkIdx - 1 + items.length) % Math.max(items.length, 1); return true; }
-						if (event.key === 'Enter') { event.preventDefault(); if (items[linkIdx]) execLink(items[linkIdx].title); return true; }
+						if (event.key === 'Enter') { event.preventDefault(); if (items[linkIdx]) execLink(items[linkIdx]); return true; }
 						if (event.key === 'Escape') { event.preventDefault(); closeLink(); return true; }
 						return false;
 					}
@@ -675,13 +707,13 @@
 				class="slash-item"
 				class:selected={i === linkIdx}
 				onmouseenter={() => linkIdx = i}
-				onclick={() => execLink(doc.title)}
+				onclick={() => execLink(doc)}
 			>
 				<span class="slash-icon">{doc.collection_icon ?? '📄'}</span>
-				<span class="slash-title">{doc.title}</span>
 				{#if formatItemRef(doc)}
 					<span class="slash-ref">{formatItemRef(doc)}</span>
 				{/if}
+				<span class="slash-title">{doc.title}</span>
 			</button>
 		{:else}
 			<div class="slash-item" style="color: var(--text-muted); cursor: default;">No matching documents</div>
@@ -1,5 +1,6 @@
 <script lang="ts">
 	import type { Editor } from '@tiptap/core';
+	import { goto } from '$app/navigation';

 	let {
 		editor,
@@ -106,6 +107,27 @@
 		}
 	}

+	// Navigate in the current tab. Uses SvelteKit's goto() for internal
+	// (path-relative) URLs so the transition is a SPA navigation; external
+	// URLs fall back to a full-page load. The handler is attached to a
+	// real <a> element so middle-click / cmd-click / "copy link" behave
+	// naturally.
+	function handleHrefClick(e: MouseEvent) {
+		if (!href) return;
+		// Let the browser handle new-tab modifiers and middle-click itself.
+		if (e.button !== 0 || e.ctrlKey || e.metaKey || e.shiftKey) return;
+		e.preventDefault();
+		e.stopPropagation();
+		const target = href;
+		visible = false;
+		editing = false;
+		if (target.startsWith('/') && !target.startsWith('//')) {
+			goto(target);
+		} else {
+			window.location.assign(target);
+		}
+	}
+
 	function startEdit() {
 		editValue = href;
 		editing = true;
@@ -179,7 +201,13 @@

 		{#if !editing}
 			<div class="link-display">
-				<span class="link-href" title={href}>{truncatedHref}</span>
+				<a
+					class="link-href"
+					href={href}
+					title="Open link in this tab — {href}"
+					onclick={handleHrefClick}
+					onmousedown={(e) => e.stopPropagation()}
+				>{truncatedHref}</a>
 				<div class="link-actions">
 					<button class="link-btn" onclick={openUrl} title="Open link">
 						<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round">
@@ -273,6 +301,10 @@
 	}

 	.link-href {
+		display: inline-block;
+		text-decoration: none;
+		cursor: pointer;
+
 		font-size: 0.82em;
 		color: var(--text-muted);
 		font-family: var(--font-mono);
@@ -280,7 +312,17 @@
 		overflow: hidden;
 		text-overflow: ellipsis;
 		max-width: 180px;
-		padding: var(--space-1) 0;
+		padding: var(--space-1);
+		border-radius: var(--radius-sm);
+	}
+	.link-href:hover {
+		color: var(--accent-blue);
+		text-decoration: underline;
+		background: var(--bg-hover);
+	}
+	.link-href:focus-visible {
+		outline: 2px solid var(--accent-blue);
+		outline-offset: 1px;
 	}

 	.link-actions {
@@ -70,61 +70,170 @@ export function unescapeDocLinks(markdown: string): string {
 	return markdown.replace(/\\\[\\\[([^\]]+)\\\]\\\]/g, '[[$1]]');
 }

+// Wiki-link reference pattern: uppercase/alphanumeric prefix, hyphen, digits.
+// Matches the item ref format produced by formatItemRef (e.g. TASK-5, BUG-585).
+// Anchored so it rejects anything else and falls back to title-based lookup,
+// which keeps legacy [[Title]] links working unchanged.
+const REF_PATTERN = /^[A-Za-z][A-Za-z0-9]*-\d+$/;
+
 /**
- * Convert [[Item Title]] to markdown links for Tiptap rendering.
- * Tiptap doesn't understand [[]] syntax, so we convert to standard
- * markdown links before feeding content to the editor.
+ * Convert wiki-link storage syntax into markdown links for Tiptap rendering.
+ * Supports three forms, in preference order:
+ *   - [[REF-123]]              → ref lookup; visible text = current item title
+ *   - [[REF-123|Display Text]] → ref lookup; visible text = Display Text
+ *   - [[Title]]                → legacy title lookup (also accepts [[coll/Title]])
+ * The ref-based forms are safe for titles containing any characters (brackets,
+ * slashes, quotes, etc.) because the stored key is the opaque item ref.
 */
 export function wikiLinksToMarkdown(content: string, items: Item[], workspaceSlug: string, username?: string): string {
-	return content.replace(/\[\[([^\]]+)\]\]/g, (_match, title: string) => {
-		// Support optional collection/ prefix: [[tasks/My Task]]
-		let searchTitle = title;
-		let collFilter: string | null = null;
-		if (title.includes('/')) {
-			const [coll, ...rest] = title.split('/');
-			collFilter = coll;
-			searchTitle = rest.join('/');
+	// Body may contain backslash-escaped chars (`\]`, `\\`, `\|`) so the tokens
+	// we emit can carry arbitrary display text. The capture is (\\.|[^\]\\])+,
+	// i.e. "a backslash-escaped char OR any non-`]`/non-`\` char".
+	return content.replace(/\[\[((?:\\.|[^\]\\])+)\]\]/g, (_match, body: string) => {
+		const prefix = username ? `/${username}/${workspaceSlug}` : `/${workspaceSlug}`;
+
+		// Split optional display override on the FIRST unescaped pipe.
+		const { key: rawKey, displayOverride: rawDisplay } = splitWikiBody(body);
+		const key = unescapeWikiBody(rawKey);
+		const displayOverride = rawDisplay == null ? null : unescapeWikiBody(rawDisplay);
+
+		// 1. Ref-based lookup (e.g. [[BUG-585]]) — the preferred form.
+		if (REF_PATTERN.test(key.trim())) {
+			const ref = key.trim();
+			const byRef = items.find(i =>
+				!!i.item_number && !!i.collection_prefix &&
+				`${i.collection_prefix}-${i.item_number}`.toLowerCase() === ref.toLowerCase()
+			);
+			if (byRef && byRef.collection_slug) {
+				const text = displayOverride ?? byRef.title;
+				return `[${escapeMarkdownLinkText(text)}](${prefix}/${byRef.collection_slug}/${itemUrlId(byRef)})`;
+			}
+			// Unresolved ref — keep the original [[…]] so it round-trips.
+			return _match;
 		}

-		const item = items.find(i => {
-			const titleMatch = i.title.toLowerCase() === searchTitle.toLowerCase();
-			if (collFilter && i.collection_slug) {
-				return titleMatch && i.collection_slug === collFilter;
+		// 2. Legacy: exact full-title match. Titles can contain slashes, so
+		//    this must win over the collection-filter fallback below.
+		const titleLower = key.toLowerCase();
+		let item = items.find(i => i.title.toLowerCase() === titleLower);
+		let displayText = displayOverride ?? key;
+
+		// 3. Legacy: the [[collection/Title]] disambiguation syntax.
+		if (!item && key.includes('/')) {
+			const [collFilter, ...rest] = key.split('/');
+			const searchTitle = rest.join('/');
+			const found = items.find(i =>
+				i.title.toLowerCase() === searchTitle.toLowerCase() &&
+				i.collection_slug === collFilter
+			);
+			if (found) {
+				item = found;
+				if (displayOverride == null) displayText = searchTitle;
 			}
-			return titleMatch;
-		});
+		}

 		if (item && item.collection_slug) {
-			const prefix = username ? `/${username}/${workspaceSlug}` : `/${workspaceSlug}`;
-				return `[${searchTitle}](${prefix}/${item.collection_slug}/${itemUrlId(item)})`;
+			return `[${escapeMarkdownLinkText(displayText)}](${prefix}/${item.collection_slug}/${itemUrlId(item)})`;
 		}
-		// Unresolved — render as styled text (editor will show it as plain text)
-		return `[${searchTitle}](broken)`;
+		// Unresolved: leave the original [[X]] text alone. Emitting a
+		// [text](broken) link here would hijack content that legitimately
+		// contains `[[` — for example a `[[` that appears inside another
+		// markdown link's text span. The regex is greedy and may grab a
+		// range that was never intended as a wiki-link, so the safe thing
+		// on miss is to restore the match verbatim.
+		return _match;
 	});
 }

 /**
- * Convert markdown links back to [[Item Title]] syntax for storage.
- * Reverses wikiLinksToMarkdown() so we store [[]] not []() in the database.
+ * Convert markdown links back to wiki-link storage syntax.
+ * When the link's URL resolves to an item with a ref, emit [[REF]] (or
+ * [[REF|Display]] if the visible text differs from the item's current
+ * title). Ref-based storage is preferred because it survives item renames
+ * and is robust against special characters in titles.
+ * Items without a ref fall back to the legacy [[Title]] form.
 */
 export function markdownToWikiLinks(markdown: string, items: Item[]): string {
-	// Match [Title](/username/workspace/collection/slug-or-REF) or [Title](/workspace/collection/slug-or-REF) pattern
-	return markdown.replace(/\[([^\]]+)\]\(\/(?:[^/]+\/){2,3}([^)]+)\)/g, (_match, title: string, slugOrRef: string) => {
+	// Match [Title](/username/workspace/collection/slug-or-REF). Title may
+	// contain backslash-escaped chars (\[, \], \\) that tiptap-markdown emits
+	// when serializing link text. The capture allows `\.` sequences so we
+	// don't terminate on an escaped `]` that's really part of the display.
+	return markdown.replace(/\[((?:\\.|[^\]\\])+)\]\(\/(?:[^/]+\/){2,3}([^)]+)\)/g, (_match, rawText: string, slugOrRef: string) => {
 		const item = items.find(i => {
 			if (i.slug === slugOrRef) return true;
-			// Also match PREFIX-NUMBER refs
 			if (i.item_number && i.collection_prefix) {
 				return `${i.collection_prefix}-${i.item_number}` === slugOrRef;
 			}
 			return false;
 		});
-		if (item) {
-			return `[[${title}]]`;
+		if (!item) return _match;
+
+		// tiptap-markdown emits backslash-escaped brackets in the link text
+		// (e.g. "Use \[\[ to link"); unescape before comparing/emitting.
+		const displayText = unescapeMarkdownLinkText(rawText);
+
+		const ref = (item.item_number && item.collection_prefix)
+			? `${item.collection_prefix}-${item.item_number}`
+			: null;
+
+		if (ref) {
+			// Prefer ref-based storage. Omit |Display if it matches the
+			// current item title (renaming the item updates the link text
+			// automatically on next load).
+			if (displayText === item.title) {
+				return `[[${ref}]]`;
+			}
+			return `[[${ref}|${escapeWikiBody(displayText)}]]`;
 		}
-		return _match;
+		// Legacy fallback for items without a ref.
+		return `[[${escapeWikiBody(displayText)}]]`;
 	});
 }

+// Escape the characters that would terminate or unbalance a markdown link's
+// text span. `\` must be doubled first so it doesn't interfere with the
+// subsequent bracket escapes.
+function escapeMarkdownLinkText(s: string): string {
+	return s.replace(/\\/g, '\\\\').replace(/([\[\]])/g, '\\$1');
+}
+
+// Escape the characters that would terminate a [[...]] wiki-link body, or
+// collide with the `|` display separator. Order matters: backslash first.
+function escapeWikiBody(s: string): string {
+	return s.replace(/\\/g, '\\\\').replace(/([\]|])/g, '\\$1');
+}
+
+// Inverse of escapeWikiBody. Accepts `\]`, `\|`, and `\\` escapes.
+function unescapeWikiBody(s: string): string {
+	return s.replace(/\\(\\|\]|\|)/g, '$1');
+}
+
+// Split a wiki-link body on the FIRST unescaped `|`. Returns the raw key
+// and the raw display override (both still escape-encoded — caller should
+// unescape them). If there's no pipe, displayOverride is null.
+function splitWikiBody(body: string): { key: string; displayOverride: string | null } {
+	let i = 0;
+	while (i < body.length) {
+		const ch = body[i];
+		if (ch === '\\' && i + 1 < body.length) {
+			i += 2;
+			continue;
+		}
+		if (ch === '|') {
+			return { key: body.slice(0, i), displayOverride: body.slice(i + 1) };
+		}
+		i++;
+	}
+	return { key: body, displayOverride: null };
+}
+
+// Inverse of escapeMarkdownLinkText. Also undoes the \[\[ / \]\] escapes that
+// tiptap-markdown inserts to prevent its own output from looking like our
+// wiki-link sentinels.
+function unescapeMarkdownLinkText(s: string): string {
+	return s.replace(/\\(\[|\]|\\)/g, '$1');
+}
+
 /**
 * Convert [[broken]] placeholder links back to wiki syntax
 */