[WEB-3698] fix: comments refactor #6759
Conversation
WalkthroughThe changes modify how issue comments are updated and managed across both the backend API and frontend components. The API now conditionally updates the comment’s edit timestamp, while the TypeScript definitions have been extended to support reactions and various comment operations. The frontend has seen the introduction of new and renamed components for displaying, creating, and reacting to comments, along with updates to editor components and services. Additionally, helper hooks and store updates streamline comment operations and metadata tracking throughout the system. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ViewSet
participant Serializer
participant ActivityHandler
Client->>ViewSet: PATCH /issue/{id} with comment_html
ViewSet->>ViewSet: Check if comment_html exists and has changed
alt Edited Content Changed
ViewSet->>Serializer: save(edited_at = now)
else No Change Detected
ViewSet->>Serializer: save()
end
Serializer-->>ViewSet: return updated comment
ViewSet->>ActivityHandler: trigger activity delay
ActivityHandler-->>ViewSet: activity processed
ViewSet-->>Client: return response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![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
🔭 Outside diff range comments (1)
web/core/store/issue/issue-details/comment_reaction.store.ts (1)
182-182:⚠️ Potential issuePotential bug in reaction removal.
The removal code is deleting
this.commentReactionMap[reaction]but should likely be deletingthis.commentReactionMap[currentReaction.id]. This could be removing entire reaction categories rather than specific reaction instances.- delete this.commentReactionMap[reaction]; + delete this.commentReactionMap[currentReaction.id];
🧹 Nitpick comments (11)
packages/types/src/issues/activity/issue_comment.d.ts (3)
28-28: Redundant type definitionThe
edited_atproperty has both the optional property notation (?) and the union withundefined, which is redundant since the optional property notation already implies that the property could be undefined.- edited_at?: string | undefined; + edited_at?: string;
33-33: Consider using a more specific typeThe
comment_reactionsproperty is typed asany[], which doesn't provide type safety. Consider using the newly definedTCommentReaction[]instead for better type checking.- comment_reactions: any[]; + comment_reactions: TCommentReaction[];
42-57: Well-defined operations interface with some inconsistenciesThe
TCommentsOperationsinterface provides a comprehensive set of operations for comment management. However, there's an inconsistency in parameter types:deleteCommentReactionacceptsuserReactions: TCommentReaction[]whilereactacceptsuserReactions: string[].Consider making these parameter types consistent based on your intended usage pattern.
web/core/components/editor/lite-text-editor/lite-text-editor.tsx (1)
109-135: Consider simplifying toolbar visibility logic.While the conditional rendering based on
showToolbaris good, the component still uses bothshowToolbarandisFocusedstates to control toolbar visibility. This dual mechanism could be confusing.You might want to simplify the logic:
- {showToolbar && ( - <div - className={cn( - "transition-all duration-300 ease-out origin-top overflow-hidden", - isFocused ? "max-h-[200px] opacity-100 scale-y-100 mt-3" : "max-h-0 opacity-0 scale-y-0 invisible" - )} - > + {showToolbar && ( + <div + className={cn( + "transition-all duration-300 ease-out origin-top overflow-hidden", + isFocused ? "max-h-[200px] opacity-100 scale-y-100 mt-3" : "max-h-0 opacity-0 scale-y-0 invisible" + )} + >Consider adding a more explicit comment explaining how
showToolbar(render/don't render) andisFocused(animate visibility) work together.web/core/store/issue/issue-details/comment_reaction.store.ts (1)
147-147: Consider addressing the ESLint warning properly.Adding a disable comment for
no-useless-catchsuggests there's an underlying issue with the error handling pattern. Consider either removing the try-catch block if it's not adding value, or implementing proper error handling.- // eslint-disable-next-line no-useless-catch try { // existing code } catch (error) { - throw error; + // Add proper error handling here + console.error("Failed to create reaction:", error); + throw error; }web/core/components/comments/comments.tsx (1)
38-59: Consider handling empty state and improving type checking.The component handles both string IDs and direct comment objects, but could benefit from:
- An empty state UI when there are no comments
- More explicit handling of the mixed types in the comments array
- A fallback UI for when
getCommentByIdreturns undefined<div className="flex-grow py-4 overflow-y-auto"> + {comments?.length === 0 && ( + <div className="flex justify-center items-center h-full text-custom-text-300 text-sm"> + No comments yet + </div> + )} {comments?.map((r, index) => { let comment; if (typeof r === "string") { comment = getCommentById?.(r); } else { comment = r; } - if (!comment) return null; + if (!comment) return <div className="text-custom-text-300 text-sm">Comment not found</div>; return ( <CommentCard key={comment.id} workspaceSlug={workspaceSlug} comment={comment as TIssueComment} activityOperations={activityOperations} disabled={!isEditingAllowed} ends={index === 0 ? "top" : index === comments.length - 1 ? "bottom" : undefined} projectId={projectId} /> ); })} </div>web/core/components/comments/comment-reaction.tsx (1)
38-64: Fix unnecessary fragment and use optional chaining.There are a couple of small issues in the reaction rendering code:
- The fragment on lines 42-62 is unnecessary since it wraps a single child element.
- Missing optional chaining when accessing reactionIds length.
{reactionIds && Object.keys(reactionIds || {}).map( (reaction: string) => reactionIds[reaction]?.length > 0 && ( - <> <Tooltip tooltipContent={activityOperations.getReactionUsers(reaction, reactionIds)}> <button type="button" onClick={() => !disabled && activityOperations.react(comment.id, reaction, userReactions)} key={reaction} className={cn( "flex h-full items-center gap-1 rounded-md px-2 py-1 text-sm text-custom-text-100", userReactions.includes(reaction) ? "bg-custom-primary-100/10" : "bg-custom-background-80", { "cursor-not-allowed": disabled, } )} > <span>{renderEmoji(reaction)}</span> <span className={userReactions.includes(reaction) ? "text-custom-primary-100" : ""}> - {(reactionIds || {})[reaction].length}{" "} + {(reactionIds || {})[reaction]?.length}{" "} </span> </button> </Tooltip> - </> ) )}🧰 Tools
🪛 Biome (1.9.4)
[error] 58-58: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 42-62: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (1)
53-62: Add null check for getCommentById result.The component directly uses the result of
getCommentByIdwithout checking if it returns a valid comment object. This could lead to runtime errors if the comment doesn't exist.{filteredActivityComments.map((activityComment, index) => activityComment.activity_type === "COMMENT" ? ( + (() => { + const comment = getCommentById(activityComment.id); + return comment ? ( <CommentCard key={activityComment.id} workspaceSlug={workspaceSlug} - comment={getCommentById(activityComment.id)} + comment={comment} activityOperations={activityOperations} ends={index === 0 ? "top" : index === filteredActivityComments.length - 1 ? "bottom" : undefined} showAccessSpecifier={showAccessSpecifier} disabled={disabled} projectId={projectId} /> + ) : null; + })() ) : activityComment.activity_type === "ACTIVITY" ? (web/core/components/comments/comment-create.tsx (1)
28-29: Consider adding error handling aroundfileServiceoperations
Currently, any failure withinFileServicecalls is neither caught here nor signaled upstream. You might want to handle or propagate errors to provide more robust feedback.const fileService = new FileService(); export const CommentCreate: FC<TCommentCreate> = observer((props) => { + // Consider try/catch around fileService call sites for reliabilityweb/core/components/comments/comment-card.tsx (1)
125-131: Optional confirmation before removing comments
Currently, the delete action is triggered immediately. Consider adding a confirmation dialog or an undo flow to avoid accidental deletion.web/core/components/issues/issue-detail/issue-activity/helper.tsx (1)
1-159: Overall design ofuseCommentOperationsis solid
The dedicated hook centralizes comment creation, updates, and reactions gracefully. One minor note: operations likecreateCommentreturnundefinedon error, so calling code won’t easily detect a failure. Consider re-throwing or returning a rejected promise to ensure upstream handlers can react to errors.+ } catch (error) { + setToast({...}); + throw error; // re-throw to allow callers to handle the error too + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
apiserver/plane/app/views/issue/comment.py(1 hunks)packages/types/src/issues/activity/issue_comment.d.ts(3 hunks)web/ce/components/comments/comment-block.tsx(1 hunks)web/ce/components/comments/index.ts(1 hunks)web/core/components/comments/comment-card.tsx(6 hunks)web/core/components/comments/comment-create.tsx(5 hunks)web/core/components/comments/comment-reaction.tsx(1 hunks)web/core/components/comments/comments.tsx(1 hunks)web/core/components/comments/index.ts(1 hunks)web/core/components/editor/lite-text-editor/lite-text-editor.tsx(4 hunks)web/core/components/editor/lite-text-editor/lite-text-read-only-editor.tsx(1 hunks)web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx(3 hunks)web/core/components/issues/issue-detail/issue-activity/activity/actions/helpers/activity-block.tsx(1 hunks)web/core/components/issues/issue-detail/issue-activity/comments/comment-block.tsx(0 hunks)web/core/components/issues/issue-detail/issue-activity/comments/index.ts(0 hunks)web/core/components/issues/issue-detail/issue-activity/comments/root.tsx(0 hunks)web/core/components/issues/issue-detail/issue-activity/helper.tsx(1 hunks)web/core/components/issues/issue-detail/issue-activity/index.ts(0 hunks)web/core/components/issues/issue-detail/issue-activity/root.tsx(5 hunks)web/core/services/file.service.ts(1 hunks)web/core/services/issue/issue_comment.service.ts(1 hunks)web/core/store/issue/issue-details/comment.store.ts(1 hunks)web/core/store/issue/issue-details/comment_reaction.store.ts(1 hunks)web/ee/components/comments/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- web/core/components/issues/issue-detail/issue-activity/index.ts
- web/core/components/issues/issue-detail/issue-activity/comments/root.tsx
- web/core/components/issues/issue-detail/issue-activity/comments/index.ts
- web/core/components/issues/issue-detail/issue-activity/comments/comment-block.tsx
🧰 Additional context used
🧬 Code Definitions (8)
web/ce/components/comments/comment-block.tsx (1)
packages/types/src/issues/activity/issue_comment.d.ts (1)
TIssueComment(17-40)
web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (2)
packages/types/src/issues/activity/issue_comment.d.ts (1)
TCommentsOperations(42-57)web/core/components/comments/comment-card.tsx (1)
CommentCard(33-214)
web/core/components/issues/issue-detail/issue-activity/helper.tsx (1)
packages/types/src/issues/activity/issue_comment.d.ts (2)
TCommentsOperations(42-57)TIssueComment(17-40)
web/core/components/comments/comments.tsx (3)
packages/types/src/issues/activity/issue_comment.d.ts (2)
TCommentsOperations(42-57)TIssueComment(17-40)web/core/components/comments/comment-create.tsx (1)
CommentCreate(30-128)web/core/components/comments/comment-card.tsx (1)
CommentCard(33-214)
web/core/components/comments/comment-reaction.tsx (1)
packages/types/src/issues/activity/issue_comment.d.ts (2)
TIssueComment(17-40)TCommentsOperations(42-57)
packages/types/src/issues/activity/issue_comment.d.ts (1)
packages/types/src/users.d.ts (1)
IUserLite(6-14)
web/core/components/editor/lite-text-editor/lite-text-editor.tsx (1)
packages/editor/src/core/types/config.ts (1)
TFileHandler(8-20)
web/core/components/issues/issue-detail/issue-activity/root.tsx (2)
web/core/components/issues/issue-detail/issue-activity/helper.tsx (1)
useCommentOperations(9-158)web/core/components/comments/comment-create.tsx (1)
CommentCreate(30-128)
🪛 Biome (1.9.4)
web/core/components/comments/comment-reaction.tsx
[error] 58-58: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 42-62: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (41)
packages/types/src/issues/activity/issue_comment.d.ts (1)
11-16: Well-structured type for comment reactionsThe
TCommentReactiontype is well-designed, providing a clear structure for comment reactions with appropriate properties including the user details.web/ce/components/comments/index.ts (1)
1-1: Clean barrel exportThis is a good pattern for creating a centralized entry point for the comment block components, making imports cleaner throughout the codebase.
web/core/components/comments/index.ts (1)
1-1: Clean barrel exportThis re-export pattern provides a centralized access point for importing all comment-related components, which aligns well with the refactoring goals mentioned in the PR.
web/ee/components/comments/index.ts (1)
1-1: Good code reuse across editionsThis export statement enables code reuse between the Community Edition (CE) and Enterprise Edition (EE) components, which is a good practice for maintaining consistency while allowing for edition-specific extensions.
web/core/components/issues/issue-detail/issue-activity/activity/actions/helpers/activity-block.tsx (1)
51-51: Enhanced time display stylingThe addition of
text-custom-text-350class to the time display provides better visual hierarchy, making the timestamp appear in a lighter shade to differentiate it from the main content.web/core/components/editor/lite-text-editor/lite-text-read-only-editor.tsx (1)
20-20: Good enhancement to component flexibilityMaking
projectIdoptional allows this component to be used in more contexts where project information might not be available or necessary.web/core/store/issue/issue-details/comment.store.ts (1)
158-161: Improved timestamp handlingThe added code properly updates the comment's
updated_atandedited_attimestamps from the API response, ensuring the UI displays accurate timing information for edited comments. The use ofrunInActioncorrectly follows MobX best practices for handling state updates.web/core/services/file.service.ts (1)
112-124: Good addition of bulk workspace assets update functionalityThis new method complements the existing project-level method by providing similar functionality at the workspace level, supporting operations where multiple file assets need to be processed together.
The implementation follows the same pattern as other similar methods in this service, maintaining consistency in the codebase.
web/core/services/issue/issue_comment.service.ts (1)
66-66: Good return type update for better type safety.Changing the return type from
Promise<void>toPromise<TIssueComment>correctly aligns with the implementation, which already returns the response data. This type correction enables consumers of this method to access the updated comment data, including fields likeedited_atorupdated_at.apiserver/plane/app/views/issue/comment.py (1)
108-114: Good implementation of edited_at tracking.This change intelligently updates the
edited_attimestamp only when the actual comment content changes, not for other types of updates. This approach matches user expectations since users typically expect the "edited" indicator to appear only when content has been modified.web/core/components/editor/lite-text-editor/lite-text-editor.tsx (4)
26-26: Making projectId optional improves component flexibility.Making
projectIdoptional allows the component to be used in contexts where a project may not be relevant or available, enhancing the component's reusability.
33-37: Good addition of customization options.Adding
showToolbarandparentClassNameprops increases the component's flexibility, allowing for different UI configurations and custom styling depending on where it's used.
53-54: Sensible default values for new props.Setting sensible defaults (
showToolbar = trueandparentClassName = "") ensures backward compatibility while enabling the new customization options.
84-84: Good use of utility function for className composition.Using the
cnutility function for combining classNames with the newparentClassNameprop is a clean and maintainable approach to styling composition.web/core/store/issue/issue-details/comment_reaction.store.ts (2)
153-153: Good defensive programming.Adding a null check before accessing the reactions object prevents potential runtime errors when working with a new comment that doesn't have any reactions yet.
155-155: Improved object path handling.Changing from array notation to template string syntax for updating nested objects is more readable and consistent with the lodash
updatefunction's expectations.web/core/components/comments/comments.tsx (3)
12-19: Props type defined clearly with good TypeScript typing.The
TCommentsWrappertype is well-structured and provides clear typing for the component's props. The optional props are marked appropriately with?.
21-26: Properly structured component with clean parameter handling.The component setup is clean with good destructuring of props and proper retrieval of the workspace slug from router parameters.
27-36: Conditionally rendering CommentCreate based on permissions.The implementation correctly renders the comment creation form only when editing is allowed, and passes all necessary props to the
CommentCreatecomponent.web/ce/components/comments/comment-block.tsx (4)
11-16: Well-defined type structure for the CommentBlock component.The component props are clearly defined with appropriate types for each property.
18-25: Component setup with proper null checks.The component correctly handles cases where the comment or user details are not available by returning an empty fragment.
26-46: Clean UI implementation with responsive styling.The component includes responsive styling based on the
endsprop and properly displays user avatars. The use of conditional styling is well-implemented.
47-70: Well-implemented comment metadata display with tooltip.The component effectively displays user information and comment timestamps with a helpful tooltip showing formatted date and time. The edited status is also nicely handled.
web/core/components/comments/comment-reaction.tsx (3)
14-18: Clear props definition for the CommentReactions component.The component props are well-defined with appropriate types for each property.
20-26: Good initialization and null handling.The component retrieves user reactions and properly returns null when no reactions are found.
27-36: Well-implemented reaction selector component.The conditional rendering of the ReactionSelector based on disabled state is appropriate, and the component is well-configured with necessary props.
web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (3)
6-7: Updated imports to use new components.The imports have been appropriately updated to use the new
CommentCardcomponent andTCommentsOperationstype.
15-24: Type definition updated for TIssueActivityCommentRoot.The type definition has been properly updated to use
TCommentsOperationsinstead ofTActivityOperations, aligning with the new comment operation structure.
38-41: Updated hook destructuring to directly extract getCommentById.The destructuring from
useIssueDetailhas been updated to directly access thegetCommentByIdfunction from thecommentobject.web/core/components/comments/comment-create.tsx (3)
2-3: Minor import updates look fine
No issues observed with adding these imports.
20-26: Confirm handling ofprojectIdwhen absent
The newTCommentCreatetype optionally includesprojectId, but ensure the rest of the flow gracefully handles scenarios whereprojectIdisn't provided.Would you like a quick script to find all call sites of
<CommentCreate>to confirm thatprojectIdis always passed when needed?
79-79: Nicely handled empty comment check
TheisCommentEmptyhelper is a clean approach to blocking submissions for empty content.web/core/components/issues/issue-detail/issue-activity/root.tsx (4)
3-20: Refactored imports are consistent with the new comment architecture
The shift to importCommentCreateanduseCommentOperationsaligns well with the removal of the old issue-based components.
80-81: Good approach consolidating comment logic
Usingconst activityOperations = useCommentOperations(...)is a clean way to keep comment features decoupled from the rest of the component logic.
125-130:CommentCreateusage looks consistent
ReplacingIssueCommentCreatewithCommentCreatewhile passingentityId,showToolbarInitially, andprojectIdmatches the new design.
51-51:❓ Verification inconclusive
Ensure you still need this store usage after refactor
It appears you’re pullingissue: { getIssueById }fromuseIssueDetail; confirm that all references to the old issue store remain necessary.
🏁 Script executed:
#!/bin/bash # Searching references to getIssueById in the codebase rg -A 4 'getIssueById'Length of output: 66680
Double-check the necessity of using the legacy issue store via
getIssueByIdin this componentIt looks like you’re still extracting
{ getIssueById }fromuseIssueDetailin this file. Since our search confirms thatgetIssueByIdis widely used across the codebase—including in this component—please verify that its usage remains intentional after the refactor. In particular, ensure that:
- The reliance on the legacy issue store isn’t duplicating new store functionalities.
- No redundant or outdated references persist that could be removed.
web/core/components/comments/comment-card.tsx (5)
19-21: Updated imports reflect new comment component structure
Moving fromIssueCommentCard/IssueCommentBlocktoCommentBlockaligns with the ongoing refactor.
23-31: Check for undefinedcommentedge cases
commentcan be undefined, so ensure you handle calls that might implicitly assume valid data (e.g.,comment.id).
33-66: NewCommentCardstructure is clearer
Extracting workspace ID fromcommentand gating with early returns prevents null references. This also simplifies props usage.
139-177: Inline edit logic is well-structured
UsingisEditingto toggle between the read-only and editable states, plus hooking the form submission to key events, is a nice approach for user experience.
190-210:CommentReactionsintegration is seamless
Enabling reactions directly in the comment display fosters quick interactions, with clean toggling of private/public statuses.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
web/ce/components/comments/comment-block.tsx (6)
18-25: Consider adding error handling for userDetails retrievalThe component correctly checks for null/undefined values of
commentanduserDetails, but there's no error handling ifgetUserDetails()fails for other reasons. Consider adding a try/catch block or handling error states explicitly.Also, there's no explanation or fallback UI when returning an empty fragment, which might confuse developers or users.
const userDetails = getUserDetails(comment?.actor); - if (!comment || !userDetails) return <></>; + // Return a more descriptive fallback or log the reason + if (!comment || !userDetails) { + console.debug("CommentBlock: Missing comment data or user details"); + return <></>; + }
28-28: Consider using thecnutility for conditional class namesYou're using string templating for conditional classes here but using the
cnutility elsewhere in the component. For consistency, consider using thecnutility throughout the component.- className={`relative flex gap-3 ${ends === "top" ? `pb-2` : ends === "bottom" ? `pt-2` : `py-2`}`} + className={cn( + "relative flex gap-3", + ends === "top" ? "pb-2" : ends === "bottom" ? "pt-2" : "py-2" + )}
56-66: Improve timestamp display logicCurrently, you're showing the time based on
updated_atwhile marking edited comments with "(edited)". However, this might be confusing when a comment has been edited as the displayed time would reflect the edit time, not the original creation time. Consider either:
- Using
created_atfor the timestamp display and adding "(edited X ago)" for edits- Clarifying in the tooltip that the time shown is when the comment was last updated
Additionally, consider extracting the hardcoded "commented" text for internationalization.
- <div className="text-xs text-custom-text-300"> - commented{" "} - <Tooltip - tooltipContent={`${renderFormattedDate(comment.created_at)} at ${renderFormattedTime(comment.created_at)}`} - position="bottom" - > - <span className="text-custom-text-350"> - {calculateTimeAgo(comment.updated_at)} - {comment.edited_at && " (edited)"} - </span> - </Tooltip> - </div> + <div className="text-xs text-custom-text-300"> + commented{" "} + <Tooltip + tooltipContent={ + `${renderFormattedDate(comment.created_at)} at ${renderFormattedTime(comment.created_at)}` + + (comment.edited_at ? ` (edited on ${renderFormattedDate(comment.edited_at)})` : '') + } + position="bottom" + > + <span className="text-custom-text-350"> + {calculateTimeAgo(comment.created_at)} + {comment.edited_at && ` (edited ${calculateTimeAgo(comment.edited_at)})`} + </span> + </Tooltip> + </div>
31-34: Enhance accessibility for the timeline indicatorThe vertical timeline indicator has
aria-hiddenwhich is good, but consider adding more context to the comment structure for screen readers by adding appropriate ARIA attributes to the container.<div - className={`relative flex gap-3 ${ends === "top" ? `pb-2` : ends === "bottom" ? `pt-2` : `py-2`}`} + className={`relative flex gap-3 ${ends === "top" ? `pb-2` : ends === "bottom" ? `pt-2` : `py-2`}`} + role="article" + aria-label={`Comment from ${userDetails.display_name}`} ref={commentBlockRef} >
70-70: Consider making the comment content more semantically correctThe comment content is currently wrapped in a simple div with text styling. Consider using a more semantically appropriate element that represents the content better.
- <div className="text-base mb-2">{children}</div> + <section className="text-base mb-2" aria-label="Comment content">{children}</section>
21-21: Unused refThe
commentBlockRefis defined but never used in the component's functionality. If it's intended for future use or for parent component access, consider adding a comment explaining its purpose or remove it if unnecessary.- const commentBlockRef = useRef<HTMLDivElement>(null); + // Ref used for scrolling to comment or measuring visibility + const commentBlockRef = useRef<HTMLDivElement>(null);Or remove it if it's not needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/ce/components/comments/comment-block.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
web/ce/components/comments/comment-block.tsx (1)
packages/types/src/issues/activity/issue_comment.d.ts (1)
TIssueComment(17-40)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
web/ce/components/comments/comment-block.tsx (1)
51-54: 🛠️ Refactor suggestionAdd safety check for actor_detail
You're using optional chaining on individual properties of
actor_detailbut not on the object itself. Ifactor_detailis undefined, this could cause runtime errors.- <div className="text-xs font-medium"> - {comment?.actor_detail?.is_bot - ? comment?.actor_detail?.first_name + " Bot" - : comment?.actor_detail?.display_name || userDetails.display_name} - </div> + <div className="text-xs font-medium"> + {comment?.actor_detail + ? (comment.actor_detail.is_bot + ? comment.actor_detail.first_name + " Bot" + : comment.actor_detail.display_name) + : userDetails.display_name} + </div>Likely an incorrect or invalid review comment.
web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/i18n/src/locales/en/translations.json (1)
334-335: New Translation Keys Added: "edited" and "bot".
The keys"edited": "Modifié"and"bot": "Bot"have been successfully added. However, please verify that the value for"edited"is correct for the English locale since it is set to a French word ("Modifié"). If this is intentional (for example, to match a design decision or align with other locale mappings), then it's fine; otherwise, consider updating it (e.g., to"Edited").packages/i18n/src/locales/pl/translations.json (1)
502-503: Localization Keys "edited" and "bot" Added Successfully
The new key-value pairs"edited": "Edytowano"and"bot": "Bot"are clear and consistent with common UI terminology. Ensure that these translations are harmonized with the respective keys in other locale files for consistency across the application.packages/i18n/src/locales/zh-CN/translations.json (1)
504-505: New Translation Keys AddedThe keys
"edited": "已编辑"and"bot": "机器人"have been added correctly. Please ensure that these new entries are consistently mirrored in other locale files (as noted in the PR summary) and that the UI components consuming these strings properly reflect the intended meanings.packages/i18n/src/locales/fr/translations.json (1)
504-505: New Translation Entries for Comments Component
The newly added keys"edited": "Modifié"and"bot": "Bot"provide the required French translations for the comment-related functionality. They are succinct and consistent with the expected terminology. Please ensure these entries are kept in sync with the corresponding entries in the other locale files.web/core/components/comments/comment-create.tsx (1)
78-78: Simplify null check for commentHTMLThe use of
??withundefinedas the fallback is redundant since ifcommentHTMLis null or undefined, this will still evaluate toundefined.- const isEmpty = isCommentEmpty(commentHTML ?? undefined); + const isEmpty = isCommentEmpty(commentHTML);web/core/components/comments/comment-card.tsx (1)
67-67: Create a clear isDisabled variable for better readabilityDefine a clear variable for the disabled state to improve code readability.
- const isDisabled = isSubmitting || isEmpty || isSubmitButtonDisabled; + // Define this variable with a clearer name for its usage on buttons + const isSubmitButtonDisabled = isSubmitting || isEmpty || !isEditorReadyToDiscard;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
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/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/ru/translations.json(1 hunks)packages/i18n/src/locales/sk/translations.json(1 hunks)packages/i18n/src/locales/ua/translations.json(1 hunks)packages/i18n/src/locales/zh-CN/translations.json(1 hunks)packages/i18n/src/locales/zh-TW/translations.json(1 hunks)web/ce/components/comments/comment-block.tsx(1 hunks)web/core/components/comments/comment-card.tsx(6 hunks)web/core/components/comments/comment-create.tsx(5 hunks)web/core/components/comments/comments.tsx(1 hunks)web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx(4 hunks)web/core/store/issue/issue-details/comment_reaction.store.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/core/store/issue/issue-details/comment_reaction.store.ts
- web/ce/components/comments/comment-block.tsx
- web/core/components/comments/comments.tsx
🧰 Additional context used
🧬 Code Definitions (1)
web/core/components/comments/comment-card.tsx (3)
packages/types/src/issues/activity/issue_comment.d.ts (2)
TIssueComment(17-40)TCommentsOperations(42-57)web/ce/components/comments/comment-block.tsx (1)
CommentBlock(19-76)web/core/components/comments/comment-reaction.tsx (1)
CommentReactions(20-67)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (16)
packages/i18n/src/locales/ru/translations.json (1)
502-503: Add new translation keys for comment components
The new keys"edited": "Редактировано"and"bot": "Бот"provide the necessary labels for the updated comments UI. The translations are clear and consistent with similar entries in other locale files. Make sure these keys are referenced uniformly across the frontend components related to comment display and interaction.packages/i18n/src/locales/ko/translations.json (1)
504-505: New Translation Entries for "edited" and "bot".The additions provide the necessary Korean translations for the comment component updates. The values
"수정됨"for "edited" and"봇"for "bot" are consistent with the terminology used in other locale files. Please ensure that these keys are referenced in the codebase wherever the comment update functionality or bot-related displays occur.packages/i18n/src/locales/es/translations.json (1)
506-507: New Translation Keys Added: "edited" and "bot"The new entries
"edited": "Modificado"and"bot": "Bot"have been introduced to enhance the Spanish localization. They are correctly formatted, follow the naming convention of the JSON file, and appear consistent with similar keys added in other locales.It would be ideal to double-check that these keys are also consistently referenced in the frontend components that display edited comments and bot indicators.
packages/i18n/src/locales/zh-TW/translations.json (1)
504-505: New Translation Keys AddedThe addition of the
"edited": "已編輯"and"bot": "機器人"entries is clear and straightforward. These keys are consistent with the similar entries added in other locale files, ensuring a uniform translation experience across the application. Please verify that these translations semantically match their intended usage in the UI.packages/i18n/src/locales/sk/translations.json (1)
502-503: New Translation Entries Added for "edited" and "bot".
The two new keys"edited": "Upravené"and"bot": "Bot"are correctly added and consistent with the translations in similar locale files. This enhances our Slovak localization for comment metadata.packages/i18n/src/locales/it/translations.json (1)
503-504: New Translation Entries for Comments ComponentThe addition of
"edited": "Modificato"and"bot": "Bot"is clear and consistent with similar keys in other locale files. These entries will support the updated comments interface by distinguishing edited content and bot-generated messages.packages/i18n/src/locales/ja/translations.json (1)
504-505: Added Translation Entries for Comments Refactor.The new entries
"edited": "編集済み"and"bot": "ボット"are correctly added and formatted. They align with the corresponding changes in other locale files, ensuring consistency across the application.packages/i18n/src/locales/ua/translations.json (1)
502-504: New Translation Keys Added for Comments FunctionalityThe keys
"edited": "Редагувано"and"bot": "Бот"have been correctly introduced and are consistent with the style and naming conventions used throughout the file. These entries will support UI updates related to comment edits and bot functionalities as outlined by the PR objectives.web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (2)
51-53: Store the comment in a variable before the conditional checkThe comment object is fetched inside the map function and then immediately used in a conditional. This could lead to redundant lookups if the comment is referenced multiple times.
- {filteredActivityComments.map((activityComment, index) => { - const comment = getCommentById(activityComment.id); - return activityComment.activity_type === "COMMENT" ? ( + {filteredActivityComments.map((activityComment, index) => { + const comment = getCommentById(activityComment.id); + return activityComment.activity_type === "COMMENT" ? (
6-7: LGTM: Type and component updates properly refactoredThe changes to types, imports, and component props look good. The refactoring replaces
IssueCommentCardwith the more genericCommentCardand updates type definitions accordingly.Also applies to: 20-20, 40-40, 57-58, 62-62
web/core/components/comments/comment-create.tsx (3)
52-75: Avoid mixing await and .then() in the same callIt's clearer to consistently use
async/awaitwith try/catch instead of mixing them with promise chaining for better readability and error handling.- activityOperations - .createComment(formData) - .then(async () => { - if (uploadedAssetIds.length > 0) { - if (projectId) { - await fileService.updateBulkProjectAssetsUploadStatus(workspaceSlug, projectId.toString(), entityId, { - asset_ids: uploadedAssetIds, - }); - } else { - await fileService.updateBulkWorkspaceAssetsUploadStatus(workspaceSlug, entityId, { - asset_ids: uploadedAssetIds, - }); - } - setUploadedAssetIds([]); - } - }) - .finally(() => { - reset({ - comment_html: "<p></p>", - }); - editorRef.current?.clearEditor(); - }); + try { + await activityOperations.createComment(formData); + if (uploadedAssetIds.length > 0) { + if (projectId) { + await fileService.updateBulkProjectAssetsUploadStatus(workspaceSlug, projectId.toString(), entityId, { + asset_ids: uploadedAssetIds, + }); + } else { + await fileService.updateBulkWorkspaceAssetsUploadStatus(workspaceSlug, entityId, { + asset_ids: uploadedAssetIds, + }); + } + setUploadedAssetIds([]); + } + } finally { + reset({ + comment_html: "<p></p>", + }); + editorRef.current?.clearEditor(); + }
19-30: LGTM: Type definition properly updated to generalize commentsThe change from
TIssueCommentCreatetoTCommentCreatewithentityIdinstead ofissueIdmakes the component more reusable across different entities that can have comments.
56-65: LGTM: Asset upload handling correctly updatedThe code correctly handles asset uploads for both project-specific and workspace-level contexts based on whether projectId is available.
web/core/components/comments/comment-card.tsx (3)
171-174: Use the cn helper for complex className logicThe complex conditional className strings would be more readable using the imported
cnhelper function.- disabled={isDisabled} - className={`group rounded border border-green-500 bg-green-500/20 p-2 shadow-md duration-300 ${ - isEmpty ? "cursor-not-allowed bg-gray-200" : "hover:bg-green-500" - }`} + disabled={isDisabled} + className={cn( + "group rounded border border-green-500 bg-green-500/20 p-2 shadow-md duration-300", + isEmpty ? "cursor-not-allowed bg-gray-200" : "hover:bg-green-500" + )}
208-208: Don't use type assertion, use toString() insteadType assertion with
asis not the recommended way to handle optional values. Use the toString() method for consistency with other parts of the codebase.- projectId={(projectId as string) ?? ""} + projectId={projectId?.toString() ?? ""}
10-11: LGTM: Component structure properly refactoredThe component has been successfully refactored to use the new
CommentBlockcomponent and handle comment reactions withCommentReactions. The changes make the code more maintainable and consistent with the rest of the application.Also applies to: 19-21, 23-26, 36-37, 62-63, 87-88, 190-211
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
Description
References
[WEB-3698]