[WIKI-830] fix: copy clipboard functionality in the editor #8229
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBroadened duplicate-asset permissions to include guests and made asset lookup workspace-agnostic; added EditorRef.copyMarkdownToClipboard() producing a custom clipboard MIME; paste now prefers that MIME and runs asset-duplication handlers; hardened image node loading/visibility and adjusted related state handling. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Editor UI (EditorRef)
participant EditorView as Editor View
participant Clipboard
participant PasteHandler as handlePaste / processAssetDuplication
participant Server as Asset API
User->>UI: trigger copy (copyMarkdownToClipboard)
UI->>EditorView: read current HTML & metadata
EditorView-->>UI: return HTML & metadata
UI->>Clipboard: intercept copy event & set formats
Clipboard-->>Clipboard: set "text/plain" (markdown)
Clipboard-->>Clipboard: set "text/html" (html)
Clipboard-->>Clipboard: set "text/plane-editor-html" (custom html)
User->>EditorView: paste
EditorView->>PasteHandler: handlePaste(event)
PasteHandler->>Clipboard: read "text/plane-editor-html"
alt custom MIME present
PasteHandler->>PasteHandler: processAssetDuplication(html)
PasteHandler->>EditorView: view.pasteHTML(processedHtml)
Note right of Server: asset duplication requests may be made during processing
EditorView->>Server: duplicate asset request (id, is_uploaded)
Server-->>EditorView: duplication result
else
PasteHandler-->>EditorView: return false (fallback to default paste)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,mts,cts}📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
Files:
🪛 ast-grep (0.40.0)packages/editor/src/core/helpers/paste-asset.ts[warning] 5-5: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first. (dom-content-modification) [warning] 22-22: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first. (dom-content-modification) [warning] 5-5: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify. (unsafe-html-content-assignment) [warning] 22-22: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify. (unsafe-html-content-assignment) ⏰ 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). (2)
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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes clipboard functionality in the editor by improving image handling during copy/paste operations and removes a workspace-specific constraint from the backend asset duplication logic.
Key Changes
- Enhanced clipboard handling to support a custom MIME type (
text/plane-editor-html) for better internal copy/paste operations - Improved error handling and state management in the image node view component
- Removed workspace filtering from asset lookup during duplication to allow cross-workspace asset copying
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/editor/src/core/plugins/paste-asset.ts | Added support for custom text/plane-editor-html MIME type with charset metadata |
| packages/editor/src/core/plugins/markdown-clipboard.ts | Set clipboard data with custom MIME type during copy operations |
| packages/editor/src/core/extensions/custom-image/components/node-view.tsx | Enhanced image loading with error handling, state resets, and improved validation logic |
| apps/api/plane/app/views/asset/v2.py | Removed workspace filter from asset lookup to enable cross-workspace duplication |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/editor/src/core/extensions/custom-image/components/node-view.tsx
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/plane/app/views/asset/v2.py (1)
793-801: Cross‑workspace asset duplication weakens workspace isolation; confirm this is intentional and gated.By dropping the
workspace__slug=slugfilter here, any uploaded asset with a knownasset_idcan now be duplicated into the current workspace, regardless of which workspace owns the original asset. The only permission check is on the destination workspace (via@allow_permission(..., level="WORKSPACE")onslug), not on the source asset’s workspace.That means:
- A user with access to workspace A but not workspace B cannot directly access B’s assets, but
- A user with access to workspace B can duplicate an uploaded asset from any workspace they can learn an
asset_idfor, effectively copying the underlying S3 object across workspaces.If workspace is meant to be a hard multi‑tenant boundary, this is a significant relaxation. Consider:
- Either restoring a workspace constraint on the lookup, or
- Explicitly validating that
request.useralso has appropriate access tooriginal_asset.workspacebefore allowing duplication.If cross‑workspace duplication is intended, it’d be good to document this behavior and confirm that asset IDs are treated as non‑guessable secrets and cannot leak across tenants inadvertently.
🧹 Nitpick comments (2)
packages/editor/src/core/extensions/custom-image/components/node-view.tsx (1)
52-75: Defensive image loading + visibility logic look solid; consider guarding against async races.The new flow around
resolvedSrc/resolvedDownloadSrc,failedToLoadImage, andhasValidImageSourceis coherent and should make uploader vs. block rendering much more predictable, especially after duplication succeeds or an image load fails.One thing you might optionally tighten later: in the
getImageSourceeffect, multiple rapidimgNodeSrcchanges could let an older async call overwriteresolvedSrcfor a newerimgNodeSrc. Capturing the currentimgNodeSrcin a local variable and bailing out if it no longer matches before setting state would avoid that race.Otherwise the duplication success/reset and
shouldShowBlockconditions look good.Also applies to: 88-100, 120-124, 127-130
packages/editor/src/core/plugins/paste-asset.ts (1)
11-19: TypehtmlContentexplicitly and (optionally) make the processed marker more specific.The new logic for preferring
text/plane-editor-htmland injecting a UTF‑8 meta tag looks good functionally and cooperates with the copy plugin.Two small refinements:
- Avoid implicit
anyonhtmlContentin TS:let htmlContent: string | null = null; if (event.clipboardData.getData("text/plane-editor-html")) { htmlContent = event.clipboardData.getData("text/plane-editor-html"); const metaTag = document.createElement("meta"); metaTag.setAttribute("charset", "utf-8"); htmlContent = metaTag.outerHTML + htmlContent; } else { htmlContent = event.clipboardData.getData("text/html"); } if (!htmlContent || htmlContent.includes('data-uploaded="true"')) return false;
- Optional: consider using a more specific marker like
data-plane-asset-uploaded="true"instead of the genericdata-uploaded="true"to reduce the chance of skipping processing for third‑party HTML that happens to use the same attribute.Also applies to: 20-21
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/plane/app/views/asset/v2.py(1 hunks)packages/editor/src/core/extensions/custom-image/components/node-view.tsx(3 hunks)packages/editor/src/core/plugins/markdown-clipboard.ts(1 hunks)packages/editor/src/core/plugins/paste-asset.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/plugins/paste-asset.tspackages/editor/src/core/extensions/custom-image/components/node-view.tsxpackages/editor/src/core/plugins/markdown-clipboard.ts
🧬 Code graph analysis (1)
packages/editor/src/core/extensions/custom-image/components/node-view.tsx (1)
packages/editor/src/core/extensions/custom-image/utils.ts (1)
hasImageDuplicationFailed(63-64)
⏰ 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: CodeQL analysis (javascript-typescript)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (1)
packages/editor/src/core/plugins/markdown-clipboard.ts (1)
33-36: LGTM – custom clipboard MIME type integrates cleanly with existing copy behavior.Emitting
text/plane-editor-htmlalongsidetext/plainandtext/htmlis backward‑compatible and lines up with the paste plugin’s new behavior, without changing existing consumers.
im-vipin
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/core/components/core/description-versions/modal.tsx (1)
13-79: Clipboard handler wiring withcopyMarkdownToClipboardis solidThe modal now correctly:
- Pulls
{ markdown, html }fromeditorRef.current.getMarkDown().- Uses
copyMarkdownToClipboard({ markdown, html })and surfaces success/failure via toasts.- Guards against a missing
editorRef.current.Implementation is consistent with the new clipboard utility and editor ref API. If you want to polish later, you could (optionally) make
handleCopyMarkdownasyncand usetry/catchinstead of.then/.catch, and consider localizing the toast messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/core/components/core/description-versions/modal.tsx(2 hunks)apps/web/core/components/pages/editor/toolbar/options-dropdown.tsx(2 hunks)apps/web/core/components/pages/modals/export-page-modal.tsx(1 hunks)packages/editor/src/core/helpers/editor-ref.ts(1 hunks)packages/editor/src/core/types/editor.ts(1 hunks)packages/utils/src/string.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/helpers/editor-ref.tsapps/web/core/components/pages/editor/toolbar/options-dropdown.tsxpackages/editor/src/core/types/editor.tsapps/web/core/components/pages/modals/export-page-modal.tsxapps/web/core/components/core/description-versions/modal.tsxpackages/utils/src/string.ts
🧠 Learnings (4)
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use copying array methods (`toSorted`, `toSpliced`, `with`) for immutable array operations (TypeScript 5.2+)
Applied to files:
apps/web/core/components/pages/editor/toolbar/options-dropdown.tsxpackages/utils/src/string.ts
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/pages/editor/toolbar/options-dropdown.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/core/description-versions/modal.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/core/description-versions/modal.tsx
🧬 Code graph analysis (3)
packages/editor/src/core/helpers/editor-ref.ts (1)
packages/utils/src/editor/markdown-parser/root.ts (1)
convertHTMLToMarkdown(112-143)
apps/web/core/components/core/description-versions/modal.tsx (3)
packages/utils/src/string.ts (1)
copyMarkdownToClipboard(387-414)packages/propel/src/toast/toast.tsx (1)
setToast(202-222)packages/i18n/src/store/index.ts (1)
t(223-244)
packages/utils/src/string.ts (1)
apps/space/helpers/string.helper.ts (1)
copyTextToClipboard(25-31)
⏰ 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). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
packages/editor/src/core/helpers/editor-ref.ts (1)
81-91: UpdatedgetMarkDownshape and null handling look correct
getMarkDownnow returns{ markdown, html }and gracefully handles!editorwith{ markdown: "", html: "" }, matching the updatedEditorRefApitype and avoiding null/undefined at call sites.apps/web/core/components/pages/modals/export-page-modal.tsx (1)
157-167: Markdown export correctly adapts to newgetMarkDownAPIDestructuring
{ markdown: markdownContent } = editorRef?.getMarkDown() ?? { markdown: "" }is type-safe with the new return shape and avoids runtime errors wheneditorRefis null, while preserving previous behavior for empty content.packages/editor/src/core/types/editor.ts (1)
100-139:EditorRefApi.getMarkDowntype now matches runtime behaviorUpdating
getMarkDownto return{ markdown: string; html: string }aligns the type with the new implementation and all updated call sites, preventing type mismatches and clarifying the API contract.apps/web/core/components/pages/editor/toolbar/options-dropdown.tsx (1)
8-91: Page options “Copy markdown” action correctly migrated to new clipboard APIThe extra menu option now:
- Safely exits if
editorRefis missing.- Uses
editorRef.getMarkDown()to obtain{ markdown, html }.- Delegates to
copyMarkdownToClipboard({ markdown, html })and surfaces success/failure via toasts.This is consistent with the updated EditorRef API and shared clipboard logic used elsewhere.
packages/utils/src/string.ts
Outdated
| /** | ||
| * @description Copies markdown and HTML content to clipboard with multiple formats including custom plane-editor format | ||
| * @param {Object} params - Parameters object | ||
| * @param {string} params.markdown - Markdown text to copy | ||
| * @param {string} params.html - HTML content to copy | ||
| * @returns {Promise<void>} Promise that resolves when copying is complete | ||
| * @example | ||
| * await copyMarkdownToClipboard({ markdown: "# Hello", html: "<h1>Hello</h1>" }) | ||
| */ | ||
| export const copyMarkdownToClipboard = async ({ | ||
| markdown, | ||
| html, | ||
| }: { | ||
| markdown: string; | ||
| html: string; | ||
| }): Promise<void> => { | ||
| // Try using the copy event to support custom MIME types | ||
| try { | ||
| const handleCopy = (event: ClipboardEvent) => { | ||
| event.preventDefault(); | ||
| event.clipboardData?.clearData(); | ||
| event.clipboardData?.setData("text/plain", markdown); | ||
| event.clipboardData?.setData("text/html", html); | ||
| event.clipboardData?.setData("text/plane-editor-html", html); | ||
| document.removeEventListener("copy", handleCopy); | ||
| }; | ||
|
|
||
| document.addEventListener("copy", handleCopy); | ||
| document.execCommand("copy"); | ||
| return; | ||
| } catch (error) { | ||
| console.warn("Failed to copy with custom formats, trying standard clipboard API:", error); | ||
| } | ||
|
|
||
| // Final fallback to simple text copy | ||
| await copyTextToClipboard(markdown); | ||
| }; |
There was a problem hiding this comment.
Harden copyMarkdownToClipboard against execCommand failures and listener leaks
The current implementation can leave the "copy" listener attached and report success even when document.execCommand("copy") silently fails:
- If
execCommand("copy")throws before the event fires,handleCopyis never called, soremoveEventListeneris never run and the listener will hijack future copy events. - If
execCommand("copy")returnsfalse(copy blocked/unsupported) without throwing, youreturnearly, never fall back tocopyTextToClipboard, and the listener also stays attached.
This can corrupt subsequent clipboard behavior across the app.
You can make this more robust by always cleaning up the listener and treating a false result as failure, e.g.:
export const copyMarkdownToClipboard = async ({
markdown,
html,
}: {
markdown: string;
html: string;
}): Promise<void> => {
- // Try using the copy event to support custom MIME types
- try {
- const handleCopy = (event: ClipboardEvent) => {
- event.preventDefault();
- event.clipboardData?.clearData();
- event.clipboardData?.setData("text/plain", markdown);
- event.clipboardData?.setData("text/html", html);
- event.clipboardData?.setData("text/plane-editor-html", html);
- document.removeEventListener("copy", handleCopy);
- };
-
- document.addEventListener("copy", handleCopy);
- document.execCommand("copy");
- return;
- } catch (error) {
- console.warn("Failed to copy with custom formats, trying standard clipboard API:", error);
- }
-
- // Final fallback to simple text copy
- await copyTextToClipboard(markdown);
+ const handleCopy = (event: ClipboardEvent) => {
+ event.preventDefault();
+ event.clipboardData?.clearData();
+ event.clipboardData?.setData("text/plain", markdown);
+ event.clipboardData?.setData("text/html", html);
+ event.clipboardData?.setData("text/plane-editor-html", html);
+ };
+
+ // Try using the copy event to support custom MIME types
+ try {
+ document.addEventListener("copy", handleCopy, { once: true });
+
+ const success = document.execCommand("copy");
+ if (!success) {
+ throw new Error("document.execCommand('copy') returned false");
+ }
+ return;
+ } catch (error) {
+ console.warn(
+ "Failed to copy with custom formats, falling back to plain-text clipboard copy:",
+ error
+ );
+ } finally {
+ document.removeEventListener("copy", handleCopy);
+ }
+
+ // Final fallback to simple text copy
+ await copyTextToClipboard(markdown);
};This keeps the listener from leaking and guarantees you fall back when the custom path doesn’t actually copy anything.
🤖 Prompt for AI Agents
In packages/utils/src/string.ts around lines 378 to 414, the copy handler can
leak if document.execCommand("copy") throws or returns false; ensure the "copy"
listener is always removed and treat a false return as failure so you fall back
to copyTextToClipboard. Implement execCommand call inside a try/catch/finally
where you: add the listener, call execCommand and capture its boolean result, on
success let the handler remove itself, but in finally always call
document.removeEventListener("copy", handleCopy) to guard against leaks, and if
execCommand threw or returned false then call await
copyTextToClipboard(markdown) as the fallback.
| metaTag.setAttribute("charset", "utf-8"); | ||
| htmlContent = metaTag.outerHTML + htmlContent; | ||
| } else { | ||
| htmlContent = event.clipboardData.getData("text/html"); |
There was a problem hiding this comment.
There's no need to handle for the else condition.
| getDocumentInfo: () => TDocumentInfo; | ||
| getHeadings: () => IMarking[]; | ||
| getMarkDown: () => string; | ||
| getMarkDown: () => { markdown: string; html: string }; |
There was a problem hiding this comment.
Revert the getMarkdown function to its original state. Crate a new copyMarkdown function which will take care of copying content of all types.
packages/utils/src/string.ts
Outdated
| * @example | ||
| * await copyMarkdownToClipboard({ markdown: "# Hello", html: "<h1>Hello</h1>" }) | ||
| */ | ||
| export const copyMarkdownToClipboard = async ({ |
There was a problem hiding this comment.
Remove this function and move it to editor.
| ); | ||
| const { markdown, html } = editorRef.current.getMarkDown(); | ||
|
|
||
| copyMarkdownToClipboard({ markdown, html }) |
There was a problem hiding this comment.
Use editorRef's copy function.
| words: editor?.storage.characterCount?.words?.() ?? 0, | ||
| }), | ||
| getHeadings: () => (editor ? editor.storage.headingsList?.headings : []), | ||
| getMarkDown: () => { |
…board for simplified clipboard operations
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/api/plane/app/views/asset/v2.py (1)
769-799: Workspace scoping removed from source asset lookup – possible cross‑workspace exposureBy changing the lookup to:
original_asset = FileAsset.objects.filter( id=asset_id, is_uploaded=True ).first()the source asset is no longer constrained to the current
workspace(fromslug). Combined with allowing[ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST]atlevel="WORKSPACE", any user with access to a workspace can duplicate any uploaded asset they can guess/obtain theasset_idfor, regardless of the asset’s original workspace.That effectively bypasses the usual
workspace__slug=...scoping used elsewhere in this file and may allow cross‑workspace data access via server‑side S3 copy.If the intent is not to allow arbitrary cross‑workspace duplication, please reintroduce workspace scoping (or an equivalent authorization check), e.g.:
- storage = S3Storage(request=request) - original_asset = FileAsset.objects.filter( - id=asset_id, is_uploaded=True - ).first() + storage = S3Storage(request=request) + original_asset = FileAsset.objects.filter( + id=asset_id, + is_uploaded=True, + workspace=workspace, + ).first()and optionally revisit whether
ROLE.GUESTshould be allowed here.apps/web/core/components/core/description-versions/modal.tsx (1)
13-69: UseeditorRef.current.copyMarkdownToClipboard()for the copy actionRight now the copy button still calls
copyTextToClipboard(editorRef.current.getMarkDown()), so:
- Only
text/plainis set on the clipboard.- The new multi‑format clipboard flow (
text/html+text/plane-editor-html) and paste plugin won’t kick in.- The newly imported
copyMarkdownToClipboardfrom@plane/utilsisn’t used.Since
EditorRefApinow exposescopyMarkdownToClipboard(), it’s cleaner to delegate to that and then show the toast:-import { calculateTimeAgo, cn, copyMarkdownToClipboard, copyTextToClipboard, getFileURL } from "@plane/utils"; +import { calculateTimeAgo, cn, getFileURL } from "@plane/utils"; ... const handleCopyMarkdown = useCallback(() => { if (!editorRef.current) return; - copyTextToClipboard(editorRef.current.getMarkDown()).then(() => + editorRef.current.copyMarkdownToClipboard().then(() => setToast({ type: TOAST_TYPE.SUCCESS, title: t("toast.success"), message: "Markdown copied to clipboard.", }) ); }, [t]);This hooks the modal into the same clipboard pipeline as the rest of the editor and avoids unused imports.
🧹 Nitpick comments (2)
packages/editor/src/core/plugins/paste-asset.ts (1)
11-21: Minor cleanup of clipboard read logicThe logic is fine, but you can simplify and make the type clearer by avoiding repeated
getDatacalls and using a single, well-typed value:
- Read
"text/plane-editor-html"once.- Early-return if it’s empty.
- Build a separate
htmlWithMeta(or reassign) and pass that downstream.For example:
- let htmlContent; - if (event.clipboardData.getData("text/plane-editor-html")) { - htmlContent = event.clipboardData.getData("text/plane-editor-html"); - const metaTag = document.createElement("meta"); - metaTag.setAttribute("charset", "utf-8"); - htmlContent = metaTag.outerHTML + htmlContent; - } else { - return false; - } + const planeEditorHtml = event.clipboardData.getData("text/plane-editor-html"); + if (!planeEditorHtml) return false; + + const metaTag = document.createElement("meta"); + metaTag.setAttribute("charset", "utf-8"); + const htmlContent = metaTag.outerHTML + planeEditorHtml;
htmlContentthen continues to work with the existing checks andprocessAssetDuplicationcall.packages/editor/src/core/helpers/editor-ref.ts (1)
81-112: copyMarkdownToClipboard implementation aligns with the new clipboard flowThe implementation correctly:
- Derives Markdown from the current HTML + meta.
- Sets
text/plainto Markdown and bothtext/htmlandtext/plane-editor-htmlto the HTML.- Uses a one‑shot
copylistener and cleans it up immediately after.To reduce the chance of future typos, consider centralizing the
"text/plane-editor-html"MIME string in a shared constant used here and in the paste/clipboard plugins, rather than hardcoding it in multiple places.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/api/plane/app/views/asset/v2.py(2 hunks)apps/web/core/components/core/description-versions/modal.tsx(1 hunks)packages/editor/src/core/helpers/editor-ref.ts(1 hunks)packages/editor/src/core/plugins/paste-asset.ts(1 hunks)packages/editor/src/core/types/editor.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/types/editor.tspackages/editor/src/core/plugins/paste-asset.tspackages/editor/src/core/helpers/editor-ref.tsapps/web/core/components/core/description-versions/modal.tsx
🧠 Learnings (3)
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
Repo: makeplane/plane PR: 7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/asset/v2.py
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/core/description-versions/modal.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/core/description-versions/modal.tsx
🧬 Code graph analysis (1)
packages/editor/src/core/helpers/editor-ref.ts (1)
packages/utils/src/editor/markdown-parser/root.ts (1)
convertHTMLToMarkdown(112-143)
⏰ 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). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (1)
packages/editor/src/core/types/editor.ts (1)
100-122: NewcopyMarkdownToClipboardAPI signature looks consistentAdding
copyMarkdownToClipboard: () => Promise<void>;to
EditorRefApimatches the helper implementation and cleanly exposes the new clipboard behavior to consumers.
…board with copyMarkdownToClipboard in editor components
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/core/components/core/description-versions/modal.tsx (1)
60-69: Good adoption of the new API, but consider adding error handling.This change correctly addresses the past review feedback to use
editorRef's copy function. The implementation properly null-checks the ref and shows a success toast.However, consider adding error handling to inform users if the clipboard operation fails:
const handleCopyMarkdown = useCallback(() => { if (!editorRef.current) return; editorRef.current.copyMarkdownToClipboard().then(() => setToast({ type: TOAST_TYPE.SUCCESS, title: t("toast.success"), message: "Markdown copied to clipboard.", }) - ); + ).catch(() => + setToast({ + type: TOAST_TYPE.ERROR, + title: t("toast.error"), + message: "Failed to copy markdown to clipboard.", + }) + ); }, [t]);apps/web/core/components/pages/editor/toolbar/options-dropdown.tsx (1)
71-86: API adoption looks good; consider adding error handling.The change correctly adopts the new
copyMarkdownToClipboard()API with proper null-checking and success feedback.Consider adding error handling to provide user feedback if the clipboard operation fails:
{ key: "copy-markdown", action: () => { if (!editorRef) return; editorRef.copyMarkdownToClipboard().then(() => setToast({ type: TOAST_TYPE.SUCCESS, title: "Success!", message: "Markdown copied to clipboard.", }) - ); + ).catch(() => + setToast({ + type: TOAST_TYPE.ERROR, + title: "Error!", + message: "Failed to copy markdown to clipboard.", + }) + ); }, title: "Copy markdown", icon: Clipboard, shouldRender: true, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/core/components/core/description-versions/modal.tsx(1 hunks)apps/web/core/components/pages/editor/toolbar/options-dropdown.tsx(1 hunks)packages/editor/src/core/plugins/paste-asset.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/plugins/paste-asset.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/core/description-versions/modal.tsxapps/web/core/components/pages/editor/toolbar/options-dropdown.tsx
⏰ 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). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
aaryan610
changed the title
…handling and streamlining HTML processing
| const handleCopyMarkdown = useCallback(() => { | ||
| if (!editorRef.current) return; | ||
| copyTextToClipboard(editorRef.current.getMarkDown()).then(() => | ||
| editorRef.current.copyMarkdownToClipboard().then(() => |
| }); | ||
| return markdown; | ||
| }, | ||
| copyMarkdownToClipboard: async () => { |
…ipboard functionality
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/editor/src/core/helpers/paste-asset.ts (1)
24-25: Consider optimizing the repeated DOM updates.The pattern of updating
tempDiv.innerHTMLafter processing each component type (line 25) is functionally correct but inefficient. Each update re-parses the entire HTML string. Since the handlers modify HTML strings rather than DOM nodes directly, this approach is necessary for the current architecture.For better performance with large documents or many handlers, consider refactoring to work with DOM nodes directly instead of serializing and re-parsing HTML strings on each iteration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/editor/src/core/extensions/utility.ts(0 hunks)packages/editor/src/core/helpers/paste-asset.ts(1 hunks)packages/editor/src/core/props.ts(2 hunks)
💤 Files with no reviewable changes (1)
- packages/editor/src/core/extensions/utility.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/helpers/paste-asset.tspackages/editor/src/core/props.ts
🧬 Code graph analysis (1)
packages/editor/src/core/helpers/paste-asset.ts (1)
packages/editor/src/ce/helpers/asset-duplication.ts (1)
assetDuplicationHandlers(38-40)
🪛 ast-grep (0.40.0)
packages/editor/src/core/helpers/paste-asset.ts
[warning] 5-5: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: tempDiv.innerHTML = htmlContent
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 24-24: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: tempDiv.innerHTML = processedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 5-5: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: tempDiv.innerHTML = htmlContent
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 24-24: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: tempDiv.innerHTML = processedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
packages/editor/src/core/props.ts
[warning] 46-46: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: tempDiv.innerHTML = processedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 46-46: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: tempDiv.innerHTML = processedHtml
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
⏰ 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). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/editor/src/core/props.ts (1)
1-1: LGTM! Imports support the new paste handling logic.The new imports for
DOMParserandprocessAssetDuplicationare correctly added to support the custom paste handling functionality.Also applies to: 5-6
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/editor/src/core/helpers/editor-ref.ts (2)
92-112: Prefer modern Clipboard API over deprecatedexecCommand.While the current implementation works,
document.execCommand("copy")is deprecated. The modernnavigator.clipboard.write()API withClipboardItemsupports custom MIME types (contrary to the earlier discussion) and provides better error handling and permissions support.Consider this async alternative:
-copyMarkdownToClipboard: () => { +copyMarkdownToClipboard: async () => { if (!editor) return; const html = editor.getHTML(); const metaData = getEditorMetaData(html); const markdown = convertHTMLToMarkdown({ description_html: html, metaData, }); - const copyHandler = (event: ClipboardEvent) => { - event.preventDefault(); - event.clipboardData?.setData("text/plain", markdown); - event.clipboardData?.setData("text/html", html); - event.clipboardData?.setData("text/plane-editor-html", html); - document.removeEventListener("copy", copyHandler); - }; - - document.addEventListener("copy", copyHandler); - document.execCommand("copy"); + try { + const item = new ClipboardItem({ + "text/plain": new Blob([markdown], { type: "text/plain" }), + "text/html": new Blob([html], { type: "text/html" }), + "text/plane-editor-html": new Blob([html], { type: "text/plane-editor-html" }), + }); + await navigator.clipboard.write([item]); + } catch (error) { + console.error("Failed to copy to clipboard:", error); + throw error; + } },Note: This requires updating the calling code in
modal.tsxandoptions-dropdown.tsxto handle the async nature (e.g.,await editorRef.copyMarkdownToClipboard()or.then()), and updating the TypeScript signature in the EditorRefApi type definition.
102-112: Consider adding error handling for clipboard failures.The current implementation lacks error handling for cases where:
document.execCommand("copy")returnsfalse(copy failed)- Clipboard access is denied by the browser
clipboardDataisnull(silent failure)If keeping the current approach, consider:
const copyHandler = (event: ClipboardEvent) => { event.preventDefault(); - event.clipboardData?.setData("text/plain", markdown); - event.clipboardData?.setData("text/html", html); - event.clipboardData?.setData("text/plane-editor-html", html); + const clipboardData = event.clipboardData; + if (!clipboardData) { + console.error("Clipboard data unavailable"); + return; + } + clipboardData.setData("text/plain", markdown); + clipboardData.setData("text/html", html); + clipboardData.setData("text/plane-editor-html", html); document.removeEventListener("copy", copyHandler); }; document.addEventListener("copy", copyHandler); -document.execCommand("copy"); +const success = document.execCommand("copy"); +if (!success) { + document.removeEventListener("copy", copyHandler); + console.error("Copy command failed"); +}This provides better observability when clipboard operations fail.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/core/components/core/description-versions/modal.tsx(1 hunks)apps/web/core/components/pages/editor/toolbar/options-dropdown.tsx(1 hunks)packages/editor/src/core/helpers/editor-ref.ts(1 hunks)packages/editor/src/core/types/editor.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/editor/src/core/types/editor.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/core/description-versions/modal.tsxpackages/editor/src/core/helpers/editor-ref.tsapps/web/core/components/pages/editor/toolbar/options-dropdown.tsx
🧬 Code graph analysis (3)
apps/web/core/components/core/description-versions/modal.tsx (2)
packages/propel/src/toast/toast.tsx (1)
setToast(202-222)packages/i18n/src/store/index.ts (1)
t(223-244)
packages/editor/src/core/helpers/editor-ref.ts (1)
packages/utils/src/editor/markdown-parser/root.ts (1)
convertHTMLToMarkdown(112-143)
apps/web/core/components/pages/editor/toolbar/options-dropdown.tsx (1)
packages/propel/src/toast/toast.tsx (1)
setToast(202-222)
⏰ 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). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/web/core/components/core/description-versions/modal.tsx (1)
60-68: LGTM! Simplified clipboard flow.The refactored implementation using
editorRef.current.copyMarkdownToClipboard()is cleaner and more direct than the previous promise-based approach. The synchronous toast feedback provides immediate user confirmation.apps/web/core/components/pages/editor/toolbar/options-dropdown.tsx (1)
73-81: LGTM! Consistent with editor API updates.The migration to
editorRef.copyMarkdownToClipboard()maintains consistency with the changes in the description versions modal and simplifies the copy flow.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/editor/src/core/props.ts (1)
1-1: Remove unused import.The
DOMParserimport from@tiptap/pm/modelis not used anywhere in this file. This appears to be leftover from a previous implementation that has since been refactored to useview.pasteHTMLinstead.Apply this diff to remove the unused import:
-import { DOMParser } from "@tiptap/pm/model"; - import type { EditorProps } from "@tiptap/pm/view";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/editor/src/core/props.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/props.ts
🧬 Code graph analysis (1)
packages/editor/src/core/props.ts (1)
packages/editor/src/core/helpers/paste-asset.ts (1)
processAssetDuplication(4-30)
⏰ 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). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
| handlePaste: (view, event) => { | ||
| if (!event.clipboardData) return false; | ||
|
|
||
| const htmlContent = event.clipboardData.getData("text/plane-editor-html"); | ||
| if (!htmlContent) return false; | ||
|
|
||
| const { processedHtml } = processAssetDuplication(htmlContent); | ||
| event.preventDefault(); | ||
| view.pasteHTML(processedHtml); | ||
| return true; | ||
| }, |
There was a problem hiding this comment.
Add error handling and respect the hasChanges flag.
The implementation has two concerns:
-
Ignoring the
hasChangesflag: TheprocessAssetDuplicationfunction returns bothprocessedHtmlandhasChanges, but the code only usesprocessedHtml. This means the handler always prevents the default paste behavior and re-pastes the content even when no asset processing occurred, adding unnecessary overhead. IfhasChangesis false, the default paste behavior should be allowed. -
Missing error handling: If
processAssetDuplicationorview.pasteHTMLthrows an error, it will propagate uncaught, potentially breaking the paste functionality.
Apply this diff to respect the hasChanges flag and add error handling:
handlePaste: (view, event) => {
- if (!event.clipboardData) return false;
-
- const htmlContent = event.clipboardData.getData("text/plane-editor-html");
- if (!htmlContent) return false;
-
- const { processedHtml } = processAssetDuplication(htmlContent);
- event.preventDefault();
- view.pasteHTML(processedHtml);
- return true;
+ try {
+ if (!event.clipboardData) return false;
+
+ const htmlContent = event.clipboardData.getData("text/plane-editor-html");
+ if (!htmlContent) return false;
+
+ const { processedHtml, hasChanges } = processAssetDuplication(htmlContent);
+ if (!hasChanges) return false;
+
+ event.preventDefault();
+ view.pasteHTML(processedHtml);
+ return true;
+ } catch (error) {
+ console.error("Error handling paste:", error);
+ return false;
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| handlePaste: (view, event) => { | |
| if (!event.clipboardData) return false; | |
| const htmlContent = event.clipboardData.getData("text/plane-editor-html"); | |
| if (!htmlContent) return false; | |
| const { processedHtml } = processAssetDuplication(htmlContent); | |
| event.preventDefault(); | |
| view.pasteHTML(processedHtml); | |
| return true; | |
| }, | |
| handlePaste: (view, event) => { | |
| try { | |
| if (!event.clipboardData) return false; | |
| const htmlContent = event.clipboardData.getData("text/plane-editor-html"); | |
| if (!htmlContent) return false; | |
| const { processedHtml, hasChanges } = processAssetDuplication(htmlContent); | |
| if (!hasChanges) return false; | |
| event.preventDefault(); | |
| view.pasteHTML(processedHtml); | |
| return true; | |
| } catch (error) { | |
| console.error("Error handling paste:", error); | |
| return false; | |
| } | |
| }, |
🤖 Prompt for AI Agents
In packages/editor/src/core/props.ts around lines 33 to 43, update the paste
handler to respect the hasChanges flag returned by processAssetDuplication and
to guard against exceptions: call processAssetDuplication and check its
hasChanges boolean — only call event.preventDefault() and
view.pasteHTML(processedHtml) when hasChanges is true (if false, return false to
allow default paste), and wrap the processing and paste calls in a try/catch; on
error log the exception (or call an existing logger) and return false so paste
doesn't crash the app.
…for simplified asset processing
Description
This PR update the image handling and clipboard functionality in the editor, as well as a minor fix to remove workspace specific check in the backend asset lookup logic for duplication.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.