Redesigned comment moderation list layout#4
Open
tomerqodo wants to merge 23 commits intocoderabbit_full_base_redesigned_comment_moderation_list_layout_pr4from
Conversation
ref https://linear.app/ghost/issue/FEA-491/design-truncation-and-display-for-long-comments - Longer comments in the comments UI need better truncation and display treatment. The PR clamps the original comment to show max 2 lines and make it possible to view the full comment.
…ests" This reverts commit 10e1f16.
📝 WalkthroughWalkthroughThe changes extend the comment system to support post feature images. The API type definitions and backend serializer are updated to expose the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
![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
🤖 Fix all issues with AI agents
In `@apps/posts/src/views/comments/components/comments-list.tsx`:
- Around line 318-346: The confirm delete dialog never opens because
commentToDelete is never set; update the CommentsList component to add a delete
action in the DropdownMenu that calls setCommentToDelete(item) (or the existing
setter for commentToDelete) when selected (e.g., via onClick/onSelect on a new
DropdownMenuItem labeled "Delete"), and ensure the menu closes after selection
so the confirm dialog can appear; reference the DropdownMenu/DropdownMenuItem
block around the LucideIcon.Ellipsis and the commentToDelete state/handler to
locate where to insert the new delete menu item and wire it to the existing
confirm dialog logic.
- Around line 60-71: formatDate currently calls new Date(dateString) and passes
it straight to Intl.DateTimeFormat, which will throw for invalid dates; in
function formatDate add a check after creating the Date (use
isNaN(date.getTime()) or Number.isNaN(+date)) and return a safe fallback (e.g.,
empty string or a placeholder like 'Invalid date') when the date is invalid
instead of attempting to format it so the tooltip won't crash; update formatDate
to perform this guard and return the formatted string only for valid dates.
🧹 Nitpick comments (1)
apps/posts/src/views/comments/components/comments-list.tsx (1)
351-356: Consider lazy-loading feature images in the list.
Helps avoid fetching many thumbnails during scrolling.⚙️ Optional tweak
<img alt={item.post.title || 'Post feature image'} className="hidden aspect-video w-32 rounded object-cover lg:block" + loading="lazy" + decoding="async" src={item.post.feature_image} />
Comment on lines
60
to
+71
| function formatDate(dateString: string): string { | ||
| const date = new Date(dateString); | ||
| return new Intl.DateTimeFormat('en-US', { | ||
| const formatted = new Intl.DateTimeFormat('en-US', { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| year: 'numeric', | ||
| hour: 'numeric', | ||
| minute: 'numeric' | ||
| }).format(date); | ||
| // Remove comma between day and year (e.g., "Dec 17, 2025" -> "Dec 17 2025") | ||
| return formatted.replace(/(\d+),(\s+\d{4})/, '$1$2'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/posts/src/views/comments/components/comments-list.tsx | head -100Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 3843
🏁 Script executed:
cd apps/posts/src/views/comments/components && grep -n "formatDate" comments-list.tsx | head -20Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 245
🏁 Script executed:
cat -n apps/posts/src/views/comments/components/comments-list.tsx | sed -n '220,235p'Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 1381
🏁 Script executed:
node <<'EOF'
// Test Intl.DateTimeFormat behavior with invalid dates
// Valid date
try {
const validDate = new Date('2025-01-15');
const formatter = new Intl.DateTimeFormat('en-US', {
month: 'short',
day: 'numeric',
year: 'numeric',
hour: 'numeric',
minute: 'numeric'
});
console.log('Valid date result:', formatter.format(validDate));
} catch (e) {
console.log('Valid date error:', e.message);
}
// Invalid date string
try {
const invalidDate = new Date('invalid-date-string');
const formatter = new Intl.DateTimeFormat('en-US', {
month: 'short',
day: 'numeric',
year: 'numeric',
hour: 'numeric',
minute: 'numeric'
});
console.log('Invalid date result:', formatter.format(invalidDate));
} catch (e) {
console.log('Invalid date error:', e.message);
}
// NaN date
try {
const nanDate = new Date(NaN);
const formatter = new Intl.DateTimeFormat('en-US', {
month: 'short',
day: 'numeric',
year: 'numeric',
hour: 'numeric',
minute: 'numeric'
});
console.log('NaN date result:', formatter.format(nanDate));
} catch (e) {
console.log('NaN date error:', e.message);
}
// Test Number.isNaN behavior
console.log('\nNumber.isNaN checks:');
console.log('Valid date getTime():', new Date('2025-01-15').getTime(), 'isNaN:', Number.isNaN(new Date('2025-01-15').getTime()));
console.log('Invalid date getTime():', new Date('invalid').getTime(), 'isNaN:', Number.isNaN(new Date('invalid').getTime()));
EOFRepository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 321
🏁 Script executed:
rg "interface Comment|type Comment" --type ts --type tsx | head -5Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 122
🏁 Script executed:
rg "interface Comment|type Comment" --type tsRepository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 1141
🏁 Script executed:
cat -n apps/admin-x-framework/src/api/comments.ts | head -50Repository: agentic-review-benchmarks/Ghost-combined-coderabbit
Length of output: 1614
Guard against invalid timestamps in tooltip formatter.
Intl.DateTimeFormat throws on invalid dates. While item.created_at && protects against null/undefined, a malformed date string from the backend would still crash the tooltip. Add a NaN check before formatting:
function formatDate(dateString: string): string {
const date = new Date(dateString);
+ if (Number.isNaN(date.getTime())) {
+ return 'Unknown';
+ }
const formatted = new Intl.DateTimeFormat('en-US', {
month: 'short',
day: 'numeric',
year: 'numeric',
hour: 'numeric',
minute: 'numeric'
}).format(date);
// Remove comma between day and year (e.g., "Dec 17, 2025" -> "Dec 17 2025")
return formatted.replace(/(\d+),(\s+\d{4})/, '$1$2');
}📝 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.
Suggested change
| function formatDate(dateString: string): string { | |
| const date = new Date(dateString); | |
| return new Intl.DateTimeFormat('en-US', { | |
| const formatted = new Intl.DateTimeFormat('en-US', { | |
| month: 'short', | |
| day: 'numeric', | |
| year: 'numeric', | |
| hour: 'numeric', | |
| minute: 'numeric' | |
| }).format(date); | |
| // Remove comma between day and year (e.g., "Dec 17, 2025" -> "Dec 17 2025") | |
| return formatted.replace(/(\d+),(\s+\d{4})/, '$1$2'); | |
| } | |
| function formatDate(dateString: string): string { | |
| const date = new Date(dateString); | |
| if (Number.isNaN(date.getTime())) { | |
| return 'Unknown'; | |
| } | |
| const formatted = new Intl.DateTimeFormat('en-US', { | |
| month: 'short', | |
| day: 'numeric', | |
| year: 'numeric', | |
| hour: 'numeric', | |
| minute: 'numeric' | |
| }).format(date); | |
| // Remove comma between day and year (e.g., "Dec 17, 2025" -> "Dec 17 2025") | |
| return formatted.replace(/(\d+),(\s+\d{4})/, '$1$2'); | |
| } |
🤖 Prompt for AI Agents
In `@apps/posts/src/views/comments/components/comments-list.tsx` around lines 60 -
71, formatDate currently calls new Date(dateString) and passes it straight to
Intl.DateTimeFormat, which will throw for invalid dates; in function formatDate
add a check after creating the Date (use isNaN(date.getTime()) or
Number.isNaN(+date)) and return a safe fallback (e.g., empty string or a
placeholder like 'Invalid date') when the date is invalid instead of attempting
to format it so the tooltip won't crash; update formatDate to perform this guard
and return the formatted string only for valid dates.
Comment on lines
+318
to
+346
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button | ||
| className="relative z-10 ml-1" | ||
| size="sm" | ||
| variant="ghost" | ||
| > | ||
| <LucideIcon.Trash2 className="mr-2 size-4" /> | ||
| Delete comment | ||
| </DropdownMenuItem> | ||
| </>} | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| <LucideIcon.Ellipsis /> | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent align="start"> | ||
| {item.post?.url && ( | ||
| <DropdownMenuItem asChild> | ||
| <a href={item.post.url} rel="noopener noreferrer" target="_blank"> | ||
| <LucideIcon.ExternalLink className="mr-2 size-4" /> | ||
| View post | ||
| </a> | ||
| </DropdownMenuItem> | ||
| )} | ||
| {item.member?.id && ( | ||
| <DropdownMenuItem asChild> | ||
| <a href={`#/members/${item.member.id}`}> | ||
| <LucideIcon.User className="mr-2 size-4" /> | ||
| View member | ||
| </a> | ||
| </DropdownMenuItem> | ||
| )} | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> |
There was a problem hiding this comment.
Delete dialog is unreachable (no trigger sets commentToDelete).
commentToDelete is never set, so the confirm dialog can’t open. Add a delete action in the menu (or elsewhere).
🧩 Proposed fix
<DropdownMenuContent align="start">
{item.post?.url && (
<DropdownMenuItem asChild>
<a href={item.post.url} rel="noopener noreferrer" target="_blank">
<LucideIcon.ExternalLink className="mr-2 size-4" />
View post
</a>
</DropdownMenuItem>
)}
{item.member?.id && (
<DropdownMenuItem asChild>
<a href={`#/members/${item.member.id}`}>
<LucideIcon.User className="mr-2 size-4" />
View member
</a>
</DropdownMenuItem>
)}
+ <DropdownMenuItem
+ className="text-red-600 focus:text-red-600"
+ onSelect={() => setCommentToDelete(item)}
+ >
+ <LucideIcon.Trash2 className="mr-2 size-4" />
+ Delete comment
+ </DropdownMenuItem>
</DropdownMenuContent>🤖 Prompt for AI Agents
In `@apps/posts/src/views/comments/components/comments-list.tsx` around lines 318
- 346, The confirm delete dialog never opens because commentToDelete is never
set; update the CommentsList component to add a delete action in the
DropdownMenu that calls setCommentToDelete(item) (or the existing setter for
commentToDelete) when selected (e.g., via onClick/onSelect on a new
DropdownMenuItem labeled "Delete"), and ensure the menu closes after selection
so the confirm dialog can appear; reference the DropdownMenu/DropdownMenuItem
block around the LucideIcon.Ellipsis and the commentToDelete state/handler to
locate where to insert the new delete menu item and wire it to the existing
confirm dialog logic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from agentic-review-benchmarks#4
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.