fix(text-editor): remove custom linkpaste plugin due to formatting issues #3678
Conversation
…sues The custom linkPaste plugin only addressed the automatic parsing of plain text URLs into links when pasted, i.e., converting text links that were not already formatted as links. However, it introduced several other issues, most notably the loss of line breaks in pasted content on serialization, which negatively impacted formatting and user experience. Removing the plugin resolves these formatting issues.
📝 WalkthroughWalkthroughRemoved all paste-handling and URL-to-link conversion from the link plugin. Eliminated regex matching and node creation for pasted URLs and the handlePaste hook. Retained click interactions (mod-click open, selection, link menu, double-click) and updateLink behavior. No changes to exported API. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Editor
participant LinkPlugin
rect rgb(240,245,255)
note right of User: Old flow (before)
User->>Editor: Paste text containing URL
Editor->>LinkPlugin: handlePaste(event)
LinkPlugin->>LinkPlugin: Match URLs, create link nodes
LinkPlugin-->>Editor: Replace with link-marked content
end
rect rgb(245,240,240)
note right of User: New flow (now)
User->>Editor: Paste text containing URL
Editor->>LinkPlugin: handlePaste(event)
LinkPlugin-->>Editor: No special handling (removed)
Editor-->>User: Plain text inserted (no auto-linking)
end
sequenceDiagram
autonumber
actor User
participant Editor
participant LinkPlugin
participant Browser
note over User,LinkPlugin: Click interactions (unchanged)
User->>Editor: Cmd/Ctrl+Click on link
Editor->>LinkPlugin: click event
LinkPlugin->>Browser: Open href in new tab/window
User->>Editor: Single click on link
Editor->>LinkPlugin: click event
LinkPlugin-->>Editor: Select link / show link menu
User->>Editor: Double click on link
Editor->>LinkPlugin: dblclick event
LinkPlugin-->>Editor: Trigger edit behavior (updateLink callback)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3678/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/components/text-editor/prosemirror-adapter/plugins/link/link-plugin.ts (7)
102-122: Guard posAtCoords null and avoid passing undefined into nodeAt; also type the return.
Currentlypos?.poscan be undefined;doc.nodeAt(undefined)is unsafe. Also destructuring the result upstream without null checks can crash.-const getLinkDataAtPosition = (view: EditorView, event: MouseEvent) => { - const pos = view.posAtCoords({ left: event.clientX, top: event.clientY }); - const node = view.state.doc.nodeAt(pos?.pos); +const getLinkDataAtPosition = ( + view: EditorView, + event: MouseEvent +): { href: string; text: string; from: number; to: number } | null => { + const posInfo = view.posAtCoords({ + left: event.clientX, + top: event.clientY, + }); + if (!posInfo) { + return null; + } + const pos = posInfo.pos; + const node = view.state.doc.nodeAt(pos); if (!node) { return null; } - const linkMark = node.marks.find( - (mark) => mark.type.name === EditorMenuTypes.Link - ); + const linkMark = node.marks.find((mark) => mark.type.name === 'link'); if (!linkMark) { return null; } const href = linkMark.attrs.href; - const from = findStart(view.state.doc, pos.pos, href); - const to = findEnd(view.state.doc, pos.pos, href); + const from = findStart(view.state.doc, pos, href); + const to = findEnd(view.state.doc, pos, href); const text = view.state.doc.textBetween(from, to, ' '); return { href: href, text: text, from: from, to: to }; };
124-133: Fix NPE on mod-click; add protocol allowlist and noopener to prevent tabnabbing.
const { href } = getLinkDataAtPosition(...)throws when not on a link. Also usenoopenerand allow only safe schemes.-const processModClickEvent = (view: EditorView, event: MouseEvent): boolean => { - const { href } = getLinkDataAtPosition(view, event); - if (href) { - window.open(href, '_blank'); - - return true; - } - - return false; -}; +const processModClickEvent = ( + view: EditorView, + event: MouseEvent +): boolean => { + const linkData = getLinkDataAtPosition(view, event); + if (!linkData?.href) { + return false; + } + const href = linkData.href; + try { + const url = new URL(href, window.location.href); + const allowed = ['http:', 'https:', 'mailto:', 'tel:']; + if (!allowed.includes(url.protocol)) { + return false; + } + const w = window.open(url.href, '_blank', 'noopener'); + if (w) { + w.opener = null; + } + } catch { + return false; + } + return true; +};
49-67: Type the helper and standardize on mark name 'link'.
Mixed use ofEditorMenuTypes.Linkvs'link'risks mismatches; use the actual mark name. Also add types for safety.-const findStart = (doc, pos, href) => { +const findStart = (doc: PMNode, pos: number, href: string): number => { while (pos > 0) { const node = doc.nodeAt(pos - 1); if ( !node?.isText || - !node.marks.some( - (mark: Mark) => - mark.type.name === EditorMenuTypes.Link && - mark.attrs.href === href - ) + !node.marks.some( + (mark: Mark) => mark.type.name === 'link' && mark.attrs.href === href + ) ) { break; } pos--; } return pos; };
76-94: Same as above for findEnd: add types and use 'link' mark name.-const findEnd = (doc, pos, href) => { +const findEnd = (doc: PMNode, pos: number, href: string): number => { while (pos < doc.content.size) { const node = doc.nodeAt(pos); if ( !node?.isText || - !node.marks.some( - (mark) => - mark.type.name === EditorMenuTypes.Link && - mark.attrs.href === href - ) + !node.marks.some( + (mark: Mark) => mark.type.name === 'link' && mark.attrs.href === href + ) ) { break; } pos++; } return pos; };
148-173: Don’t swallow normal single-clicks on non-links.
mousedownreturnstrueeven when no link is under the cursor, suppressing PM’s default cursor placement. Returnfalsewhen not on a link.- clickTimeout = setTimeout(() => { - const linkData = getLinkDataAtPosition(view, event); - if (linkData) { - const { href, text, from, to } = linkData; - const transaction = view.state.tr.setSelection( - TextSelection.create(view.state.doc, from, to) - ); - view.dispatch(transaction); - openLinkMenu(view, href, text); - } - }, DOUBLE_CLICK_DELAY); - - return true; + const linkData = getLinkDataAtPosition(view, event); + if (!linkData) { + // Let ProseMirror handle regular cursor placement/selection. + return false; + } + clickTimeout = setTimeout(() => { + const { href, text, from, to } = linkData; + const transaction = view.state.tr.setSelection( + TextSelection.create(view.state.doc, from, to) + ); + view.dispatch(transaction); + openLinkMenu(view, href, text); + }, DOUBLE_CLICK_DELAY); + return true;
147-147: Type the timer handle.
Improves TS safety across browser/node envs.-let clickTimeout; +let clickTimeout: ReturnType<typeof setTimeout>;
144-147: Per-editor state to avoid cross-editor interference.
lastClickTime/clickTimeoutare module-scoped; multiple editors on one page will share state. Prefer a WeakMap keyed byEditorView.I can provide a patch that introduces:
const clickState = new WeakMap<EditorView, { lastClickTime: number; clickTimeout: ReturnType<typeof setTimeout> | null }>();processClickEventloads/updates state viaclickState.get(view)/clickState.set(view, state).Want me to draft this change?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/text-editor/prosemirror-adapter/plugins/link/link-plugin.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
Files:
src/components/text-editor/prosemirror-adapter/plugins/link/link-plugin.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Docs / Publish Docs
- GitHub Check: Test
- GitHub Check: Build
🔇 Additional comments (1)
src/components/text-editor/prosemirror-adapter/plugins/link/link-plugin.ts (1)
175-215: Sanity checks after paste-plugin removal — verification
- Repo search did not find remaining link-paste handlers (handlePaste / pasteAsLink / linkPaste / URL_REGEX) in the runs performed.
- Remaining "paste" occurrences are image-paste handling and UI/example strings: src/components/text-editor/text-editor.tsx (imagePasted), src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx (createImageInserterPlugin), plus examples under src/components/text-editor/examples and menu example labels.
- Link mark is still defined at src/components/text-editor/prosemirror-adapter/menu/menu-commands.ts (schema.marks.link).
- Line-break preservation on paste cannot be validated via static search — run the integration/manual paste test (paste multi-line plain text) or provide test output to confirm no regression.
src/components/text-editor/prosemirror-adapter/plugins/link/link-plugin.ts
Show resolved
Hide resolved
I understand, just to be clear, just can still paste hyperlinks. In my opinion parsing normal links adds more headache than it brings value, because the parsing is not easy. I'm not saying the text-editor is Screen.Recording.2025-09-19.at.11.06.57.mov |
|
Closing as the issue is fixed here |
3 participants
The custom linkPaste plugin only addressed the automatic parsing of plain text URLs into links when pasted, i.e., converting text links that were not already formatted as hyperlinks.
However, it introduced several other issues, most notably the loss of line breaks in pasted content on serialization, which negatively impacted formatting and user experience.
Removing the plugin resolves these formatting issues.
Fixes: https://github.com/Lundalogik/limepkg-email/issues/1689
Summary by CodeRabbit
How To Test This:
Paste content in to the text editor, both with and without URLS.
Whatever you paste should be what you see in the text editor and also what you get when the document is serialized. The document is serialized when you for example press Save/Create in Lime CRM.
You can also test this with both
markdownmode andhtmlmode on the text-editor.Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: