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.
WalkthroughThis PR adds feature image support to comments throughout the system—from the API type definitions to the serialization layer—and comprehensively refactors the comments list UI with expandable content, author filtering, status controls, and enhanced visual presentation including post feature images and formatted metrics. 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
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: 1
🤖 Fix all issues with AI agents
In `@apps/posts/src/views/comments/components/comments-list.tsx`:
- Around line 319-326: The ellipsis menu trigger uses DropdownMenuTrigger and a
Button rendering LucideIcon.Ellipsis but lacks an accessible name; add an
accessible label by supplying aria-label (for example aria-label="More options")
or include visually hidden text inside the Button to give screen readers a name.
Update the Button component usage (the Button within DropdownMenuTrigger) to
include the aria-label or hidden label so the menu is announced properly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
ghost/core/test/e2e-api/admin/__snapshots__/activity-feed.test.js.snapis excluded by!**/*.snapghost/core/test/e2e-api/admin/__snapshots__/comments.test.js.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
apps/admin-x-framework/src/api/comments.tsapps/posts/src/views/comments/components/comments-list.tsxghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js
🧰 Additional context used
🧬 Code graph analysis (1)
apps/posts/src/views/comments/components/comments-list.tsx (3)
apps/shade/src/components/ui/avatar.tsx (2)
Avatar(48-48)AvatarFallback(48-48)apps/shade/src/components/ui/tooltip.tsx (4)
TooltipProvider(33-33)Tooltip(33-33)TooltipTrigger(33-33)TooltipContent(33-33)apps/shade/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(189-189)DropdownMenuTrigger(190-190)DropdownMenuContent(191-191)DropdownMenuItem(192-192)
🪛 ast-grep (0.40.5)
apps/posts/src/views/comments/components/comments-list.tsx
[warning] 109-109: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/posts/src/views/comments/components/comments-list.tsx
[error] 110-110: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (9)
apps/admin-x-framework/src/api/comments.ts (1)
23-29: Looks good: optionalfeature_imagefield added toComment.post.
This keeps the post payload backwards-compatible while enabling the new UI.ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js (1)
34-40: Post serializer now includesfeature_image— LGTM.
This aligns with the expanded comment post shape and UI usage.apps/posts/src/views/comments/components/comments-list.tsx (7)
1-32: Imports and hooks align with the new layout + formatting utilities.
No concerns with the added UI primitives and React hooks usage here.
60-71: Date tooltip formatting looks good.
TheIntl.DateTimeFormat+ comma removal provides a clean tooltip value.
73-85:ExpandButtonextraction is clean and reusable.
Keeps the content component focused.
122-136:onAddFilteris now required — verify all callers updated.
This is a breaking prop change; please ensure everyCommentsListusage supplies it.
165-317: New row layout, filters, and metrics read well.
The author/post filtering links, hidden-state indicator, and per-item metrics are coherent and consistent.
350-357: Feature image column rendering looks good.
Conditional rendering + alt text fallback are sensible.
87-116: The HTML is already sanitized by the backend API before being sent to the client.In
ghost/core/core/server/models/comment.js, the comment HTML is sanitized on save using thesanitize-htmllibrary with a strict whitelist (onlyp,br,a, andblockquotetags are allowed). Link attributes are also hardened withtarget="_blank"andrel="ugc noopener noreferrer nofollow". Since theitem.htmlcomes from this sanitized backend data, usingdangerouslySetInnerHTMLhere is safe and no additional client-side sanitization is needed.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <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> |
There was a problem hiding this comment.
Add an accessible label to the ellipsis menu button.
Icon-only buttons should have an accessible name for screen readers.
♿ Suggested fix
- <Button
+ <Button
className="relative z-10 ml-1"
size="sm"
variant="ghost"
+ aria-label="More actions"
>📝 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.
| <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 asChild> | |
| <Button | |
| className="relative z-10 ml-1" | |
| size="sm" | |
| variant="ghost" | |
| aria-label="More actions" | |
| > | |
| <LucideIcon.Ellipsis /> | |
| </Button> |
🤖 Prompt for AI Agents
In `@apps/posts/src/views/comments/components/comments-list.tsx` around lines 319
- 326, The ellipsis menu trigger uses DropdownMenuTrigger and a Button rendering
LucideIcon.Ellipsis but lacks an accessible name; add an accessible label by
supplying aria-label (for example aria-label="More options") or include visually
hidden text inside the Button to give screen readers a name. Update the Button
component usage (the Button within DropdownMenuTrigger) to include the
aria-label or hidden label so the menu is announced properly.
3 participants
Benchmark PR from qodo-benchmark#228
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.