Redesigned comment moderation list layout#25745
Conversation
WalkthroughReworks the comments list UI in apps/posts: adds Avatar, Tooltip, ExpandButton and CommentContent helpers to render clamped/expandable HTML comment content with dynamic height handling; replaces previous per-row rendering with a grid row showing author/avatar (clickable to add filter), short timestamp with full-date tooltip, post link and optional post.feature_image, per-row metrics (replies/likes/reports) with tooltips, status indicators (e.g., hidden), inline actions (hide/show, delete confirmation dialog, compact More menu with “View post”/“View member”), and refactors delete flow. Changes Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/posts/src/views/comments/components/comments-list.tsx(5 hunks)apps/shade/src/components/ui/avatar.stories.tsx(1 hunks)apps/shade/src/components/ui/avatar.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/shade/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
Atomic UI components should be placed in
src/components/ui/*and each component must have a corresponding*.stories.tsxfile next to it for Storybook documentation
Files:
apps/shade/src/components/ui/avatar.tsxapps/shade/src/components/ui/avatar.stories.tsx
apps/shade/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
apps/shade/src/components/**/*.{ts,tsx}: UsePascalCasefor component identifiers in filenames while keeping ShadCN-generated files in kebab-case
Always forward and mergeclassNameprop withcn(...)utility function
Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Prefer compound subcomponents (e.g.,Header.Title,Header.Meta,Header.Actions) for multi-region components instead of many props
Use Tailwind CSS scoped via.shadeclass; dark mode uses.dark
Files:
apps/shade/src/components/ui/avatar.tsxapps/shade/src/components/ui/avatar.stories.tsx
apps/shade/src/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
apps/shade/src/**/*.{ts,tsx,js}: UsecamelCasefor function and variable names
Use the@alias for internal imports (e.g.,@/lib/utils)
Files:
apps/shade/src/components/ui/avatar.tsxapps/shade/src/components/ui/avatar.stories.tsx
apps/shade/{src,test}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
apps/shade/{src,test}/**/*.{ts,tsx,js}: Runyarn lintafter making changes to fix any ESLint errors and warnings before committing
Follow ESLint andtailwindcss/*plugin rules when writing styles
Files:
apps/shade/src/components/ui/avatar.tsxapps/shade/src/components/ui/avatar.stories.tsx
apps/shade/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
apps/shade/**/*.stories.{ts,tsx}: Storybook stories for new components should include an overview explaining what the component does and its primary use case
Storybook stories should demonstrate key variants and states (sizes, disabled/loading, critical props) with minimal but representative examples
Files:
apps/shade/src/components/ui/avatar.stories.tsx
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/**/*.stories.{ts,tsx} : Storybook stories should demonstrate key variants and states (sizes, disabled/loading, critical props) with minimal but representative examples
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/shade/src/components/ui/avatar.tsxapps/shade/src/components/ui/avatar.stories.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/index.ts : Place new UI components under `src/components/ui` and export them from `src/index.ts`
Applied to files:
apps/shade/src/components/ui/avatar.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use Tailwind CSS scoped via `.shade` class; dark mode uses `.dark`
Applied to files:
apps/shade/src/components/ui/avatar.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Prefer compound subcomponents (e.g., `Header.Title`, `Header.Meta`, `Header.Actions`) for multi-region components instead of many props
Applied to files:
apps/shade/src/components/ui/avatar.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/**/*.stories.{ts,tsx} : Storybook stories should demonstrate key variants and states (sizes, disabled/loading, critical props) with minimal but representative examples
Applied to files:
apps/shade/src/components/ui/avatar.tsxapps/shade/src/components/ui/avatar.stories.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use `PascalCase` for component identifiers in filenames while keeping ShadCN-generated files in kebab-case
Applied to files:
apps/shade/src/components/ui/avatar.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/ui/**/*.{ts,tsx} : Atomic UI components should be placed in `src/components/ui/*` and each component must have a corresponding `*.stories.tsx` file next to it for Storybook documentation
Applied to files:
apps/shade/src/components/ui/avatar.tsxapps/shade/src/components/ui/avatar.stories.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/**/*.{ts,tsx,js} : Use the `@` alias for internal imports (e.g., `@/lib/utils`)
Applied to files:
apps/shade/src/components/ui/avatar.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Always forward and merge `className` prop with `cn(...)` utility function
Applied to files:
apps/shade/src/components/ui/avatar.tsx
📚 Learning: 2025-12-09T12:37:23.267Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25501
File: apps/shade/src/hooks/use-mobile.tsx:5-10
Timestamp: 2025-12-09T12:37:23.267Z
Learning: In the Ghost repository, this guideline applies to the shade admin app. For files under apps/shade that access browser globals (navigator, window, document) at module load time, SSR is not used, so typeof guards are not required. Reviewers should verify that such files remain client-side only and that no SSR context is introduced; apply this understanding to similarly structured files under apps/shade.
Applied to files:
apps/shade/src/components/ui/avatar.tsxapps/shade/src/components/ui/avatar.stories.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/features/**/*.{ts,tsx} : Higher-level, opinionated components (e.g., PostShareModal, SourceTabs) should be placed in `src/components/features/*`
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:48.725Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/Growth.tsx:199-204
Timestamp: 2025-11-12T22:26:48.725Z
Learning: In apps/stats/src/views/Stats/Growth/Growth.tsx, the loading skeleton for the top posts/pages table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the main content column (which contains post/page titles with dates).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:03.622Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx:189-199
Timestamp: 2025-11-12T22:26:03.622Z
Learning: In apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx, the loading skeleton for the GrowthSources table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the source column which contains the main content (source names with icons).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/**/*.stories.{ts,tsx} : Storybook stories for new components should include an overview explaining what the component does and its primary use case
Applied to files:
apps/shade/src/components/ui/avatar.stories.tsx
🧬 Code graph analysis (1)
apps/posts/src/views/comments/components/comments-list.tsx (6)
apps/admin-x-framework/src/api/comments.ts (1)
Comment(8-33)apps/shade/src/components/ui/table.tsx (2)
TableHead(148-148)TableCell(151-151)apps/shade/src/components/ui/avatar.tsx (3)
Avatar(87-87)AvatarImage(87-87)AvatarFallback(87-87)apps/shade/src/components/ui/tooltip.tsx (4)
TooltipProvider(33-33)Tooltip(33-33)TooltipTrigger(33-33)TooltipContent(33-33)apps/comments-ui/src/utils/admin-api.ts (2)
hideComment(59-61)showComment(62-64)apps/shade/src/components/ui/dropdown-menu.tsx (1)
DropdownMenuItem(192-192)
🪛 ast-grep (0.40.0)
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)
⏰ 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). (5)
- GitHub Check: ActivityPub tests
- GitHub Check: Admin-X Settings tests
- GitHub Check: Lint
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (13)
apps/shade/src/components/ui/avatar.stories.tsx (2)
70-84: LGTM! Clean migration to size-based variant API.The refactoring correctly uses the new CVA-based size props, replacing hard-coded classes with explicit variant values. The size prop is properly passed to both Avatar and AvatarFallback components.
89-115: Excellent addition of size variant stories.These new stories effectively demonstrate all available size variants with minimal, representative examples. This aligns well with the Storybook best practices for showcasing component variants.
Based on learnings, Storybook stories should demonstrate key variants and states with minimal but representative examples.
apps/shade/src/components/ui/avatar.tsx (4)
3-23: Excellent use of CVA for size variants.The implementation follows coding guidelines by using Class Variance Authority for component variants. The size scale is comprehensive (xs to xl) and uses consistent Tailwind utilities. The base classes and default variant are well-chosen.
As per coding guidelines, use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration.
25-38: LGTM! Proper TypeScript typing and className merging.The AvatarProps interface correctly combines Radix primitives with variant props, and the component implementation properly merges the size variant with custom className using the
cn()utility function.As per coding guidelines, always forward and merge
classNameprop withcn(...)utility function.
53-84: Well-designed fallback variants with icon sizing.The implementation cleverly uses the
[&_svg]selector to automatically size icons within the fallback, creating a cohesive size system. The text and icon size progressions are well-balanced across all variants.
87-87: LGTM! Comprehensive exports.Exporting both the components and the CVA variants allows consumers to reuse the variant styling in other contexts if needed.
apps/posts/src/views/comments/components/comments-list.tsx (7)
10-32: LGTM! Necessary imports for the new features.All imports are correctly sourced and use the
@alias for internal imports as per coding guidelines.
60-71: LGTM! Date formatting adjustment.The regex pattern correctly removes the comma between day and year for a cleaner date format.
73-85: LGTM! Clean expand/collapse button.The component provides clear user feedback with descriptive text and appropriate icons.
167-169: LGTM! Invisible headers for layout structure.The zero-height headers provide column width constraints for the table layout while remaining visually hidden.
221-232: LGTM! Excellent tooltip UX pattern.The tooltip implementation provides a good user experience by showing relative time (via
formatTimestamp) as the trigger text and displaying the absolute formatted date on hover.
257-274: LGTM! Improved button labels and status indicators.Adding text labels to the Hide/Show buttons improves accessibility and clarity. The "Hidden" status indicator with icon provides clear visual feedback.
295-300: LGTM! Useful navigation to member details.The dropdown item provides convenient access to member details using the appropriate routing format.
| function CommentContent({item}: {item: Comment}) { | ||
| const contentRef = useRef<HTMLDivElement>(null); | ||
| const [isClamped, setIsClamped] = useState(false); | ||
| const [isExpanded, setIsExpanded] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| const checkIfClamped = () => { | ||
| if (contentRef.current) { | ||
| // Check if the content is clamped by comparing scrollHeight with clientHeight | ||
| setIsClamped(contentRef.current.scrollHeight > contentRef.current.clientHeight); | ||
| } | ||
| }; | ||
|
|
||
| checkIfClamped(); | ||
| // Recheck on window resize | ||
| window.addEventListener('resize', checkIfClamped); | ||
| return () => window.removeEventListener('resize', checkIfClamped); | ||
| }, [item.html]); | ||
|
|
||
| return ( | ||
| <div className="mt-1 flex flex-col gap-2"> | ||
| <div className="flex max-w-[720px] flex-col items-start"> | ||
| <div | ||
| dangerouslySetInnerHTML={{__html: item.html || ''}} | ||
| ref={contentRef} | ||
| className={`prose flex-1 text-base leading-[1.45em] ${isExpanded ? '[&_p]:mb-[0.85em]' : 'mb-1 line-clamp-2 [&_*]:m-0 [&_*]:inline'} ${item.status === 'hidden' && 'text-muted-foreground'}`} | ||
| /> | ||
| {isClamped && ( | ||
| <ExpandButton expanded={isExpanded} onClick={() => setIsExpanded(!isExpanded)} /> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Address XSS vulnerability in HTML content rendering.
Line 110 uses dangerouslySetInnerHTML to render comment HTML without visible sanitization. This exposes users to XSS attacks if the item.html field contains malicious scripts.
Ensure that item.html is sanitized server-side before it reaches the client, or sanitize it on the client using a library like DOMPurify:
import DOMPurify from 'dompurify';
// In the component:
<div
dangerouslySetInnerHTML={{__html: DOMPurify.sanitize(item.html || '')}}
ref={contentRef}
className={...}
/>Consider performance impact of resize listeners.
Lines 92-104 attach a global resize event listener for each comment in the list. With many comments, this could impact performance. Consider using a single resize observer or debouncing the resize handler.
useEffect(() => {
const checkIfClamped = () => {
if (contentRef.current) {
setIsClamped(contentRef.current.scrollHeight > contentRef.current.clientHeight);
}
};
checkIfClamped();
// Debounce the resize handler
let timeoutId: NodeJS.Timeout;
const debouncedCheck = () => {
clearTimeout(timeoutId);
timeoutId = setTimeout(checkIfClamped, 150);
};
window.addEventListener('resize', debouncedCheck);
return () => {
window.removeEventListener('resize', debouncedCheck);
clearTimeout(timeoutId);
};
}, [item.html]);🧰 Tools
🪛 ast-grep (0.40.0)
[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)
[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)
🤖 Prompt for AI Agents
In apps/posts/src/views/comments/components/comments-list.tsx around lines 87 to
120, the component renders raw HTML via dangerouslySetInnerHTML without
sanitization (XSS risk) and attaches a per-comment window resize listener
(potential performance issue); fix by sanitizing item.html before passing to
dangerouslySetInnerHTML (use a vetted sanitizer like DOMPurify on the client or
guarantee server-side sanitization) and replace the per-instance immediate
resize handler with a debounced resize handler or a shared ResizeObserver so
checks are rate-limited or centralized; ensure you import/configure the
sanitizer once and clean up any timeouts/observers on unmount.
There was a problem hiding this comment.
Comment HTML is sanitised server-side in the Comment model here https://github.com/TryGhost/Ghost/blob/main/ghost/core/core/server/models/comment.js#L104-L119
I think the resize listener performance concern is valid but I don't think it's a blocker at this stage. @rob-ghost, @jonatansberg?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
While I think a signle resize handler would be better in many ways, I'm not sure it's worth the extra effort. Since the list is virtualized, we only render a handful of comment items at any single time.
There was a problem hiding this comment.
Fix icon size override that conflicts with avatar variant.
Line 205 uses className='!size-3' with the important flag, overriding the avatarFallbackVariants size-xs setting of [&_svg]:size-2. The size={12} prop is also redundant since Tailwind classes handle sizing.
Remove the className override and size prop to respect the variant's intended sizing:
-<LucideIcon.User className='!size-3 text-muted-foreground' size={12} />
+<LucideIcon.User className='text-muted-foreground' />This ensures consistent sizing across all avatar fallback icons as defined by the avatarFallbackVariants.
📝 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.
| <Avatar size="xs"> | |
| {item.member.avatar_image && ( | |
| <AvatarImage alt={item.member.name} src={item.member.avatar_image} /> | |
| )} | |
| <AvatarFallback> | |
| <LucideIcon.User className='!size-3 text-muted-foreground' size={12} /> | |
| </AvatarFallback> | |
| </Avatar> | |
| {item.member.name || 'Unknown'} | |
| <Avatar size="xs"> | |
| {item.member.avatar_image && ( | |
| <AvatarImage alt={item.member.name} src={item.member.avatar_image} /> | |
| )} | |
| <AvatarFallback> | |
| <LucideIcon.User className='text-muted-foreground' /> | |
| </AvatarFallback> | |
| </Avatar> | |
| {item.member.name || 'Unknown'} |
🤖 Prompt for AI Agents
In apps/posts/src/views/comments/components/comments-list.tsx around lines 200
to 208, the AvatarFallback is overriding the avatar variant sizing by using
className='!size-3' and passing size={12} to the LucideIcon.User; remove the
className override and the size prop so the fallback SVG inherits sizing from
avatarFallbackVariants ([_&svg]:size-2) to ensure consistent avatar icon sizes.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/posts/src/views/comments/components/comments-list.tsx (1)
149-157: Remove unreachable delete functionality or add the missing triggerThe delete infrastructure is fully implemented—
deleteCommenthook,commentToDeletestate,confirmDeletefunction, andAlertDialog—but it's unreachable. The DropdownMenu (lines 275–303) contains only "View post" and "View member" options; there's no delete button or any UI trigger to callsetCommentToDelete. Either the delete button was removed during redesign and should be restored in the DropdownMenuContent, or all unused delete-related code should be cleaned up.
♻️ Duplicate comments (2)
apps/posts/src/views/comments/components/comments-list.tsx (2)
87-120: XSS vulnerability and resize listener performance already flagged.The concerns about
dangerouslySetInnerHTMLwithout sanitization (XSS risk) and per-component resize listeners (performance) were already raised in a previous review. Please address those comments.
200-208: Icon size override already flagged.The icon sizing conflict with
className='!size-3'overriding the avatar variant was already raised in a previous review.
🧹 Nitpick comments (2)
apps/posts/src/views/comments/components/comments-list.tsx (2)
221-232: Consider hoistingTooltipProviderto reduce overhead.Creating a new
TooltipProviderper row adds unnecessary overhead. Move it to wrap the entireTableorCommentsListcomponent once, then useTooltipdirectly here.- <TooltipProvider> - <Tooltip> - <TooltipTrigger asChild> - <span className="cursor-default text-sm text-muted-foreground"> - {formatTimestamp(item.created_at)} - </span> - </TooltipTrigger> - <TooltipContent> - {formatDate(item.created_at)} - </TooltipContent> - </Tooltip> - </TooltipProvider> + <Tooltip> + <TooltipTrigger asChild> + <span className="cursor-default text-sm text-muted-foreground"> + {formatTimestamp(item.created_at)} + </span> + </TooltipTrigger> + <TooltipContent> + {formatDate(item.created_at)} + </TooltipContent> + </Tooltip>Then wrap the table at a higher level:
<TooltipProvider> <Table ...> {/* existing content */} </Table> </TooltipProvider>
237-243: Minor: Double space in className.There's a double space in the className string.
- className="block h-auto truncate p-0 font-medium text-primary hover:opacity-70" + className="block h-auto truncate p-0 font-medium text-primary hover:opacity-70"
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/posts/src/views/comments/components/comments-list.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-27T11:59:33.968Z
Learnt from: peterzimon
Repo: TryGhost/Ghost PR: 25261
File: apps/admin/src/layout/app-sidebar/UserMenu.tsx:24-29
Timestamp: 2025-10-27T11:59:33.968Z
Learning: In apps/admin/src/layout/app-sidebar/UserMenu.tsx, the hardcoded placeholder data (avatar URL, initials "US", name "User Name", email "userexample.com") is a known limitation and should not be flagged for replacement with real user data.
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:48.725Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/Growth.tsx:199-204
Timestamp: 2025-11-12T22:26:48.725Z
Learning: In apps/stats/src/views/Stats/Growth/Growth.tsx, the loading skeleton for the top posts/pages table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the main content column (which contains post/page titles with dates).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:03.622Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx:189-199
Timestamp: 2025-11-12T22:26:03.622Z
Learning: In apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx, the loading skeleton for the GrowthSources table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the source column which contains the main content (source names with icons).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
🧬 Code graph analysis (1)
apps/posts/src/views/comments/components/comments-list.tsx (4)
apps/admin-x-framework/src/api/comments.ts (1)
Comment(8-33)apps/shade/src/components/ui/avatar.tsx (3)
Avatar(87-87)AvatarImage(87-87)AvatarFallback(87-87)apps/shade/src/components/ui/tooltip.tsx (4)
TooltipProvider(33-33)Tooltip(33-33)TooltipTrigger(33-33)TooltipContent(33-33)apps/comments-ui/src/utils/admin-api.ts (2)
hideComment(59-61)showComment(62-64)
🪛 ast-grep (0.40.0)
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)
⏰ 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). (4)
- GitHub Check: Admin-X Settings tests
- GitHub Check: ActivityPub tests
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (2)
apps/posts/src/views/comments/components/comments-list.tsx (2)
60-71: LGTM!The
formatDatefunction correctly formats timestamps and removes the comma between day and year as intended.
73-85: LGTM!The
ExpandButtoncomponent is a clean, accessible toggle with clear visual feedback through icon direction changes.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/posts/src/views/comments/components/comments-list.tsx (1)
149-157: Dead code: delete confirmation dialog is unreachable.The
commentToDeletestate,confirmDeletefunction, andAlertDialogexist but nothing triggers them—the delete action was removed from the dropdown menu. Either:
- Re-add the delete action to the dropdown menu:
<DropdownMenuItem className="text-destructive" onClick={() => setCommentToDelete(item)} > <LucideIcon.Trash2 className="mr-2 size-4" /> Delete </DropdownMenuItem>
- Or remove the dead code (lines 150, 152-157, 313-331) if delete functionality is intentionally disabled.
Also applies to: 313-331
♻️ Duplicate comments (3)
apps/posts/src/views/comments/components/comments-list.tsx (3)
87-120: XSS and performance concerns previously flagged.The
dangerouslySetInnerHTMLusage and per-instance resize listeners were addressed in prior review comments. Ensure those recommendations (DOMPurify sanitization, debounced/shared resize handling) are implemented.
200-208: Icon size override previously flagged.The
!size-3override conflicting with avatar variant sizing was addressed in prior review.
294-301: UnnecessaryonAddFiltercheck previously flagged.The condition including
onAddFilterfor the "View member" link was addressed in prior review.
🧹 Nitpick comments (2)
apps/posts/src/views/comments/components/comments-list.tsx (2)
60-71: Consider usingdateStyleoption for cleaner date formatting.The regex replacement approach works but is fragile. The
Intl.DateTimeFormatAPI offers more control via formatting parts:function formatDate(dateString: string): string { const date = new Date(dateString); - 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'); + const formatter = new Intl.DateTimeFormat('en-US', { + month: 'short', + day: 'numeric', + year: 'numeric', + hour: 'numeric', + minute: 'numeric' + }); + return formatter.formatToParts(date) + .filter(part => part.type !== 'literal' || part.value !== ', ') + .map(part => part.value) + .join(''); }This avoids regex and handles edge cases more robustly.
236-248: Minor: double space in className string.- className="block h-auto truncate p-0 font-medium text-primary hover:opacity-70" + className="block h-auto truncate p-0 font-medium text-primary hover:opacity-70"
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/posts/src/views/comments/components/comments-list.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.{ts,tsx} : Prefer less comments and give things clear names
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-10-27T11:59:33.968Z
Learnt from: peterzimon
Repo: TryGhost/Ghost PR: 25261
File: apps/admin/src/layout/app-sidebar/UserMenu.tsx:24-29
Timestamp: 2025-10-27T11:59:33.968Z
Learning: In apps/admin/src/layout/app-sidebar/UserMenu.tsx, the hardcoded placeholder data (avatar URL, initials "US", name "User Name", email "userexample.com") is a known limitation and should not be flagged for replacement with real user data.
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:48.725Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/Growth.tsx:199-204
Timestamp: 2025-11-12T22:26:48.725Z
Learning: In apps/stats/src/views/Stats/Growth/Growth.tsx, the loading skeleton for the top posts/pages table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the main content column (which contains post/page titles with dates).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:03.622Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx:189-199
Timestamp: 2025-11-12T22:26:03.622Z
Learning: In apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx, the loading skeleton for the GrowthSources table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the source column which contains the main content (source names with icons).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
🪛 ast-grep (0.40.0)
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 (3)
apps/posts/src/views/comments/components/comments-list.tsx (3)
1-34: Imports look appropriate for the new UI components.The new Avatar, Tooltip, and formatting utility imports align well with the redesigned layout requirements.
73-85: Clean expand/collapse toggle implementation.The
ExpandButtoncomponent is well-structured with clear visual states and accessible labels.
122-145: Component hooks and virtual scroll setup look correct.The infinite virtual scroll integration and mutation hooks are properly initialized.
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.
peterzimon
force-pushed
the
commentmod-clamp-comments
branch
from
2b38d60 to
c44f20a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/posts/src/views/comments/components/comments-list.tsx (3)
88-121: Previously flagged: XSS and performance concerns.These issues were already raised in previous reviews:
- Line 111:
dangerouslySetInnerHTMLusage without visible client-side sanitization (XSS risk)- Lines 93-104: Per-comment resize listeners (performance impact)
206-206: Previously flagged: Icon size override conflict.The use of
className='!size-3'andsize={12}props on the User icon was already flagged in previous reviews as conflicting with avatar variant sizing.
168-169: Previously flagged: Empty table headers.The accessibility concern about empty
<TableHead>elements was already raised and marked as addressed in previous reviews. The current empty state may be intentional for the visual layout.
🧹 Nitpick comments (2)
apps/posts/src/views/comments/components/comments-list.tsx (2)
222-233: Optimize tooltip provider usage for better performance.Creating a new
TooltipProviderfor each tooltip instance is inefficient. A singleTooltipProvidershould wrap the entire comments list instead of creating multiple providers per row.Move the
TooltipProviderto wrap the entireCommentsListcomponent:function CommentsList({...}) { ... return ( + <TooltipProvider> <div ref={parentRef} className="overflow-hidden"> <Table...> ... <div className='flex flex-wrap items-baseline gap-1 text-muted-foreground'> {item.created_at && ( - <TooltipProvider> <Tooltip> <TooltipTrigger asChild> <span className="cursor-default text-sm text-muted-foreground"> {formatTimestamp(item.created_at)} </span> </TooltipTrigger> <TooltipContent> {formatDate(item.created_at)} </TooltipContent> </Tooltip> - </TooltipProvider> )}Apply the same pattern to remove
TooltipProviderfrom the replies, likes, and reports tooltips (lines 277, 291, 305).Also applies to: 277-289, 291-303, 305-317
352-358: Improve feature image alt text for better accessibility.The alt text falls back to a generic 'Post feature image' when the post title is unavailable. Since
item.post.titleis checked earlier and could be an empty string, the alt should handle this case more gracefully.Apply this diff to improve the alt text:
{item.post?.feature_image ? ( <img - alt={item.post.title || 'Post feature image'} + alt={item.post.title?.trim() || 'Feature image'} className="hidden aspect-video w-32 rounded object-cover lg:block" src={item.post.feature_image} /> ) : null}This ensures whitespace-only titles are treated as empty and provides a more concise fallback.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/admin-x-framework/src/api/comments.ts(1 hunks)apps/posts/src/views/comments/components/comments-list.tsx(5 hunks)ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-05-16T06:39:39.055Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23363
File: apps/admin-x-framework/src/api/posts.ts:14-14
Timestamp: 2025-05-16T06:39:39.055Z
Learning: In the Post type definition, feature_image should be marked as optional (with ?) to safely handle posts without images and prevent runtime errors.
Applied to files:
apps/admin-x-framework/src/api/comments.tsghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.{ts,tsx} : Prefer less comments and give things clear names
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-10-27T11:59:33.968Z
Learnt from: peterzimon
Repo: TryGhost/Ghost PR: 25261
File: apps/admin/src/layout/app-sidebar/UserMenu.tsx:24-29
Timestamp: 2025-10-27T11:59:33.968Z
Learning: In apps/admin/src/layout/app-sidebar/UserMenu.tsx, the hardcoded placeholder data (avatar URL, initials "US", name "User Name", email "userexample.com") is a known limitation and should not be flagged for replacement with real user data.
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:48.725Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/Growth.tsx:199-204
Timestamp: 2025-11-12T22:26:48.725Z
Learning: In apps/stats/src/views/Stats/Growth/Growth.tsx, the loading skeleton for the top posts/pages table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the main content column (which contains post/page titles with dates).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:03.622Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx:189-199
Timestamp: 2025-11-12T22:26:03.622Z
Learning: In apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx, the loading skeleton for the GrowthSources table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the source column which contains the main content (source names with icons).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-06-18T10:56:19.906Z
Learnt from: 55sketch
Repo: TryGhost/Ghost PR: 23894
File: ghost/core/core/server/api/endpoints/comments.js:16-22
Timestamp: 2025-06-18T10:56:19.906Z
Learning: The validateCommentData function in ghost/core/core/server/api/endpoints/comments.js is not duplicated elsewhere in the codebase - it's a unique validation function that ensures either post_id or parent_id is provided for comment creation.
Applied to files:
ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js
🧬 Code graph analysis (1)
apps/posts/src/views/comments/components/comments-list.tsx (5)
apps/admin-x-framework/src/api/comments.ts (1)
Comment(8-35)ghost/core/core/server/models/comment.js (1)
Comment(28-359)apps/shade/src/components/ui/avatar.tsx (3)
Avatar(48-48)AvatarImage(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.0)
apps/posts/src/views/comments/components/comments-list.tsx
[warning] 110-110: 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] 111-111: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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). (8)
- GitHub Check: Ghost-CLI tests
- GitHub Check: ActivityPub tests
- GitHub Check: Legacy tests (Node 22.18.0, mysql8)
- GitHub Check: Legacy tests (Node 22.18.0, sqlite3)
- GitHub Check: Lint
- GitHub Check: Admin-X Settings tests
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Build & Push Docker Image
🔇 Additional comments (6)
apps/admin-x-framework/src/api/comments.ts (1)
28-28: LGTM! Correct use of optional field.The
feature_imagefield is correctly marked as optional, which safely handles posts without images and prevents runtime errors. Based on learnings, this is the recommended approach.ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js (1)
38-39: LGTM! Field addition aligns with frontend requirements.Adding
feature_imagetopostFieldsenables the frontend to display post images in the comments list, supporting the UI enhancements in this PR.apps/posts/src/views/comments/components/comments-list.tsx (4)
10-30: LGTM! Imports support new UI features.The new Avatar, Tooltip, and formatting utilities are appropriately imported and used throughout the refactored component.
61-72: LGTM! Date formatting implementation is sound.The
formatDatefunction correctly usesIntl.DateTimeFormatfor localization and includes a clear comment explaining the comma removal.
74-86: LGTM! Clean expand/collapse button implementation.The
ExpandButtoncomponent is straightforward and properly toggles between expanded and collapsed states with appropriate icons.
136-136: No action required: onAddFilter prop is properly provided at all call sites.The
onAddFilterprop is required and the single internal usage ofCommentsListincludes this prop, so there is no breaking change within the codebase. All usage sites are consistent with the current signature.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/posts/src/views/comments/components/comments-list.tsx (1)
206-206: Icon size override still present despite previous fix.A past review comment flagged the
!size-3override andsize={12}prop on this icon, which was reportedly addressed in commit 9f56431. However, the override is still present in the current code.Remove the className override and size prop to respect the avatar variant's intended sizing:
- <LucideIcon.User className='!size-3 text-muted-foreground' size={12} /> + <LucideIcon.User className='text-muted-foreground' />
🧹 Nitpick comments (2)
apps/posts/src/views/comments/components/comments-list.tsx (2)
202-204: Remove or uncomment the avatar image code.The
AvatarImagerendering is commented out. If avatar images are not yet implemented, consider removing this commented code to keep the codebase clean. If they will be implemented soon, add a TODO comment explaining the plan.<Avatar className={`size-5 ${item.status === 'hidden' && 'opacity-40'}`}> - {/* {item.member.avatar_image && ( - <AvatarImage alt={item.member.name} src={item.member.avatar_image} /> - )} */} + {item.member.avatar_image && ( + <AvatarImage alt={item.member.name} src={item.member.avatar_image} /> + )} <AvatarFallback>
237-237: Remove redundantonAddFiltercheck.Since
onAddFilteris now a required prop (line 136), checking for its existence is redundant.- {item.post?.id && item.post?.title && onAddFilter ? ( + {item.post?.id && item.post?.title ? ( <Button className="block h-auto truncate p-0 font-medium text-primary hover:opacity-70" variant="link" onClick={() => onAddFilter('post', item.post!.id)} > {item.post.title} </Button> ) : (
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/posts/src/views/comments/components/comments-list.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.{ts,tsx} : Prefer less comments and give things clear names
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/features/**/*.{ts,tsx} : Higher-level, opinionated components (e.g., PostShareModal, SourceTabs) should be placed in `src/components/features/*`
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-10-27T11:59:33.968Z
Learnt from: peterzimon
Repo: TryGhost/Ghost PR: 25261
File: apps/admin/src/layout/app-sidebar/UserMenu.tsx:24-29
Timestamp: 2025-10-27T11:59:33.968Z
Learning: In apps/admin/src/layout/app-sidebar/UserMenu.tsx, the hardcoded placeholder data (avatar URL, initials "US", name "User Name", email "userexample.com") is a known limitation and should not be flagged for replacement with real user data.
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:48.725Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/Growth.tsx:199-204
Timestamp: 2025-11-12T22:26:48.725Z
Learning: In apps/stats/src/views/Stats/Growth/Growth.tsx, the loading skeleton for the top posts/pages table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the main content column (which contains post/page titles with dates).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:03.622Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx:189-199
Timestamp: 2025-11-12T22:26:03.622Z
Learning: In apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx, the loading skeleton for the GrowthSources table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the source column which contains the main content (source names with icons).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
🧬 Code graph analysis (1)
apps/posts/src/views/comments/components/comments-list.tsx (5)
apps/shade/src/components/ui/table.tsx (2)
TableHead(148-148)TableCell(151-151)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/comments-ui/src/utils/admin-api.ts (2)
hideComment(59-61)showComment(62-64)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.0)
apps/posts/src/views/comments/components/comments-list.tsx
[warning] 110-110: 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] 111-111: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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). (5)
- GitHub Check: Admin-X Settings tests
- GitHub Check: Ghost-CLI tests
- GitHub Check: ActivityPub tests
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: Lint
🔇 Additional comments (10)
apps/posts/src/views/comments/components/comments-list.tsx (10)
1-31: LGTM!The new imports for Avatar, Tooltip, and formatting utilities are appropriate for the redesigned comment list UI.
61-72: LGTM!The custom date formatter correctly removes the comma between day and year for the desired format.
74-86: LGTM!Clean implementation of the expand/collapse button with appropriate styling and icons.
88-121: LGTM!The expandable comment content implementation correctly handles clamped text detection and state management. The XSS concern with
dangerouslySetInnerHTMLhas been addressed through server-side sanitization, and the resize listener approach was acknowledged in previous reviews.
168-170: LGTM!The table header structure is appropriate for the redesigned layout.
264-275: LGTM!The show/hide button logic correctly toggles based on the comment status.
277-318: LGTM!The metrics display (replies, likes, reports) with tooltips is well-implemented. The conditional styling for reports (yellow when present) provides good visual feedback.
319-347: LGTM!The dropdown menu provides appropriate quick actions for viewing the post and member profile.
351-359: LGTM!The post feature image column is well-implemented with appropriate responsive behavior and accessibility (alt text).
136-136: AllCommentsListusages have been verified to provide theonAddFilterprop. There is only one call site (inapps/posts/src/views/comments/comments.tsxat line 143), and it correctly passesonAddFilter={handleAddFilter}. The prop signature change from optional to required is safe.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/posts/src/views/comments/components/comments-list.tsx (1)
205-205: Icon size override conflicts with avatar variant.Line 205 uses
className='!size-3'with the important flag andsize={12}prop, overriding the intended avatar fallback styling. This is a duplicate of a previous review comment that has not been addressed.Apply this diff to respect the variant's intended sizing:
-<LucideIcon.User className='!size-3 text-muted-foreground' size={12} /> +<LucideIcon.User className='text-muted-foreground' />
🧹 Nitpick comments (2)
apps/posts/src/views/comments/components/comments-list.tsx (2)
201-203: Remove or implement commented AvatarImage code.The commented-out
AvatarImagesuggests incomplete implementation. Either implement avatar image support or remove the commented code to keep the codebase clean.
221-316: Consolidate TooltipProvider to component root.Multiple
TooltipProviderinstances (lines 221, 276, 290, 304) create unnecessary React context overhead. According to Radix UI best practices,TooltipProvidershould wrap the entire component tree once, not individual tooltips.Move
TooltipProviderto wrap the entireCommentsListcomponent:function CommentsList({ items, totalItems, hasNextPage, isFetchingNextPage, fetchNextPage, onAddFilter }: { items: Comment[]; totalItems: number; hasNextPage?: boolean; isFetchingNextPage?: boolean; fetchNextPage: () => void; onAddFilter: (field: string, value: string, operator?: string) => void; }) { // ... state declarations ... return ( + <TooltipProvider> <div ref={parentRef} className="overflow-hidden"> <Table className="flex table-fixed flex-col lg:table" data-testid="comments-list" > {/* ... table content ... */} </Table> {/* ... AlertDialog ... */} </div> + </TooltipProvider> ); }Then remove the individual
TooltipProviderwrappers around eachTooltipcomponent (lines 221-233, 276-288, 290-302, 304-316), keeping only theTooltip,TooltipTrigger, andTooltipContentcomponents.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/posts/src/views/comments/components/comments-list.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: minimaluminium
Repo: TryGhost/Ghost PR: 24798
File: apps/admin-x-activitypub/src/components/feed/layouts/ReplyLayout.tsx:122-122
Timestamp: 2025-09-02T07:58:42.097Z
Learning: In apps/admin-x-activitypub ReplyLayout component, renderFeedAttachment for Article-type objects intentionally receives onClick (card click handler) instead of onImageClick to ensure clicking article images opens the article rather than triggering the image lightbox.
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.{ts,tsx} : Prefer less comments and give things clear names
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/features/**/*.{ts,tsx} : Higher-level, opinionated components (e.g., PostShareModal, SourceTabs) should be placed in `src/components/features/*`
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-10-27T11:59:33.968Z
Learnt from: peterzimon
Repo: TryGhost/Ghost PR: 25261
File: apps/admin/src/layout/app-sidebar/UserMenu.tsx:24-29
Timestamp: 2025-10-27T11:59:33.968Z
Learning: In apps/admin/src/layout/app-sidebar/UserMenu.tsx, the hardcoded placeholder data (avatar URL, initials "US", name "User Name", email "userexample.com") is a known limitation and should not be flagged for replacement with real user data.
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:48.725Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/Growth.tsx:199-204
Timestamp: 2025-11-12T22:26:48.725Z
Learning: In apps/stats/src/views/Stats/Growth/Growth.tsx, the loading skeleton for the top posts/pages table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the main content column (which contains post/page titles with dates).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
📚 Learning: 2025-11-12T22:26:03.622Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx:189-199
Timestamp: 2025-11-12T22:26:03.622Z
Learning: In apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx, the loading skeleton for the GrowthSources table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the source column which contains the main content (source names with icons).
Applied to files:
apps/posts/src/views/comments/components/comments-list.tsx
🧬 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.0)
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)
⏰ 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). (5)
- GitHub Check: Ghost-CLI tests
- GitHub Check: Admin-X Settings tests
- GitHub Check: Unit tests (Node 22.18.0)
- GitHub Check: ActivityPub tests
- GitHub Check: Lint
3 participants
ref. https://linear.app/ghost/issue/FEA-465/allow-individual-banning-and-moderation-of-commenters