Improving chat input aesthetics#2561
Conversation
|
@BugBot run |
Summary of ChangesHello @azizmejri1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user interface and experience of the chat input area by applying a series of aesthetic improvements. The changes aim to create a more polished, modern, and consistent look and feel across various interactive elements, improving visual harmony and user feedback without altering core functionality. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a series of aesthetic improvements to the chat input and related components, resulting in a cleaner and more consistent user interface. The changes effectively refactor CSS classes using cn, standardize spacing with gap, and update component styles for better visual harmony.
I've identified a minor but important issue with invalid Tailwind CSS syntax for an arbitrary background color value in both ChatInput.tsx and HomeChatInput.tsx. I've left specific comments with suggestions to correct this, which will improve the code's robustness and prevent potential issues with future Tailwind versions or tooling.
| isDraggingOver ? "ring-2 ring-blue-500 border-blue-500" : "" | ||
| } ${showBanner ? "rounded-t-none border-t-0" : ""}`} | ||
| className={cn( | ||
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", |
There was a problem hiding this comment.
The Tailwind CSS class bg-(--background-lighter) uses invalid syntax for arbitrary values. It should be bg-[--background-lighter] to correctly reference the CSS variable. This will ensure it's parsed correctly by Tailwind and avoid potential issues with tooling or future updates.
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | |
| "relative flex flex-col border border-border rounded-2xl bg-[--background-lighter] transition-colors duration-200", |
| isDraggingOver ? "ring-2 ring-blue-500 border-blue-500" : "" | ||
| }`} | ||
| className={cn( | ||
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", |
There was a problem hiding this comment.
The Tailwind CSS class bg-(--background-lighter) uses invalid syntax for arbitrary values. It should be bg-[--background-lighter] to correctly reference the CSS variable. This will ensure it's parsed correctly by Tailwind and avoid potential issues with tooling or future updates.
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | |
| "relative flex flex-col border border-border rounded-2xl bg-[--background-lighter] transition-colors duration-200", |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile OverviewGreptile SummaryPolished the chat input UI for a cleaner, more modern appearance with consistent visual hierarchy. Key improvements:
All changes are purely visual/CSS updates with no logic modifications. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ChatInput
participant Controls as ChatInputControls
participant ModeSelector as ChatModeSelector
participant ModelPicker
participant ProSelector as ProModeSelector
participant McpPicker as McpToolsPicker
participant LexicalInput as LexicalChatInput
User->>ChatInput: Focus input
ChatInput->>ChatInput: Apply focus-within ring
ChatInput->>LexicalInput: Render text editor (15px text)
User->>Controls: View control bar
Controls->>ModeSelector: Render mode selector
ModeSelector->>ModeSelector: Show icon + mode name
Note over ModeSelector: Purple accent for Ask<br/>Blue accent for Plan
Controls->>ModelPicker: Render model picker
ModelPicker->>ModelPicker: Show "Model: Auto" (muted)
Controls->>ProSelector: Render Pro selector
ProSelector->>ProSelector: Show Sparkles icon + "Pro"
alt MCP enabled in Build mode
Controls->>McpPicker: Render MCP tools picker
McpPicker->>McpPicker: Show Wrench icon (size 3.5)
end
User->>ModeSelector: Click mode selector
ModeSelector->>ModeSelector: Show dropdown with icons
User->>ModeSelector: Select new mode
ModeSelector->>ModeSelector: Update with new icon/color
User->>ChatInput: Hover send button
ChatInput->>ChatInput: Change to primary color
User->>ChatInput: Click send
ChatInput->>ChatInput: Submit message
|
🔍 Multi-Agent Code ReviewFound 3 new issue(s) flagged by 3 independent reviewers (consensus voting). Summary
Issues to Address
🟢 Low Priority Issues (2 items)
See inline comments for details on MEDIUM issues. Generated by multi-agent consensus review (3 agents, 2+ agreement threshold) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| className={cn( | ||
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | ||
| "focus-within:border-primary/30 focus-within:ring-1 focus-within:ring-primary/20", | ||
| isDraggingOver && "ring-2 ring-blue-500 border-blue-500", | ||
| showBanner && "rounded-t-none border-t-0", | ||
| )} |
There was a problem hiding this comment.
🟡 MEDIUM — CSS specificity conflict: focus-within ring vs drag-over ring
2/3 agents flagged this (both MEDIUM)
When isDraggingOver is true, ring-2 ring-blue-500 border-blue-500 is applied alongside the always-present focus-within:ring-1 focus-within:ring-primary/20. If the user is typing (input focused) and drags a file over, both ring styles activate simultaneously. The focus-within ring color (ring-primary/20) may bleed through or conflict with the drag-over blue ring, producing an inconsistent visual.
The same pattern exists in HomeChatInput.tsx.
Suggested fix: Conditionally omit the focus-within ring when dragging:
| className={cn( | |
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | |
| "focus-within:border-primary/30 focus-within:ring-1 focus-within:ring-primary/20", | |
| isDraggingOver && "ring-2 ring-blue-500 border-blue-500", | |
| showBanner && "rounded-t-none border-t-0", | |
| )} | |
| className={cn( | |
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | |
| !isDraggingOver && "focus-within:border-primary/30 focus-within:ring-1 focus-within:ring-primary/20", | |
| isDraggingOver && "ring-2 ring-blue-500 border-blue-500", | |
| showBanner && "rounded-t-none border-t-0", | |
| )} |
| <DropdownMenu open={isOpen} onOpenChange={setIsOpen}> | ||
| <DropdownMenuTrigger | ||
| className="inline-flex items-center justify-center whitespace-nowrap rounded-xl text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 hover:bg-muted bg-primary/10 text-primary cursor-pointer h-8 px-2" | ||
| className="inline-flex items-center justify-center whitespace-nowrap rounded-full text-sm font-medium transition-all duration-200 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 hover:bg-primary/20 hover:scale-105 bg-primary/10 text-primary cursor-pointer h-8 w-8" |
There was a problem hiding this comment.
🟡 MEDIUM — hover:scale-105 causes layout jank and ignores prefers-reduced-motion
3/3 agents flagged this (1 MEDIUM, 2 LOW)
hover:scale-105 on a flex-child button can cause visual jitter in adjacent elements. Additionally, transition-all duration-200 animates all CSS properties (including layout properties like width/height), which can cause unwanted animation when the dropdown opens/closes.
More importantly, this scale animation doesn't respect prefers-reduced-motion, which is an accessibility concern.
Suggested fix:
| className="inline-flex items-center justify-center whitespace-nowrap rounded-full text-sm font-medium transition-all duration-200 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 hover:bg-primary/20 hover:scale-105 bg-primary/10 text-primary cursor-pointer h-8 w-8" | |
| className="inline-flex items-center justify-center whitespace-nowrap rounded-full text-sm font-medium transition-colors duration-200 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 hover:bg-primary/20 motion-safe:hover:scale-105 bg-primary/10 text-primary cursor-pointer h-8 w-8" |
Use transition-colors instead of transition-all, and prefix scale with motion-safe: to respect accessibility preferences.
| <PopoverTrigger className="inline-flex items-center justify-center whitespace-nowrap rounded-lg text-xs font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 border-none bg-transparent shadow-none text-primary/80 hover:text-primary hover:bg-primary/10 h-7 px-2 gap-1 cursor-pointer" /> | ||
| } | ||
| > | ||
| <Sparkles className="h-4 w-4 text-primary" /> | ||
| <span className="text-primary font-medium text-xs-sm">Pro</span> | ||
| <Sparkles className="h-3.5 w-3.5" /> | ||
| <span className="font-medium">Pro</span> | ||
| </TooltipTrigger> |
There was a problem hiding this comment.
🟡 MEDIUM — Pro selector lost distinctive visual treatment, reducing discoverability
2/3 agents flagged this (1 MEDIUM, 1 LOW)
The old ProModeSelector had a visible border (border border-primary/50), shadow (shadow-sm shadow-primary/10), and hover shadow effect that made it stand out as a premium feature. The new version uses border-none bg-transparent shadow-none, making it visually identical to the ModelPicker and McpToolsPicker.
Additionally, the Sparkles icon lost its explicit text-primary class and the "Pro" span lost text-primary — they now inherit from the parent's text-primary/80, making the branding subtly dimmer.
If the intent is to de-emphasize Pro, this is fine. But if Pro should remain visually distinctive for discoverability/conversion, consider keeping some visual differentiation (e.g., a subtle background tint or retaining the explicit text-primary on the icon and label).
wwwillchen
left a comment
wwwillchen
left a comment
There was a problem hiding this comment.
feedback from mtg: every mode should have an icon. for non-agent/build mode, they should get a distinct color so that users are aware they are in a non-standard mode.
azizmejri1
force-pushed
the
improve-chat-input-ui
branch
from
8a2c918 to
c82df6d
Compare
azizmejri1
force-pushed
the
improve-chat-input-ui
branch
from
c82df6d to
5f81e7c
Compare
|
@BugBot run |
1 similar comment
|
@BugBot run |
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 specialized agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
🟢 Low Priority Notes (3 items)
Generated by Dyadbot swarm code review (3 agents, with cross-review discussion) |
| <div className="flex items-center gap-0.5"> | ||
| <ChatModeSelector /> | ||
| {showMcpToolsPicker && ( | ||
| <> |
There was a problem hiding this comment.
🟡 MEDIUM | consistency / dead-code | Found by: All 3 reviewers (unanimous)
Leftover spacer div inconsistent with new gap approach
The PR adds gap-0.5 to the parent flex container and removes several spacer <div> elements, but leaves one <div className="w-1.5"></div> between the MCP tools section and ModelPicker. This spacer now stacks with the gap, producing inconsistent spacing: gap-0.5 (2px) between most items, but gap-0.5 + w-1.5 (2px + 6px = 8px) between McpToolsPicker and ModelPicker.
💡 Suggestion: Remove the remaining spacer div and let gap-0.5 handle all spacing uniformly (or increase gap if more spacing is desired between all items).
| } | ||
| aria-label={t("sendMessage")} | ||
| className="px-2 py-2 mt-1 mr-1 hover:bg-(--background-darkest) text-(--sidebar-accent-fg) rounded-lg disabled:opacity-50" | ||
| className="px-2 py-2 mb-0.5 mr-1 text-muted-foreground hover:text-primary rounded-lg transition-colors duration-150 disabled:opacity-30 disabled:hover:text-muted-foreground cursor-pointer disabled:cursor-default" |
There was a problem hiding this comment.
🟡 MEDIUM | accessibility | Found by: UX Wizard, Endorsed by: Correctness, Code Health
disabled:opacity-30 may fail WCAG 2.1 contrast requirements
The disabled send button uses disabled:opacity-30 (down from disabled:opacity-50). Combined with the already-muted text-muted-foreground color, the resulting contrast ratio likely falls below the WCAG 2.1 SC 1.4.11 minimum of 3:1 for non-text UI components. Users may not realize the send button exists when disabled.
The same pattern appears in HomeChatInput.tsx.
💡 Suggestion: Use disabled:opacity-40 or disabled:opacity-50 to maintain sufficient visibility. Test contrast against WCAG 3:1 minimum on both light and dark themes.
|
@BugBot run |
@wwwillchen done |
🔍 Dyadbot Code Review SummaryVerdict: 🤔 NOT SURE - Potential issues Reviewed by 3 specialized agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
🟢 Low Priority Notes (2 items)
✅ Already Flagged by Other Reviewers (7 items)These issues were already identified by other review bots and confirmed by our reviewers:
Generated by Dyadbot code review |
| <span className="text-xs text-muted-foreground ml-[22px]"> | ||
| Better at bigger tasks and debugging | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
🟡 MEDIUM | consistency / maintainability | Found by: Code Health, Endorsed by: UX, Correctness
Brittle ml-[22px] magic number repeated 5 times
The description text uses ml-[22px] to indent below the icon+label row. This hardcoded pixel value appears 5 times and is tightly coupled to the icon size (14px) + gap (1.5 = 6px) + some rounding. If the icon size or gap changes, all 5 instances must be updated manually.
💡 Suggestion: Use pl-[calc(14px+0.375rem)] to make the coupling explicit, or better yet, wrap each item's layout in a shared component/pattern that handles the indent structurally (e.g., using a grid or a shared padding class).
| <> | ||
| <div className="p-4" data-testid="home-chat-input-container"> | ||
| <div | ||
| className={`relative flex flex-col space-y-2 border border-border rounded-lg bg-(--background-lighter) shadow-sm ${ | ||
| isDraggingOver ? "ring-2 ring-blue-500 border-blue-500" : "" | ||
| }`} | ||
| className={cn( | ||
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | ||
| "hover:border-primary/30", | ||
| "focus-within:border-primary/30 focus-within:ring-1 focus-within:ring-primary/20", | ||
| isDraggingOver && "ring-2 ring-blue-500 border-blue-500", | ||
| )} | ||
| onDragOver={handleDragOver} | ||
| onDragLeave={handleDragLeave} |
There was a problem hiding this comment.
🟡 MEDIUM | correctness / visual regression | Found by: Correctness, Endorsed by: UX
space-y-2 removed without replacement
The old code used space-y-2 on the input container to create vertical spacing between the text area row and the controls row. The new code removes it without adding an equivalent gap, relying solely on the children's own padding. Verify this doesn't collapse the spacing between the input area and the controls bar — the ChatInput.tsx version doesn't have this issue because it never had space-y-2.
🔍 Dyadbot Code Review Summary (Corrected)
Verdict: 🤔 NOT SURE - Potential issues Reviewed by 3 specialized agents: Correctness Expert, Code Health Expert, UX Wizard. New Issues (not previously flagged)
🟢 Low Priority Notes (4 items)
✅ Already Flagged by Other Reviewers (7 items — all confirmed)These issues were already identified by other review bots and confirmed by our review team:
Generated by Dyadbot swarm code review (3 agents: Correctness, Code Health, UX) |
wwwillchen-bot
force-pushed
the
improve-chat-input-ui
branch
from
c6f8c93 to
b4f4eeb
Compare
wwwillchen-bot
temporarily deployed
to
ai-bots
GitHub Actions
Inactive
wwwillchen-bot
temporarily deployed
to
ai-bots
GitHub Actions
Inactive
|
@BugBot run |
🔍 Multi-Agent Code ReviewFound 1 new consensus issue flagged by 3 independent reviewers (consensus voting, 2+ agreement threshold). Summary
Issues to Address
🟢 Low Priority Issues (2 items — 2+ agents agreed but all LOW severity)
✅ Already Flagged by Other Reviewers (13 items — all confirmed)All 13 previously identified issues were confirmed by at least 2 of our 3 reviewers:
See inline comment for details on the MEDIUM issue. Generated by multi-agent consensus review (3 agents, 2+ agreement threshold) |
| return ( | ||
| <div | ||
| className="mx-auto max-w-3xl px-3 py-1.5 rounded-t-md border-t border-l border-r border-amber-500/30 bg-amber-500/10 flex items-center justify-between gap-3 text-xs text-amber-600 dark:text-amber-500" | ||
| className="mx-auto max-w-3xl px-3 py-1.5 rounded-t-2xl border-t border-l border-r border-amber-500/30 bg-amber-500/10 flex items-center justify-between gap-3 text-xs text-amber-600 dark:text-amber-500" |
There was a problem hiding this comment.
🟡 MEDIUM | visual / layout | Found by: Correctness Expert, Endorsed by: UX Wizard
ContextLimitBanner width constraint mismatches ChatInput container
2/3 agents flagged this (1 MEDIUM, 1 LOW)
The ContextLimitBanner uses max-w-3xl mx-auto (768px, centered), but the ChatInput wrapper div below it has no max-w-3xl constraint. When the banner is shown, the ChatInput applies rounded-t-none border-t-0 to visually join with the banner above. However, if the parent container is wider than 768px, the banner will be narrower and centered while the input stretches full width — the side borders won't align, breaking the seamless visual connection.
The previous rounded-t-md had the same structural issue, but the larger rounded-t-2xl radius makes the misalignment more noticeable.
Additionally, the focus-within:ring-1 on the ChatInput will still render on the top edge even when border-t-0 is set, creating a subtle colored line where the banner and input should seamlessly join.
💡 Suggestion: Either add max-w-3xl mx-auto to the ChatInput wrapper when the banner is shown, or remove it from the ContextLimitBanner. Also consider conditionally disabling the focus-within ring on the top edge when showBanner is true.
|
@BugBot run |
wwwillchen
left a comment
wwwillchen
left a comment
There was a problem hiding this comment.
thanks! looks a lot better.
i made a couple tweaks around spacing and making the color a little darker. appreciate the change!
|
@BugBot run |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | ||
| "focus-within:border-primary/30 focus-within:ring-1 focus-within:ring-primary/20", | ||
| isDraggingOver && "ring-2 ring-blue-500 border-blue-500", | ||
| showBanner && "rounded-t-none border-t-0", |
There was a problem hiding this comment.
🟡 CSS specificity: focus-within ring overrides drag-over ring indicator
When the chat input is focused (which is nearly always the case) and a user drags a file over the input, the drag-and-drop visual indicator (blue ring-2 + border-blue-500) is overridden by the focus-within ring styles because pseudo-class variants have higher CSS specificity than plain utility classes.
Root Cause
The new classes focus-within:border-primary/30 focus-within:ring-1 focus-within:ring-primary/20 are always active when the text input has focus. The drag indicator classes ring-2 ring-blue-500 border-blue-500 are conditionally applied when isDraggingOver is true. Because :focus-within pseudo-class variants have higher CSS specificity than bare utilities, focus-within:ring-1 wins over ring-2, and focus-within:border-primary/30 wins over border-blue-500.
The same issue exists in src/components/chat/HomeChatInput.tsx:89-93.
Previously (before this PR), there were no focus-within ring styles on the container, so the drag-over blue ring always displayed correctly.
Actual: During drag-over while focused, user sees a subtle 1px primary-colored ring instead of the intended prominent 2px blue ring.
Expected: The drag-over indicator (2px blue ring/border) should always be visible regardless of focus state, as it's a temporary high-priority visual signal.
Impact: Drag-and-drop file attachment visual feedback is degraded in the most common scenario (input is focused), making it harder for users to see the drop target.
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | |
| "focus-within:border-primary/30 focus-within:ring-1 focus-within:ring-primary/20", | |
| isDraggingOver && "ring-2 ring-blue-500 border-blue-500", | |
| showBanner && "rounded-t-none border-t-0", | |
| "relative flex flex-col border border-border rounded-2xl bg-(--background-lighter) transition-colors duration-200", | |
| !isDraggingOver && "focus-within:border-primary/30 focus-within:ring-1 focus-within:ring-primary/20", | |
| isDraggingOver && "ring-2 ring-blue-500 border-blue-500", | |
| showBanner && "rounded-t-none border-t-0", | |
Was this helpful? React with 👍 or 👎 to provide feedback.
🔍 Dyadbot Code Review Summary3 independent reviewers analyzed this PR. Found 2 new issue(s) after consensus voting. Methodology
New Issues
🟢 Low Priority Issues (not consensus — mentioned by single agent only)
See inline comments for details on the 2 consensus issues. Generated by Dyadbot multi-agent code review (3x Claude Opus) |
| const getModeIcon = (mode: ChatMode) => { | ||
| switch (mode) { | ||
| case "build": | ||
| case "agent": | ||
| return <Hammer size={14} />; | ||
| case "ask": | ||
| return <MessageCircle size={14} />; | ||
| case "local-agent": | ||
| return <Bot size={14} />; | ||
| case "plan": | ||
| return <Lightbulb size={14} />; | ||
| default: | ||
| return <Hammer size={14} />; | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM | consistency / maintainability | Found by: Code Health, UX Wizard
getModeIcon duplicates the icon-to-mode mapping that's repeated in each SelectItem below
The icon for each mode is defined in two places:
getModeIcon(lines 100–113) — used in the trigger- Each
SelectItem(lines 155–220) — with additional color classes
If a mode's icon changes, both must be updated. The default case also silently returns <Hammer />, masking any missing mapping for future modes.
Suggestion: Extract a single modeConfig record mapping each mode to { icon, displayName, color } and derive both the trigger icon and the dropdown items from it. For the default case, consider returning a generic icon so unknown modes are visually distinct.
| @@ -357,7 +357,7 @@ export const MessagesList = forwardRef<HTMLDivElement, MessagesListProps>( | |||
| } | |||
| return ( | |||
| <div | |||
| className="absolute inset-0 overflow-y-auto p-4" | |||
| className="absolute inset-0 overflow-y-auto p-4 pb-0 pr-0" | |||
| ref={ref} | |||
| data-testid="messages-list" | |||
| > | |||
| @@ -375,7 +375,7 @@ export const MessagesList = forwardRef<HTMLDivElement, MessagesListProps>( | |||
| if (isTestMode) { | |||
| return ( | |||
| <div | |||
| className="absolute inset-0 p-4 overflow-y-auto" | |||
| className="absolute inset-0 p-4 pb-0 pr-0 overflow-y-auto" | |||
| ref={ref} | |||
| data-testid="messages-list" | |||
| > | |||
| @@ -394,7 +394,7 @@ export const MessagesList = forwardRef<HTMLDivElement, MessagesListProps>( | |||
|
|
|||
| return ( | |||
| <div | |||
| className="absolute inset-0 overflow-y-auto p-4" | |||
| className="absolute inset-0 overflow-y-auto p-4 pb-0 mb-2 pr-0" | |||
There was a problem hiding this comment.
🟡 MEDIUM | consistency / duplication | Found by: Code Health, UX Wizard
Messages list wrapper div class duplicated 4 times with inconsistent variance
Four render paths use nearly identical class strings:
- Line 350:
"absolute inset-0 overflow-y-auto p-4 pb-0 pr-0" - Line 360:
"absolute inset-0 overflow-y-auto p-4 pb-0 pr-0" - Line 378:
"absolute inset-0 p-4 pb-0 pr-0 overflow-y-auto"(different class order) - Line 397:
"absolute inset-0 overflow-y-auto p-4 pb-0 mb-2 pr-0"(addsmb-2)
The mb-2 is only on the Virtuoso path, which is likely intentional, but the duplication makes it easy for these to drift. The pr-0 removal also means content will extend to the right edge of the container, which may look cramped on platforms with always-visible scrollbars (Windows/Linux).
Suggestion: Extract a shared constant like:
const messagesListBaseClass = "absolute inset-0 overflow-y-auto p-4 pb-0 pr-0";and use cn(messagesListBaseClass, isVirtualized && "mb-2") for the variant.
🎭 Playwright Test Results❌ Some tests failed
Summary: 692 passed, 3 failed, 12 flaky, 222 skipped Failed Tests🍎 macOS
📋 Re-run Failing Tests (macOS)Copy and paste to re-run all failing spec files locally: npm run e2e \
e2e-tests/problems.spec.ts \
e2e-tests/security_review.spec.ts
|
<!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2561" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Refreshes the chat input UI for a cleaner look and consistent controls. Adds per‑mode icons and clearer visual states across inputs, pickers, and actions. - **UI Improvements** - Unified control sizing (h-7, text-xs, rounded-lg); vertical align center; input row aligns to end; removed spacer divs. - Borderless triggers for Mode/Model/Tools/Pro with subtle hover; smaller icons; muted “Model:”; mode chips show icons for all modes; Ask/Plan get color accents. - Input boxes rounded-2xl with focus-within ring; home input gets hover border; context banner rounded-t-2xl. - Send/Cancel use text-color hovers with clearer disabled; Cancel turns destructive on hover; Auxiliary Actions is circular with gentle hover scale. - Increased input padding and 15px text with repositioned placeholder; trimmed outer and MessagesList padding for tighter alignment; home send icon switched to SendHorizontal. <sup>Written for commit 301875e. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: Will Chen <willchen90@gmail.com>
<!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2561" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Refreshes the chat input UI for a cleaner look and consistent controls. Adds per‑mode icons and clearer visual states across inputs, pickers, and actions. - **UI Improvements** - Unified control sizing (h-7, text-xs, rounded-lg); vertical align center; input row aligns to end; removed spacer divs. - Borderless triggers for Mode/Model/Tools/Pro with subtle hover; smaller icons; muted “Model:”; mode chips show icons for all modes; Ask/Plan get color accents. - Input boxes rounded-2xl with focus-within ring; home input gets hover border; context banner rounded-t-2xl. - Send/Cancel use text-color hovers with clearer disabled; Cancel turns destructive on hover; Auxiliary Actions is circular with gentle hover scale. - Increased input padding and 15px text with repositioned placeholder; trimmed outer and MessagesList padding for tighter alignment; home send icon switched to SendHorizontal. <sup>Written for commit 301875e. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: Will Chen <willchen90@gmail.com>
3 participants
Summary by cubic
Refreshes the chat input UI for a cleaner look and consistent controls, adding per‑mode icons and clearer visual states across inputs, pickers, and actions. Also updates the git workflow with a rebase tip for React wrapper class conflicts.
Written for commit d99bf48. Summary will update on new commits.