From 8d6e5f2bcc663d9e6ca4e54caf07bdb99c47335d Mon Sep 17 00:00:00 2001 From: Naiyuan Qing <145280634+NevilleQingNY@users.noreply.github.com> Date: Wed, 15 Apr 2026 09:27:01 +0800 Subject: [PATCH] fix(editor): hover card bug, view crash, perf, and link handler cleanup - Fix issue mention cards incorrectly triggering Link Hover Card - Guard editor.view access in BubbleMenu against unmounted/destroyed view Proxy (fixes desktop Inbox fast-switching crash) - Use useEditorState for precise formatting state subscriptions in BubbleMenu instead of relying on parent re-renders - Add markdownTokenizer to FileCard for unambiguous !file[name](url) roundtrip syntax (legacy CDN hostname matching kept for compat) - Extract shared openLink/isMentionHref into utils/link-handler.ts Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/views/editor/bubble-menu.tsx | 72 ++++++++++++++----- packages/views/editor/content-editor.tsx | 16 ++--- .../views/editor/extensions/file-card.tsx | 36 ++++++++-- packages/views/editor/link-hover-card.tsx | 20 ++---- packages/views/editor/readonly-content.tsx | 12 +--- packages/views/editor/utils/link-handler.ts | 22 ++++++ packages/views/editor/utils/preprocess.ts | 29 +++++++- 7 files changed, 148 insertions(+), 59 deletions(-) create mode 100644 packages/views/editor/utils/link-handler.ts diff --git a/packages/views/editor/bubble-menu.tsx b/packages/views/editor/bubble-menu.tsx index 472108b05..384c486f3 100644 --- a/packages/views/editor/bubble-menu.tsx +++ b/packages/views/editor/bubble-menu.tsx @@ -12,6 +12,7 @@ import { useState, useEffect, useCallback, useRef } from "react"; import { BubbleMenu } from "@tiptap/react/menus"; +import { useEditorState } from "@tiptap/react"; import type { Editor } from "@tiptap/core"; import { NodeSelection } from "@tiptap/pm/state"; import type { EditorState } from "@tiptap/pm/state"; @@ -112,12 +113,14 @@ function MarkButton({ icon: Icon, label, shortcut, + isActive, }: { editor: Editor; mark: InlineMark; icon: React.ComponentType<{ className?: string }>; label: string; shortcut: string; + isActive: boolean; }) { return ( @@ -125,7 +128,7 @@ function MarkButton({ render={ toggleMarkActions[mark](editor)} onMouseDown={(e) => e.preventDefault()} /> @@ -239,9 +242,8 @@ function LinkEditBar({ // Heading Dropdown // --------------------------------------------------------------------------- -function HeadingDropdown({ editor, onOpenChange }: { editor: Editor; onOpenChange: (open: boolean) => void }) { +function HeadingDropdown({ editor, onOpenChange, activeLevel }: { editor: Editor; onOpenChange: (open: boolean) => void; activeLevel: number | undefined }) { const [open, setOpen] = useState(false); - const activeLevel = [1, 2, 3].find((l) => editor.isActive("heading", { level: l })); const label = activeLevel ? `H${activeLevel}` : "Text"; const items = [ { label: "Normal Text", icon: Type, active: !activeLevel, action: () => editor.chain().focus().setParagraph().run() }, @@ -296,10 +298,8 @@ function HeadingDropdown({ editor, onOpenChange }: { editor: Editor; onOpenChang // List Dropdown // --------------------------------------------------------------------------- -function ListDropdown({ editor, onOpenChange }: { editor: Editor; onOpenChange: (open: boolean) => void }) { +function ListDropdown({ editor, onOpenChange, isBullet, isOrdered }: { editor: Editor; onOpenChange: (open: boolean) => void; isBullet: boolean; isOrdered: boolean }) { const [open, setOpen] = useState(false); - const isBullet = editor.isActive("bulletList"); - const isOrdered = editor.isActive("orderedList"); const handleOpenChange = useCallback((next: boolean) => { setOpen(next); @@ -360,9 +360,40 @@ function EditorBubbleMenu({ editor }: { editor: Editor }) { const [mode, setMode] = useState<"toolbar" | "link-edit">("toolbar"); const [scrollTarget, setScrollTarget] = useState(window); - // Find the real scroll container once on mount + // Precise subscription to formatting state — only re-renders when these + // values actually change, replacing direct editor.isActive() calls that + // relied on the parent re-rendering on every transaction. + const fmt = useEditorState({ + editor, + selector: ({ editor: e }) => ({ + bold: e.isActive("bold"), + italic: e.isActive("italic"), + strike: e.isActive("strike"), + code: e.isActive("code"), + link: e.isActive("link"), + blockquote: e.isActive("blockquote"), + bulletList: e.isActive("bulletList"), + orderedList: e.isActive("orderedList"), + heading1: e.isActive("heading", { level: 1 }), + heading2: e.isActive("heading", { level: 2 }), + heading3: e.isActive("heading", { level: 3 }), + }), + }); + + // Find the real scroll container once the editor view is ready. + // editor.view.dom throws if the view hasn't been mounted yet or has been + // destroyed — the Proxy only stubs state/isDestroyed, everything else throws. + // This race happens on fast page transitions in Desktop (Inbox switching) + // because useEditor delays destruction via setTimeout(..., 1) for StrictMode + // survival (TipTap issue #7346). useEffect(() => { - setScrollTarget(getScrollParent(editor.view.dom)); + const detect = () => { + if (!editor.isInitialized) return; // view not ready yet + setScrollTarget(getScrollParent(editor.view.dom)); + }; + detect(); + editor.on("create", detect); + return () => { editor.off("create", detect); }; }, [editor]); // Hide when the selection scrolls outside the scroll container's @@ -384,7 +415,14 @@ function EditorBubbleMenu({ editor }: { editor: Editor }) { } return; } - const coords = editor.view.coordsAtPos(editor.state.selection.from); + // editor.view.coordsAtPos throws if the view has been destroyed + // during a fast unmount race (same Proxy guard as view.dom above). + let coords: { top: number }; + try { + coords = editor.view.coordsAtPos(editor.state.selection.from); + } catch { + return; + } const rect = el.getBoundingClientRect(); const visible = coords.top >= rect.top && coords.top <= rect.bottom; if (scrollHiddenRef.current !== !visible) { @@ -440,25 +478,25 @@ function EditorBubbleMenu({ editor }: { editor: Editor }) { ) : (
- - - - + + + + setMode("link-edit")} onMouseDown={(e) => e.preventDefault()} /> + setMode("link-edit")} onMouseDown={(e) => e.preventDefault()} /> }> Link - - + + editor.chain().focus().toggleBlockquote().run()} onMouseDown={(e) => e.preventDefault()} /> + editor.chain().focus().toggleBlockquote().run()} onMouseDown={(e) => e.preventDefault()} /> }> diff --git a/packages/views/editor/content-editor.tsx b/packages/views/editor/content-editor.tsx index 71b717fd5..53588cd90 100644 --- a/packages/views/editor/content-editor.tsx +++ b/packages/views/editor/content-editor.tsx @@ -39,6 +39,7 @@ import { useQueryClient } from "@tanstack/react-query"; import { createEditorExtensions } from "./extensions"; import { uploadAndInsertFile } from "./extensions/file-upload"; import { preprocessMarkdown } from "./utils/preprocess"; +import { openLink, isMentionHref } from "./utils/link-handler"; import { EditorBubbleMenu } from "./bubble-menu"; import { useLinkHover, LinkHoverCard } from "./link-hover-card"; import "./content-editor.css"; @@ -122,6 +123,9 @@ const ContentEditor = forwardRef( const editor = useEditor({ immediatelyRender: false, + // Note: in v3.22.1 the default is already false/undefined (same behavior). + // Explicit for clarity — the real perf win is useEditorState in BubbleMenu. + shouldRerenderOnTransaction: false, editable, content: defaultValue ? preprocessMarkdown(defaultValue) : "", contentType: defaultValue ? "markdown" : undefined, @@ -152,18 +156,10 @@ const ContentEditor = forwardRef( const link = target.closest("a"); const href = link?.getAttribute("href"); - if (!href || href.startsWith("mention://")) return false; + if (!href || isMentionHref(href)) return false; - // Open the link. Internal paths use multica:navigate - // (Electron hash-router safe), external open in new tab. event.preventDefault(); - if (href.startsWith("/")) { - window.dispatchEvent( - new CustomEvent("multica:navigate", { detail: { path: href } }), - ); - } else { - window.open(href, "_blank", "noopener,noreferrer"); - } + openLink(href); return true; }, }, diff --git a/packages/views/editor/extensions/file-card.tsx b/packages/views/editor/extensions/file-card.tsx index 6a0de7c04..3dc44102b 100644 --- a/packages/views/editor/extensions/file-card.tsx +++ b/packages/views/editor/extensions/file-card.tsx @@ -4,9 +4,14 @@ * FileCard — Tiptap node extension for rendering uploaded non-image files * as styled cards instead of plain markdown links. * - * Markdown serialization: `[filename](href)` — standard link syntax. - * Preprocessing in preprocess.ts converts standalone CDN file links back - * to fileCard HTML on load, completing the roundtrip. + * Markdown serialization: `!file[filename](href)` — custom syntax that is + * unambiguous (standard `[name](url)` is indistinguishable from regular links). + * + * Loading pipeline: preprocessFileCards in preprocess.ts converts both the + * new `!file[name](url)` syntax AND legacy `[name](cdnUrl)` lines into HTML + * divs BEFORE @tiptap/markdown parses the content. The markdownTokenizer + * below acts as a fallback for any direct markdown parsing that bypasses + * preprocessing. */ import { Node, mergeAttributes } from "@tiptap/core"; @@ -146,10 +151,31 @@ export const FileCardExtension = Node.create({ ]; }, - // Markdown serialization: fileCard → [filename](href) + // Markdown: custom !file[name](url) syntax for unambiguous roundtrip. + // Standard [name](url) is indistinguishable from regular links — the old + // regex-based CDN hostname matching in preprocessFileCards was fragile. + markdownTokenizer: { + name: "fileCard", + level: "block" as const, + start(src: string) { + return src.search(/^!file\[/m); + }, + tokenize(src: string) { + const match = src.match(/^!file\[([^\]]*)\]\((https?:\/\/[^)]+)\)/); + if (!match) return undefined; + return { + type: "fileCard", + raw: match[0], + attributes: { filename: match[1], href: match[2] }, + }; + }, + }, + parseMarkdown: (token: any, helpers: any) => { + return helpers.createNode("fileCard", token.attributes); + }, renderMarkdown: (node: any) => { const { href, filename } = node.attrs || {}; - return `[${filename || "file"}](${href})`; + return `!file[${filename || "file"}](${href})`; }, addNodeView() { diff --git a/packages/views/editor/link-hover-card.tsx b/packages/views/editor/link-hover-card.tsx index a129a804d..a9ad890b4 100644 --- a/packages/views/editor/link-hover-card.tsx +++ b/packages/views/editor/link-hover-card.tsx @@ -15,20 +15,7 @@ import { computePosition, offset, flip, shift } from "@floating-ui/dom"; import { ExternalLink, Copy } from "lucide-react"; import { toast } from "sonner"; import { Button } from "@multica/ui/components/ui/button"; - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -function openLink(href: string) { - if (href.startsWith("/")) { - window.dispatchEvent( - new CustomEvent("multica:navigate", { detail: { path: href } }), - ); - } else { - window.open(href, "_blank", "noopener,noreferrer"); - } -} +import { openLink, isMentionHref } from "./utils/link-handler"; function truncateUrl(url: string, max = 48): string { if (url.length <= max) return url; @@ -77,7 +64,10 @@ function useLinkHover(containerRef: React.RefObject, disable const link = target.closest("a") as HTMLAnchorElement | null; if (!link) return; const href = link.getAttribute("href"); - if (!href || href.startsWith("mention://")) return; + if (!href || isMentionHref(href)) return; + // Issue mention cards render as — they + // display their own rich info, a URL hover card is redundant. + if (link.classList.contains("issue-mention")) return; clearTimeout(hideTimer.current); showTimer.current = window.setTimeout(() => { diff --git a/packages/views/editor/readonly-content.tsx b/packages/views/editor/readonly-content.tsx index 43b2c045b..3d5bd7f77 100644 --- a/packages/views/editor/readonly-content.tsx +++ b/packages/views/editor/readonly-content.tsx @@ -34,6 +34,7 @@ import { useNavigation } from "../navigation"; import { IssueMentionCard } from "../issues/components/issue-mention-card"; import { ImageLightbox } from "./extensions/image-view"; import { useLinkHover, LinkHoverCard } from "./link-hover-card"; +import { openLink, isMentionHref } from "./utils/link-handler"; import { preprocessMarkdown } from "./utils/preprocess"; import "./content-editor.css"; @@ -112,7 +113,7 @@ function IssueMentionLink({ issueId, label }: { issueId: string; label?: string const components: Partial = { // Links — route mention:// to mention components, others show preview card a: ({ href, children }) => { - if (href?.startsWith("mention://")) { + if (isMentionHref(href)) { const match = href.match( /^mention:\/\/(member|agent|issue|all)\/(.+)$/, ); @@ -135,14 +136,7 @@ const components: Partial = { href={href} onClick={(e) => { e.preventDefault(); - if (!href) return; - if (href.startsWith("/")) { - window.dispatchEvent( - new CustomEvent("multica:navigate", { detail: { path: href } }), - ); - } else { - window.open(href, "_blank", "noopener,noreferrer"); - } + if (href) openLink(href); }} > {children} diff --git a/packages/views/editor/utils/link-handler.ts b/packages/views/editor/utils/link-handler.ts new file mode 100644 index 000000000..6ffe46526 --- /dev/null +++ b/packages/views/editor/utils/link-handler.ts @@ -0,0 +1,22 @@ +/** + * Shared link handling utilities for the editor system. + * + * Used by content-editor (ProseMirror click handler), readonly-content + * (react-markdown link component), and link-hover-card (Open button). + */ + +/** Open a link — internal paths dispatch multica:navigate, external open new tab. */ +export function openLink(href: string): void { + if (href.startsWith("/")) { + window.dispatchEvent( + new CustomEvent("multica:navigate", { detail: { path: href } }), + ); + } else { + window.open(href, "_blank", "noopener,noreferrer"); + } +} + +/** Check if a href is a mention protocol link (should not be opened as a regular link). */ +export function isMentionHref(href: string | null | undefined): href is string { + return !!href && href.startsWith("mention://"); +} diff --git a/packages/views/editor/utils/preprocess.ts b/packages/views/editor/utils/preprocess.ts index f9da68337..c9bf326a3 100644 --- a/packages/views/editor/utils/preprocess.ts +++ b/packages/views/editor/utils/preprocess.ts @@ -24,29 +24,52 @@ export function preprocessMarkdown(markdown: string): string { } /** - * Convert standalone `[name](cdnUrl)` lines into HTML that Tiptap's fileCard - * parseHTML can recognise. Only matches non-image CDN URLs on their own line. + * LEGACY MIGRATION: Convert standalone `[name](cdnUrl)` lines into HTML that + * Tiptap's fileCard parseHTML can recognise. Only matches non-image CDN URLs + * on their own line. + * + * New file cards are saved as `!file[name](url)` via the fileCard extension's + * markdownTokenizer, which is unambiguous and doesn't need this preprocessing. + * This function remains for backward compatibility with content saved before + * the `!file` syntax was introduced. As users edit old content, it auto-migrates + * to the new syntax on save. * * Input: `[report.pdf](https://multica-static.copilothub.ai/xxx.pdf)` * Output: `
` */ +/** New syntax: !file[name](url) — unambiguous, no hostname matching needed. */ +const NEW_FILE_CARD_RE = /^!file\[([^\]]*)\]\((https?:\/\/[^)]+)\)$/; + +/** Legacy syntax: [name](cdnUrl) on its own line — matched by CDN hostname. */ const FILE_LINK_LINE = /^\[([^\]]+)\]\((https?:\/\/[^)]+)\)$/; function escapeAttr(s: string): string { return s.replace(/&/g, "&").replace(/"/g, """).replace(/
`; +} + function preprocessFileCards(markdown: string): string { return markdown .split("\n") .map((line) => { const trimmed = line.trim(); + + // New syntax: !file[name](url) — always a file card, no hostname check needed. + const newMatch = trimmed.match(NEW_FILE_CARD_RE); + if (newMatch) { + return toFileCardHtml(newMatch[1]!, newMatch[2]!); + } + + // Legacy: [name](cdnUrl) on its own line — CDN hostname matching. const match = trimmed.match(FILE_LINK_LINE); if (!match) return line; const filename = match[1]!; const url = match[2]!; if (!isFileCardUrl(url)) return line; - return `
`; + return toFileCardHtml(filename, url); }) .join("\n"); }