UI Modernization: ChatGPT-Style Design System Foundation #116
Conversation
This commit implements the foundation for UI modernization, transforming the internal system style into a modern AI tool interface inspired by ChatGPT's design language. Key Changes: - Install shadcn/ui dependencies (Radix UI primitives, lucide-react, CVA, tailwind-merge) - Configure shadcn/ui with components.json and updated Tailwind config - Introduce ChatGPT-inspired color scheme with new CSS variables for light/dark modes - Create core shadcn/ui components: Button, Input, Textarea, Select, Dialog - Add utility function library (cn helper) for class name management - Remove Ant Design custom styles from globals.css (repository-selector, custom-message) - Update Tailwind config to support shadcn/ui animations and border radius system - Add content path for new components directory Technical Details: - Light mode: Uses ChatGPT green (#10a37f) as primary color with clean backgrounds - Dark mode: Adopts ChatGPT's signature dark theme (#212121 base, #343541 surface) - All UI components follow ChatGPT design patterns (rounded corners, subtle shadows) - Maintains full theme compatibility with existing dark/light mode switching This is the first phase of the modernization effort. Subsequent commits will apply these components to chat interfaces and replace legacy UI library usage.
|
Caution Review failedThe pull request is closed. WalkthroughThis PR establishes a new UI component library using shadcn/ui and Radix UI primitives, replacing Ant Design components with custom-styled alternatives. Updates include new reusable UI components (Button, Dialog, Input, Select, Textarea), integrated lucide-react icons globally, refreshed theme tokens in globals.css, and refactored numerous existing components to use the new UI system. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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
🧹 Nitpick comments (4)
frontend/src/components/ui/input.tsx (1)
12-27: Consider React 19's ref as prop feature.React 19 changed ref handling — ref is now a regular prop and no longer requires
forwardRef. WhileforwardRefstill works in React 19 for backward compatibility, you can simplify this component:-const Input = React.forwardRef<HTMLInputElement, InputProps>( - ({ className, type, ...props }, ref) => { +const Input = ({ className, type, ref, ...props }: InputProps & { ref?: React.Ref<HTMLInputElement> }) => { return ( <input type={type} className={cn( 'flex h-10 w-full rounded-lg border border-border bg-surface px-4 py-3 text-sm ring-offset-base file:border-0 file:bg-transparent file:text-sm file:font-medium placeholder:text-text-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50', className )} ref={ref} {...props} /> ) - } -) -Input.displayName = 'Input' +}Based on learnings
frontend/src/components/ui/textarea.tsx (1)
14-30: Consider React 19's ref as prop feature.Similar to the Input component, React 19 allows ref as a regular prop without forwardRef. You can simplify this component while maintaining the same functionality:
-const Textarea = React.forwardRef<HTMLTextAreaElement, TextareaProps>( - ({ className, chatStyle = false, ...props }, ref) => { +const Textarea = ({ className, chatStyle = false, ref, ...props }: TextareaProps & { ref?: React.Ref<HTMLTextAreaElement> }) => { return ( <textarea className={cn( chatStyle ? 'flex min-h-[60px] w-full rounded-2xl border border-border bg-surface px-4 py-3 text-sm ring-offset-base placeholder:text-text-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:border-primary disabled:cursor-not-allowed disabled:opacity-50 resize-none shadow-sm' : 'flex min-h-[80px] w-full rounded-lg border border-border bg-surface px-4 py-3 text-sm ring-offset-base placeholder:text-text-muted focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary disabled:cursor-not-allowed disabled:opacity-50', className )} ref={ref} {...props} /> ) - } -) -Textarea.displayName = 'Textarea' +}Based on learnings
frontend/src/components/ui/button.tsx (1)
42-54: Consider React 19's ref as prop feature.React 19 allows ref as a regular prop without forwardRef. While the current implementation works, you could simplify it to align with React 19 patterns:
-const Button = React.forwardRef<HTMLButtonElement, ButtonProps>( - ({ className, variant, size, asChild = false, ...props }, ref) => { +const Button = ({ className, variant, size, asChild = false, ref, ...props }: ButtonProps & { ref?: React.Ref<HTMLButtonElement> }) => { const Comp = asChild ? Slot : 'button' return ( <Comp className={cn(buttonVariants({ variant, size, className }))} ref={ref} {...props} /> ) - } -) -Button.displayName = 'Button' +}Note: The
asChildpattern allows this Button to render as a child component (e.g.,<Button asChild><Link href="...">...</Link></Button>), which is useful for preserving button styling while rendering a different element.Based on learnings
frontend/package.json (1)
18-32: Multiple icon libraries suggest incomplete migration.The project now has four icon libraries installed:
@heroicons/react(line 18)@tabler/icons-react(line 24)react-icons(line 37)lucide-react(line 32, newly added)According to the PR objectives, the next steps include "migrate icons to lucide-react". Consider creating a tracking issue to consolidate icon usage and remove unused icon libraries once the migration is complete.
Would you like me to generate a script to find which icon libraries are currently in use across the codebase?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
frontend/components.json(1 hunks)frontend/package.json(1 hunks)frontend/src/app/globals.css(1 hunks)frontend/src/components/ui/button.tsx(1 hunks)frontend/src/components/ui/dialog.tsx(1 hunks)frontend/src/components/ui/input.tsx(1 hunks)frontend/src/components/ui/select.tsx(1 hunks)frontend/src/components/ui/textarea.tsx(1 hunks)frontend/src/lib/utils.ts(1 hunks)frontend/tailwind.config.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/src/components/ui/button.tsx (1)
frontend/src/lib/utils.ts (1)
cn(8-10)
frontend/src/components/ui/dialog.tsx (1)
frontend/src/lib/utils.ts (1)
cn(8-10)
frontend/src/components/ui/input.tsx (1)
frontend/src/lib/utils.ts (1)
cn(8-10)
frontend/src/components/ui/textarea.tsx (1)
frontend/src/lib/utils.ts (1)
cn(8-10)
frontend/src/components/ui/select.tsx (1)
frontend/src/lib/utils.ts (1)
cn(8-10)
🔇 Additional comments (11)
frontend/src/lib/utils.ts (1)
8-10: LGTM!This is the standard shadcn/ui utility for merging Tailwind classes. The implementation correctly uses
clsxfor conditional class composition andtwMergefor Tailwind-specific deduplication.frontend/src/app/globals.css (2)
35-48: Dark theme updates align with ChatGPT-inspired design.The new dark theme color scheme (#212121 base, #343541 surface, #40414f muted) successfully implements the ChatGPT-inspired design mentioned in the PR objectives. The teal primary color (#10a37f) is consistent across both themes.
14-26: Verification complete: color token changes are intentional design system updates with confirmed component usage.The git diff confirms this is part of the "feat: introduce shadcn/ui and ChatGPT-style design system" commit. All color tokens in lines 14-26 are actively used:
- --color-text-muted, --color-text-primary, --color-border: Used in QuotaUsage.tsx, TeamEdit.tsx, ChatArea.tsx, and other components
- --color-scrollbar-track, --color-scrollbar-thumb, --color-border-strong: Used in scrollbar.css and tasks.css
- --color-primary: Used in animations and UI components throughout the codebase
The changes shift the palette from blue-tinted neutrals to green/teal primary with neutral grays. Most significant change: primary color shifts from blue (37 99 235) to green/teal (16 163 127), affecting --color-focus-ring identically.
Components will not break functionally—they'll render with the new color values. The visual appearance will change substantially, but this is the intended design system refresh.
frontend/src/components/ui/button.tsx (1)
11-34: Button variants are well-designed.The button variants (default, secondary, ghost, link) and sizes (default, sm, lg, icon) provide good flexibility for different UI contexts. The use of
class-variance-authorityenables type-safe variant selection.frontend/package.json (1)
27-27: Ant Design dependency remains while custom styles were removed.The PR removes Ant Design custom styles from
globals.cssbut retains theantddependency. This is appropriate for a phased migration approach as mentioned in the PR objectives ("remove Ant Design from chat pages" is listed as a next step). Ensure that existing Ant Design components still render correctly without the removed custom styles.frontend/tailwind.config.js (3)
14-14: Content path correctly includes new components directory.Adding
'./src/components/**/*.{js,ts,jsx,tsx,mdx}'ensures Tailwind processes the new UI components for tree-shaking and class generation. This aligns with the component structure introduced in this PR.
31-49: Border radius and animation tokens are well-integrated.The border radius system (
lg,md,sm) derives from the--radiusCSS variable defined inglobals.css(line 30, 52), and the accordion animations support Radix UI primitives. This creates a cohesive design token system.
52-52: LGTM!The
tailwindcss-animateplugin addition aligns with the new animation tokens and is a standard requirement for shadcn/ui components.frontend/components.json (1)
12-15: Path aliases are correctly configured—no action required.The verification confirms that
tsconfig.jsonproperly defines path resolution with"@/*": ["./src/*"], which covers both aliases (@/componentsand@/lib/utils). The wildcard pattern enables TypeScript resolution and editor autocomplete as intended.frontend/src/components/ui/dialog.tsx (1)
5-126: Dialog composition looks solid and idiomaticThe Dialog wrapper, overlay, content, and header/footer/title/description helpers follow Radix + shadcn patterns, forward refs are correctly typed, props are forwarded, and styling matches the new design tokens (rounded-2xl, border-border, bg-base/surface, focus rings, data-state animations). I don’t see functional or accessibility issues here.
frontend/src/components/ui/select.tsx (1)
39-164: Rest of Select composition aligns well with Radix patterns and design tokensScroll buttons, Content (with
positiondefaulting to'popper'), Label, Item, Separator, and exports all look correct and consistent with Radix Select APIs and your design system (rounded corners, border-border, bg-surface, focus state on items, animated open/close). No additional issues spotted beyond the placeholder styling noted above.
| --color-success: 34 197 94; | ||
| --color-error: 239 68 68; | ||
| --shadow-popover: 0 12px 32px rgba(15, 23, 42, 0.12); | ||
| --radius: 0.5rem; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify removed Ant Design styles aren't breaking existing components.
The PR summary mentions removing Ant Design custom styles (repository-selector, custom-message, custom-message-notice-content), but these removals aren't visible in the diff (only modified lines are shown). Since Ant Design is still a dependency in package.json, verify that removing these custom styles doesn't break existing Ant Design components that may still be in use.
Run the following script to check for usage of removed style classes and Ant Design components:
🏁 Script executed:
#!/bin/bash
# Description: Check for usage of removed Ant Design styles and components
# Search for references to removed custom classes
rg -n 'repository-selector|custom-message|custom-message-notice-content' \
frontend/src --type tsx --type ts
# Find Ant Design component imports (Select, message, etc.)
rg -n "from ['\"]antd['\"]" frontend/src --type tsx --type ts -A 1
# Look for RepositorySelector component usage
ast-grep --pattern 'import { $$$ } from "antd"'Length of output: 55
🏁 Script executed:
#!/bin/bash
# Description: Check for usage of removed Ant Design styles and components (corrected)
echo "=== Searching for removed style class references ==="
rg -n 'repository-selector|custom-message|custom-message-notice-content' frontend/src -g '*.tsx' -g '*.ts' -g '*.jsx' -g '*.js' -g '*.css' -g '*.scss'
echo -e "\n=== Searching for Ant Design imports ==="
rg -n "from ['\"]antd['\"]" frontend/src -g '*.tsx' -g '*.ts' -g '*.jsx' -g '*.js' -A 1
echo -e "\n=== Checking globals.css for removed styles ==="
cat frontend/src/app/globals.css | head -80Length of output: 10097
🏁 Script executed:
#!/bin/bash
# Search for removed style definitions in globals.css
echo "=== Searching for removed style classes in globals.css ==="
rg -n 'repository-selector|custom-message|custom-message-notice-content' frontend/src/app/globals.css
echo -e "\n=== Total lines in globals.css ==="
wc -l frontend/src/app/globals.css
echo -e "\n=== Searching for .repository-selector definition ==="
rg -n '\.(repository-selector|custom-message)' frontend/src/app/globals.cssLength of output: 182
Update components to remove references to deleted CSS classes or restore the styles.
The verification confirms the review concern: the removed style classes repository-selector and custom-message are actively referenced in component files (TeamSelector.tsx:155/164, RepositorySelector.tsx:276/280, BranchSelector.tsx:163/167, and AntdProvider.tsx:194), but these styles do not exist in globals.css. This will break the visual appearance of Ant Design Select dropdowns and message components.
Fix by either:
- Removing the className and prefixCls references from components, OR
- Restoring the removed CSS definitions in globals.css
| <SelectPrimitive.Trigger | ||
| ref={ref} | ||
| className={cn( | ||
| 'flex h-10 w-full items-center justify-between rounded-lg border border-border bg-transparent px-3 py-2 text-sm ring-offset-base placeholder:text-text-muted focus:outline-none focus:ring-2 focus:ring-primary focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50', | ||
| className | ||
| )} | ||
| {...props} | ||
| > | ||
| {children} | ||
| <SelectPrimitive.Icon asChild> | ||
| <ChevronDown className="h-4 w-4 opacity-50" /> | ||
| </SelectPrimitive.Icon> | ||
| </SelectPrimitive.Trigger> | ||
| )) |
There was a problem hiding this comment.
🧩 Analysis chain
placeholder: utility on SelectTrigger is ineffective; target the value’s placeholder instead
SelectTrigger is ultimately rendered as a <button> by Radix, so placeholder:text-text-muted on the trigger won’t affect the text color of SelectValue’s placeholder span. As a result, placeholder text likely isn’t visually distinguished from a selected value.
You can instead style the internal placeholder span (Radix adds data-placeholder to it). For example:
- className={cn(
- 'flex h-10 w-full items-center justify-between rounded-lg border border-border bg-transparent px-3 py-2 text-sm ring-offset-base placeholder:text-text-muted focus:outline-none focus:ring-2 focus:ring-primary focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50',
- className
- )}
+ className={cn(
+ 'flex h-10 w-full items-center justify-between rounded-lg border border-border bg-transparent px-3 py-2 text-sm ring-offset-base focus:outline-none focus:ring-2 focus:ring-primary focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 [&_[data-placeholder]]:text-text-muted',
+ className
+ )}This keeps the same overall styling while correctly muting only the placeholder text.
🌐 Web query:
In Radix UI Select, how is the placeholder rendered inside `SelectValue`, and what attributes/classes (e.g., `data-placeholder`) does it use that we can target for styling?
💡 Result:
-
The placeholder is provided via the placeholder prop on Select.Value — when no value/defaultValue is set that placeholder node/text is rendered inside the SelectValue element. (radix-ui.com)
-
For styling hooks:
- Select.Trigger receives a data-placeholder attribute when the Select is rendering a placeholder (you can target [data-placeholder]). (radix-ui.com)
- Other Trigger attributes you can use: data-state ("open" | "closed") and data-disabled (when disabled). (radix-ui.com)
-
Notes:
- Select.Value supports asChild (so the rendered tag may vary), but the placeholder behavior and the Trigger’s data-placeholder attribute remain the intended styling hooks. (radix-ui.com)
Citations:
- 1: https://www.radix-ui.com/primitives/docs/components/select?utm_source=openai
- 2: https://www.radix-ui.com/primitives/docs/components/select?utm_source=openai
- 3: https://www.radix-ui.com/primitives/docs/components/select?utm_source=openai
- 4: https://www.radix-ui.com/primitives/docs/components/select?utm_source=openai
placeholder: utility is ineffective, but the proposed selector needs correction
The original concern is valid: placeholder:text-text-muted won't work on a button. However, the Radix UI Select.Trigger receives a data-placeholder attribute when rendering a placeholder, which is the intended styling hook.
The issue with the proposed fix: since the Trigger itself receives the attribute (not a child element), the selector should target the element directly, not a descendant:
- className={cn(
- 'flex h-10 w-full items-center justify-between rounded-lg border border-border bg-transparent px-3 py-2 text-sm ring-offset-base placeholder:text-text-muted focus:outline-none focus:ring-2 focus:ring-primary focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50',
- className
- )}
+ className={cn(
- 'flex h-10 w-full items-center justify-between rounded-lg border border-border bg-transparent px-3 py-2 text-sm ring-offset-base focus:outline-none focus:ring-2 focus:ring-primary focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 [&[data-placeholder]]:text-text-muted',
+ className
+ )}Use [&[data-placeholder]] instead of [&_[data-placeholder]] to correctly target the Trigger when displaying a placeholder.
📝 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.
| <SelectPrimitive.Trigger | |
| ref={ref} | |
| className={cn( | |
| 'flex h-10 w-full items-center justify-between rounded-lg border border-border bg-transparent px-3 py-2 text-sm ring-offset-base placeholder:text-text-muted focus:outline-none focus:ring-2 focus:ring-primary focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50', | |
| className | |
| )} | |
| {...props} | |
| > | |
| {children} | |
| <SelectPrimitive.Icon asChild> | |
| <ChevronDown className="h-4 w-4 opacity-50" /> | |
| </SelectPrimitive.Icon> | |
| </SelectPrimitive.Trigger> | |
| )) | |
| <SelectPrimitive.Trigger | |
| ref={ref} | |
| className={cn( | |
| 'flex h-10 w-full items-center justify-between rounded-lg border border-border bg-transparent px-3 py-2 text-sm ring-offset-base focus:outline-none focus:ring-2 focus:ring-primary focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 [&[data-placeholder]]:text-text-muted', | |
| className | |
| )} | |
| {...props} | |
| > | |
| {children} | |
| <SelectPrimitive.Icon asChild> | |
| <ChevronDown className="h-4 w-4 opacity-50" /> | |
| </SelectPrimitive.Icon> | |
| </SelectPrimitive.Trigger> | |
| )) |
🤖 Prompt for AI Agents
In frontend/src/components/ui/select.tsx around lines 23 to 36, the current
Tailwind class `placeholder:text-text-muted` is ineffective because the Trigger
button receives a `data-placeholder` attribute (not a placeholder
pseudo-element) when a placeholder is shown; replace that class with a selector
that targets the Trigger itself when it has the attribute, e.g. add
`[&[data-placeholder]]:text-text-muted` (or swap `placeholder:text-text-muted`
for this) inside the cn(...) classes so the Trigger element becomes muted when
`data-placeholder` is present.
This commit applies the shadcn/ui components and ChatGPT design language to the actual chat pages, completing the visual transformation. Key Changes: **MessagesArea.tsx** - Replace react-icons with lucide-react icons (User, Bot, Copy, Check, Download) - Replace Ant Design Button with shadcn/ui Button component - Update message card styling: - Increase border radius to rounded-2xl (16px) for ChatGPT aesthetic - Enhance padding from p-3 to p-5 for better breathing room - Add subtle shadow-sm for depth - Increase message spacing from space-y-4 to space-y-8 (2rem) - Modernize button interactions: - Use ghost variant for tool buttons - Consistent icon sizing (h-4 w-4) - Improved hover states with bg-muted **ChatArea.tsx** - Replace PaperAirplaneIcon (Heroicons) with Send icon (lucide-react) - Replace Ant Design Button with shadcn/ui Button for send action - Update send button design: - Round button (rounded-full) for ChatGPT-style aesthetic - Ghost variant with hover effect (hover:bg-primary/10) - Consistent sizing (h-10 w-10) - Update input card border radius to rounded-2xl Visual Impact: - Messages now have ChatGPT-like appearance with larger spacing and rounded corners - Send button matches ChatGPT's circular icon button design - All buttons use consistent shadcn/ui variants - Icons are now from lucide-react, providing a more modern and cohesive look - Improved visual hierarchy with better spacing and padding This maintains full backward compatibility while significantly modernizing the UI.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/features/tasks/components/ChatArea.tsx (1)
421-439: Consider adding accessible label for icon-only button.The Button migration looks correct with the new
variant="ghost"andsize="icon"API. However, for better accessibility, consider adding anaria-labelattribute to help screen reader users understand the button's purpose.Apply this diff to add an accessible label:
<Button variant="ghost" size="icon" onClick={handleSendMessage} + aria-label={isLoading || selectedTaskDetail?.status === 'PENDING' || selectedTaskDetail?.status === 'RUNNING' ? t('messages.sending') : t('messages.send')} disabled={ isLoading || selectedTaskDetail?.status === 'PENDING' || selectedTaskDetail?.status === 'RUNNING' } className="h-10 w-10 rounded-full hover:bg-primary/10" >Note: You'll need to add corresponding translation keys for
messages.sendingandmessages.sendif they don't already exist.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/features/tasks/components/ChatArea.tsx(4 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/features/tasks/components/ChatArea.tsx (1)
frontend/src/features/tasks/components/LoadingDots.tsx (1)
LoadingDots(7-24)
frontend/src/features/tasks/components/MessagesArea.tsx (1)
frontend/src/components/ui/button.tsx (1)
Button(56-56)
🔇 Additional comments (10)
frontend/src/features/tasks/components/ChatArea.tsx (2)
8-8: LGTM: Icon and Button migration.The import changes correctly reflect the migration from heroicons to lucide-react for icons and the introduction of the new shadcn/ui Button component. The continued use of antd's App context (line 19) is appropriate during this phased migration.
Also applies to: 20-20
398-398: LGTM: Border radius increased per design system.The change from
rounded-xltorounded-2xlaligns with the ChatGPT-style design system principles specified in the PR objectives.frontend/src/features/tasks/components/MessagesArea.tsx (8)
10-11: LGTM: Complete icon library migration.The migration from react-icons (RiRobot2Line, RiUser3Line, FiCopy, FiCheck, FiDownload) to lucide-react is complete and consistent with the changes in ChatArea.tsx.
78-91: LGTM: CopyButton migrated correctly with good accessibility.The Button migration is correct with
variant="ghost"andsize="icon". Thetitleattribute provides good accessibility for the icon-only button. The icon size consistency (h-4 w-4) and conditional rendering (Copy ↔ Check) work well.Note: The className pattern
className={className ?? 'h-8 w-8 hover:bg-muted'}provides a sensible default while allowing override.
108-108: LGTM: Spacing adjustment aligns with design system.The change from
bottom-1tobottom-2increases the bottom padding from 4px to 8px, consistent with the overall spacing improvements in this PR.
111-121: LGTM: BubbleTools Button usage consistent.The Button API usage is correct and consistent with CopyButton. The
titleattribute provides accessibility, and the icon rendering pattern is clean.
382-382: LGTM: Download icon migrated with consistent sizing.The Download icon from lucide-react uses consistent sizing (h-4 w-4) and color (text-text-muted) with other toolbar icons.
682-682: LGTM: Message spacing increased per design goals.The change from
space-y-4tospace-y-8doubles the vertical spacing between messages (16px → 32px), directly addressing the "increase message spacing" goal mentioned in the PR objectives and improving readability.
686-686: LGTM: Bubble styling modernized per design system.The styling updates align well with the ChatGPT-style design principles:
- Increased padding (p-3 pb-8 → p-5 pb-10) improves content spacing
- Border radius (rounded-lg → rounded-2xl) matches the design system's 2xl standard
- Added
shadow-smprovides the subtle depth specified in the design principles
691-694: LGTM: Header icons migrated consistently.The User and Bot icons from lucide-react are appropriately sized (w-4 h-4) for the message header context and semantically correct for their roles.
Address user feedback for better ChatGPT visual alignment: **1. Fix Task Sidebar Icon Style** - Replace react-icons (fa6) with lucide-react in TaskListSection - Icons updated: - FaRegCircleCheck → CheckCircle2 - FaRegCircleXmark → XCircle - FaRegCircleStop → StopCircle - FaRegCirclePause → PauseCircle - ArrowPathIcon (Heroicons) → RotateCw - Provides consistent icon style across the entire interface - Better visual harmony with the modernized chat area **2. Fix Input Box Background Color** - Change input card background from bg-surface (gray #F7F7F8) to bg-base (white) - Matches ChatGPT's white input box with border design - Previously: Gray background looked inconsistent with ChatGPT - Now: Pure white background with border for authentic ChatGPT look Visual Impact: - Task list sidebar now uses modern, consistent lucide-react icons - Input box matches ChatGPT's signature white-with-border aesthetic - Overall interface feels more cohesive and polished
Address user feedback for better visual harmony and ChatGPT-style consistency: **1. Update Sidebar Buttons to shadcn/ui** - Replace Ant Design Button with shadcn/ui Button in TaskSidebar - New Task button: Now uses primary variant with proper styling - Settings button: Changed to ghost variant for subtle appearance - Migrate icons from Heroicons to lucide-react: - PlusIcon → Plus - MagnifyingGlassIcon → Search - XMarkIcon → X - Cog6ToothIcon → Settings - All buttons now have consistent shadcn/ui styling **2. Remove Borders from Selectors** - Add variant="borderless" to all three selectors: - TeamSelector - RepositorySelector - BranchSelector - Creates cleaner, ChatGPT-style appearance - Reduces visual clutter in input area **3. Reduce Color Contrast for Better Harmony** - Lighten --color-bg-muted in light mode: - Before: rgb(236, 236, 241) - too dark, high contrast - After: rgb(249, 250, 251) - subtle, gentle on eyes - Messages and UI elements now have softer transitions - Better aligns with ChatGPT's minimalist aesthetic - Reduces eye strain from harsh color differences Visual Impact: - Sidebar now uses modern shadcn/ui buttons instead of Ant Design - Selector dropdowns appear borderless for cleaner look - Overall color palette is more harmonious with reduced contrast - Interface feels more cohesive and polished
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/features/tasks/components/BranchSelector.tsx (1)
154-154: Verify alignment with shadcn/ui migration strategy.Similar to TeamSelector.tsx, this component continues using Ant Design's Select with
variant="borderless". Per the PR's migration objectives toward shadcn/ui, consider whether this selector should be migrated to the new Select component atfrontend/src/components/ui/select.tsxfor consistency.
🧹 Nitpick comments (2)
frontend/src/features/tasks/components/TaskSidebar.tsx (2)
175-189: Improve clear-search button semantics and future‑proofingTwo small tweaks worth considering here:
- Add
type="button"on the clear button so it won’t accidentally submit if this input is ever wrapped in a<form>.- Add an accessible label, e.g.
aria-label={t('tasks.clear_search')}, since theXicon alone isn’t announced meaningfully by screen readers.Example:
- {localSearchTerm && ( - <button - onClick={handleClearSearch} - className="absolute right-2 top-1/2 transform -translate-y-1/2" - > + {localSearchTerm && ( + <button + type="button" + onClick={handleClearSearch} + aria-label={t('tasks.clear_search')} + className="absolute right-2 top-1/2 transform -translate-y-1/2" + > <X className="h-3.5 w-3.5 text-text-muted hover:text-text-primary" /> </button>
196-203: New Task Button usage looks correct;size="default"likely redundantUsing the shared
ButtonwithclassName="w-full"and a leadingPlusicon matches the new design system nicely. If your Button component follows the typical shadcn default variants,size="default"is implied and can be dropped to reduce noise, but that’s purely optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/app/globals.css(1 hunks)frontend/src/features/tasks/components/BranchSelector.tsx(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(4 hunks)frontend/src/features/tasks/components/RepositorySelector.tsx(1 hunks)frontend/src/features/tasks/components/TaskListSection.tsx(2 hunks)frontend/src/features/tasks/components/TaskSidebar.tsx(5 hunks)frontend/src/features/tasks/components/TeamSelector.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/features/tasks/components/RepositorySelector.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/globals.css
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/features/tasks/components/TaskSidebar.tsx (1)
frontend/src/config/paths.ts (1)
paths(5-44)
frontend/src/features/tasks/components/ChatArea.tsx (1)
frontend/src/features/tasks/components/LoadingDots.tsx (1)
LoadingDots(7-24)
🔇 Additional comments (9)
frontend/src/features/tasks/components/TaskListSection.tsx (2)
10-16: LGTM! Clean migration to lucide-react icons.The import statement correctly brings in all necessary status icons from lucide-react. The icon choices are semantically appropriate for their respective task statuses.
133-153: Verify visual consistency of the new icons.The icon mappings are correct and the implementation looks good. The new lucide-react icons are semantically appropriate replacements for the previous ones.
Please verify that the visual appearance of the new icons (especially
RotateCwreplacingArrowPathIconfor RUNNING status andPauseCirclereplacingFaRegCirclePausefor PENDING) maintains visual consistency with the previous design in both light and dark modes.frontend/src/features/tasks/components/TaskSidebar.tsx (2)
8-13: Button + lucide imports align with new design systemSwitch to the local
Buttonand lucide-react icon imports looks consistent with the new shadcn-based stack; all imported icons are used and there’s no obvious dead code here.
265-277: Settings Button variant and styling are consistent
variant="ghost"withsize="sm"and custom text color classes fits the “link-like settings action” use case and keeps the visual hierarchy below the primary “New Task” button. The icon + label composition is clear and accessible.frontend/src/features/tasks/components/ChatArea.tsx (4)
421-439: Verify Button children rendering and conditional logic.The Button now uses children for icon rendering with conditional logic showing either
LoadingDotsorSendicon. The disabled state checksselectedTaskDetail?.statusfor 'PENDING' or 'RUNNING'. Ensure:
- The LoadingDots component is properly imported (line 14 shows it is)
- The conditional logic correctly prevents multiple clicks during task execution
- The Send icon renders with appropriate size/color
The implementation looks correct, but visual testing is recommended to confirm the icon sizing and loading state appearance.
Manual testing checklist:
- Light and dark mode rendering of Send icon
- LoadingDots animation during task execution
- Button disabled state visual feedback
- Icon alignment and sizing within the rounded button
8-8: The script output reveals important findings. Let me run a corrected verification to fully assess the situation:The original review comment is incorrect.
Based on verification:
lucide-react IS listed in package.json: The initial script output clearly shows
"^0.554.0"in the dependencies section, contradicting the review's claim that "this library is not listed in the package manifest."The import is valid: lucide-react supports importing individual icons as React components, and the
Sendicon is a standard export from this package.node_modules absence is not the issue: The sandbox environment does not include installed dependencies, which is a limitation of the verification environment, not a problem with the actual codebase.
The code will function correctly when dependencies are properly installed (
npm installor equivalent).
398-398: All CSS tokens are properly defined and available for use.The verification confirms:
bg-baseis mapped to--color-bg-basewith CSS variables defined in globals.css for both light and dark modesbg-surfaceis mapped to--color-bg-surfacewith CSS variables defined; it's already extensively used throughout the codebase (60+ occurrences)rounded-2xlis a standard Tailwind class that's already used in 4 places in the codebase (textarea, dialog, MessagesArea, ChatArea components)No styling issues or missing definitions will result from these changes. The code is correct as written.
20-20: Button component compatibility verified — no issues found.The new Button component from
@/components/ui/buttonfully supports all props and patterns used in ChatArea.tsx (lines 421-439):
variant="ghost"is defined in buttonVariantssize="icon"is defined in buttonVariants- Children-based icon rendering works via React.ButtonHTMLAttributes
- Additional className styling is supported
- All event handlers (onClick) and disabled states work as expected
The import change is safe and compatible.
frontend/src/features/tasks/components/TeamSelector.tsx (1)
131-131: Clarify the shadcn/ui migration strategy and scope for Select components.The concern is factually valid. TeamSelector.tsx (line 131) and other task components (BranchSelector, RepositorySelector) continue using Ant Design's Select with
variant="borderless", while a new shadcn/ui Select component exists atfrontend/src/components/ui/select.tsxbut is currently unused throughout the codebase.The migration appears selective: shadcn/ui Button is being adopted in some places, yet Ant Design Select (and other Ant Design components) persists in task-related features. Since the two component APIs are fundamentally incompatible, clarify whether:
- The migration should be completed for Select components (requiring substantial refactoring)
- Ant Design is intentionally retained for these components (document the decision)
- The shadcn/ui Select should be removed if not part of the plan
- Updated all form buttons to use ghost/secondary variants for minimal visual impact - Changed "New Task" button from primary to outline variant - Replaced all react-icons with lucide-react in ThinkingComponent - Removed large color blocks from interactive elements to match ChatGPT aesthetic - Updated ClarificationForm submit button to secondary variant - Updated FinalPromptMessage buttons to ghost/secondary variants
- Replace all remaining FiMaximize2/FiMinimize2 with Maximize2/Minimize2 - Ensures all react-icons are fully migrated to lucide-react - Fixes ReferenceError for undefined FiMaximize2
- Reduce chat input default height from 3 rows to 1 row (auto-expands on input) - Move 'New Task' button above search box in sidebar for better UX - Reduce top navigation height (py-6/py-8 → py-3/py-4, min-h-60 → min-h-48) - Improves screen space efficiency while maintaining ChatGPT-like aesthetics
- Add bottom border to TopNavigation component - Set explicit background color to ensure header stands out - Improves visual hierarchy when viewing chat messages
- Reduce spacing between 'New Task' button and search box in sidebar (mb-3 → mb-2) - Fix team selector vertical alignment (items-baseline → items-center) - Further reduce header height (py-3/py-4 → py-2/py-3, min-h-48 → min-h-44) - Improves visual consistency and compactness
修复了多个UI对齐和样式问题: - 新建任务按钮:移除加粗样式,添加outline变体,修复与logo和任务列表的对齐 - 顶部导航:修复导航链接和GitHub按钮的垂直对齐 - 输入框:减少内边距至单行高度(px-3 py-2) - 选择器宽度:统一团队和仓库选择器的响应式宽度(移动端150px,桌面端200px)
本次修复包含4个ChatGPT风格的交互优化: 1. 搜索框交互改进 - 默认透明背景,鼠标移上去显示输入框样式 - 使用group hover实现平滑过渡效果 - 聚焦时自动显示背景和边框 2. 统一选择器宽度 - 团队和仓库选择器paddingRight统一为20px - 确保视觉宽度完全一致 3. 新建任务按钮样式 - 从outline变体改为ghost变体 - 去除边框,更符合ChatGPT的极简设计 4. 文本悬停效果 - 任务标题和时间添加hover:text-text-primary - 鼠标移上去文本颜色变深 - 与ChatGPT的交互反馈保持一致
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/features/tasks/components/ThinkingComponent.tsx (1)
9-15: Lucide icon migration is consistent; consider marking icons decorative for screen readersThe swap to
lucide-react(Brain, chevrons, maximize/minimize) is applied consistently across all expand/collapse controls and the scroll button with matching sizes and colors, so behavior is unchanged. Since every icon here is paired with a textual label (e.g., “Expand”, “Collapse”, “Thinking”), you could optionally addaria-hidden="true"to these<Icon />usages to avoid redundant announcements for screen readers.Also applies to: 416-436, 555-567, 690-701, 824-835, 1059-1073, 1111-1125, 1171-1177, 993-1006
frontend/src/features/tasks/components/ClarificationForm.tsx (1)
8-11: Clarification submit Button migration keeps behavior; consider clearer submitting feedbackSwitching to the local
Buttonwithvariant="secondary",size="lg", anddisabled={isSubmitting}preserves the previous “block double-submit” behavior. You might optionally enhance UX by showing a spinner or changing the label (e.g., to “Submitting…”) whileisSubmittingis true so users get clearer feedback that their answers are being sent.Also applies to: 312-322
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
frontend/src/components/ui/button.tsx(1 hunks)frontend/src/features/layout/TopNavigation.tsx(2 hunks)frontend/src/features/layout/components/DesktopNavLinks.tsx(2 hunks)frontend/src/features/tasks/components/ChatInput.tsx(1 hunks)frontend/src/features/tasks/components/ClarificationForm.tsx(2 hunks)frontend/src/features/tasks/components/FinalPromptMessage.tsx(3 hunks)frontend/src/features/tasks/components/RepositorySelector.tsx(3 hunks)frontend/src/features/tasks/components/TaskSidebar.tsx(4 hunks)frontend/src/features/tasks/components/TeamSelector.tsx(1 hunks)frontend/src/features/tasks/components/ThinkingComponent.tsx(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/features/tasks/components/RepositorySelector.tsx
- frontend/src/features/tasks/components/TaskSidebar.tsx
- frontend/src/features/tasks/components/TeamSelector.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/features/tasks/components/FinalPromptMessage.tsx (1)
frontend/src/components/ui/button.tsx (1)
Button(57-57)
frontend/src/components/ui/button.tsx (1)
frontend/src/lib/utils.ts (1)
cn(8-10)
🔇 Additional comments (4)
frontend/src/features/tasks/components/ChatInput.tsx (1)
55-69: Chat input padding and minRows change look safe and align with compact ChatGPT-style UIHorizontal/vertical padding and
minRows={1}give a tighter, single-row baseline without touching key handling, disabled logic, or maxRows, so behavior remains consistent while matching the new design.frontend/src/features/layout/TopNavigation.tsx (1)
41-72: Top nav density & centering improvements are consistent and remain accessibleReduced padding plus
min-h-[44px]and centered nav (items-center) should keep hit targets comfortable while better matching the modernized, compact header styling.frontend/src/features/layout/components/DesktopNavLinks.tsx (1)
71-101: Desktop nav spacing changes align with the new header without affecting the indicatorCentering items on the container and using symmetric
py-1withleading-nonekeeps labels readable while preserving the active underline indicator’s positioning logic.frontend/src/features/tasks/components/FinalPromptMessage.tsx (1)
8-11: Final prompt header and actions migrate cleanly to the new Button + lucide systemThe move to
Star,Copy/Check, andPlusicons plusButtonvariants (ghostfor copy,secondaryfor create-task) preserves existing copy and navigation behavior while matching the shared design language.Also applies to: 91-133
| const buttonVariants = cva( | ||
| 'inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-lg text-sm ring-offset-base transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50', | ||
| { | ||
| variants: { | ||
| variant: { | ||
| default: 'bg-primary text-primary-contrast hover:bg-primary/90 font-medium', | ||
| secondary: | ||
| 'bg-transparent border border-border text-text-primary hover:bg-surface font-medium', | ||
| outline: 'bg-transparent border border-border text-text-primary hover:bg-surface', | ||
| ghost: 'hover:bg-surface text-text-primary', | ||
| link: 'text-primary underline-offset-4 hover:underline', | ||
| }, | ||
| size: { | ||
| default: 'h-10 px-4 py-2', | ||
| sm: 'h-9 rounded-md px-3', | ||
| lg: 'h-11 rounded-md px-8', | ||
| icon: 'h-10 w-10', | ||
| }, | ||
| }, | ||
| defaultVariants: { | ||
| variant: 'default', | ||
| size: 'default', | ||
| }, | ||
| } | ||
| ) | ||
|
|
||
| export interface ButtonProps | ||
| extends React.ButtonHTMLAttributes<HTMLButtonElement>, | ||
| VariantProps<typeof buttonVariants> { | ||
| asChild?: boolean | ||
| } | ||
|
|
||
| const Button = React.forwardRef<HTMLButtonElement, ButtonProps>( | ||
| ({ className, variant, size, asChild = false, ...props }, ref) => { | ||
| const Comp = asChild ? Slot : 'button' | ||
| return ( | ||
| <Comp | ||
| className={cn(buttonVariants({ variant, size, className }))} | ||
| ref={ref} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
| ) | ||
| Button.displayName = 'Button' |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Default Button type may cause unintended form submissions
Right now Button renders a plain <button> (when asChild is false) without specifying type, so inside a <form> it will default to type="submit". That’s different from AntD’s default and can accidentally submit forms for buttons intended to just trigger an action (e.g., copy, toggle, navigation).
I’d strongly recommend defaulting to type="button" while still allowing callers to override it:
-const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
- ({ className, variant, size, asChild = false, ...props }, ref) => {
+const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
+ ({ className, variant, size, asChild = false, type, ...props }, ref) => {
const Comp = asChild ? Slot : 'button'
return (
<Comp
- className={cn(buttonVariants({ variant, size, className }))}
- ref={ref}
- {...props}
+ className={cn(buttonVariants({ variant, size, className }))}
+ ref={ref}
+ // Default to "button" to avoid accidental form submissions,
+ // while still allowing an explicit type to be passed.
+ type={type ?? 'button'}
+ {...props}
/>
)
}
)This keeps the new design system intact but avoids subtle regressions when buttons are placed inside forms.
I need to verify the Button component's actual implementation and usage patterns. Let me inspect the codebase.
---
🏁 Script executed:
```shell
#!/bin/bash
# View the complete button.tsx file to see the actual implementation
cat frontend/src/components/ui/button.tsx
Length of output: 1815
🏁 Script executed:
#!/bin/bash
# Search for Button component usage, especially in form contexts
rg -nC5 "Button" frontend/src --type=tsx --type=jsx | head -100Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check if there are any forms in the codebase that use Button
rg -nP "<form" frontend/src --type=tsx --type=jsx -A10 | head -80Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for Button component imports and usage
rg -n "import.*Button" frontend/src --type=ts -A2
Length of output: 8752
🏁 Script executed:
#!/bin/bash
# Search for <form> tags in the codebase
rg -n "<form" frontend/src --type=ts -A15 -B2Length of output: 1931
🏁 Script executed:
#!/bin/bash
# Find all .tsx and .jsx files to understand structure
fd -e tsx -e jsx frontend/src | head -20Length of output: 353
---
🏁 Script executed:
```shell
#!/bin/bash
# Check ClarificationForm.tsx to see Button usage in form context
cat frontend/src/features/tasks/components/ClarificationForm.tsx
Length of output: 12032
🏁 Script executed:
#!/bin/bash
# Search more broadly for <Button inside <form patterns
rg -n "Button" frontend/src --type=ts -B10 | grep -A10 -B10 "form"Length of output: 3993
🏁 Script executed:
#!/bin/bash
# Check TaskSidebar for Button usage
cat frontend/src/features/tasks/components/TaskSidebar.tsx | head -80Length of output: 2321
---
🏁 Script executed:
```shell
#!/bin/bash
# Explicitly search for new Button component inside form elements
# Look for patterns where @/components/ui/button is imported and used near <form
rg -n "import.*Button.*@/components/ui/button" frontend/src --type=ts -A200 | grep -B5 -A5 "<form\|<Form"
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Double-check: search all tsx files that import new Button
# and contain <form tag
for file in $(rg -l "@/components/ui/button" frontend/src --type=ts); do
if grep -q "<form" "$file"; then
echo "=== $file ==="
grep -n "<form\|<Form\|Button" "$file" | head -30
fi
doneLength of output: 0
Default Button type should be "button" to prevent accidental form submissions
HTML <button> elements default to type="submit" inside forms. Currently, the Button component doesn't set an explicit type, so if used within a form without passing type explicitly, it will unexpectedly submit the form instead of just triggering an action.
While this isn't currently manifesting in the codebase, it's a common footgun that will likely occur as the new Button component sees wider adoption. I recommend adding a sensible default:
const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
- ({ className, variant, size, asChild = false, ...props }, ref) => {
+ ({ className, variant, size, asChild = false, type = 'button', ...props }, ref) => {
const Comp = asChild ? Slot : 'button'
return (
<Comp
className={cn(buttonVariants({ variant, size, className }))}
ref={ref}
+ type={type}
{...props}
/>
)
}
)This ensures safe defaults while allowing explicit overrides when type="submit" is actually needed.
🤖 Prompt for AI Agents
In frontend/src/components/ui/button.tsx around lines 11 to 55, the Button
component doesn't set an explicit type so HTML buttons default to "submit"
inside forms; update the component to ensure the rendered element receives
type="button" unless a type prop is explicitly provided (i.e., derive type =
props.type ?? 'button' and pass that into <Comp ... />) so consumers can still
override with type="submit" when needed.
本次修复包含ChatGPT风格的文本样式统一和布局优化:
1. 统一左侧文本样式
- Logo、新建任务、搜索框、任务历史标题统一为text-sm font-medium text-text-primary
- 颜色从浅灰色改为更偏黑色,与ChatGPT保持一致
- 新建任务按钮内边距从px-3改为px-2,修复偏右问题
2. 优化搜索框
- 去掉placeholder的省略号("搜索任务..."改为"搜索任务")
- 中英文翻译文件同步更新
- placeholder颜色改为text-text-primary,字体为font-medium
3. 增加输入框宽度
- 右侧输入框和消息区域从max-w-3xl(768px)改为max-w-4xl(896px)
- 参考ChatGPT的宽度设计,提供更好的输入体验
完全参考ChatGPT的左侧边栏样式: 1. 移除所有加粗样式 - Logo文字: 去掉font-medium - 新建任务按钮: 去掉font-medium - 搜索框: 去掉font-medium和placeholder:font-medium - 任务历史标题: 去掉font-medium 2. 统一悬停背景色 - 新建任务按钮: hover:bg-muted - 任务列表项: hover:bg-muted - 确保所有可交互元素的悬停效果一致 3. 移除文本悬停变色 - 任务标题和时间不再有hover:text-text-primary - 只保留背景悬停效果,符合ChatGPT交互方式 4. 统一padding和对齐 - 新建任务按钮: px-2 - 任务列表项: px-2 py-2 - 所有元素左边距完全一致
本次修复完善了所有左侧边栏的样式细节: 1. 间距优化 - Logo与新建任务间距增加(pb-1 → pb-3) - 新建任务与搜索框间距统一(mb-2/mb-3 → mb-1) - 与任务列表项间距保持一致 2. 文本对齐修复 - 新建任务icon间距调整(mr-2 → mr-1.5) - 修复文本偏右问题,与其他元素完美对齐 3. 悬停背景色统一 - 从浅色bg-muted改为深色bg-surface/80 - 新建任务按钮: hover:bg-surface/80 - 搜索框: group-hover:bg-surface/80 - 任务列表项: hover:bg-surface/80 - 完全符合ChatGPT的深灰色悬停效果 4. 文本颜色统一 - 搜索框placeholder: text-text-muted → text-text-primary - 任务标题: text-text-muted → text-text-primary - 所有主要文本统一为深色 5. 字号统一 - 任务标题: text-xs → text-sm - 与新建任务、搜索框保持一致的字号
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/tailwind.config.js (1)
5-56: Fix module system mismatch: replace ES import with CommonJS requireYour Tailwind config uses an ES module
importin a CommonJS context. Sincepackage.jsondoesn't have"type": "module", Node treatstailwind.config.jsas CommonJS, making theimportstatement invalid syntax. This will cause a SyntaxError and break the Tailwind build.Change the import to CommonJS:
-import tailwindcssAnimate from 'tailwindcss-animate'; +const tailwindcssAnimate = require('tailwindcss-animate');Alternatively, you could rename the file to
tailwind.config.mjsor add"type": "module"topackage.jsonand useexport default, but usingrequirekeeps the file aligned with your existing CommonJS setup.The new theme extensions (colors, radius, accordion animations) are all correct and look good.
frontend/src/features/tasks/components/TeamSelector.tsx (1)
124-161: Fixtext-sxtypo totext-xsin placeholder span; width changes reasonableThe placeholder span uses
text-sx, which is not a default Tailwind utility. The valid Tailwind text-size class istext-xs(as already used in the Tag component on line 145). This needs to be corrected:- placeholder={ - <span className="text-sx truncate h-2"> + placeholder={ + <span className="text-xs truncate h-2"> {isLoading ? 'Loading...' : t('teams.select_team')} </span> }The other changes are sound: switching to
items-centerfor better vertical alignment, the conditionalanimate-pulseon the icon for loading state, and the fixed width (isMobile ? 150 : 200) matching the modernized selector style. The fixed width will cause more truncation for long team names in certain locales, but that's acceptable with the truncate and title attributes in place.
♻️ Duplicate comments (1)
frontend/src/app/globals.css (1)
443-457: Partial restoration of removed Ant Design styles—verify full coverage.This restores the
repository-selectorarrow visibility control that was removed earlier, which is good. However, the previous review's verification confirmed that multiple removed CSS classes are still actively referenced in components:
custom-messageandcustom-message-notice-content(removed per AI summary but not yet restored)- Additional selectors in
BranchSelector,TeamSelector, andAntdProviderVerify that all removed custom Ant Design styles have been either:
- Restored in globals.css, OR
- Removed from the components that reference them
Run this verification to confirm no broken references remain:
#!/bin/bash # Description: Check for unreferenced Ant Design custom classes echo "=== Searching for removed style class references in components ===" rg -n 'custom-message|custom-message-notice-content' frontend/src \ -g '*.tsx' -g '*.ts' -g '*.jsx' -g '*.js' -C 2 echo -e "\n=== Checking if custom-message styles exist in globals.css ===" rg -n 'custom-message' frontend/src/app/globals.cssIf components still reference these classes, either restore the CSS definitions or remove the className props from the components to prevent visual regressions.
🧹 Nitpick comments (3)
frontend/src/features/tasks/components/TaskSidebar.tsx (2)
49-70: Consider extracting useDebounce to a shared utility.The useDebounce hook is functionally correct, but since debouncing is a common pattern, extracting it to a shared hooks utility (e.g.,
@/hooks/useDebounce.ts) would improve code reusability and maintainability across the codebase.
194-194: Consider using the cn() utility for complex className strings.The className string is quite long and complex. The PR introduces a
cn()helper utility (from tailwind-merge) for managing className combinations. Using it here would improve readability and maintainability.Example refactor:
import { cn } from '@/lib/utils'; // Then use: className={cn( "w-full pl-8 pr-8 py-1.5", "bg-transparent group-hover:bg-hover", "border border-transparent group-hover:border-border", "rounded text-sm text-text-primary placeholder:text-text-primary", "focus:outline-none focus:ring-2 focus:ring-primary/40 focus:border-transparent focus:bg-hover", "cursor-text" )}frontend/src/features/tasks/components/ChatArea.tsx (1)
8-21: Shared Button + icon send behavior looks correct; add an accessible label
- Importing the shared Button and using
variant="ghost"withsize="icon"plus aclassName="h-10 w-10 rounded-full hover:bg-primary/10"is in line with the new button API.- The disabled/loading conditions are preserved, and the
LoadingDotsvs icon swap cleanly reflects pending/running states.Since this is now an icon‑only control, consider adding an accessible label (or visually hidden text) so screen readers know what the button does, for example:
- <Button + <Button variant="ghost" size="icon" onClick={handleSendMessage} disabled={ isLoading || selectedTaskDetail?.status === 'PENDING' || selectedTaskDetail?.status === 'RUNNING' } className="h-10 w-10 rounded-full hover:bg-primary/10" + aria-label="Send message" > {isLoading || selectedTaskDetail?.status === 'PENDING' || selectedTaskDetail?.status === 'RUNNING' ? ( <LoadingDots /> ) : ( <Send className="h-5 w-5 text-text-muted" /> )} </Button>This keeps the UI the same while improving accessibility.
Also applies to: 421-439
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
frontend/src/app/globals.css(2 hunks)frontend/src/features/layout/GithubStarButton.tsx(1 hunks)frontend/src/features/layout/UserMenu.tsx(1 hunks)frontend/src/features/layout/components/DesktopNavLinks.tsx(2 hunks)frontend/src/features/layout/components/MobileNavTabs.tsx(2 hunks)frontend/src/features/tasks/components/ChatArea.tsx(6 hunks)frontend/src/features/tasks/components/RepositorySelector.tsx(3 hunks)frontend/src/features/tasks/components/TaskListSection.tsx(5 hunks)frontend/src/features/tasks/components/TaskSidebar.tsx(5 hunks)frontend/src/features/tasks/components/TeamSelector.tsx(2 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)frontend/tailwind.config.js(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/features/layout/UserMenu.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/features/tasks/components/TaskListSection.tsx
- frontend/src/features/tasks/components/RepositorySelector.tsx
- frontend/src/features/layout/components/DesktopNavLinks.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/features/tasks/components/ChatArea.tsx (2)
frontend/src/features/tasks/components/LoadingDots.tsx (1)
LoadingDots(7-24)frontend/src/components/ui/button.tsx (1)
Button(57-57)
frontend/src/features/tasks/components/TaskSidebar.tsx (2)
frontend/src/components/ui/button.tsx (1)
Button(57-57)frontend/src/config/paths.ts (1)
paths(5-44)
🔇 Additional comments (12)
frontend/src/i18n/locales/zh-CN/common.json (1)
232-232: Text refinement is consistent across locales.The ellipsis removal from
tasks.search_placeholderis mirrored identically in the English locale, maintaining consistency. This aligns well with the ChatGPT-inspired design system modernization goals.frontend/src/i18n/locales/en/common.json (1)
232-232: Text refinement is consistent across locales.The ellipsis removal from
tasks.search_placeholderis mirrored identically in the Chinese locale, maintaining consistency. This aligns well with the ChatGPT-inspired design system modernization goals.frontend/src/app/globals.css (2)
14-31: Color scheme updates align with PR objectives.The light theme color palette correctly reflects the ChatGPT-inspired design system:
- Primary color:
16 163 127(#10a37f, teal) ✓- Surface, border, and text hierarchy updated appropriately
- New
--color-bg-hovertoken (line 16) added for interactive states
36-54: Dark theme palette correctly implements ChatGPT-style colors.Dark mode variables match PR specifications:
- Base:
33 33 33(#212121) ✓- Surface:
52 53 65(#343541) ✓- Muted:
64 65 79(#40414f) ✓- Primary remains consistent across themes ✓
frontend/src/features/layout/components/MobileNavTabs.tsx (2)
42-45: LGTM: Formatting improvement.The multi-line formatting of the div element improves readability with no functional changes.
57-57: LGTM: Typography refinement aligns with design modernization.The lighter font weight (
font-normalvsfont-medium) for non-active tabs improves visual hierarchy and aligns with the ChatGPT-inspired design system introduced in this PR.frontend/src/features/tasks/components/TaskSidebar.tsx (4)
172-183: LGTM! Clean implementation of the new Button component.The "New Task" button properly uses the shadcn/ui Button component with appropriate props (variant, size, className) and includes the lucide-react Plus icon with consistent sizing.
266-279: LGTM! Proper migration to the new Button component.The Settings button correctly uses the shadcn/ui Button component with appropriate props and maintains the data-tour attribute for user onboarding functionality.
286-291: LGTM! Clean sidebar container structure.The desktop sidebar container is properly structured with appropriate classes and the data-tour attribute is correctly positioned for user onboarding functionality.
8-12: Imports verified as correct.All four icons (Search, Plus, Settings, X) are available in lucide-react v0.554.0, which matches the version specified in package.json. The imports are valid and safe to use.
frontend/src/features/layout/GithubStarButton.tsx (1)
88-93: Typography tweak looks goodUsing
text-sm font-normalsoftens the button styling without affecting behavior; consistent with the lighter design system direction.frontend/src/features/tasks/components/ChatArea.tsx (1)
363-398: Chat area width and card styling changes are consistent with the new design system
- Bumping the inner containers to
max-w-4xlgives the chat a more spacious feel without affecting scroll or floating input logic.- The updated card styling (
rounded-2xl border border-border bg-base shadow-lg) matches the ChatGPT-style surfaces and keeps the structure unchanged.No functional issues spotted here.
| <button | ||
| onClick={handleClearSearch} | ||
| className="absolute right-2 top-1/2 transform -translate-y-1/2" | ||
| > | ||
| <XMarkIcon className="h-3.5 w-3.5 text-text-muted hover:text-text-primary" /> | ||
| <X className="h-3.5 w-3.5 text-text-muted hover:text-text-primary" /> | ||
| </button> |
There was a problem hiding this comment.
Add accessible label to the clear search button.
The clear search button lacks an accessible name for screen readers, which violates WCAG 2.1 accessibility guidelines. Users relying on assistive technology won't understand the button's purpose.
Apply this diff to add an accessible label:
{localSearchTerm && (
<button
onClick={handleClearSearch}
className="absolute right-2 top-1/2 transform -translate-y-1/2"
+ aria-label={t('tasks.clear_search') || 'Clear search'}
>
<X className="h-3.5 w-3.5 text-text-muted hover:text-text-primary" />
</button>
)}📝 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.
| <button | |
| onClick={handleClearSearch} | |
| className="absolute right-2 top-1/2 transform -translate-y-1/2" | |
| > | |
| <XMarkIcon className="h-3.5 w-3.5 text-text-muted hover:text-text-primary" /> | |
| <X className="h-3.5 w-3.5 text-text-muted hover:text-text-primary" /> | |
| </button> | |
| <button | |
| onClick={handleClearSearch} | |
| className="absolute right-2 top-1/2 transform -translate-y-1/2" | |
| aria-label={t('tasks.clear_search') || 'Clear search'} | |
| > | |
| <X className="h-3.5 w-3.5 text-text-muted hover:text-text-primary" /> | |
| </button> |
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/TaskSidebar.tsx around lines 197 to
202, the clear search button has no accessible name; add an accessible label so
screen readers understand its purpose. Modify the button element to include an
appropriate accessible name (for example add aria-label="Clear search" or
aria-label={`Clear search`} and ensure the element remains a button role/type if
needed). If you prefer visible text for accessibility, include a visually-hidden
span with the label text instead of or in addition to aria-label. Ensure the
label is concise and matches the button action.
| {totalUnreadCount > 0 && ( | ||
| <div className="px-3 mb-2"> | ||
| <button | ||
| onClick={handleMarkAllAsViewed} | ||
| className="w-full text-xs text-text-muted hover:text-text-primary py-1 px-2 rounded hover:bg-muted transition-colors text-center" | ||
| className="w-full text-xs text-text-primary hover:text-text-primary py-1 px-2 rounded hover:bg-hover transition-colors text-center" | ||
| > | ||
| {t('tasks.mark_all_read')} ({totalUnreadCount}) | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Update to use the new Button component for consistency.
The "Mark All As Read" button is still implemented as a plain HTML button element, which is inconsistent with the other buttons in this component (New Task, Settings) that now use the shadcn/ui Button component. This undermines the UI modernization effort.
Apply this diff to align with the new design system:
{totalUnreadCount > 0 && (
<div className="px-3 mb-2">
- <button
+ <Button
+ variant="ghost"
+ size="sm"
onClick={handleMarkAllAsViewed}
- className="w-full text-xs text-text-primary hover:text-text-primary py-1 px-2 rounded hover:bg-hover transition-colors text-center"
+ className="w-full text-xs"
>
{t('tasks.mark_all_read')} ({totalUnreadCount})
- </button>
+ </Button>
</div>
)}📝 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.
| {totalUnreadCount > 0 && ( | |
| <div className="px-3 mb-2"> | |
| <button | |
| onClick={handleMarkAllAsViewed} | |
| className="w-full text-xs text-text-muted hover:text-text-primary py-1 px-2 rounded hover:bg-muted transition-colors text-center" | |
| className="w-full text-xs text-text-primary hover:text-text-primary py-1 px-2 rounded hover:bg-hover transition-colors text-center" | |
| > | |
| {t('tasks.mark_all_read')} ({totalUnreadCount}) | |
| </button> | |
| </div> | |
| {totalUnreadCount > 0 && ( | |
| <div className="px-3 mb-2"> | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| onClick={handleMarkAllAsViewed} | |
| className="w-full text-xs" | |
| > | |
| {t('tasks.mark_all_read')} ({totalUnreadCount}) | |
| </Button> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/TaskSidebar.tsx around lines 208 to
216, the "Mark All As Read" control is a raw HTML <button> while other buttons
use the shadcn/ui Button component; replace the HTML button with the Button
component (import it at the top if not present), pass the
onClick={handleMarkAllAsViewed}, render the label {t('tasks.mark_all_read')}
({totalUnreadCount}) inside the Button, and apply equivalent styling/props (full
width layout, small/text-xs appearance or appropriate variant/size, rounded,
hover behavior) to match the existing Button usage so the visual and
accessibility behavior remains the same; keep the conditional wrapper
{totalUnreadCount > 0 && (...)} unchanged.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/features/tasks/components/TeamShareHandler.tsx (1)
64-67: Fix: catch block missing error parameter.The
catchblock on line 64 has no parameter, so theerrorreference on line 65 resolves to the state variable (line 40) rather than the actual caught exception. This means the real fetch error is never logged, making debugging difficult.Apply this diff to capture and log the actual error:
- } catch { - console.error('Failed to fetch team share info:', error); + } catch (err) { + console.error('Failed to fetch team share info:', err); message.error(t('teams.share.fetch_info_failed')); cleanupUrlParams();
🧹 Nitpick comments (4)
frontend/src/app/(tasks)/code/page.tsx (1)
83-104: Align thinking extraction with MessagesArea to avoid missing timelinesRight now
thinkingDataonly looks for a top‑levelresult.thinkingarray on the latest subtask, so you’ll miss cases where:
thinkingis nested underresult.value, orvalueis a JSON string that containsthinking,which you already handle in
MessagesArea(viaResultWithThinkingparsing). That will make the Workbench Execution Timeline disagree with the inline Thinking panel for some tasks.Consider:
- Reusing a shared helper (e.g.,
extractThinkingFromResult(result)) that encapsulates the same parsing logic used inMessagesArea, and- Optionally scanning subtasks from the end to the first one that actually has thinking, instead of assuming the last subtask always does.
This keeps CodePage / Workbench behavior consistent with the chat view.
Also applies to: 192-193
frontend/src/features/tasks/components/ThinkingComponent.tsx (1)
87-99: Minor robustness improvements for tool stats and duration helpersThe new
TOOL_ICONS,extractToolCalls, andcalculateDurationhelpers are a nice addition, but a couple of small tweaks would make them more robust:
calculateDurationcurrently:
- Only considers steps where
details.message?.idis truthy; steps that havetimestamp/created_atbut nomessage.idare ignored.- Uses
if (!startTime || !endTime) return null;, which technically treats0orNaNas “missing”.Consider:
- for (const step of thinkingSteps) { - if (step.details?.message?.id) { - const timestamp = step.details.timestamp || step.details.created_at; + for (const step of thinkingSteps) { + const timestamp = step.details?.timestamp || step.details?.created_at; if (timestamp) { startTime = new Date(timestamp).getTime(); break; } } ... - if (!startTime || !endTime) return null; + if (startTime == null || endTime == null || !Number.isFinite(startTime) || !Number.isFinite(endTime)) { + return null; + }This keeps behavior the same for current data but is more tolerant of slightly different shapes and avoids falsy checks on numeric timestamps.
Also applies to: 217-282
frontend/src/features/tasks/components/MessagesArea.tsx (1)
352-401: Optional: make downloaded markdown filename more descriptiveThe download tool always saves as
message.md. If users often download multiple results, a slightly richer name (e.g., including task or subtask id, or a timestamp) would reduce confusion:- a.download = 'message.md'; + a.download = `task-${selectedTaskDetail?.id ?? 'message'}.md`;(or similar, depending on what identifiers you want exposed).
frontend/src/features/tasks/components/Workbench.tsx (1)
337-361: Consider refactoring complex nested conditionals.The tool name extraction logic (lines 342-361) has deeply nested conditionals that check multiple object shapes. While functionally correct, this could be simplified for maintainability.
Consider extracting to a helper function:
const extractToolName = (details?: { type?: string; name?: string; message?: unknown; [key: string]: unknown; }): string | null => { // Direct tool_use type if (details?.type === 'tool_use' && typeof details?.name === 'string') { return details.name; } // Nested in message.content if (details?.message && typeof details.message === 'object' && details.message !== null) { const message = details.message as { content?: Array<{ type: string; name?: string }> }; if (Array.isArray(message.content)) { const toolUse = message.content.find(c => c.type === 'tool_use' && c.name); if (toolUse?.name) return toolUse.name; } } return null; }; // Then in buildTimeline: thinkingSteps.forEach(step => { const toolName = extractToolName(step.details); // ... rest of logic });This separates concerns and makes the extraction logic more testable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
frontend/src/app/(tasks)/code/page.tsx(2 hunks)frontend/src/components/ui/input.tsx(1 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(8 hunks)frontend/src/features/tasks/components/TeamShareHandler.tsx(2 hunks)frontend/src/features/tasks/components/ThinkingComponent.tsx(37 hunks)frontend/src/features/tasks/components/Workbench.tsx(6 hunks)frontend/src/i18n/locales/en/tasks.json(1 hunks)frontend/src/i18n/locales/zh-CN/tasks.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/ui/input.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/features/tasks/components/MessagesArea.tsx (2)
frontend/src/components/ui/button.tsx (1)
Button(57-57)frontend/src/features/tasks/components/ThinkingComponent.tsx (1)
ThinkingComponent(100-1327)
frontend/src/features/tasks/components/ThinkingComponent.tsx (1)
shared/models/task.py (1)
ThinkingStep(45-55)
🔇 Additional comments (9)
frontend/src/features/tasks/components/TeamShareHandler.tsx (1)
45-49: Good practice: memoizing the cleanup function.The
useCallbackmemoization preventscleanupUrlParamsfrom being recreated on every render, which is important since it's included in the effect's dependency array (line 74). Without memoization, the effect would re-run unnecessarily on every render.frontend/src/i18n/locales/zh-CN/tasks.json (1)
49-66: New execution timeline & thinking translations look consistentThe added
execution_timelineandthinking.*keys (includingtool_actions) mirror the English structure and read naturally in Chinese; no issues from an i18n or structure standpoint.frontend/src/i18n/locales/en/tasks.json (1)
53-70: Execution timeline & thinking keys are well-structuredThe new
execution_timelineandthinking.*/tool_actionsentries are clearly named, align with their usage in the UI, and match the Chinese locale structure. No issues from an i18n or data-shape perspective.frontend/src/features/tasks/components/MessagesArea.tsx (1)
21-37: Per-subtask status + thinking wiring looks correctThe Message shape updates (
subtaskStatus) and the way you passmsg.thinkingandmsg.subtaskStatusintoThinkingComponentintegrate cleanly with the new thinking UI; status is now per-subtask rather than global, which matches how messages are generated.The Button + icon refactors for
CopyButton,BubbleTools, and the message bubbles themselves are also consistent with the new design system and don’t introduce any obvious behavioral changes.Also applies to: 257-267, 678-709
frontend/src/features/tasks/components/Workbench.tsx (5)
30-41: LGTM: Clean tool icon mapping.The emoji-based icon mapping is simple, dependency-free, and provides good visual differentiation for different tool types.
110-118: LGTM: Well-typed public API extension.The
thinkingprop is properly typed as optional, and the use ofunknownfor additional properties indetailsis a good defensive TypeScript practice.
407-414: LGTM: Clean derived state and UX-friendly auto-collapse.The timeline auto-collapse on task completion is a nice UX touch that keeps the interface clean. The effect dependencies are correctly specified.
417-427: LGTM: Concise summary generation.The summary format (
icon×count) provides a quick visual overview of tool usage in the collapsed state.
626-699: LGTM: Well-structured timeline UI with good UX patterns.The timeline implementation follows the existing component patterns well:
- Collapsible design reduces visual clutter
- Timeline connector lines provide clear visual flow
- Translation integration with graceful fallback to raw tool names
- Responsive to task completion status
The translation fallback pattern (lines 655-658) is a common approach, though it assumes the translation system returns the key itself when missing—which is typical behavior for most i18n libraries.
| url.searchParams.delete('teamShare'); | ||
| router.replace(url.pathname + url.search); | ||
| }; | ||
| }, [searchParams, message, t, cleanupUrlParams, error]); |
There was a problem hiding this comment.
Remove error from the dependency array.
The error state variable is not used within the effect logic (the reference on line 65 should be the catch parameter, not the state variable). Including it in the dependency array causes the effect to re-run whenever the error state changes elsewhere in the component (lines 95, 115, 124), which is unintended behavior.
After fixing the catch block, apply this diff:
- }, [searchParams, message, t, cleanupUrlParams, error]);
+ }, [searchParams, message, t, cleanupUrlParams]);📝 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.
| }, [searchParams, message, t, cleanupUrlParams, error]); | |
| }, [searchParams, message, t, cleanupUrlParams]); |
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/TeamShareHandler.tsx around line 74,
the effect's dependency array incorrectly includes the component state variable
error and the catch block incorrectly references that state instead of the catch
parameter; change the catch parameter name (e.g., err) and update the catch body
to use that parameter, then remove error from the useEffect dependency array so
the effect does not re-run when the component error state changes.
| // Initialize isOpen based on taskStatus | ||
| const shouldBeCollapsed = | ||
| taskStatus === 'COMPLETED' || taskStatus === 'FAILED' || taskStatus === 'CANCELLED'; | ||
| const [isOpen, setIsOpen] = useState(!shouldBeCollapsed); | ||
|
|
There was a problem hiding this comment.
CANCELLED status currently displays as “Execution Completed”
You’ve added CANCELLED into the terminal state checks (shouldBeCollapsed, isThinkingCompleted, and auto‑collapse), but the expanded header still uses a single execution_completed label whenever isThinkingCompleted is true. For a CANCELLED subtask this will render:
- Expanded: “Execution Completed”
- Collapsed: correctly localized via
thinking.execution_cancelled
which is inconsistent and misleading.
Recommend updating the header text to branch on taskStatus explicitly, e.g.:
- {!isOpen && isThinkingCompleted
- ? formatCollapsedTitle()
- : isThinkingCompleted
- ? tTasks('thinking.execution_completed')
- : tChat('messages.thinking') || 'Thinking'}
+ {!isOpen && isThinkingCompleted
+ ? formatCollapsedTitle()
+ : taskStatus === 'COMPLETED'
+ ? tTasks('thinking.execution_completed')
+ : taskStatus === 'FAILED'
+ ? tTasks('thinking.execution_failed')
+ : taskStatus === 'CANCELLED'
+ ? tTasks('thinking.execution_cancelled')
+ : tChat('messages.thinking') || 'Thinking'}so all terminal states use the correct, localized label in both expanded and collapsed variants.
Also applies to: 140-153, 318-320, 335-340
🤖 Prompt for AI Agents
frontend/src/features/tasks/components/ThinkingComponent.tsx lines 105-109,
140-153, 318-320, 335-340: the header text currently uses a single
execution_completed label whenever isThinkingCompleted is true, which makes
CANCELLED subtasks display “Execution Completed” when expanded; change the
header logic to branch on taskStatus (COMPLETED, FAILED, CANCELLED) and select
the appropriate localization key (e.g. thinking.execution_completed,
thinking.execution_failed, thinking.execution_cancelled) for both expanded and
collapsed states (or replace the boolean isThinkingCompleted usage with a small
helper that returns the correct label key/string based on taskStatus), and
update any auto-collapse label sites to use that mapping so expanded and
collapsed headers match.
| // Build execution timeline from thinking data | ||
| interface TimelineStep { | ||
| toolName: string; | ||
| count: number; | ||
| timestamp: string; | ||
| } | ||
|
|
||
| const buildTimeline = ( | ||
| thinkingSteps: Array<{ | ||
| title: string; | ||
| next_action: string; | ||
| details?: { | ||
| timestamp?: string; | ||
| created_at?: string; | ||
| [key: string]: unknown; | ||
| }; | ||
| }> | ||
| ): TimelineStep[] => { | ||
| if (!thinkingSteps || thinkingSteps.length === 0) return []; | ||
|
|
||
| const timeline: TimelineStep[] = []; | ||
| let currentTool: string | null = null; | ||
| let currentCount = 0; | ||
| let currentTimestamp = ''; | ||
|
|
||
| thinkingSteps.forEach(step => { | ||
| let toolName: string | null = null; | ||
| let timestamp = ''; | ||
|
|
||
| // Extract tool name from details | ||
| if (step.details?.type === 'tool_use' && typeof step.details?.name === 'string') { | ||
| toolName = step.details.name; | ||
| } else if ( | ||
| step.details && | ||
| 'message' in step.details && | ||
| typeof step.details.message === 'object' && | ||
| step.details.message !== null | ||
| ) { | ||
| const message = step.details.message as { | ||
| content?: Array<{ type: string; name?: string }>; | ||
| }; | ||
| if (message.content && Array.isArray(message.content)) { | ||
| for (const content of message.content) { | ||
| if (content.type === 'tool_use' && content.name) { | ||
| toolName = content.name; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Extract timestamp | ||
| const details = step.details; | ||
| if (details?.timestamp && typeof details.timestamp === 'string') { | ||
| timestamp = new Date(details.timestamp).toLocaleTimeString('en-US', { | ||
| hour12: false, | ||
| }); | ||
| } else if (details?.created_at && typeof details.created_at === 'string') { | ||
| timestamp = new Date(details.created_at).toLocaleTimeString('en-US', { | ||
| hour12: false, | ||
| }); | ||
| } | ||
|
|
||
| if (toolName) { | ||
| if (toolName === currentTool) { | ||
| // Same tool, increment count | ||
| currentCount++; | ||
| } else { | ||
| // Different tool, save previous and start new | ||
| if (currentTool) { | ||
| timeline.push({ | ||
| toolName: currentTool, | ||
| count: currentCount, | ||
| timestamp: currentTimestamp, | ||
| }); | ||
| } | ||
| currentTool = toolName; | ||
| currentCount = 1; | ||
| currentTimestamp = timestamp || currentTimestamp; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| // Add last group | ||
| if (currentTool) { | ||
| timeline.push({ | ||
| toolName: currentTool, | ||
| count: currentCount, | ||
| timestamp: currentTimestamp, | ||
| }); | ||
| } | ||
|
|
||
| return timeline; | ||
| }; |
There was a problem hiding this comment.
Use i18n locale instead of hardcoded 'en-US'.
Lines 366 and 370 use toLocaleTimeString('en-US', ...) with a hardcoded locale, which is inconsistent with the app's i18n system. This means timestamps will always display in US format regardless of the user's language preference.
Consider accessing the current locale from your i18n context:
+ const { t, i18n } = useTranslation('tasks');
+ // or use i18n.language to get current locale
+
const buildTimeline = (
thinkingSteps: Array<{
...
}>
): TimelineStep[] => {
...
if (details?.timestamp && typeof details.timestamp === 'string') {
- timestamp = new Date(details.timestamp).toLocaleTimeString('en-US', {
+ timestamp = new Date(details.timestamp).toLocaleTimeString(i18n.language, {
hour12: false,
});
} else if (details?.created_at && typeof details.created_at === 'string') {
- timestamp = new Date(details.created_at).toLocaleTimeString('en-US', {
+ timestamp = new Date(details.created_at).toLocaleTimeString(i18n.language, {
hour12: false,
});
}Note: You'll need to pass i18n or the locale into buildTimeline if you move it outside the component scope, or keep it as an inline function with access to the hook.
📝 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.
| // Build execution timeline from thinking data | |
| interface TimelineStep { | |
| toolName: string; | |
| count: number; | |
| timestamp: string; | |
| } | |
| const buildTimeline = ( | |
| thinkingSteps: Array<{ | |
| title: string; | |
| next_action: string; | |
| details?: { | |
| timestamp?: string; | |
| created_at?: string; | |
| [key: string]: unknown; | |
| }; | |
| }> | |
| ): TimelineStep[] => { | |
| if (!thinkingSteps || thinkingSteps.length === 0) return []; | |
| const timeline: TimelineStep[] = []; | |
| let currentTool: string | null = null; | |
| let currentCount = 0; | |
| let currentTimestamp = ''; | |
| thinkingSteps.forEach(step => { | |
| let toolName: string | null = null; | |
| let timestamp = ''; | |
| // Extract tool name from details | |
| if (step.details?.type === 'tool_use' && typeof step.details?.name === 'string') { | |
| toolName = step.details.name; | |
| } else if ( | |
| step.details && | |
| 'message' in step.details && | |
| typeof step.details.message === 'object' && | |
| step.details.message !== null | |
| ) { | |
| const message = step.details.message as { | |
| content?: Array<{ type: string; name?: string }>; | |
| }; | |
| if (message.content && Array.isArray(message.content)) { | |
| for (const content of message.content) { | |
| if (content.type === 'tool_use' && content.name) { | |
| toolName = content.name; | |
| break; | |
| } | |
| } | |
| } | |
| } | |
| // Extract timestamp | |
| const details = step.details; | |
| if (details?.timestamp && typeof details.timestamp === 'string') { | |
| timestamp = new Date(details.timestamp).toLocaleTimeString('en-US', { | |
| hour12: false, | |
| }); | |
| } else if (details?.created_at && typeof details.created_at === 'string') { | |
| timestamp = new Date(details.created_at).toLocaleTimeString('en-US', { | |
| hour12: false, | |
| }); | |
| } | |
| if (toolName) { | |
| if (toolName === currentTool) { | |
| // Same tool, increment count | |
| currentCount++; | |
| } else { | |
| // Different tool, save previous and start new | |
| if (currentTool) { | |
| timeline.push({ | |
| toolName: currentTool, | |
| count: currentCount, | |
| timestamp: currentTimestamp, | |
| }); | |
| } | |
| currentTool = toolName; | |
| currentCount = 1; | |
| currentTimestamp = timestamp || currentTimestamp; | |
| } | |
| } | |
| }); | |
| // Add last group | |
| if (currentTool) { | |
| timeline.push({ | |
| toolName: currentTool, | |
| count: currentCount, | |
| timestamp: currentTimestamp, | |
| }); | |
| } | |
| return timeline; | |
| }; | |
| // Build execution timeline from thinking data | |
| const { i18n } = useTranslation('tasks'); | |
| interface TimelineStep { | |
| toolName: string; | |
| count: number; | |
| timestamp: string; | |
| } | |
| const buildTimeline = ( | |
| thinkingSteps: Array<{ | |
| title: string; | |
| next_action: string; | |
| details?: { | |
| timestamp?: string; | |
| created_at?: string; | |
| [key: string]: unknown; | |
| }; | |
| }> | |
| ): TimelineStep[] => { | |
| if (!thinkingSteps || thinkingSteps.length === 0) return []; | |
| const timeline: TimelineStep[] = []; | |
| let currentTool: string | null = null; | |
| let currentCount = 0; | |
| let currentTimestamp = ''; | |
| thinkingSteps.forEach(step => { | |
| let toolName: string | null = null; | |
| let timestamp = ''; | |
| // Extract tool name from details | |
| if (step.details?.type === 'tool_use' && typeof step.details?.name === 'string') { | |
| toolName = step.details.name; | |
| } else if ( | |
| step.details && | |
| 'message' in step.details && | |
| typeof step.details.message === 'object' && | |
| step.details.message !== null | |
| ) { | |
| const message = step.details.message as { | |
| content?: Array<{ type: string; name?: string }>; | |
| }; | |
| if (message.content && Array.isArray(message.content)) { | |
| for (const content of message.content) { | |
| if (content.type === 'tool_use' && content.name) { | |
| toolName = content.name; | |
| break; | |
| } | |
| } | |
| } | |
| } | |
| // Extract timestamp | |
| const details = step.details; | |
| if (details?.timestamp && typeof details.timestamp === 'string') { | |
| timestamp = new Date(details.timestamp).toLocaleTimeString(i18n.language, { | |
| hour12: false, | |
| }); | |
| } else if (details?.created_at && typeof details.created_at === 'string') { | |
| timestamp = new Date(details.created_at).toLocaleTimeString(i18n.language, { | |
| hour12: false, | |
| }); | |
| } | |
| if (toolName) { | |
| if (toolName === currentTool) { | |
| // Same tool, increment count | |
| currentCount++; | |
| } else { | |
| // Different tool, save previous and start new | |
| if (currentTool) { | |
| timeline.push({ | |
| toolName: currentTool, | |
| count: currentCount, | |
| timestamp: currentTimestamp, | |
| }); | |
| } | |
| currentTool = toolName; | |
| currentCount = 1; | |
| currentTimestamp = timestamp || currentTimestamp; | |
| } | |
| } | |
| }); | |
| // Add last group | |
| if (currentTool) { | |
| timeline.push({ | |
| toolName: currentTool, | |
| count: currentCount, | |
| timestamp: currentTimestamp, | |
| }); | |
| } | |
| return timeline; | |
| }; |
🤖 Prompt for AI Agents
frontend/src/features/tasks/components/Workbench.tsx around lines 312 to 405:
the code uses a hardcoded locale ('en-US') in two toLocaleTimeString calls
(lines ~366 and ~370); change the function to use the app i18n locale instead by
either (A) keeping buildTimeline inside the component so it can call the i18n
hook (e.g., const { i18n } = useTranslation()) and replacing 'en-US' with
i18n.language, or (B) add a locale parameter to buildTimeline (locale: string)
and pass the current i18n.locale when calling it; update both timestamp
formatting calls to use that locale variable and ensure the function
signature/call sites are adjusted accordingly.
Summary
This PR introduces the foundation for UI modernization, transforming Wegent's frontend from a traditional internal system style to a modern AI tool interface inspired by ChatGPT's design language.
Key Changes
1. shadcn/ui Integration
2. ChatGPT-Inspired Color Scheme
3. Core UI Components Created
Button: Multiple variants (default, secondary, ghost, link) with proper focus statesInput: Clean, rounded inputs with ChatGPT-style bordersTextarea: Dual mode support (regular and chat-style with rounded-2xl)Select: Radix UI-based dropdown with smooth animationsDialog: Modern modal with backdrop and animations4. Configuration Updates
5. Cleanup
Design System Principles
This foundation follows ChatGPT's design language:
Next Steps
This is Phase 1 of the UI modernization. Future PRs will:
Testing Checklist
Screenshots
Note: Visual changes are minimal in this PR as it focuses on infrastructure. UI changes will be visible in subsequent PRs when components are applied to pages.
Migration Strategy: This PR maintains backward compatibility. Existing components continue to work while new shadcn/ui components are available for gradual adoption.
Summary by CodeRabbit
New Features
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.