refactor(frontend): Enhance chat input and sidebar styling#705
Merged
feifei325 merged 8 commits intowecode-ai:mainfrom Mar 10, 2026
Merged
refactor(frontend): Enhance chat input and sidebar styling#705feifei325 merged 8 commits intowecode-ai:mainfrom
feifei325 merged 8 commits intowecode-ai:mainfrom
Conversation
…for various chat input components.
Optimize chat area layout spacing and input component structure: - Adjust ChatArea spacing (pb-6 -> pb-10, marginBottom 20vh -> 12vh) - Constrain input container max-width to 820px for better readability - Simplify QuickAccessCards implementation - Refactor ChatInputCard layout structure - Update ChatInputControls button layout - Polish SendButton and TeamSelectorButton styles - Improve UnifiedRepositorySelector component Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ith new theme variables
…d improved hover effects
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTheme tokens updated to a purple Wegent palette; added Agent and Model SVG icon components; introduced a TeamSelectorButton and propagated teams into chat input controls; QuickAccessCards refactored to a horizontal scroller; numerous ActionButton/SendButton styling and layout adjustments; i18n keys added (EN/ZH). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TeamSelectorButton
participant Popover
participant ChatInputControls
participant Router
User->>TeamSelectorButton: Click agent button
TeamSelectorButton->>Popover: open()
Popover->>User: show search & team list
User->>Popover: select team item
Popover->>TeamSelectorButton: setSelectedTeam(team)
TeamSelectorButton->>ChatInputControls: update selectedTeam
TeamSelectorButton->>Popover: close()
User->>ChatInputControls: see selected team applied
User->>Router: (optional) click settings link
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
…ng state - Fix ESLint no-unused-vars errors by prefixing unused variables with underscore: - TaskSidebar: totalUnreadCount, handleMarkAllAsViewed - QuickAccessCards: hideSelected - Fix QuickAccessCards alignment in loading state to match ChatInputCard - Add w-full and mx-auto to loading skeleton container Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/src/features/tasks/components/selector/ModelSelector.tsx (1)
248-257:⚠️ Potential issue | 🟠 MajorIncrease the selector trigger to a 44px mobile touch target.
Line 249 sets
h-9, so this button is only 36px tall on mobile. That misses the repo’s minimum touch size and makes the model picker harder to hit on touch devices. Please use a mobile-firsth-11 min-w-[44px]baseline and keep the smaller height behind anmd:override if you still want the denser desktop layout.♿ Suggested class update
- 'flex items-center gap-1 min-w-0 rounded-[24px] pl-2.5 pr-3 py-2.5 h-9', + 'flex items-center gap-1 rounded-[24px] pl-2.5 pr-3 h-11 min-w-[44px] md:h-9 md:min-w-0',As per coding guidelines, "All interactive touch elements on mobile must be at least 44px × 44px: use
h-11 min-w-[44px]for buttons".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/selector/ModelSelector.tsx` around lines 248 - 257, The selector's touch target is too small: in ModelSelector.tsx update the className on the trigger (the JSX that currently includes 'h-9') to use a mobile-first size of 'h-11 min-w-[44px]' and preserve the denser desktop height with an override such as 'md:h-9'; ensure this replacement sits with the existing class list (including focus/disabled/animate classes) so ordering and responsive classes apply correctly.frontend/src/i18n/locales/en/chat.json (1)
103-124:⚠️ Potential issue | 🟠 MajorDuplicate
thinkingkey at top level and duplicatetoolskey within the firstthinkingblock.The JSON file contains two separate
"thinking"keys at the same nesting level. In valid JSON, the second occurrence overwrites the first, meaning all keys in the first"thinking"block (lines 103-124) are lost during parsing:tool_group,tools(string),completed,running,has_errors,tool_input,tool_output,tool_error, andtool_executing.Additionally, the first
"thinking"block contains a conflicting duplicate key:"tools": "tools"(line 105) is immediately followed by"tools": {...}(lines 113-123), where the object value overwrites the string value.Consolidate both
"thinking"blocks into a single object. In the consolidated block, ensure each key appears only once. For thetoolskey specifically, decide whether the string value or the object definition is needed—the current structure has both competing for the same key name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/i18n/locales/en/chat.json` around lines 103 - 124, There are duplicate top-level "thinking" objects and a conflicting duplicate "tools" key; fix by merging both "thinking" blocks into one object (keep all unique keys such as "tool_group", "completed", "running", "has_errors", "tool_input", "tool_output", "tool_error", "tool_executing") and remove the conflicting string "tools": "tools" so that "tools" is a single object mapping (bash, read, edit, write, grep, glob, todo, kb_search, web_search); ensure no duplicate keys remain and the final "thinking" contains the consolidated values.frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx (1)
511-512:⚠️ Potential issue | 🟠 MajorRemove the unused
totalUnreadCountprop to unblock lint.The pipeline is already failing on this prop. If it is no longer needed, drop it from the call site, prop type, and destructuring; otherwise rename the destructured arg to
_totalUnreadCount.🧹 Suggested cleanup
- totalUnreadCount={totalUnreadCount} handleMarkAllAsViewed={handleMarkAllAsViewed}- totalUnreadCount: number handleMarkAllAsViewed: () => void- totalUnreadCount, handleMarkAllAsViewed,Also applies to: 589-590, 619-620
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx` around lines 511 - 512, The component is passing an unused prop totalUnreadCount which trips the linter; remove totalUnreadCount from all call sites where TaskSidebar (and any child Sidebar components) are invoked, delete it from the prop type/interface and from the props/destructuring in TaskSidebar, and keep handleMarkAllAsViewed as-is; alternatively, if you want to preserve the value for future use, rename the destructured prop to _totalUnreadCount (and update the prop type name accordingly) so the linter ignores it—apply this change consistently for the other occurrences of the same prop usage in this file.frontend/src/features/tasks/components/chat/QuickAccessCards.tsx (1)
255-264:⚠️ Potential issue | 🟡 MinorThe click glow still flashes the old accent.
pulse-glowis pinned torgba(20, 184, 166, ...), so card clicks will show teal even after the rest of this PR moves the UI to the new primary theme. Please drive this from a theme token or CSS variable instead of a raw color literal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx` around lines 255 - 264, The pulse-glow keyframes in QuickAccessCards currently use a hard-coded teal RGBA (rgba(20, 184, 166, ...)); change the animation to use the theme CSS variable (for example a primary/accent token like --color-primary or --color-accent with appropriate alpha) instead of the literal color so clicks follow the app theme; update all occurrences inside the `@keyframes` pulse-glow (0%, 50%, 100%) to use the CSS variable (and ensure the variable is referenced in the component’s CSS scope or a global theme so the animation picks up the new primary color).
🧹 Nitpick comments (7)
frontend/src/components/icons/ModelIcon.tsx (1)
18-41: Rename this new component file to kebab-case.This component is being added in a new file, so this is the cheapest point to align it with the repo convention. Please rename
frontend/src/components/icons/ModelIcon.tsxtofrontend/src/components/icons/model-icon.tsxand update the import sites before more references land.As per coding guidelines, "Component names must use PascalCase, component files must use kebab-case in React".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/icons/ModelIcon.tsx` around lines 18 - 41, The new React component file uses a PascalCase filename; rename the file containing the ModelIcon component (exported default ModelIcon and the function ModelIcon) to kebab-case (model-icon.tsx) and update all import sites to import the same default export from './model-icon' (or the appropriate relative path); ensure the exported component name remains ModelIcon (PascalCase) inside the file so component usage stays unchanged.frontend/src/components/ui/action-button.tsx (1)
22-27: Consider updating the JSDoc to reflect the new dual-mode behavior.The comment states "consistent size (36px)" but the component now supports two modes: icon-only (36x36 circle) and with-label (36px height, variable width). Consider updating the documentation to describe both modes.
📝 Suggested JSDoc update
/** * ActionButton Component * - * A unified circular action button component with consistent size (36px) and styling. - * Designed for use across the application wherever a circular icon button is needed. + * A unified action button component with consistent height (36px) and styling. + * Supports two modes: + * - Icon-only: 36x36 circular button + * - With label: 36px height, variable width pill-shaped button + * + * Designed for use across the application for icon buttons and icon+label buttons. * Supports clickable, outline, and loading/static states.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ui/action-button.tsx` around lines 22 - 27, Update the JSDoc for the ActionButton component to describe its dual-mode behavior: an icon-only circular mode (36x36px) and a with-label mode (36px height, variable width), and note which props control each mode (e.g., presence of a label/children or a prop like "iconOnly"/"variant" if applicable); mention supported states (clickable, outline, loading/static) and any accessibility notes (aria-label when icon-only). Ensure the comment above the ActionButton component and its props block reflects these details so consumers know when size is fixed vs. fluid.frontend/src/components/icons/AgentIcon.tsx (1)
1-47: Rename this component file to kebab-case.
AgentIcon.tsxbreaks the repo’s React file naming convention. Please switch it to something likeagent-icon.tsxbefore more imports land.As per coding guidelines, "
**/*.{ts,tsx}: ... component files must use kebab-case in React"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/icons/AgentIcon.tsx` around lines 1 - 47, The component file is named AgentIcon.tsx which violates the repo's kebab-case React component filename rule; rename the file to agent-icon.tsx and update any imports to reference the new filename, ensuring the exported symbol AgentIcon (and the default export) remains unchanged so components importing AgentIcon continue to work once their import paths are updated.frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx (1)
307-310: Use the shared hover token here instead of hard-coded RGB.The new
hover:bg-[rgb(238,238,238)]classes bypass the theme token work in this PR and force a separate dark override.hover:bg-hoverkeeps both themes aligned from a single source.As per coding guidelines, "
**/*.{ts,tsx}: Use Tailwind CSS classes for styling:className=\"bg-base text-text-primary\"for page background,className=\"bg-surface border-border\"for cards/panels,className=\"bg-primary text-white\"for primary buttons"Also applies to: 329-335
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx` around lines 307 - 310, Replace the hard-coded hover background utility with the shared theme token: update the Button in TaskSidebar (the Button that uses onClick={handleNewAgentClick}) to remove the explicit hover:bg-[rgb(238,238,238)] and its dark override and use hover:bg-hover instead so both light and dark themes derive from the single token; apply the same replacement for the other similar Button/className block later in this file (the second Button around the same sidebar area) to ensure consistent theme token usage across both instances.frontend/src/features/tasks/components/selector/TeamSelectorButton.tsx (1)
35-35: Unused prop:taskDetailis accepted but never used.The
taskDetailprop is destructured but not referenced anywhere in the component. Either remove it from the interface and destructuring, or document why it's needed for future use.♻️ Remove unused prop
interface TeamSelectorButtonProps { selectedTeam: Team | null setSelectedTeam: (team: Team | null) => void teams: Team[] disabled: boolean - taskDetail?: TaskDetail | null hideSettingsLink?: boolean /** Current mode for filtering teams by bind_mode */ currentMode?: TaskType } export default function TeamSelectorButton({ selectedTeam, setSelectedTeam, teams, disabled, hideSettingsLink = false, currentMode = 'chat', }: TeamSelectorButtonProps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/selector/TeamSelectorButton.tsx` at line 35, The prop taskDetail is declared and destructured in the TeamSelectorButton component but never used; remove taskDetail from the component's props/interface (and the destructuring in TeamSelectorButton) to eliminate the unused prop, or if it's intended for future use, add a brief comment explaining its planned purpose and keep it referenced where needed; update the props type/interface that declares taskDetail and the TeamSelectorButton function signature/destructuring accordingly (look for the TaskDetail type reference and the TeamSelectorButton props declaration).frontend/src/features/tasks/components/input/ChatInputControls.tsx (1)
474-489: TeamSelectorButton integration looks correct, but consider null handling.The conditional rendering logic (
teams.length > 0 && _onTeamChange && !hasMessages) correctly shows the team selector only for new conversations.However, the
setSelectedTeamcallback filters out null values:setSelectedTeam={(team: Team | null) => { if (team) { _onTeamChange(team) } }}This is fine, but note that
TeamSelectorButtonreturnsnullearly when!selectedTeam, so the null case in this callback is effectively unreachable. Consider simplifying to just pass_onTeamChangedirectly if the API allows.💡 Optional: Simplify callback if TeamSelectorButton never passes null
<TeamSelectorButton selectedTeam={selectedTeam} - setSelectedTeam={(team: Team | null) => { - if (team) { - _onTeamChange(team) - } - }} + setSelectedTeam={_onTeamChange as (team: Team | null) => void} teams={teams} disabled={isLoading || isStreaming} taskDetail={selectedTaskDetail} hideSettingsLink={true} currentMode={taskType} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/input/ChatInputControls.tsx` around lines 474 - 489, The inline setSelectedTeam callback contains an unreachable null branch because TeamSelectorButton already returns early when !selectedTeam; replace the wrapper with passing _onTeamChange directly (i.e. setSelectedTeam={_onTeamChange}) or, if prop types require, change the prop signature on TeamSelectorButton and its consumers from (team: Team | null) => void to (team: Team) => void so you can remove the unnecessary null guard; update function/type references setSelectedTeam, TeamSelectorButton, and _onTeamChange accordingly.frontend/src/features/tasks/components/chat/QuickAccessCards.tsx (1)
36-349: QuickAccessCards is still doing too much in one function.Fetching quick-access data, reconciling API models, scroll state, click timing, and all rendering/styles now live in a 300+ line component. Please extract the data hook and the card/scroller subcomponents before adding more branches here.
As per coding guidelines "Function length should not exceed 50 lines (preferred)" and "Manage complexity through extraction: if a module is already complex, prioritize extracting common logic into utilities or creating new modules rather than adding more complexity".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx` around lines 36 - 349, The QuickAccessCards component is too large and mixes data fetching (fetchQuickAccess/quickAccessTeams/isQuickAccessLoading), reconciliation logic, scroll handling (scrollContainerRef/checkScrollState/scrollLeft/scrollRight), click timing (handleTeamClick/clickedTeamId) and rendering (renderTeamCard) in one file; extract responsibilities: create a useQuickAccessTeams hook that contains the userApis.getQuickAccess call and manages quickAccessTeams/isQuickAccessLoading and reconciliation with filtered teams, create a QuickAccessCard component that encapsulates renderTeamCard and click animation/timing, and create a QuickAccessScroller component that owns scrollContainerRef, checkScrollState, scrollLeft/scrollRight and pagination UI; then have QuickAccessCards compose these three smaller units, passing only the minimal props (teams/selectedTeam/onTeamSelect/currentMode/defaultTeam) to keep the top-level function under ~50 lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/globals.css`:
- Line 17: The CSS custom property --color-bg-hover currently includes an
opacity component ("93 94 201 / 0.06") which breaks Tailwind's withOpacity()
wrapper; remove the "/ 0.06" so the value is just the raw RGB triplet ("93 94
201"). Do the same for any other tokens using the "/ <alpha>" pattern (e.g.,
--color-accent if present as "118 119 218 / 0.06") so all color custom
properties store only the RGB triplet and allow Tailwind's withOpacity()/utility
opacity modifiers to supply alpha values.
In `@frontend/src/features/tasks/components/chat/AddContextButton.tsx`:
- Around line 17-20: The file AddContextButton.tsx contains a docblock and a
hard-coded Chinese label "知识库" — update the comment text to be entirely English
(e.g., replace "知识库" with "Knowledge Base") and remove any Chinese in the
file-level docblock; also ensure the visible label used by the AddContextButton
UI (and any references to ActionButton or BookOpenText within AddContextButton)
uses the project's i18n/localization API rather than a hard-coded string if one
exists, or otherwise replace the literal with the English string "Knowledge
Base" to comply with the comment-language rule.
- Around line 30-35: The labeled ActionButton variant renders at h-9 which is
too small for mobile; update the shared ActionButton component (the labeled
rendering path in ActionButton) to use mobile-accessible sizing by replacing the
small height/width classes with h-11 and min-w-[44px] (and ensure any related
padding/alignment classes still work with these dimensions) so all uses like the
AddContextButton calling ActionButton (icon, label, title props) meet the 44×44
requirement.
In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx`:
- Around line 14-21: The carousel uses fixed constants (CONTAINER_WIDTH,
CONTAINER_HEIGHT, CARD_WIDTH, CARD_GAP, CARDS_PER_PAGE, PAGE_SCROLL_AMOUNT)
which lock it to desktop sizes; change the layout to be responsive by replacing
fixed pixel container/viewport sizing with mobile-first styling (e.g., use
w-full with a max-w fallback such as max-w-[880px] or computed CSS percentages)
and compute PAGE_SCROLL_AMOUNT from the actual rendered card width or use
percent-based widths so the carousel adapts to narrow viewports; increase arrow
button targets to at least h-11 and min-w-[44px] and adjust the arrow
click/visible area in the component that renders the controls so touch targets
meet the guideline; when behavior/layout differs significantly between mobile
and desktop, split into a MobileQuickAccessCards variant (mobile-first) and
DesktopQuickAccessCards (reuse CARD_* constants for desktop if needed) to follow
the component-separation rule.
- Around line 29-33: QuickAccessCards was refactored to ignore the props
hideSelected, isTeamsLoading, onRefreshTeams, and showWizardButton while
ChatArea still passes them, causing silent dropped intent and a lint error for
hideSelected; either (A) wire the props back up: restore hideSelected filtering
logic in QuickAccessCards where teams are mapped/filtered (use hideSelected to
exclude the selected team), and use
isTeamsLoading/onRefreshTeams/showWizardButton to control loading state, refresh
behavior and wizard button rendering respectively, or (B) remove these props
from QuickAccessCardsProps and also remove the corresponding prop usages in the
caller ChatArea so the contract matches; reference the QuickAccessCards
component and its QuickAccessCardsProps type and the ChatArea caller when making
the change.
- Around line 209-249: In renderTeamCard, replace the outer div with a semantic
<button type="button"> element (keep the same className string, inline style
object, CARD_WIDTH, height, padding and borderRadius), use the existing
onClick={() => !isClicked && handleTeamClick(team)} as the button's onClick, set
aria-pressed={isSelected} to expose selection to assistive tech, and when
isClicked apply disabled={true} instead of pointer-events-none so keyboard users
can't interact while the clicking animation runs; keep isSelected/isClicked
logic and other visuals unchanged.
In `@frontend/src/features/tasks/components/input/ChatInputCard.tsx`:
- Around line 227-235: Replace the hardcoded Chinese strings in the drag overlay
inside ChatInputCard.tsx with i18n calls: import and call the useTranslation
hook (e.g., const { t } = useTranslation()), add translation keys such as
"dragDrop.release_to_upload" and "dragDrop.supported_formats" to the locale
files, and replace the literal texts "释放以上传文件" and "支持 PDF, Word, TXT, Markdown
等格式" with t('dragDrop.release_to_upload') and t('dragDrop.supported_formats')
respectively so the overlay (rendered when isDragging is true) uses
translations.
In `@frontend/src/features/tasks/components/input/SendButton.tsx`:
- Around line 16-17: Update the SendButton component to meet mobile touch-target
sizing: adjust its classes from the current 34×34 sizing to use h-11 and
min-w-[44px] (e.g., add/replace class names to include "h-11 min-w-[44px]") so
the rendered button is at least 44×44px; leave the deprecated compact prop
untouched (do not destructure or use it) and ensure SendButton's className/size
logic is updated where the button element is rendered.
In `@frontend/src/features/tasks/components/selector/TeamSelectorButton.tsx`:
- Around line 137-140: The conditional rendering in TeamSelectorButton uses a
duplicated ternary (searchQuery ? t('common:teams.no_match') :
t('common:teams.no_match')) which is likely a copy-paste mistake; update the
branch to either use the correct translation keys (e.g., when searchQuery is
truthy show t('common:teams.no_match') and when falsy show
t('common:teams.no_teams') or another appropriate key) or simplify by removing
the ternary and directly returning a single translation (e.g.,
t('common:teams.no_match')) if both states should show the same message; look
for filteredTeams, searchQuery and the t(...) call within TeamSelectorButton to
make the change.
- Around line 5-10: Update the TeamSelectorButton component's documentation and
displayed label to English: replace the Chinese text "智能体" in the doc comment
and the visible label within TeamSelectorButton with "Agent" (or the agreed
English term). Locate the TeamSelectorButton component and the string literal
used alongside AgentIcon and change both the comment and the UI label to the
English word, ensuring the docstring and any inline comments referencing the
label are updated accordingly.
In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx`:
- Around line 809-815: The search button in TaskSidebar uses p-0.5 and is below
the 44×44 mobile touch target; update the button that calls
handleOpenSearchDialog (the one rendering the Search icon) to use a mobile-first
hit area such as h-11 min-w-[44px] and center the icon (e.g., make it flex
items-center justify-center) instead of p-0.5, and if you need to visually
shrink the touch area on larger screens apply smaller sizing via responsive
classes (e.g., md:h-auto md:min-w-0 or similar) so the button meets the 44px
mobile minimum while keeping the existing aria-label and Search icon rendering.
---
Outside diff comments:
In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx`:
- Around line 255-264: The pulse-glow keyframes in QuickAccessCards currently
use a hard-coded teal RGBA (rgba(20, 184, 166, ...)); change the animation to
use the theme CSS variable (for example a primary/accent token like
--color-primary or --color-accent with appropriate alpha) instead of the literal
color so clicks follow the app theme; update all occurrences inside the
`@keyframes` pulse-glow (0%, 50%, 100%) to use the CSS variable (and ensure the
variable is referenced in the component’s CSS scope or a global theme so the
animation picks up the new primary color).
In `@frontend/src/features/tasks/components/selector/ModelSelector.tsx`:
- Around line 248-257: The selector's touch target is too small: in
ModelSelector.tsx update the className on the trigger (the JSX that currently
includes 'h-9') to use a mobile-first size of 'h-11 min-w-[44px]' and preserve
the denser desktop height with an override such as 'md:h-9'; ensure this
replacement sits with the existing class list (including focus/disabled/animate
classes) so ordering and responsive classes apply correctly.
In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx`:
- Around line 511-512: The component is passing an unused prop totalUnreadCount
which trips the linter; remove totalUnreadCount from all call sites where
TaskSidebar (and any child Sidebar components) are invoked, delete it from the
prop type/interface and from the props/destructuring in TaskSidebar, and keep
handleMarkAllAsViewed as-is; alternatively, if you want to preserve the value
for future use, rename the destructured prop to _totalUnreadCount (and update
the prop type name accordingly) so the linter ignores it—apply this change
consistently for the other occurrences of the same prop usage in this file.
In `@frontend/src/i18n/locales/en/chat.json`:
- Around line 103-124: There are duplicate top-level "thinking" objects and a
conflicting duplicate "tools" key; fix by merging both "thinking" blocks into
one object (keep all unique keys such as "tool_group", "completed", "running",
"has_errors", "tool_input", "tool_output", "tool_error", "tool_executing") and
remove the conflicting string "tools": "tools" so that "tools" is a single
object mapping (bash, read, edit, write, grep, glob, todo, kb_search,
web_search); ensure no duplicate keys remain and the final "thinking" contains
the consolidated values.
---
Nitpick comments:
In `@frontend/src/components/icons/AgentIcon.tsx`:
- Around line 1-47: The component file is named AgentIcon.tsx which violates the
repo's kebab-case React component filename rule; rename the file to
agent-icon.tsx and update any imports to reference the new filename, ensuring
the exported symbol AgentIcon (and the default export) remains unchanged so
components importing AgentIcon continue to work once their import paths are
updated.
In `@frontend/src/components/icons/ModelIcon.tsx`:
- Around line 18-41: The new React component file uses a PascalCase filename;
rename the file containing the ModelIcon component (exported default ModelIcon
and the function ModelIcon) to kebab-case (model-icon.tsx) and update all import
sites to import the same default export from './model-icon' (or the appropriate
relative path); ensure the exported component name remains ModelIcon
(PascalCase) inside the file so component usage stays unchanged.
In `@frontend/src/components/ui/action-button.tsx`:
- Around line 22-27: Update the JSDoc for the ActionButton component to describe
its dual-mode behavior: an icon-only circular mode (36x36px) and a with-label
mode (36px height, variable width), and note which props control each mode
(e.g., presence of a label/children or a prop like "iconOnly"/"variant" if
applicable); mention supported states (clickable, outline, loading/static) and
any accessibility notes (aria-label when icon-only). Ensure the comment above
the ActionButton component and its props block reflects these details so
consumers know when size is fixed vs. fluid.
In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx`:
- Around line 36-349: The QuickAccessCards component is too large and mixes data
fetching (fetchQuickAccess/quickAccessTeams/isQuickAccessLoading),
reconciliation logic, scroll handling
(scrollContainerRef/checkScrollState/scrollLeft/scrollRight), click timing
(handleTeamClick/clickedTeamId) and rendering (renderTeamCard) in one file;
extract responsibilities: create a useQuickAccessTeams hook that contains the
userApis.getQuickAccess call and manages quickAccessTeams/isQuickAccessLoading
and reconciliation with filtered teams, create a QuickAccessCard component that
encapsulates renderTeamCard and click animation/timing, and create a
QuickAccessScroller component that owns scrollContainerRef, checkScrollState,
scrollLeft/scrollRight and pagination UI; then have QuickAccessCards compose
these three smaller units, passing only the minimal props
(teams/selectedTeam/onTeamSelect/currentMode/defaultTeam) to keep the top-level
function under ~50 lines.
In `@frontend/src/features/tasks/components/input/ChatInputControls.tsx`:
- Around line 474-489: The inline setSelectedTeam callback contains an
unreachable null branch because TeamSelectorButton already returns early when
!selectedTeam; replace the wrapper with passing _onTeamChange directly (i.e.
setSelectedTeam={_onTeamChange}) or, if prop types require, change the prop
signature on TeamSelectorButton and its consumers from (team: Team | null) =>
void to (team: Team) => void so you can remove the unnecessary null guard;
update function/type references setSelectedTeam, TeamSelectorButton, and
_onTeamChange accordingly.
In `@frontend/src/features/tasks/components/selector/TeamSelectorButton.tsx`:
- Line 35: The prop taskDetail is declared and destructured in the
TeamSelectorButton component but never used; remove taskDetail from the
component's props/interface (and the destructuring in TeamSelectorButton) to
eliminate the unused prop, or if it's intended for future use, add a brief
comment explaining its planned purpose and keep it referenced where needed;
update the props type/interface that declares taskDetail and the
TeamSelectorButton function signature/destructuring accordingly (look for the
TaskDetail type reference and the TeamSelectorButton props declaration).
In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx`:
- Around line 307-310: Replace the hard-coded hover background utility with the
shared theme token: update the Button in TaskSidebar (the Button that uses
onClick={handleNewAgentClick}) to remove the explicit
hover:bg-[rgb(238,238,238)] and its dark override and use hover:bg-hover instead
so both light and dark themes derive from the single token; apply the same
replacement for the other similar Button/className block later in this file (the
second Button around the same sidebar area) to ensure consistent theme token
usage across both instances.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9df9fdf-b04d-4282-bdae-7c4886fc54aa
📒 Files selected for processing (26)
frontend/src/app/globals.cssfrontend/src/components/icons/AgentIcon.tsxfrontend/src/components/icons/ModelIcon.tsxfrontend/src/components/ui/action-button.tsxfrontend/src/features/layout/components/UserFloatingMenu.tsxfrontend/src/features/projects/components/ProjectSection.tsxfrontend/src/features/tasks/components/AttachmentButton.tsxfrontend/src/features/tasks/components/CorrectionModeToggle.tsxfrontend/src/features/tasks/components/chat/AddContextButton.tsxfrontend/src/features/tasks/components/chat/ChatArea.tsxfrontend/src/features/tasks/components/chat/QuickAccessCards.tsxfrontend/src/features/tasks/components/clarification/ClarificationToggle.tsxfrontend/src/features/tasks/components/input/ChatInputCard.tsxfrontend/src/features/tasks/components/input/ChatInputControls.tsxfrontend/src/features/tasks/components/input/SendButton.tsxfrontend/src/features/tasks/components/selector/ModelSelector.tsxfrontend/src/features/tasks/components/selector/SkillSelectorPopover.tsxfrontend/src/features/tasks/components/selector/TeamSelectorButton.tsxfrontend/src/features/tasks/components/selector/UnifiedRepositorySelector.tsxfrontend/src/features/tasks/components/sidebar/ResizableSidebar.tsxfrontend/src/features/tasks/components/sidebar/TaskSidebar.tsxfrontend/src/i18n/locales/en/chat.jsonfrontend/src/i18n/locales/en/common.jsonfrontend/src/i18n/locales/zh-CN/chat.jsonfrontend/src/i18n/locales/zh-CN/common.jsonfrontend/tailwind.config.js
💤 Files with no reviewable changes (1)
- frontend/src/features/tasks/components/AttachmentButton.tsx
| --color-text-primary: 26 26 26; | ||
| --color-text-secondary: 102 102 102; | ||
| --color-text-muted: 160 160 160; | ||
| --color-bg-hover: 93 94 201 / 0.06; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expected result:
# - tailwind.config.js shows withOpacity() returning rgb(var(--token) / <alpha-value>)
# - globals.css shows --color-bg-hover already contains "/ 0.06" and "/ 0.1"
rg -n -C2 'withOpacity|--color-bg-hover' frontend/tailwind.config.js frontend/src/app/globals.cssRepository: wecode-ai/Wegent
Length of output: 5516
🏁 Script executed:
# Search for usages of bg-hover utility and opacity modifiers
rg -r 'bg-hover|accent-\[|accent/|accent-DEFAULT' frontend/src --type tsx --type ts --type jsx --type html -B2 -A2Repository: wecode-ai/Wegent
Length of output: 87
🏁 Script executed:
# Check all CSS custom properties in globals.css to see if others have opacity values
rg '^\s*--.*:.*\/\s*[\d.]+;' frontend/src/app/globals.css -nRepository: wecode-ai/Wegent
Length of output: 160
🏁 Script executed:
# Look for other color variable definitions that might have the same pattern
rg '^\s*--color-.*:' frontend/src/app/globals.css -n | head -30Repository: wecode-ai/Wegent
Length of output: 1174
🏁 Script executed:
# Search for usages of bg-hover, accent colors, and --color-bg-hover in JSX/TSX
fd '\.(js|jsx|ts|tsx)$' frontend/src --exec rg -l 'bg-hover|accent|--color-bg-hover' {} \;Repository: wecode-ai/Wegent
Length of output: 5042
🏁 Script executed:
# Get actual usage patterns
fd '\.(js|jsx|ts|tsx)$' frontend/src --exec rg 'bg-hover|accent-' {} + | head -20Repository: wecode-ai/Wegent
Length of output: 31226
🏁 Script executed:
# Check if there are any Tailwind opacity modifiers used with these colors
rg '\b(bg-hover|accent)/' frontend/src -A2 -B2Repository: wecode-ai/Wegent
Length of output: 725
Remove opacity values from --color-bg-hover CSS custom properties.
Tailwind's withOpacity() helper at frontend/tailwind.config.js:53 wraps --color-bg-hover as rgb(var(--color-bg-hover) / <alpha-value>). Defining the token as 93 94 201 / 0.06 creates invalid CSS like rgb(93 94 201 / 0.06 / 1) when utilities like bg-hover or opacity modifiers (bg-hover/50, bg-accent/50) are applied. Store only the raw RGB triplet (93 94 201 and 118 119 218) and let Tailwind apply opacity through standard utility modifiers.
Also applies to: 49-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/app/globals.css` at line 17, The CSS custom property
--color-bg-hover currently includes an opacity component ("93 94 201 / 0.06")
which breaks Tailwind's withOpacity() wrapper; remove the "/ 0.06" so the value
is just the raw RGB triplet ("93 94 201"). Do the same for any other tokens
using the "/ <alpha>" pattern (e.g., --color-accent if present as "118 119 218 /
0.06") so all color custom properties store only the RGB triplet and allow
Tailwind's withOpacity()/utility opacity modifiers to supply alpha values.
Comment on lines
17
to
+20
| /** | ||
| * Add Context Button - Icon-only button that opens knowledge base selector | ||
| * Always displays "#" symbol with tooltip on hover | ||
| * Uses ActionButton for consistent 36px size with other control buttons | ||
| * Add Context Button - Button with icon and label that opens knowledge base selector | ||
| * Displays BookOpenText icon with "知识库" label | ||
| * Uses ActionButton for consistent styling with other control buttons |
Contributor
There was a problem hiding this comment.
Keep the docblock fully in English.
Line 19 hard-codes 知识库, which violates the repo rule for source comments and will drift from the localized label anyway.
As per coding guidelines, "**/*.{py,ts,tsx,js,jsx}: All code comments MUST be written in English"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/chat/AddContextButton.tsx` around
lines 17 - 20, The file AddContextButton.tsx contains a docblock and a
hard-coded Chinese label "知识库" — update the comment text to be entirely English
(e.g., replace "知识库" with "Knowledge Base") and remove any Chinese in the
file-level docblock; also ensure the visible label used by the AddContextButton
UI (and any references to ActionButton or BookOpenText within AddContextButton)
uses the project's i18n/localization API rather than a hard-coded string if one
exists, or otherwise replace the literal with the English string "Knowledge
Base" to comply with the comment-language rule.
Comment on lines
30
to
35
| <ActionButton | ||
| variant="outline" | ||
| onClick={onClick} | ||
| icon={<span className="text-base font-medium text-text-primary">#</span>} | ||
| icon={<BookOpenText className="h-4 w-4" />} | ||
| label={t('knowledge:tooltip')} | ||
| title={t('knowledge:tooltip')} | ||
| className="border-border bg-base text-text-primary hover:bg-hover" | ||
| /> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Update the shared labeled ActionButton sizing before using it in the composer.
frontend/src/components/ui/action-button.tsx:66-115 renders labeled buttons at h-9, so this new control is only 36px tall on mobile. That misses the repo’s 44×44 minimum and the same shared issue will hit the other newly labeled action buttons in this PR.
As per coding guidelines, "**/*.{ts,tsx}: All interactive touch elements on mobile must be at least 44px × 44px: use h-11 min-w-[44px] for buttons"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/chat/AddContextButton.tsx` around
lines 30 - 35, The labeled ActionButton variant renders at h-9 which is too
small for mobile; update the shared ActionButton component (the labeled
rendering path in ActionButton) to use mobile-accessible sizing by replacing the
small height/width classes with h-11 and min-w-[44px] (and ensure any related
padding/alignment classes still work with these dimensions) so all uses like the
AddContextButton calling ActionButton (icon, label, title props) meet the 44×44
requirement.
Comment on lines
+14
to
+21
| const CONTAINER_WIDTH = 880 | ||
| const CONTAINER_HEIGHT = 108 | ||
|
|
||
| // Card dimensions | ||
| const CARD_WIDTH = 154 | ||
| const CARD_GAP = 12 | ||
| const CARDS_PER_PAGE = 5 | ||
| const PAGE_SCROLL_AMOUNT = CARDS_PER_PAGE * CARD_WIDTH + (CARDS_PER_PAGE - 1) * CARD_GAP |
Contributor
There was a problem hiding this comment.
The carousel is locked to desktop dimensions.
Both the loading state and the final state hard-code an 880px wrapper / 818px viewport, and the arrow buttons are only 36px wide. On smaller screens this will overflow or clip, and the arrows miss the repo’s minimum touch target. Use w-full max-w-* responsive sizing and increase the arrows to at least h-11 min-w-[44px], or split a mobile-specific variant.
As per coding guidelines "Use mobile-first, component-separation architecture for responsive design: separate Mobile/Desktop components when layout differences exceed 30% or interaction patterns differ significantly" and "All interactive touch elements on mobile must be at least 44px × 44px: use h-11 min-w-[44px] for buttons".
Also applies to: 163-180, 297-344
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx` around
lines 14 - 21, The carousel uses fixed constants (CONTAINER_WIDTH,
CONTAINER_HEIGHT, CARD_WIDTH, CARD_GAP, CARDS_PER_PAGE, PAGE_SCROLL_AMOUNT)
which lock it to desktop sizes; change the layout to be responsive by replacing
fixed pixel container/viewport sizing with mobile-first styling (e.g., use
w-full with a max-w fallback such as max-w-[880px] or computed CSS percentages)
and compute PAGE_SCROLL_AMOUNT from the actual rendered card width or use
percent-based widths so the carousel adapts to narrow viewports; increase arrow
button targets to at least h-11 and min-w-[44px] and adjust the arrow
click/visible area in the component that renders the controls so touch targets
meet the guideline; when behavior/layout differs significantly between mobile
and desktop, split into a MobileQuickAccessCards variant (mobile-first) and
DesktopQuickAccessCards (reuse CARD_* constants for desktop if needed) to follow
the component-separation rule.
Comment on lines
+227
to
+235
| {isDragging && ( | ||
| <div className="absolute inset-0 z-50 rounded-3xl bg-base/95 backdrop-blur-sm flex flex-col items-center justify-center border-2 border-dashed border-primary transition-all animate-in fade-in duration-200"> | ||
| <div className="p-4 rounded-full bg-primary/10 mb-4 animate-bounce"> | ||
| <Upload className="h-8 w-8 text-primary" /> | ||
| </div> | ||
| <p className="text-lg font-medium text-primary">释放以上传文件</p> | ||
| <p className="text-sm text-text-muted mt-1">支持 PDF, Word, TXT, Markdown 等格式</p> | ||
| </div> | ||
| <p className="text-lg font-medium text-primary">释放以上传文件</p> | ||
| <p className="text-sm text-text-muted mt-1">支持 PDF, Word, TXT, Markdown 等格式</p> | ||
| </div> | ||
| )} | ||
| )} |
Contributor
There was a problem hiding this comment.
Hardcoded Chinese strings should use i18n translations.
The drag overlay contains hardcoded Chinese text:
- "释放以上传文件" (Line 232)
- "支持 PDF, Word, TXT, Markdown 等格式" (Line 233)
These should use the useTranslation hook with appropriate translation keys for internationalization support.
🌐 Proposed fix - use i18n translations
First, add the translation keys to the locale files (e.g., chat.json or common.json):
{
"dragDrop": {
"release_to_upload": "Release to upload file",
"supported_formats": "Supports PDF, Word, TXT, Markdown and more"
}
}Then update the component:
{isDragging && (
<div className="absolute inset-0 z-50 rounded-3xl bg-base/95 backdrop-blur-sm flex flex-col items-center justify-center border-2 border-dashed border-primary transition-all animate-in fade-in duration-200">
<div className="p-4 rounded-full bg-primary/10 mb-4 animate-bounce">
<Upload className="h-8 w-8 text-primary" />
</div>
- <p className="text-lg font-medium text-primary">释放以上传文件</p>
- <p className="text-sm text-text-muted mt-1">支持 PDF, Word, TXT, Markdown 等格式</p>
+ <p className="text-lg font-medium text-primary">{t('dragDrop.release_to_upload')}</p>
+ <p className="text-sm text-text-muted mt-1">{t('dragDrop.supported_formats')}</p>
</div>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/input/ChatInputCard.tsx` around lines
227 - 235, Replace the hardcoded Chinese strings in the drag overlay inside
ChatInputCard.tsx with i18n calls: import and call the useTranslation hook
(e.g., const { t } = useTranslation()), add translation keys such as
"dragDrop.release_to_upload" and "dragDrop.supported_formats" to the locale
files, and replace the literal texts "释放以上传文件" and "支持 PDF, Word, TXT, Markdown
等格式" with t('dragDrop.release_to_upload') and t('dragDrop.supported_formats')
respectively so the overlay (rendered when isDragging is true) uses
translations.
Comment on lines
+16
to
17
| /** @deprecated No longer used, kept for API compatibility */ | ||
| compact?: boolean |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all usages of SendButton with compact prop to understand impact
rg -n "SendButton.*compact" --type=tsx --type=ts -C2Repository: wecode-ai/Wegent
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Search for SendButton usages, fixing the rg syntax
rg "SendButton" --glob="*.tsx" --glob="*.ts" -C2Repository: wecode-ai/Wegent
Length of output: 7869
🏁 Script executed:
#!/bin/bash
# Check the current SendButton component implementation and size
head -70 frontend/src/features/tasks/components/input/SendButton.tsxRepository: wecode-ai/Wegent
Length of output: 1697
🏁 Script executed:
#!/bin/bash
# Search for MobileChatInputControls to see how it uses SendButton
rg "MobileChatInputControls" --glob="*.tsx" --glob="*.ts" -C3Repository: wecode-ai/Wegent
Length of output: 4470
Update SendButton to meet mobile accessibility requirements: 44px × 44px minimum.
The compact prop deprecation is correctly implemented—it's properly ignored by not being destructured. However, the button is currently 34x34px, which violates mobile accessibility guidelines requiring at least 44x44px for touch targets. Update the button size to h-11 min-w-[44px] to meet mobile accessibility standards.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/input/SendButton.tsx` around lines 16
- 17, Update the SendButton component to meet mobile touch-target sizing: adjust
its classes from the current 34×34 sizing to use h-11 and min-w-[44px] (e.g.,
add/replace class names to include "h-11 min-w-[44px]") so the rendered button
is at least 44×44px; leave the deprecated compact prop untouched (do not
destructure or use it) and ensure SendButton's className/size logic is updated
where the button element is rendered.
Comment on lines
+5
to
+10
| /** | ||
| * TeamSelectorButton Component | ||
| * | ||
| * Simplified team selector for chat input controls. | ||
| * Always displays "智能体" label with AgentIcon. | ||
| */ |
Contributor
There was a problem hiding this comment.
Doc comment contains Chinese text - should be English only.
Per coding guidelines, all code comments must be written in English. The docstring contains "智能体" which should be translated.
📝 Proposed fix
/**
* TeamSelectorButton Component
*
- * Simplified team selector for chat input controls.
- * Always displays "智能体" label with AgentIcon.
+ * Simplified team selector for chat input controls.
+ * Always displays "Agent" label with AgentIcon.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * TeamSelectorButton Component | |
| * | |
| * Simplified team selector for chat input controls. | |
| * Always displays "智能体" label with AgentIcon. | |
| */ | |
| /** | |
| * TeamSelectorButton Component | |
| * | |
| * Simplified team selector for chat input controls. | |
| * Always displays "Agent" label with AgentIcon. | |
| */ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/selector/TeamSelectorButton.tsx`
around lines 5 - 10, Update the TeamSelectorButton component's documentation and
displayed label to English: replace the Chinese text "智能体" in the doc comment
and the visible label within TeamSelectorButton with "Agent" (or the agreed
English term). Locate the TeamSelectorButton component and the string literal
used alongside AgentIcon and change both the comment and the UI label to the
English word, ensuring the docstring and any inline comments referencing the
label are updated accordingly.
Comment on lines
+137
to
+140
| {filteredTeams.length === 0 ? ( | ||
| <div className="py-4 text-center text-sm text-text-muted"> | ||
| {searchQuery ? t('common:teams.no_match') : t('common:teams.no_match')} | ||
| </div> |
Contributor
There was a problem hiding this comment.
Duplicate ternary condition - both branches return the same value.
The ternary searchQuery ? t('common:teams.no_match') : t('common:teams.no_match') returns the same translation key regardless of the condition. This appears to be a copy-paste error.
🐛 Proposed fix - differentiate empty states
{filteredTeams.length === 0 ? (
<div className="py-4 text-center text-sm text-text-muted">
- {searchQuery ? t('common:teams.no_match') : t('common:teams.no_match')}
+ {searchQuery ? t('common:teams.no_match') : t('common:teams.no_teams')}
</div>
) : (Or if both cases should show the same message, simplify to:
{filteredTeams.length === 0 ? (
<div className="py-4 text-center text-sm text-text-muted">
- {searchQuery ? t('common:teams.no_match') : t('common:teams.no_match')}
+ {t('common:teams.no_match')}
</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.
Suggested change
| {filteredTeams.length === 0 ? ( | |
| <div className="py-4 text-center text-sm text-text-muted"> | |
| {searchQuery ? t('common:teams.no_match') : t('common:teams.no_match')} | |
| </div> | |
| {filteredTeams.length === 0 ? ( | |
| <div className="py-4 text-center text-sm text-text-muted"> | |
| {t('common:teams.no_match')} | |
| </div> |
Suggested change
| {filteredTeams.length === 0 ? ( | |
| <div className="py-4 text-center text-sm text-text-muted"> | |
| {searchQuery ? t('common:teams.no_match') : t('common:teams.no_match')} | |
| </div> | |
| {filteredTeams.length === 0 ? ( | |
| <div className="py-4 text-center text-sm text-text-muted"> | |
| {searchQuery ? t('common:teams.no_match') : t('common:teams.no_teams')} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/selector/TeamSelectorButton.tsx`
around lines 137 - 140, The conditional rendering in TeamSelectorButton uses a
duplicated ternary (searchQuery ? t('common:teams.no_match') :
t('common:teams.no_match')) which is likely a copy-paste mistake; update the
branch to either use the correct translation keys (e.g., when searchQuery is
truthy show t('common:teams.no_match') and when falsy show
t('common:teams.no_teams') or another appropriate key) or simplify by removing
the ternary and directly returning a single translation (e.g.,
t('common:teams.no_match')) if both states should show the same message; look
for filteredTeams, searchQuery and the t(...) call within TeamSelectorButton to
make the change.
Comment on lines
+809
to
+815
| <button | ||
| onClick={handleOpenSearchDialog} | ||
| className="p-0.5 text-text-muted hover:text-text-primary transition-colors rounded" | ||
| aria-label={t('common:tasks.search_placeholder_chat')} | ||
| > | ||
| <Search className="h-3.5 w-3.5" /> | ||
| </button> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make the history search button a 44px touch target.
p-0.5 leaves this control far below the repo’s mobile minimum, and this header is rendered inside MobileSidebar too. Please give it a mobile-first hit area and scale it back down only on larger breakpoints if needed.
📱 Suggested sizing
- className="p-0.5 text-text-muted hover:text-text-primary transition-colors rounded"
+ className="inline-flex h-11 min-w-[44px] items-center justify-center rounded text-text-muted transition-colors hover:text-text-primary lg:h-7 lg:min-w-7"As per coding guidelines, "**/*.{ts,tsx}: All interactive touch elements on mobile must be at least 44px × 44px: use h-11 min-w-[44px] for buttons"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button | |
| onClick={handleOpenSearchDialog} | |
| className="p-0.5 text-text-muted hover:text-text-primary transition-colors rounded" | |
| aria-label={t('common:tasks.search_placeholder_chat')} | |
| > | |
| <Search className="h-3.5 w-3.5" /> | |
| </button> | |
| <button | |
| onClick={handleOpenSearchDialog} | |
| className="inline-flex h-11 min-w-[44px] items-center justify-center rounded text-text-muted transition-colors hover:text-text-primary lg:h-7 lg:min-w-7" | |
| aria-label={t('common:tasks.search_placeholder_chat')} | |
| > | |
| <Search className="h-3.5 w-3.5" /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx` around lines
809 - 815, The search button in TaskSidebar uses p-0.5 and is below the 44×44
mobile touch target; update the button that calls handleOpenSearchDialog (the
one rendering the Search icon) to use a mobile-first hit area such as h-11
min-w-[44px] and center the icon (e.g., make it flex items-center
justify-center) instead of p-0.5, and if you need to visually shrink the touch
area on larger screens apply smaller sizing via responsive classes (e.g.,
md:h-auto md:min-w-0 or similar) so the button meets the 44px mobile minimum
while keeping the existing aria-label and Search icon rendering.
Contributor
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 (2)
frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx (2)
843-854:⚠️ Potential issue | 🟡 MinorLoad more button below 44px mobile touch target.
Same issue as the expand/collapse button—
py-1.5produces a touch area under 44px.📱 Suggested sizing
- className="flex items-center gap-1 px-1 py-1.5 text-xs text-text-muted hover:text-text-primary transition-colors w-full" + className="flex items-center gap-1 px-1 h-11 text-xs text-text-muted hover:text-text-primary transition-colors w-full lg:h-auto lg:py-1.5"Based on learnings: "All interactive touch elements on mobile must be at least 44px × 44px: use
h-11 min-w-[44px]for buttons".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx` around lines 843 - 854, The "Load more" button in TaskSidebar renders with py-1.5 which yields a mobile touch target under 44px; update the button for the loadMorePersonalTasks block (the element using hasMorePersonalTasks, loadMorePersonalTasks, loadingMorePersonalTasks) to use a minimum 44px touch target by replacing vertical padding with explicit height and minimum width utilities (e.g., add h-11 and min-w-[44px]) while preserving existing flex, gap and text classes so alignment and styles remain consistent.
702-727:⚠️ Potential issue | 🟡 MinorExpand/collapse button below 44px mobile touch target.
This button uses
py-1.5resulting in a touch area well under the 44px minimum. SincesidebarContentrenders insideMobileSidebar, this affects mobile users.📱 Suggested sizing
- className="flex items-center gap-1 px-1 py-1.5 text-xs text-text-muted hover:text-text-primary transition-colors w-full" + className="flex items-center gap-1 px-1 h-11 text-xs text-text-muted hover:text-text-primary transition-colors w-full lg:h-auto lg:py-1.5"Based on learnings: "All interactive touch elements on mobile must be at least 44px × 44px: use
h-11 min-w-[44px]for buttons".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx` around lines 702 - 727, The expand/collapse button's touch area is too small (uses py-1.5) — update the button rendering (the element using onClick that references isGroupChatsExpanded, loadMoreGroupTasks, setIsGroupChatsExpanded, hasMoreGroupTasks and collapsedReadCount) to meet the 44×44px mobile minimum by replacing the size classes: remove the small vertical padding and add sizing classes such as h-11 and min-w-[44px] (keep items-center and gap-1 to preserve layout and other classes like px-1 and text-xs as needed), ensuring the visual alignment and behavior remain the same while enlarging the hit target for mobile.
♻️ Duplicate comments (3)
frontend/src/features/tasks/components/chat/QuickAccessCards.tsx (3)
13-21:⚠️ Potential issue | 🟠 MajorThe carousel is still locked to desktop dimensions.
880px/154pxsizing is baked into both the skeleton and the live rail, and the arrow controls are still only36pxwide. That will overflow on narrow viewports and misses the repo’s minimum mobile touch target. Please switch this to a responsivew-full max-w-*layout, or split mobile/desktop variants if the interaction needs to differ.Based on learnings "All interactive touch elements on mobile must be at least 44px × 44px: use
h-11 min-w-[44px]for buttons" and "Use mobile-first, component-separation architecture for responsive design: separate Mobile/Desktop components when layout differences exceed 30% or interaction patterns differ significantly".Also applies to: 163-180, 296-344
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx` around lines 13 - 21, The layout constants (CONTAINER_WIDTH, CONTAINER_HEIGHT, CARD_WIDTH, CARD_GAP, CARDS_PER_PAGE, PAGE_SCROLL_AMOUNT) are hard-coded for desktop and must be converted to a responsive implementation: replace fixed pixel sizing with a mobile-first approach (use w-full and max-w-* constraints on the container and compute card widths via CSS classes/layout rather than constants) or split into two components (QuickAccessCardsMobile and QuickAccessCardsDesktop) and select by breakpoint; update any logic that uses PAGE_SCROLL_AMOUNT to compute scroll by deriving values from rendered element widths (e.g., clientWidth) instead of CARD_WIDTH constants; and update arrow controls to meet mobile touch targets (use classes like h-11 and min-w-[44px] for the arrow buttons) — apply the same changes where these constants appear (also referenced at lines noted in the review).
42-45:⚠️ Potential issue | 🟠 MajorRestore or remove the dropped prop contract.
ChatAreastill passeshideSelected,isTeamsLoading,onRefreshTeams, andshowWizardButton, but this component now discards all four and only filtersdefaultTeam. That silently changes behavior for the only caller, so please either wire these props back up here or remove them from both sides in the same PR.Also applies to: 104-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx` around lines 42 - 45, QuickAccessCards currently accepts hideSelected, isTeamsLoading, onRefreshTeams, and showWizardButton but ignores them (only using defaultTeam), breaking ChatArea's contract; restore the original behavior by wiring these props into the component logic (use hideSelected to filter out selected cards, use isTeamsLoading to show loading state, call onRefreshTeams where refresh action occurs, and render the wizard button when showWizardButton is true) or remove these props from both QuickAccessCards and its caller in the same PR; locate QuickAccessCards (component) and update its prop handling and render path (or update ChatArea to stop passing the props) so the prop contract is consistent.
214-249:⚠️ Potential issue | 🟠 MajorUse a semantic button for the selectable card.
The outer card is still a clickable
<div>, so it is not keyboard-focusable and does not expose selection state. This should be a<button type="button">witharia-pressed={isSelected}, and the temporary click lock should usedisabledinstead ofpointer-events-none.Suggested fix
- <div - onClick={() => !isClicked && handleTeamClick(team)} + <button + type="button" + onClick={() => !isClicked && handleTeamClick(team)} + aria-pressed={isSelected} + disabled={isClicked} className={` group relative flex flex-col justify-center cursor-pointer transition-all duration-200 ${ isSelected ? 'border-l-[3px] border-l-primary border-y border-r border-border bg-primary/5' : 'border border-border bg-base' } ${isClicked ? 'clicking-card' : ''} - ${isClicked ? 'pointer-events-none' : ''} ${!isSelected ? 'hover:shadow-[0_2px_12px_0_rgba(0,0,0,0.1)]' : ''} `} style={{ width: CARD_WIDTH, height: 78, padding: '8px 12px', borderRadius: 20, flexShrink: 0, flexGrow: 0, }} > ... - </div> + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx` around lines 214 - 249, Replace the outer clickable <div> in QuickAccessCards.tsx with a semantic <button type="button">: make the element a button (still using CARD_WIDTH, height 78, padding and borderRadius styles), set aria-pressed={isSelected}, and use disabled={isClicked} instead of applying the pointer-events-none class; update the onClick to call handleTeamClick(team) (relying on the disabled prop to prevent clicks) and remove the conditional !isClicked in the inline onClick handler and any pointer-events-none class usages so keyboard focusability, accessibility and selection state are exposed correctly.
🧹 Nitpick comments (2)
frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx (1)
310-310: Use design system hover color instead of hardcoded RGB.
hover:bg-[rgb(238,238,238)]bypasses theme variables. If the theme is updated, this won't adapt. The same pattern appears on line 334.♻️ Suggested fix
- className="w-full justify-between px-3 h-9 text-sm text-text-primary hover:bg-[rgb(238,238,238)] dark:hover:bg-white/10 rounded-md group transition-all duration-200 hover:scale-[1.02]" + className="w-full justify-between px-3 h-9 text-sm text-text-primary hover:bg-hover rounded-md group transition-all duration-200 hover:scale-[1.02]"Apply the same change at line 334:
- : 'text-text-primary hover:bg-[rgb(238,238,238)] dark:hover:bg-white/10 hover:scale-[1.02]' + : 'text-text-primary hover:bg-hover hover:scale-[1.02]'As per coding guidelines: "Use Tailwind CSS classes for styling...
className='bg-surface border-border'for cards/panels".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx` at line 310, The class string in TaskSidebar.tsx contains a hardcoded hover color `hover:bg-[rgb(238,238,238)]` (and the same pattern at the second occurrence) which bypasses the design system; replace those hardcoded classes with the theme/token-based Tailwind hover utility used across the codebase (for example `hover:bg-surface` or your project's equivalent design token like `hover:bg-surface-hover` or `hover:bg-[var(--bg-surface-hover)]`) in the className on the TaskSidebar component so the hover style responds to theme updates, and make the identical replacement for the other occurrence mentioned in the review.frontend/src/features/tasks/components/chat/QuickAccessCards.tsx (1)
255-284: The click animation is still hard-coded to the old teal accent.
pulse-glowuses literalrgba(20, 184, 166, ...), so this effect will drift from the new theme palette and dark-mode tuning. Please derive the glow color from the existing primary theme token/CSS variable instead of baking the RGB value into the component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx` around lines 255 - 284, The pulse-glow keyframe hard-codes teal via rgba(20, 184, 166, ...); update pulse-glow to derive its glow color from the theme CSS variable instead of a literal RGB. Replace occurrences of rgba(20,184,166, X) in QuickAccessCards.tsx (pulse-glow / .clicking-card) with a theme-based value such as using a primary RGB var (e.g. rgba(var(--color-primary-rgb), 0.4) and rgba(var(--color-primary-rgb), 0)) or using color-mix()/color() with var(--color-primary) so the glow follows the app’s primary token and dark-mode adjustments; ensure the variable name you use matches the existing theme token in your global CSS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx`:
- Around line 313-340: The aria-labels for the left/right scroll buttons are
hard-coded English; import useTranslation from '@/hooks/useTranslation' at the
top of QuickAccessCards.tsx, call const { t } = useTranslation('tasks') (or the
same namespace used elsewhere in this component), and replace the literal
aria-label values on the buttons with t('scroll_left') and t('scroll_right') (or
similar keys), and add those keys to the tasks translations so screen readers
get localized strings; ensure you update both the left-button (scrollLeft) and
right-button (scrollRight) instances.
---
Outside diff comments:
In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx`:
- Around line 843-854: The "Load more" button in TaskSidebar renders with py-1.5
which yields a mobile touch target under 44px; update the button for the
loadMorePersonalTasks block (the element using hasMorePersonalTasks,
loadMorePersonalTasks, loadingMorePersonalTasks) to use a minimum 44px touch
target by replacing vertical padding with explicit height and minimum width
utilities (e.g., add h-11 and min-w-[44px]) while preserving existing flex, gap
and text classes so alignment and styles remain consistent.
- Around line 702-727: The expand/collapse button's touch area is too small
(uses py-1.5) — update the button rendering (the element using onClick that
references isGroupChatsExpanded, loadMoreGroupTasks, setIsGroupChatsExpanded,
hasMoreGroupTasks and collapsedReadCount) to meet the 44×44px mobile minimum by
replacing the size classes: remove the small vertical padding and add sizing
classes such as h-11 and min-w-[44px] (keep items-center and gap-1 to preserve
layout and other classes like px-1 and text-xs as needed), ensuring the visual
alignment and behavior remain the same while enlarging the hit target for
mobile.
---
Duplicate comments:
In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx`:
- Around line 13-21: The layout constants (CONTAINER_WIDTH, CONTAINER_HEIGHT,
CARD_WIDTH, CARD_GAP, CARDS_PER_PAGE, PAGE_SCROLL_AMOUNT) are hard-coded for
desktop and must be converted to a responsive implementation: replace fixed
pixel sizing with a mobile-first approach (use w-full and max-w-* constraints on
the container and compute card widths via CSS classes/layout rather than
constants) or split into two components (QuickAccessCardsMobile and
QuickAccessCardsDesktop) and select by breakpoint; update any logic that uses
PAGE_SCROLL_AMOUNT to compute scroll by deriving values from rendered element
widths (e.g., clientWidth) instead of CARD_WIDTH constants; and update arrow
controls to meet mobile touch targets (use classes like h-11 and min-w-[44px]
for the arrow buttons) — apply the same changes where these constants appear
(also referenced at lines noted in the review).
- Around line 42-45: QuickAccessCards currently accepts hideSelected,
isTeamsLoading, onRefreshTeams, and showWizardButton but ignores them (only
using defaultTeam), breaking ChatArea's contract; restore the original behavior
by wiring these props into the component logic (use hideSelected to filter out
selected cards, use isTeamsLoading to show loading state, call onRefreshTeams
where refresh action occurs, and render the wizard button when showWizardButton
is true) or remove these props from both QuickAccessCards and its caller in the
same PR; locate QuickAccessCards (component) and update its prop handling and
render path (or update ChatArea to stop passing the props) so the prop contract
is consistent.
- Around line 214-249: Replace the outer clickable <div> in QuickAccessCards.tsx
with a semantic <button type="button">: make the element a button (still using
CARD_WIDTH, height 78, padding and borderRadius styles), set
aria-pressed={isSelected}, and use disabled={isClicked} instead of applying the
pointer-events-none class; update the onClick to call handleTeamClick(team)
(relying on the disabled prop to prevent clicks) and remove the conditional
!isClicked in the inline onClick handler and any pointer-events-none class
usages so keyboard focusability, accessibility and selection state are exposed
correctly.
---
Nitpick comments:
In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx`:
- Around line 255-284: The pulse-glow keyframe hard-codes teal via rgba(20, 184,
166, ...); update pulse-glow to derive its glow color from the theme CSS
variable instead of a literal RGB. Replace occurrences of rgba(20,184,166, X) in
QuickAccessCards.tsx (pulse-glow / .clicking-card) with a theme-based value such
as using a primary RGB var (e.g. rgba(var(--color-primary-rgb), 0.4) and
rgba(var(--color-primary-rgb), 0)) or using color-mix()/color() with
var(--color-primary) so the glow follows the app’s primary token and dark-mode
adjustments; ensure the variable name you use matches the existing theme token
in your global CSS.
In `@frontend/src/features/tasks/components/sidebar/TaskSidebar.tsx`:
- Line 310: The class string in TaskSidebar.tsx contains a hardcoded hover color
`hover:bg-[rgb(238,238,238)]` (and the same pattern at the second occurrence)
which bypasses the design system; replace those hardcoded classes with the
theme/token-based Tailwind hover utility used across the codebase (for example
`hover:bg-surface` or your project's equivalent design token like
`hover:bg-surface-hover` or `hover:bg-[var(--bg-surface-hover)]`) in the
className on the TaskSidebar component so the hover style responds to theme
updates, and make the identical replacement for the other occurrence mentioned
in the review.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25e3e858-bb0c-40ff-b049-48d745e3931f
📒 Files selected for processing (2)
frontend/src/features/tasks/components/chat/QuickAccessCards.tsxfrontend/src/features/tasks/components/sidebar/TaskSidebar.tsx
Comment on lines
+313
to
+340
| aria-label="Scroll left" | ||
| > | ||
| <span className="text-xs font-normal text-text-primary">{t('teams.more')}</span> | ||
| <ChevronDownIcon | ||
| className={`w-2.5 h-2.5 text-text-muted transition-transform duration-200 ${showMoreTeams ? 'rotate-180' : ''}`} | ||
| /> | ||
| <ChevronLeftIcon className="w-5 h-5 text-text-muted" /> | ||
| </button> | ||
| )} | ||
|
|
||
| {/* Dropdown with team list */} | ||
| {showMoreTeams && ( | ||
| <div className="absolute top-full left-0 mt-2 z-50 min-w-[300px] max-w-[400px] bg-surface border border-border rounded-xl shadow-xl overflow-hidden"> | ||
| {/* Search input */} | ||
| <div className="p-2 border-b border-border"> | ||
| <div className="relative"> | ||
| <MagnifyingGlassIcon className="absolute left-3 top-1/2 -translate-y-1/2 w-4 h-4 text-text-muted" /> | ||
| <input | ||
| type="text" | ||
| value={searchQuery} | ||
| onChange={e => setSearchQuery(e.target.value)} | ||
| placeholder={t('teams.search_team')} | ||
| className="w-full pl-9 pr-3 py-2 text-sm bg-base border border-border rounded-lg focus:outline-none focus:ring-2 focus:ring-primary/20 placeholder:text-text-muted" | ||
| autoFocus | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Team list */} | ||
| <div className="max-h-[240px] overflow-y-auto"> | ||
| {isTeamsLoading ? ( | ||
| <div className="py-4 text-center text-sm text-text-muted"> | ||
| {t('actions.loading')} | ||
| </div> | ||
| ) : dropdownTeams.length === 0 ? ( | ||
| <div className="py-4 text-center text-sm text-text-muted"> | ||
| {t('teams.no_match')} | ||
| </div> | ||
| ) : ( | ||
| dropdownTeams.map(team => { | ||
| const isSelected = selectedTeam?.id === team.id | ||
| const isSharedTeam = team.share_status === 2 && team.user?.user_name | ||
| const isGroupTeamItem = team.namespace && team.namespace !== 'default' | ||
| const isPublicTeam = 'user_id' in team && team.user_id === 0 | ||
| const isPersonalTeamItem = !isPublicTeam && !isGroupTeamItem | ||
|
|
||
| return ( | ||
| <div | ||
| key={team.id} | ||
| onClick={() => handleTeamSelectFromDropdown(team)} | ||
| className={` | ||
| flex items-center gap-3 px-3 py-2 mx-1 my-0.5 rounded-md cursor-pointer | ||
| transition-colors duration-150 | ||
| ${ | ||
| isSelected | ||
| ? 'bg-primary/10 text-primary' | ||
| : 'hover:bg-hover text-text-primary' | ||
| } | ||
| `} | ||
| > | ||
| <CheckIcon | ||
| className={`w-4 h-4 flex-shrink-0 ${isSelected ? 'opacity-100 text-primary' : 'opacity-0'}`} | ||
| /> | ||
| <TeamIconDisplay | ||
| iconId={team.icon} | ||
| size="sm" | ||
| className="flex-shrink-0 text-text-muted" | ||
| /> | ||
| <span className="flex-1 text-sm font-medium truncate" title={team.name}> | ||
| {team.name} | ||
| </span> | ||
| {isPersonalTeamItem && ( | ||
| <Tag className="text-xs !m-0 flex-shrink-0" variant="info"> | ||
| {t('settings.personal')} | ||
| </Tag> | ||
| )} | ||
| {isGroupTeamItem && ( | ||
| <Tag className="text-xs !m-0 flex-shrink-0" variant="default"> | ||
| {team.namespace} | ||
| </Tag> | ||
| )} | ||
| {isSharedTeam && ( | ||
| <Tag | ||
| className="text-xs !m-0 flex-shrink-0" | ||
| variant="default" | ||
| style={sharedBadgeStyle} | ||
| > | ||
| {team.user?.user_name} | ||
| </Tag> | ||
| )} | ||
| </div> | ||
| ) | ||
| }) | ||
| )} | ||
| </div> | ||
|
|
||
| {/* Footer with settings link */} | ||
| <div | ||
| className="border-t border-border bg-base cursor-pointer group flex items-center space-x-2 px-3 py-2.5 text-xs text-text-secondary hover:bg-muted transition-colors duration-150" | ||
| onClick={() => { | ||
| setShowMoreTeams(false) | ||
| router.push(paths.settings.team.getHref()) | ||
| }} | ||
| > | ||
| <Cog6ToothIcon className="w-4 h-4 text-text-secondary group-hover:text-text-primary" /> | ||
| <span className="font-medium group-hover:text-text-primary"> | ||
| {t('teams.manage')} | ||
| </span> | ||
| </div> | ||
| </div> | ||
| )} | ||
| <div | ||
| ref={scrollContainerRef} | ||
| className="flex items-center gap-3 overflow-x-auto hide-scrollbar" | ||
| style={{ | ||
| maxWidth: CARDS_PER_PAGE * CARD_WIDTH + (CARDS_PER_PAGE - 1) * CARD_GAP, | ||
| }} | ||
| > | ||
| {displayTeams.map(team => ( | ||
| <div key={team.id}>{renderTeamCard(team)}</div> | ||
| ))} | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Divider */} | ||
| <div className="h-7 w-px bg-border mx-1" /> | ||
|
|
||
| {/* Wizard button - quick create agent (only show in chat mode) */} | ||
| {showWizardButton && ( | ||
| <TooltipProvider> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| onClick={() => setShowWizard(true)} | ||
| className="flex items-center justify-center w-8 h-8 rounded-full border border-dashed border-primary/50 bg-primary/5 hover:bg-primary/10 hover:border-primary cursor-pointer transition-all duration-200" | ||
| > | ||
| <Wand2 className="w-4 h-4 text-primary" /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="top"> | ||
| <p>{t('wizard:wizard_button_tooltip')}</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| </TooltipProvider> | ||
| )} | ||
| </div> | ||
|
|
||
| {/* Team Creation Wizard Dialog */} | ||
| {showWizardButton && ( | ||
| <TeamCreationWizard | ||
| open={showWizard} | ||
| onClose={() => setShowWizard(false)} | ||
| onSuccess={async (teamId, _) => { | ||
| // Refresh teams list first to get the new team | ||
| if (onRefreshTeams) { | ||
| const refreshedTeams = await onRefreshTeams() | ||
| // Find and select the new team from refreshed list | ||
| const newTeam = refreshedTeams.find(t => t.id === teamId) | ||
| if (newTeam) { | ||
| onTeamSelect(newTeam) | ||
| } | ||
| } else { | ||
| // Fallback: try to find in current teams (may not work for newly created) | ||
| const newTeam = teams.find(t => t.id === teamId) | ||
| if (newTeam) { | ||
| onTeamSelect(newTeam) | ||
| } | ||
| } | ||
| }} | ||
| /> | ||
| )} | ||
| {needsPagination && canScrollRight && ( | ||
| <button | ||
| onClick={scrollRight} | ||
| className="absolute right-0 z-10 flex items-center justify-center cursor-pointer hover:opacity-80 transition-opacity bg-gradient-to-l from-transparent via-base/80 to-base" | ||
| style={{ | ||
| width: 36, | ||
| height: 86, | ||
| borderRadius: 10, | ||
| }} | ||
| aria-label="Scroll right" |
Contributor
There was a problem hiding this comment.
Localize the scroll button labels.
aria-label="Scroll left" and aria-label="Scroll right" bypass the translation layer, so screen-reader users will still hear English in non-English locales. Please move these strings into the same namespace used elsewhere in this component.
Suggested fix
- aria-label="Scroll left"
+ aria-label={t('teams.scroll_left')}
...
- aria-label="Scroll right"
+ aria-label={t('teams.scroll_right')}As per coding guidelines "Use useTranslation from @/hooks/useTranslation (not from react-i18next) with a single namespace matching your feature, and use t('namespace:key') format for cross-namespace keys".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/features/tasks/components/chat/QuickAccessCards.tsx` around
lines 313 - 340, The aria-labels for the left/right scroll buttons are
hard-coded English; import useTranslation from '@/hooks/useTranslation' at the
top of QuickAccessCards.tsx, call const { t } = useTranslation('tasks') (or the
same namespace used elsewhere in this component), and replace the literal
aria-label values on the buttons with t('scroll_left') and t('scroll_right') (or
similar keys), and add those keys to the tasks translations so screen readers
get localized strings; ensure you update both the left-button (scrollLeft) and
right-button (scrollRight) instances.
Adapt selectTestTeam() helper to work with the refactored QuickAccessCards component: - Remove dependency on deleted "More" button (removed in commit 0e00c84) - Add pagination support using left/right scroll arrows - Update team card selector from button to div elements - Add support for new TeamSelectorButton component (added in commit 4646cbe) - Use role="button" instead of role="option" in TeamSelectorButton popover Fixes two failing E2E tests: - should upload image via browser and verify model receives correct image_url format - should display model response after sending image Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/e2e/tests/tasks/chat-image-browser-e2e.spec.ts (1)
514-517:⚠️ Potential issue | 🟠 MajorE2E tests should not use
test.skip()for conditional skipping.Multiple instances of
test.skip()are used throughout the tests (lines 516, 532, 539, 575, 594, 614, 689, 695, 722, 734, 749). Per repository guidelines, E2E tests must not fail gracefully usingtest.skip(),test.fixme(), or conditional skips based on environment detection.Consider one of these approaches:
- Use
test.fail()to mark known failures that should be investigated- Use proper assertions that fail the test with clear error messages
- Move prerequisite checks to
test.beforeAlland throw errors there to skip the entire suiteExample refactor for prerequisite checks
test('should verify mock model server is running', async ({ request }) => { const response = await request.get(`${MOCK_MODEL_SERVER_URL}/health`) - if (response.status() !== 200) { - test.skip() - return - } + expect(response.status(), 'Mock model server should be running').toBe(200) const data = await response.json() expect(data.status).toBe('ok') })For resource-dependent tests, fail with a descriptive message:
// Skip if team was not created if (!createdTeamId) { - console.warn('Test team was not created, skipping test') - test.skip() - return + throw new Error('Test team was not created - check beforeAll setup') }Based on learnings: "E2E tests MUST NOT fail gracefully: DO NOT use test.skip(), test.fixme(), or conditional skips based on environment detection"
Also applies to: 528-540, 572-577, 590-595, 610-615, 687-696, 719-724, 732-735, 746-750
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/e2e/tests/tasks/chat-image-browser-e2e.spec.ts` around lines 514 - 517, The conditional use of test.skip() inside the response status check (the block with if (response.status() !== 200) { test.skip() ... }) must be replaced by a hard failure or moved to a suite-level prerequisite: either throw/assert inside a test.beforeAll that validates the resource and aborts the suite on failure, or call test.fail() / an explicit expect(...).toBe(200) with a descriptive message so the test fails clearly (update the response.status() handling in the same block and replicate the change for the other occurrences mentioned).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/e2e/tests/tasks/chat-image-browser-e2e.spec.ts`:
- Around line 514-517: The conditional use of test.skip() inside the response
status check (the block with if (response.status() !== 200) { test.skip() ... })
must be replaced by a hard failure or moved to a suite-level prerequisite:
either throw/assert inside a test.beforeAll that validates the resource and
aborts the suite on failure, or call test.fail() / an explicit
expect(...).toBe(200) with a descriptive message so the test fails clearly
(update the response.status() handling in the same block and replicate the
change for the other occurrences mentioned).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23d5e24b-5f50-49e7-bda1-5905548e65d6
📒 Files selected for processing (1)
frontend/e2e/tests/tasks/chat-image-browser-e2e.spec.ts
Enhance dismissOnboardingTour() to properly handle driver.js overlay: - Press Escape multiple times to dismiss all tour steps - Verify overlay is dismissed before proceeding - Click outside overlay as fallback if still visible Add dismissOnboardingTour() call before clicking message input: - Prevents "subtree intercepts pointer events" error from driver-overlay - Use force: true as additional safety for click operations Fixes timeout errors in both image browser E2E tests caused by tour overlay blocking interactions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ractions Add dismissOnboardingTour() calls before clicking: - Team card in QuickAccessCards (both direct and after scrolling) - Model selector button - Use force: true for all clicks to bypass any remaining overlays This ensures the driver-overlay is dismissed before every interaction attempt, preventing "subtree intercepts pointer events" errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
qdaxb
approved these changes
Mar 10, 2026
feifei325
approved these changes
Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces comprehensive UI improvements to the chat interface, focusing on team/agent selection, component refactoring, and visual consistency with the design system.
Key Changes:
Changes
Files Changed
Test Plan
Screenshots
[Add screenshots showing before/after of sidebar, team selector, and chat layout]
Summary by CodeRabbit
New Features
UI & Design Updates
Localization