Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR standardizes icon sizing across message and attachment components (consolidating mixed utilities to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
📝 Coding Plan
Comment |
4f4de3a to
c2d3fbf
Compare
87dc2d8 to
3aacda9
Compare
c2d3fbf to
76cc2e9
Compare
387f8af to
ee0c0f3
Compare
d87f1f4 to
3d30c18
Compare
ee0c0f3 to
380a9b1
Compare
3d30c18 to
6a6911d
Compare
380a9b1 to
4e310a2
Compare
6a6911d to
fa396ea
Compare
4e310a2 to
e4be9d9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
ui/components/prompts/components/messagesView/attachmentViews.tsx (1)
26-32: Consider addingdata-testidattributes to removal buttons in a follow-up PR.The removal buttons in
AttachmentBadgeandAttachmentDisplaylackdata-testidattributes. Per coding guidelines, interactive elements should follow the patterndata-testid="<entity>-<element>-<qualifier>"(e.g.,attachment-remove-btn).Based on learnings: "When adding data-testid attributes in UI components, plan to include them in a dedicated PR that also updates related e2e tests, rather than sprinkling data-testid additions across feature PRs."
Also applies to: 57-62, 76-83, 97-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/prompts/components/messagesView/attachmentViews.tsx` around lines 26 - 32, Add a data-testid attribute to the removal buttons in AttachmentBadge and AttachmentDisplay using the project pattern (e.g., data-testid="attachment-remove-btn") so e2e tests can target them; update the button that calls onRemove (the element rendering <XIcon className="size-3" />) and the other similar remove buttons in the file to use the same naming convention, and put these changes in a dedicated follow-up PR that also updates related e2e tests.ui/components/prompts/fragments/sidebar.tsx (2)
220-220: Prefer explicit vertical padding utility for readability.
p-2 px-3works, butpy-2 px-3is clearer and avoids override-style class stacking.♻️ Suggested cleanup
- <div className="flex flex-col p-2 px-3"> + <div className="flex flex-col py-2 px-3">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/prompts/fragments/sidebar.tsx` at line 220, The className on the div using "p-2 px-3" is ambiguous/redundant; update the element that renders the sidebar fragment (the div with className "flex flex-col p-2 px-3") to use explicit vertical padding by replacing "p-2" with "py-2" so the final className is "flex flex-col py-2 px-3" for clearer intent and to avoid override-style stacking.
361-363: Consider completing icon size token normalization in this component.A few icons in these updated paths still use explicit
h-4 w-4; if the stack is converging on shared size tokens, finishing that pass here would improve consistency.As per coding guidelines, "
**: always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs."Also applies to: 402-402, 415-415
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/prompts/fragments/sidebar.tsx` around lines 361 - 363, Replace the explicit size classes "h-4 w-4" on icon components in this sidebar fragment with the project's shared icon size token used across the PR stack; specifically update the FolderOpen and FolderIcon usages (and the other icon instances noted in the comment) to use the shared token/class instead of hard-coded "h-4 w-4" so all icons in this component follow the common size token convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ui/components/prompts/components/messagesView/attachmentViews.tsx`:
- Around line 26-32: Add a data-testid attribute to the removal buttons in
AttachmentBadge and AttachmentDisplay using the project pattern (e.g.,
data-testid="attachment-remove-btn") so e2e tests can target them; update the
button that calls onRemove (the element rendering <XIcon className="size-3" />)
and the other similar remove buttons in the file to use the same naming
convention, and put these changes in a dedicated follow-up PR that also updates
related e2e tests.
In `@ui/components/prompts/fragments/sidebar.tsx`:
- Line 220: The className on the div using "p-2 px-3" is ambiguous/redundant;
update the element that renders the sidebar fragment (the div with className
"flex flex-col p-2 px-3") to use explicit vertical padding by replacing "p-2"
with "py-2" so the final className is "flex flex-col py-2 px-3" for clearer
intent and to avoid override-style stacking.
- Around line 361-363: Replace the explicit size classes "h-4 w-4" on icon
components in this sidebar fragment with the project's shared icon size token
used across the PR stack; specifically update the FolderOpen and FolderIcon
usages (and the other icon instances noted in the comment) to use the shared
token/class instead of hard-coded "h-4 w-4" so all icons in this component
follow the common size token convention.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d354bee-0d1d-4f68-ac7e-20e226080f32
📒 Files selected for processing (10)
ui/components/prompts/components/messagesView/assistantMessageView.tsxui/components/prompts/components/messagesView/attachmentViews.tsxui/components/prompts/components/messagesView/errorMessageView.tsxui/components/prompts/components/messagesView/messageRoleSwitcher.tsxui/components/prompts/components/messagesView/rootMessageView.tsxui/components/prompts/components/messagesView/systemMessageView.tsxui/components/prompts/components/messagesView/toolCallResultView.tsxui/components/prompts/components/messagesView/toolCallView.tsxui/components/prompts/components/messagesView/userMessageView.tsxui/components/prompts/fragments/sidebar.tsx
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @impoiler. * #2119 (comment) The following files were modified: * `ui/components/prompts/components/messagesView/assistantMessageView.tsx` * `ui/components/prompts/components/messagesView/attachmentViews.tsx` * `ui/components/prompts/components/messagesView/errorMessageView.tsx` * `ui/components/prompts/components/messagesView/messageRoleSwitcher.tsx` * `ui/components/prompts/components/messagesView/rootMessageView.tsx` * `ui/components/prompts/components/messagesView/systemMessageView.tsx` * `ui/components/prompts/components/messagesView/toolCallResultView.tsx` * `ui/components/prompts/components/messagesView/toolCallView.tsx` * `ui/components/prompts/components/messagesView/userMessageView.tsx` * `ui/components/prompts/fragments/sidebar.tsx`
fa396ea to
f49ca80
Compare
e4be9d9 to
e4300ea
Compare
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)
ui/components/prompts/fragments/sidebar.tsx (1)
458-475:⚠️ Potential issue | 🟠 MajorPrompt row is click-only and not keyboard operable.
Line 463 renders the selectable prompt row as a plain
divwith onlyonClick, so keyboard users cannot activate selection. This is an accessibility regression after removing href-based interaction.♿ Suggested fix
function DraggablePromptItem({ prompt, isSelected, onSelect, onEdit, onDelete, canUpdate, canDelete }: DraggablePromptItemProps) { const { ref, isDragging } = useDraggable({ id: `prompt-${prompt.id}`, disabled: !canUpdate }); const showActions = canUpdate || canDelete; return ( <div ref={ref} data-testid={`sidebar-prompt-${prompt.id}`} + role="button" + tabIndex={0} + aria-current={isSelected ? "true" : undefined} className={cn( "group mb-1 flex h-[30px] cursor-pointer items-center gap-2 rounded-sm px-2 last:mb-0", isSelected ? "bg-primary/10 text-primary" : "hover:bg-muted/50", isDragging && "opacity-50", )} + onKeyDown={(e) => { + if (isDragging) return; + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onSelect(); + } + }} onClick={(e) => { // Don't navigate if this was a drag if (isDragging) return; onSelect(); }} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/prompts/fragments/sidebar.tsx` around lines 458 - 475, The prompt row in DraggablePromptItem is a plain div with only onClick, making it inaccessible to keyboard users; update the root element (the div with ref and data-testid `sidebar-prompt-${prompt.id}` in function DraggablePromptItem) to be keyboard-operable by adding tabIndex={0}, role="button" (or role="option" if part of a listbox), and an onKeyDown handler that triggers onSelect() when Enter or Space is pressed (respecting the existing isDragging guard so it doesn't activate during a drag). Also ensure focus styles remain visible and keep the existing onClick behavior intact.
🧹 Nitpick comments (1)
ui/components/prompts/components/messagesView/messageRoleSwitcher.tsx (1)
26-34: Consider addingdata-testidto the button for test coverage.The button element is an interactive trigger but lacks a
data-testidattribute. This is a pre-existing omission rather than something introduced by this PR. As per coding guidelines, interactive elements should have data-testid attributes following the pattern<entity>-<element>-<qualifier>.A suitable testid would be:
data-testid="message-role-switcher-btn".🧪 Optional: Add data-testid for test coverage
<button + data-testid="message-role-switcher-btn" className={cn( "-ml-1.5 flex items-center gap-1 rounded-sm px-1.5 py-0.5 text-xs font-medium uppercase", !disabled && "hover:bg-muted cursor-pointer", )} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/prompts/components/messagesView/messageRoleSwitcher.tsx` around lines 26 - 34, The button in the MessageRoleSwitcher component lacks a test identifier; update the button element in messageRoleSwitcher.tsx (the button that renders {role} and <ChevronDown /> and uses the cn(...) call) to include data-testid="message-role-switcher-btn" so tests can target this interactive trigger; ensure the attribute is added directly on the same <button> element that currently has the className and children.
🤖 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 `@ui/components/prompts/fragments/sidebar.tsx`:
- Around line 458-475: The prompt row in DraggablePromptItem is a plain div with
only onClick, making it inaccessible to keyboard users; update the root element
(the div with ref and data-testid `sidebar-prompt-${prompt.id}` in function
DraggablePromptItem) to be keyboard-operable by adding tabIndex={0},
role="button" (or role="option" if part of a listbox), and an onKeyDown handler
that triggers onSelect() when Enter or Space is pressed (respecting the existing
isDragging guard so it doesn't activate during a drag). Also ensure focus styles
remain visible and keep the existing onClick behavior intact.
---
Nitpick comments:
In `@ui/components/prompts/components/messagesView/messageRoleSwitcher.tsx`:
- Around line 26-34: The button in the MessageRoleSwitcher component lacks a
test identifier; update the button element in messageRoleSwitcher.tsx (the
button that renders {role} and <ChevronDown /> and uses the cn(...) call) to
include data-testid="message-role-switcher-btn" so tests can target this
interactive trigger; ensure the attribute is added directly on the same <button>
element that currently has the className and children.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5108d4d0-b129-4097-bbda-6af21c37aead
📒 Files selected for processing (10)
ui/components/prompts/components/messagesView/assistantMessageView.tsxui/components/prompts/components/messagesView/attachmentViews.tsxui/components/prompts/components/messagesView/errorMessageView.tsxui/components/prompts/components/messagesView/messageRoleSwitcher.tsxui/components/prompts/components/messagesView/rootMessageView.tsxui/components/prompts/components/messagesView/systemMessageView.tsxui/components/prompts/components/messagesView/toolCallResultView.tsxui/components/prompts/components/messagesView/toolCallView.tsxui/components/prompts/components/messagesView/userMessageView.tsxui/components/prompts/fragments/sidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- ui/components/prompts/components/messagesView/assistantMessageView.tsx
- ui/components/prompts/components/messagesView/attachmentViews.tsx
- ui/components/prompts/components/messagesView/toolCallView.tsx
- ui/components/prompts/components/messagesView/rootMessageView.tsx
- ui/components/prompts/components/messagesView/toolCallResultView.tsx
- ui/components/prompts/components/messagesView/userMessageView.tsx
- ui/components/prompts/components/messagesView/systemMessageView.tsx
Merge activity
|
* docs prompt repo * 📝 Add docstrings to `03-17-prompt_playground_style_fixes` Docstrings generation was requested by @impoiler. * #2119 (comment) The following files were modified: * `ui/components/prompts/components/messagesView/assistantMessageView.tsx` * `ui/components/prompts/components/messagesView/attachmentViews.tsx` * `ui/components/prompts/components/messagesView/errorMessageView.tsx` * `ui/components/prompts/components/messagesView/messageRoleSwitcher.tsx` * `ui/components/prompts/components/messagesView/rootMessageView.tsx` * `ui/components/prompts/components/messagesView/systemMessageView.tsx` * `ui/components/prompts/components/messagesView/toolCallResultView.tsx` * `ui/components/prompts/components/messagesView/toolCallView.tsx` * `ui/components/prompts/components/messagesView/userMessageView.tsx` * `ui/components/prompts/fragments/sidebar.tsx` --------- Co-authored-by: Suresh Chaudhary <hello@suresh.im> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Akshay Deo <akshay@akshaydeo.com>

Summary
Standardizes icon sizing across the prompts UI by replacing individual
h-*andw-*classes with thesize-*utility class, and updates hover states for attachment removal buttons to use a more consistent background color.Changes
h-3 w-3,h-3.5 w-3.5, andh-4 w-4classes withsize-3for consistent icon sizing across message views, attachment displays, and sidebar componentshover:bg-destructive/20tohover:bg-cardfor better visual consistencyp-4topx-1 py-4for improved spacingp-2top-2 px-3for better alignmentgetPromptHreffunction and related parameters from sidebar componentsType of change
Affected areas
How to test
Verify that all icons maintain consistent sizing and hover states work properly across the prompts interface:
Test the following areas:
Screenshots/Recordings
N/A - Visual changes are subtle consistency improvements to icon sizing and hover states.
Breaking changes
Related issues
N/A
Security considerations
None - this is a purely cosmetic refactoring of CSS classes.
Checklist
docs/contributing/README.mdand followed the guidelines