[WIKI-509] feat: comment copy link option #7385
Conversation
WalkthroughThis update refactors the comment card functionality by splitting the previous monolithic component into separate display, edit, and quick action components. It introduces a new custom hook for hash-based scrolling, adds a comment link copying feature, updates types and translation files for multiple languages, and adjusts related imports and props accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommentCard
participant CommentQuickActions
participant useCommentOperations
participant Clipboard
participant Toast
User->>CommentCard: Opens comment options
CommentCard->>CommentQuickActions: Renders quick actions menu
User->>CommentQuickActions: Clicks "Copy Link"
CommentQuickActions->>useCommentOperations: copyCommentLink(commentId)
useCommentOperations->>Clipboard: Copies link
alt Success
Clipboard-->>useCommentOperations: Success
useCommentOperations->>Toast: Show "copy success"
else Failure
Clipboard-->>useCommentOperations: Failure
useCommentOperations->>Toast: Show "copy error"
end
sequenceDiagram
participant User
participant useHashScroll
participant DOM
User->>Browser: Navigates to URL with #commentId
Browser->>useHashScroll: hashchange event
useHashScroll->>DOM: scrollToElement(commentId)
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/web/core/components/issues/issue-detail/issue-activity/helper.tsx (1)
187-187: Critical: Incomplete dependency array in useMemo hook.The useMemo dependency array is missing several dependencies that are used within the operations object, including
issueDetails,projectDetails,currentUser,t(translation function), and various getter functions from the store hooks. This can cause stale closures where the operations object doesn't update when these dependencies change.Add the missing dependencies to prevent stale closures:
- }, [workspaceSlug, projectId, issueId, createComment, updateComment, uploadEditorAsset, removeComment]); + }, [ + workspaceSlug, + projectId, + issueId, + issueDetails, + projectDetails, + currentUser, + t, + createComment, + updateComment, + uploadEditorAsset, + removeComment, + createCommentReaction, + removeCommentReaction, + getCommentReactionsByCommentId, + commentReactionsByUser, + getCommentReactionById, + getUserDetails + ]);
🧹 Nitpick comments (12)
packages/i18n/src/locales/en/translations.json (1)
888-891: Avoid message duplication—reuse the existing generic “link copied” keysThe catalogue already exposes similar strings (
common.link_copied,common.link_copied_to_clipboard, etc.).
Adding another pair that differs only by the “Comment” prefix will fragment translations and burden all locale files.
Unless a comment-specific nuance is absolutely required, consider referencing the existing keys or hoisting these two into a shared namespace.No functional issue, just a localisation-maintenance heads-up.
packages/i18n/src/locales/zh-TW/translations.json (1)
1048-1051: Prefer “留言” over “評論” for consistencyThroughout the file, the concept comment is translated as 留言 (e.g.,
issue.comments.placeholder, create/update/remove toasts).
The newly-added copy-link messages switch to 評論, which breaks the established terminology and may confuse users.- "success": "評論連結已複製到剪貼簿", - "error": "複製評論連結時出錯。請稍後再試。" + "success": "留言連結已複製到剪貼簿", + "error": "複製留言連結時出錯。請稍後再試。"Aligning the wording keeps UX copy coherent across screens.
No other changes required.packages/i18n/src/locales/vi-VN/translations.json (1)
1045-1049: Replace “clipboard” with the localized term “bảng tạm” for consistency.Everywhere else in this locale, “clipboard” is translated as “bảng tạm” (e.g., keys 731–735, 913). Keeping the wording consistent avoids confusing users and improves the overall polish of the Vietnamese UI.
- "success": "Liên kết bình luận đã được sao chép vào clipboard", + "success": "Liên kết bình luận đã được sao chép vào bảng tạm",packages/i18n/src/locales/id/translations.json (1)
1046-1048: Favor phrasing consistency with existing “link copied” toastsOther clipboard-success strings in this locale (e.g.,
common.link_copied_to_clipboard,common.link_copied) omit the word “berhasil”. Aligning the tone across all success toasts keeps the UX consistent.- "success": "Tautan komentar berhasil disalin ke clipboard", + "success": "Tautan komentar disalin ke clipboard",packages/i18n/src/locales/pt-BR/translations.json (1)
1048-1050: Finalize punctuation for consistency with neighboring messagesMost success/error toast strings in this file terminate with a period.
"success"under"upload"(line 1046) follows that pattern, while the newly-added"copy_link.success"string omits it. Add the trailing period to align with the prevailing convention and avoid style drift.- "success": "Link do comentário copiado para a área de transferência", + "success": "Link do comentário copiado para a área de transferência.",packages/i18n/src/locales/zh-CN/translations.json (1)
1047-1049: Keep error-message wording consistent with neighbouring stringsOther error messages in the same
"issue.comments"block use “…请稍后重试。”.
For consistency, replace “再试” with “重试”.- "error": "复制评论链接时出错。请稍后再试。" + "error": "复制评论链接时出错。请稍后重试。"packages/i18n/src/locales/ko/translations.json (1)
1046-1049: Unify error-message wording & spacing with existing stringsMost other error messages use the phrasing
다시 시도해주세요.(no space between시도해and주세요).
The newly-added copy-link error has an extra space:다시 시도해 주세요.– this is inconsistent and will look jarring in the UI.- "error": "댓글 링크 복사 중 오류가 발생했습니다. 나중에 다시 시도해 주세요." + "error": "댓글 링크 복사 중 오류가 발생했습니다. 나중에 다시 시도해주세요."packages/i18n/src/locales/ja/translations.json (1)
1047-1049: Nit: adjust particle for more natural Japanese
がクリップボードにコピーされましたsounds slightly stilted compared with existing success toasts that use the direct-object particleを. Consider:- "success": "コメントリンクがクリップボードにコピーされました", + "success": "コメントリンクをクリップボードにコピーしました",(No functional impact – purely wording.)
packages/i18n/src/locales/pl/translations.json (1)
1048-1050: Prefer “skopiowano” / “linku” for style & grammar consistencyElsewhere in this file the passive-voice form „skopiowano” is used (e.g. line 312: „Link do projektu skopiowano do schowka”).
Likewise, the genitive of link is „linku”, not „linka”. Updating keeps the wording uniform and grammatically correct."copy_link": { - "success": "Link do komentarza skopiowany do schowka", - "error": "Błąd podczas kopiowania linka do komentarza. Spróbuj ponownie później." + "success": "Link do komentarza skopiowano do schowka", + "error": "Błąd podczas kopiowania linku do komentarza. Spróbuj ponownie później." }packages/i18n/src/locales/ua/translations.json (1)
1048-1050: Unify clipboard-related phrasing with existing stringsEarlier messages (e.g.
"link_copied_to_clipboard", line 733) use “до буфера обміну” and start error texts with “Не вдалося …”.
Keeping wording consistent across the locale prevents jarring UX.- "success": "Посилання на коментар скопійовано в буфер обміну", - "error": "Помилка при копіюванні посилання на коментар. Спробуйте пізніше." + "success": "Посилання на коментар скопійовано до буфера обміну", + "error": "Не вдалося скопіювати посилання на коментар. Спробуйте пізніше."packages/i18n/src/locales/ro/translations.json (1)
1044-1049: Keep terminology & phrasing consistent with existing Romanian stringsElsewhere in this file we translate “clipboard” as “memoria temporară” and we hyphen-separate “Link-ul”.
For coherence, tweak the new keys accordingly and align the error wording with the prevailing “Te rugăm să încerci din nou” pattern.- "success": "Linkul comentariului a fost copiat în clipboard", - "error": "Eroare la copierea linkului comentariului. Încercați din nou mai târziu." + "success": "Link-ul comentariului a fost copiat în memoria temporară", + "error": "Eroare la copierea link-ului comentariului. Te rugăm să încerci din nou mai târziu."apps/web/core/components/issues/issue-detail/issue-activity/helper.tsx (1)
36-62: Consider making copyCommentLink async for better error handling.The function currently doesn't return a Promise, which prevents callers from handling errors or knowing when the operation completes. Making it async would improve the API consistency with other operations.
Update the function signature and implementation:
- copyCommentLink: (id) => { + copyCommentLink: async (id) => { if (!workspaceSlug || !issueDetails) return; try { const workItemLink = generateWorkItemLink({ workspaceSlug, projectId: issueDetails.project_id, issueId, projectIdentifier: projectDetails?.identifier, sequenceId: issueDetails.sequence_id, }); const commentLink = `${workItemLink}#comment-${id}`; await copyUrlToClipboard(commentLink); setToast({ title: t("common.success"), type: TOAST_TYPE.SUCCESS, message: t("issue.comments.copy_link.success"), }); } catch (error) { console.error("Error in copying comment link:", error); setToast({ title: t("common.error.label"), type: TOAST_TYPE.ERROR, message: t("issue.comments.copy_link.error"), }); } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
apps/web/ce/components/comments/comment-block.tsx(1 hunks)apps/web/core/components/comments/card/display.tsx(1 hunks)apps/web/core/components/comments/card/edit-form.tsx(1 hunks)apps/web/core/components/comments/card/root.tsx(1 hunks)apps/web/core/components/comments/comment-card.tsx(0 hunks)apps/web/core/components/comments/comments.tsx(3 hunks)apps/web/core/components/comments/quick-actions.tsx(1 hunks)apps/web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx(2 hunks)apps/web/core/components/issues/issue-detail/issue-activity/helper.tsx(8 hunks)apps/web/core/hooks/use-hash-scroll.ts(1 hunks)packages/i18n/src/locales/cs/translations.json(1 hunks)packages/i18n/src/locales/de/translations.json(1 hunks)packages/i18n/src/locales/en/translations.json(1 hunks)packages/i18n/src/locales/es/translations.json(1 hunks)packages/i18n/src/locales/fr/translations.json(1 hunks)packages/i18n/src/locales/id/translations.json(1 hunks)packages/i18n/src/locales/it/translations.json(1 hunks)packages/i18n/src/locales/ja/translations.json(1 hunks)packages/i18n/src/locales/ko/translations.json(1 hunks)packages/i18n/src/locales/pl/translations.json(1 hunks)packages/i18n/src/locales/pt-BR/translations.json(1 hunks)packages/i18n/src/locales/ro/translations.json(1 hunks)packages/i18n/src/locales/ru/translations.json(1 hunks)packages/i18n/src/locales/sk/translations.json(1 hunks)packages/i18n/src/locales/tr-TR/translations.json(1 hunks)packages/i18n/src/locales/ua/translations.json(1 hunks)packages/i18n/src/locales/vi-VN/translations.json(1 hunks)packages/i18n/src/locales/zh-CN/translations.json(1 hunks)packages/i18n/src/locales/zh-TW/translations.json(1 hunks)packages/types/src/issues/activity/issue_comment.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/core/components/comments/comment-card.tsx
🧰 Additional context used
🧠 Learnings (4)
apps/web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (1)
Learnt from: mathalav55
PR: makeplane/plane#6107
File: web/ce/components/workspace-notifications/sidebar/notification-card/options/archive.tsx:11-14
Timestamp: 2024-11-28T07:02:54.664Z
Learning: When components are still located in `core`, it's appropriate for files to import them using `@/components/...`, and the migration to the new import paths is not necessary in such cases.
packages/i18n/src/locales/pl/translations.json (1)
Learnt from: janreges
PR: makeplane/plane#6743
File: packages/i18n/src/store/index.ts:160-161
Timestamp: 2025-03-11T19:42:41.769Z
Learning: In the Plane project, the file 'packages/i18n/src/store/index.ts' already includes support for Polish language translations with the case "pl".
apps/web/core/components/issues/issue-detail/issue-activity/helper.tsx (1)
Learnt from: vamsikrishnamathala
PR: makeplane/plane#7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
packages/i18n/src/locales/cs/translations.json (1)
Learnt from: janreges
PR: makeplane/plane#6743
File: packages/i18n/src/store/index.ts:160-161
Timestamp: 2025-03-11T19:42:41.769Z
Learning: In the Plane project, the file 'packages/i18n/src/store/index.ts' already includes support for Polish language translations with the case "pl".
🧬 Code Graph Analysis (1)
apps/web/ce/components/comments/comment-block.tsx (1)
packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (32)
packages/i18n/src/locales/de/translations.json (1)
1044-1049: Translation entries look correct and consistentThe new
copy_link.success/errormessages are well-formed, use clear German phrasing, and fit the established JSON structure. No issues spotted.packages/i18n/src/locales/es/translations.json (1)
1048-1052: LGTM! Spanish translations are accurate and well-structured.The comment copy link translations are grammatically correct and appropriately placed within the "issue.comments" section. The success and error messages provide clear feedback to Spanish-speaking users.
apps/web/core/hooks/use-hash-scroll.ts (5)
4-12: Well-defined TypeScript interfaces.The type definitions are clear and provide good structure for the hook's API. The return type clearly indicates what consumers can expect.
63-71: Robust hash parsing with good delimiter support.The
extractIdsFromHashfunction handles multiple delimiters well and includes proper string cleaning. The regex pattern and filtering logic are appropriate for parsing URL hash fragments.
77-81: Proper SSR safety check.The
getCurrentHashfunction correctly handles server-side rendering by checking forwindowavailability and safely extracts the hash value.
84-117: Well-structured effect with proper cleanup.The
useEffectimplementation correctly:
- Handles initial load and hash changes
- Includes proper event listener cleanup
- Has appropriate dependencies in the dependency array
- Manages state updates safely
The effect properly responds to route changes via the
pathnamedependency, which is important for SPA navigation.
120-135: Clear and well-documented return interface.The return object provides a clean API with descriptive JSDoc comments. The interface makes it easy for consumers to understand what each property represents.
packages/i18n/src/locales/tr-TR/translations.json (1)
1046-1052: All locale JSON files validated successfullyThe
tr-TRtranslations foruploadandcopy_linkare clear, and all JSON files parse without errors. Nothing else to address here.packages/i18n/src/locales/it/translations.json (1)
1043-1046: Add the sameissue.comments.copy_linkkeys to every base/other locale for parityThe Italian strings look correct, but make sure the equivalent keys are present in the default (
en) locale and any other supported locales; otherwise the UI will fall back or render raw keys for those languages.
No action needed in this file.packages/i18n/src/locales/ru/translations.json (1)
1047-1051: LGTM! Well-structured Russian translations for the copy link feature.The translation keys follow the established pattern and provide clear, contextually appropriate feedback messages for both success and error scenarios of the comment copy link functionality.
apps/web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (2)
7-7: LGTM! Import path correctly updated for the refactored component structure.The import path change from
"@/components/comments/comment-card"to"@/components/comments/card/root"aligns with the component modularization described in the PR.
60-62: LGTM! Good prop handling for the new comment features.The explicit boolean conversion
!!showAccessSpecifierimproves type safety, and the newshowCopyLinkOptionprop correctly enables the copy link functionality. The implicittruevalue is appropriate since this feature should be available in the issue activity context.packages/i18n/src/locales/ko/translations.json (1)
1046-1047: Sanity-check trailing comma after"upload"blockLine 1046 closes the
"upload"object and adds a comma.
If the"copy_link"block is the final property under"comments", that comma will break the JSON. Double-check the lines that follow – the safest option is:}, // keep comma only when more keys follow "copy_link": { … }Run a quick
jq . translations.jsonto ensure the file still parses.packages/i18n/src/locales/ja/translations.json (1)
1047-1049: All locale bundles includecopy_linktranslations
Verified that everytranslations.jsonfile defines bothissue.comments.copy_link.successandissue.comments.copy_link.error. No missing keys were detected.packages/i18n/src/locales/cs/translations.json (1)
1044-1048: Copy-link strings LGTMThe added
copy_linktranslations are grammatically correct, follow existing wording style, and keep the JSON structure valid. No further changes required.packages/i18n/src/locales/fr/translations.json (1)
1046-1050: Copy-link translations look good.Both success and error messages are idiomatic, consistent with earlier “Lien … copié dans le presse-papiers” patterns, and punctuation matches the surrounding file. No further action needed.
apps/web/ce/components/comments/comment-block.tsx (1)
20-29: Minor internal tidy-up acknowledged.• Added section comments improve readability.
• MovinguseTranslationlower and returningnullinstead of an empty fragment are harmless behavioural no-ops and slightly clearer.Nothing else to flag.
packages/i18n/src/locales/sk/translations.json (1)
1047-1051: Translation strings added correctly for Slovak locale.The new
copy_linktranslations underissue.commentsare well-structured and follow the existing pattern. The Slovak translations appear appropriate for success and error scenarios when copying comment links.apps/web/core/components/comments/comments.tsx (4)
10-10: Import path updated correctly.The import path change from
"./comment-card"to"./card/root"reflects the component refactoring and modularization.
21-22: New props added with appropriate typing.The optional boolean props
showAccessSpecifierandshowCopyLinkOptionare well-named and properly typed, enabling conditional feature rendering.
26-35: Props destructured cleanly with sensible defaults.The destructuring includes appropriate default values (
false) for the new props, ensuring backward compatibility while enabling the new features.
74-75: Props correctly passed down to CommentCard.The new props are properly passed to each
CommentCardinstance, enabling the conditional rendering of access specifier and copy link features.packages/types/src/issues/activity/issue_comment.ts (2)
44-44: New copyCommentLink method added correctly.The method signature is appropriate for copying comment links - takes a
commentIdstring parameter and returnsvoidsince it's a side-effect operation.
50-50: deleteCommentReaction signature update verified
All occurrences ofdeleteCommentReactionnow use two parameters (commentId,reactionEmoji), matching the updated type definition. No references to a thirduserReactionsparameter remain.apps/web/core/components/comments/card/root.tsx (3)
1-25: Well-structured component imports and type definitions.The imports are organized logically and the
TCommentCardtype interface is comprehensive with appropriate optional properties.
26-44: Clean component structure with proper state management.The component uses appropriate React patterns:
- Proper destructuring with defaults
- useRef for editor reference
- useState for editing mode
- Early return for null checks
45-85: Excellent composition pattern implementation.The component effectively composes smaller components (
CommentCardDisplay,CommentCardEditForm,CommentQuickActions) and manages the editing state transitions cleanly. The conditional rendering logic is clear and maintainable.apps/web/core/components/comments/quick-actions.tsx (3)
1-33: Excellent component setup and permissions logic.The component structure is well-organized with proper imports, type definitions, and clear permission derivation based on user authorization.
34-75: Well-structured menu items with proper optimization.The
useMemooptimization is appropriate here and the menu item configuration is clean and maintainable. Each item has proper conditional rendering logic and integrates well with the translation system.
77-113: Clean menu rendering with proper accessibility.The menu rendering logic is excellent with:
- Proper conditional rendering based on
shouldRender- Good use of styling utilities
- Accessible menu structure
- Proper icon and text rendering
apps/web/core/components/comments/card/display.tsx (1)
1-82: Well-structured component with proper React patterns.The component correctly implements hash-based comment highlighting with appropriate cleanup, conditional rendering of access specifiers, and proper integration with MobX for reactive updates. The timeout cleanup in the useEffect hook prevents memory leaks, and the component structure follows React best practices.
apps/web/core/components/comments/card/edit-form.tsx (1)
1-130: Comprehensive edit form implementation with proper form handling.The component effectively combines react-hook-form with the rich text editor, implements proper keyboard shortcuts, handles file uploads, and manages form states correctly. The async submission flow and button state management are well implemented.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/hooks/src/use-hash-scroll.ts (4)
15-21: Fix JSDoc parameter documentation mismatch.The JSDoc says
@param {TArgs} argsbut the function uses destructured parameters. Update the documentation to match the actual implementation./** * Custom hook for handling hash-based scrolling to a specific element * Supports multiple IDs in URL hash (comma-separated, space-separated, or other delimiters) * - * @param {TArgs} args - The ID of the element to scroll to + * @param {TArgs} args - Configuration object containing elementId, pathname, and optional scrollDelay * @returns {TReturnType} Object containing hash match status and scroll function */
49-50: Consider using a more appropriate error handling approach.Using
console.warnfor production errors might not be ideal. Consider using a proper error reporting mechanism or making the error handling configurable.} catch (error) { - console.warn("Hash scroll error:", error); + // Consider using a proper error reporting mechanism + if (process.env.NODE_ENV === 'development') { + console.warn("Hash scroll error:", error); + } return false; }
61-69: Optimize regex compilation for better performance.The regex pattern is being created on every function call. Consider extracting it to a constant for better performance.
+const DELIMITER_REGEX = /[,\s|;]+/; + const extractIdsFromHash = (hashString: string | null): string[] => { if (!hashString) return []; // Split by common delimiters and clean up return hashString - .split(/[,\s|;]+/) // Split by comma, space, pipe, or semicolon + .split(DELIMITER_REGEX) // Split by comma, space, pipe, or semicolon .map((id) => id.trim()) // Remove whitespace .filter((id) => id.length > 0); // Remove empty strings };
23-23: Add input validation for scrollDelay.The hook doesn't validate that
scrollDelayis a positive number, which could cause unexpected behavior.-const { elementId, pathname, scrollDelay = 200 } = args; +const { elementId, pathname, scrollDelay = 200 } = args; + +// Validate scrollDelay is a positive number +const validScrollDelay = typeof scrollDelay === 'number' && scrollDelay >= 0 ? scrollDelay : 200;Then use
validScrollDelayinstead ofscrollDelaythroughout the hook.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/core/components/comments/card/display.tsx(1 hunks)packages/hooks/src/index.ts(1 hunks)packages/hooks/src/use-hash-scroll.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/hooks/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/core/components/comments/card/display.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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/hooks/src/use-hash-scroll.ts (1)
82-115: LGTM! Well-structured effect with proper cleanup.The useEffect properly handles hash changes, initial load, and cleanup. The dependency array correctly includes
pathnameto handle route changes, and the event listener cleanup is properly implemented.
Description
This PR includes the following changes-
Type of Change
Media
Screen.Recording.2025-07-10.at.19.33.29.mov
Summary by CodeRabbit
New Features
Improvements
Bug Fixes