-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Improving chat input aesthetics #2561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import { toast } from "sonner"; | |
| import { LocalAgentNewChatToast } from "./LocalAgentNewChatToast"; | ||
| import { useAtomValue } from "jotai"; | ||
| import { chatMessagesByIdAtom } from "@/atoms/chatAtoms"; | ||
| import { Hammer, Bot, MessageCircle, Lightbulb } from "lucide-react"; | ||
|
|
||
| function NewBadge() { | ||
| return ( | ||
|
|
@@ -95,6 +96,22 @@ export function ChatModeSelector() { | |
| return "Build"; | ||
| } | ||
| }; | ||
|
|
||
| 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} />; | ||
| } | ||
|
Comment on lines
+100
to
+113
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM | consistency / maintainability | Found by: Code Health, UX Wizard
The icon for each mode is defined in two places:
If a mode's icon changes, both must be updated. The Suggestion: Extract a single |
||
| }; | ||
| const isMac = detectIsMac(); | ||
|
|
||
| return ( | ||
|
|
@@ -109,18 +126,25 @@ export function ChatModeSelector() { | |
| <MiniSelectTrigger | ||
| data-testid="chat-mode-selector" | ||
| className={cn( | ||
| "h-6 w-fit px-1.5 py-0 text-xs-sm font-medium shadow-none gap-0.5", | ||
| selectedMode === "build" || | ||
| selectedMode === "local-agent" || | ||
| selectedMode === "plan" | ||
| ? "bg-background hover:bg-muted/50 focus:bg-muted/50" | ||
| : "bg-primary/10 hover:bg-primary/20 focus:bg-primary/20 text-primary border-primary/20 dark:bg-primary/20 dark:hover:bg-primary/30 dark:focus:bg-primary/30", | ||
| "cursor-pointer w-fit px-2 py-0 text-xs font-medium border-none shadow-none gap-1 rounded-lg transition-colors", | ||
| selectedMode === "build" || selectedMode === "local-agent" | ||
| ? "text-foreground/80 hover:text-foreground hover:bg-muted/60" | ||
| : selectedMode === "ask" | ||
| ? "bg-purple-500/10 text-purple-600 hover:bg-purple-500/15 dark:bg-purple-500/15 dark:text-purple-400 dark:hover:bg-purple-500/20" | ||
| : selectedMode === "plan" | ||
| ? "bg-blue-500/10 text-blue-600 hover:bg-blue-500/15 dark:bg-blue-500/15 dark:text-blue-400 dark:hover:bg-blue-500/20" | ||
| : "text-foreground/80 hover:text-foreground hover:bg-muted/60", | ||
| )} | ||
|
Comment on lines
128
to
137
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM | consistency / complexity | Found by: Code Health, UX Build/Agent modes visually blend into toolbar; nested ternary is hard to maintain Two related issues:
💡 Suggestion: Extract a |
||
| size="sm" | ||
| /> | ||
| } | ||
| > | ||
| <SelectValue>{getModeDisplayName(selectedMode)}</SelectValue> | ||
| <SelectValue> | ||
| <span className="flex items-center gap-1.5"> | ||
| {getModeIcon(selectedMode)} | ||
| {getModeDisplayName(selectedMode)} | ||
| </span> | ||
| </SelectValue> | ||
| </TooltipTrigger> | ||
| <TooltipContent> | ||
| {`Open mode menu (${isMac ? "\u2318 + ." : "Ctrl + ."} to toggle)`} | ||
|
|
@@ -131,10 +155,11 @@ export function ChatModeSelector() { | |
| <SelectItem value="local-agent"> | ||
| <div className="flex flex-col items-start"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <Bot size={14} className="text-muted-foreground" /> | ||
| <span className="font-medium">Agent v2</span> | ||
| <NewBadge /> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground"> | ||
| <span className="text-xs text-muted-foreground ml-[22px]"> | ||
| Better at bigger tasks and debugging | ||
| </span> | ||
| </div> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM | consistency / maintainability | Found by: Code Health, Endorsed by: UX, Correctness Brittle The description text uses 💡 Suggestion: Use |
||
|
|
@@ -143,10 +168,11 @@ export function ChatModeSelector() { | |
| <SelectItem value="plan"> | ||
| <div className="flex flex-col items-start"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <Lightbulb size={14} className="text-blue-500" /> | ||
| <span className="font-medium">Plan</span> | ||
| <NewBadge /> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground"> | ||
| <span className="text-xs text-muted-foreground ml-[22px]"> | ||
| Design before you build | ||
| </span> | ||
| </div> | ||
|
|
@@ -155,13 +181,14 @@ export function ChatModeSelector() { | |
| <SelectItem value="local-agent" disabled={isQuotaExceeded}> | ||
| <div className="flex flex-col items-start"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <Bot size={14} className="text-muted-foreground" /> | ||
| <span className="font-medium">Basic Agent</span> | ||
| <span className="text-xs text-muted-foreground"> | ||
| ({isQuotaExceeded ? "0" : messagesRemaining}/5 remaining for | ||
| today) | ||
| </span> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground"> | ||
| <span className="text-xs text-muted-foreground ml-[22px]"> | ||
| {isQuotaExceeded | ||
| ? "Daily limit reached" | ||
| : "Try our AI agent for free"} | ||
|
|
@@ -171,16 +198,22 @@ export function ChatModeSelector() { | |
| )} | ||
| <SelectItem value="build"> | ||
| <div className="flex flex-col items-start"> | ||
| <span className="font-medium">Build</span> | ||
| <span className="text-xs text-muted-foreground"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <Hammer size={14} className="text-muted-foreground" /> | ||
| <span className="font-medium">Build</span> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground ml-[22px]"> | ||
| Generate and edit code | ||
| </span> | ||
| </div> | ||
| </SelectItem> | ||
| <SelectItem value="ask"> | ||
| <div className="flex flex-col items-start"> | ||
| <span className="font-medium">Ask</span> | ||
| <span className="text-xs text-muted-foreground"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <MessageCircle size={14} className="text-purple-500" /> | ||
| <span className="font-medium">Ask</span> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground ml-[22px]"> | ||
| Ask questions about the app | ||
| </span> | ||
| </div> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -92,6 +92,7 @@ import { useCountTokens } from "@/hooks/useCountTokens"; | |||||||||||||||||||||||||
| import { useChats } from "@/hooks/useChats"; | ||||||||||||||||||||||||||
| import { useRouter } from "@tanstack/react-router"; | ||||||||||||||||||||||||||
| import { showError as showErrorToast } from "@/lib/toast"; | ||||||||||||||||||||||||||
| import { cn } from "@/lib/utils"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const showTokenBarAtom = atom(false); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -424,7 +425,7 @@ export function ChatInput({ chatId }: { chatId?: number }) { | |||||||||||||||||||||||||
| {t("errorLoadingProposal", { message: proposalError.message })} | ||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||
| <div className="p-4" data-testid="chat-input-container"> | ||||||||||||||||||||||||||
| <div className="p-2 pt-0" data-testid="chat-input-container"> | ||||||||||||||||||||||||||
| {/* Show context limit banner above chat input for visibility */} | ||||||||||||||||||||||||||
| {showBanner && tokenCountResult && ( | ||||||||||||||||||||||||||
| <ContextLimitBanner | ||||||||||||||||||||||||||
|
|
@@ -433,9 +434,12 @@ export function ChatInput({ chatId }: { chatId?: number }) { | |||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||
| <div | ||||||||||||||||||||||||||
| className={`relative flex flex-col border border-border rounded-lg bg-(--background-lighter) shadow-sm ${ | ||||||||||||||||||||||||||
| 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", | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Tailwind CSS class
Suggested change
|
||||||||||||||||||||||||||
| "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", | ||||||||||||||||||||||||||
|
Comment on lines
+438
to
+441
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 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 Root CauseThe new classes The same issue exists in Previously (before this PR), there were no 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.
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||
|
Comment on lines
+437
to
+442
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM — CSS specificity conflict: focus-within ring vs drag-over ring 2/3 agents flagged this (both MEDIUM) When The same pattern exists in Suggested fix: Conditionally omit the focus-within ring when dragging:
Suggested change
|
||||||||||||||||||||||||||
| onDragOver={handleDragOver} | ||||||||||||||||||||||||||
| onDragLeave={handleDragLeave} | ||||||||||||||||||||||||||
| onDrop={handleDrop} | ||||||||||||||||||||||||||
|
|
@@ -566,7 +570,7 @@ export function ChatInput({ chatId }: { chatId?: number }) { | |||||||||||||||||||||||||
| onCancel={cancelPendingFiles} | ||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| <div className="flex items-start space-x-2 "> | ||||||||||||||||||||||||||
| <div className="flex items-end gap-1"> | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM | interaction-design | Found by: UX, Endorsed by: Correctness
The input row changed from 💡 Suggestion: Verify visually with a single line of text that the send icon aligns cleanly. May need |
||||||||||||||||||||||||||
| <LexicalChatInput | ||||||||||||||||||||||||||
| value={inputValue} | ||||||||||||||||||||||||||
| onChange={setInputValue} | ||||||||||||||||||||||||||
|
|
@@ -585,7 +589,7 @@ export function ChatInput({ chatId }: { chatId?: number }) { | |||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||||
| onClick={handleCancel} | ||||||||||||||||||||||||||
| aria-label={t("cancelGeneration")} | ||||||||||||||||||||||||||
| className="px-2 py-2 mt-1 mr-1 hover:bg-(--background-darkest) text-(--sidebar-accent-fg) rounded-lg" | ||||||||||||||||||||||||||
| className="px-2 py-2 mb-0.5 mr-1 text-muted-foreground hover:text-destructive rounded-lg transition-colors duration-150 cursor-pointer" | ||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||
|
|
@@ -604,7 +608,7 @@ export function ChatInput({ chatId }: { chatId?: number }) { | |||||||||||||||||||||||||
| disableSendButton | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| 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" | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM | accessibility | Found by: UX Wizard, Endorsed by: Correctness, Code Health
The disabled send button uses The same pattern appears in 💡 Suggestion: Use |
||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||
|
|
@@ -614,7 +618,7 @@ export function ChatInput({ chatId }: { chatId?: number }) { | |||||||||||||||||||||||||
| </Tooltip> | ||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||
| <div className="pl-2 pr-1 flex items-center justify-between pb-2"> | ||||||||||||||||||||||||||
| <div className="px-2 flex items-center justify-between pb-0.5 pt-0.5"> | ||||||||||||||||||||||||||
| <div className="flex items-center"> | ||||||||||||||||||||||||||
| <ChatInputControls showContextFilesPicker={false} /> | ||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,7 @@ export function ContextLimitBanner({ | |
|
|
||
| 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" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 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 The previous Additionally, the 💡 Suggestion: Either add |
||
| data-testid="context-limit-banner" | ||
| > | ||
| <span className="flex items-center gap-1.5"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||
| import { SendIcon, StopCircleIcon } from "lucide-react"; | ||||||
| import { SendHorizontalIcon, StopCircleIcon } from "lucide-react"; | ||||||
| import { | ||||||
| Tooltip, | ||||||
| TooltipTrigger, | ||||||
|
|
@@ -20,6 +20,7 @@ import { LexicalChatInput } from "./LexicalChatInput"; | |||||
| import { useChatModeToggle } from "@/hooks/useChatModeToggle"; | ||||||
| import { useTypingPlaceholder } from "@/hooks/useTypingPlaceholder"; | ||||||
| import { AuxiliaryActionsMenu } from "./AuxiliaryActionsMenu"; | ||||||
| import { cn } from "@/lib/utils"; | ||||||
|
|
||||||
| export function HomeChatInput({ | ||||||
| onSubmit, | ||||||
|
|
@@ -85,9 +86,12 @@ export function HomeChatInput({ | |||||
| <> | ||||||
| <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", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Tailwind CSS class
Suggested change
|
||||||
| "hover:border-primary/30", | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM | consistency | Found by: Code Health, Endorsed by: UX
HomeChatInput adds a 💡 Suggestion: Either add |
||||||
| "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} | ||||||
|
Comment on lines
86
to
96
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM | correctness / visual regression | Found by: Correctness, Endorsed by: UX
The old code used |
||||||
| onDrop={handleDrop} | ||||||
|
|
@@ -108,7 +112,7 @@ export function HomeChatInput({ | |||||
| onCancel={cancelPendingFiles} | ||||||
| /> | ||||||
|
|
||||||
| <div className="flex items-start space-x-2 "> | ||||||
| <div className="flex items-end gap-1"> | ||||||
| <LexicalChatInput | ||||||
| value={inputValue} | ||||||
| onChange={setInputValue} | ||||||
|
|
@@ -127,7 +131,7 @@ export function HomeChatInput({ | |||||
| render={ | ||||||
| <button | ||||||
| aria-label="Cancel generation (unavailable here)" | ||||||
| className="px-2 py-2 mt-1 mr-1 text-(--sidebar-accent-fg) rounded-lg opacity-50 cursor-not-allowed" | ||||||
| className="px-2 py-2 mb-0.5 mr-1 text-muted-foreground rounded-lg opacity-50 cursor-not-allowed transition-colors duration-150" | ||||||
| /> | ||||||
| } | ||||||
| > | ||||||
|
|
@@ -145,17 +149,17 @@ export function HomeChatInput({ | |||||
| onClick={handleCustomSubmit} | ||||||
| disabled={!inputValue.trim() && attachments.length === 0} | ||||||
| aria-label="Send message" | ||||||
| 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" | ||||||
| /> | ||||||
| } | ||||||
| > | ||||||
| <SendIcon size={20} /> | ||||||
| <SendHorizontalIcon size={20} /> | ||||||
| </TooltipTrigger> | ||||||
| <TooltipContent>Send message</TooltipContent> | ||||||
| </Tooltip> | ||||||
| )} | ||||||
| </div> | ||||||
| <div className="pl-2 pr-1 flex items-center justify-between pb-2"> | ||||||
| <div className="px-2 flex items-center justify-between pb-0.5 pt-0.5"> | ||||||
| <div className="flex items-center"> | ||||||
| <ChatInputControls showContextFilesPicker={false} /> | ||||||
| </div> | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM | consistency / dead-code | Found by: All 3 reviewers (unanimous)
Leftover spacer div inconsistent with new gap approach
The PR adds
gap-0.5to 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, butgap-0.5 + w-1.5(2px + 6px = 8px) between McpToolsPicker and ModelPicker.💡 Suggestion: Remove the remaining spacer div and let
gap-0.5handle all spacing uniformly (or increasegapif more spacing is desired between all items).