Fix: everyday ui bugs - #5194
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
web/oss/src/components/AgentChatSlice/AgentChatPanel.tsx (1)
1676-1680: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueCollapse animation won't play when the queue empties.
QueuedMessagesreturnsnullwhenqueued.length === 0, so when the last message is deleted or released, theRevealCollapsegrid row height snaps to 0 instantly instead of animating from1fr→0frover 300ms. TheRevealCollapsecomponent's own documentation notes: "Callers that render nothing when 'closed' should latch their last content so it persists through the leave."This is an intentional trade-off per the PR objective (preventing deleted messages from lingering), so the instant disappearance is acceptable. If you want to preserve the animation without showing deleted messages, consider latching the last non-empty
queuedarray but filtering out any individually removed message IDs from the latched copy.web/oss/src/components/AgentChatSlice/assets/markdown.tsx (1)
134-139: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMove
{...rest}before the security-critical attributes to prevent override.Spreading
{...rest}aftertarget="_blank"andrel="noopener noreferrer"means anytargetorrelvalue from the markdown renderer would silently override the safe defaults. Reversing the order ensures the security attributes always win. Also consider replacinganywithReact.ComponentProps<'a'>for type safety.♻️ Proposed fix
-const Anchor = ({href, children, ...rest}: any) => ( - <a href={href} target="_blank" rel="noopener noreferrer" {...rest}> +const Anchor = ({href, children, ...rest}: React.ComponentProps<"a">) => ( + <a {...rest} href={href} target="_blank" rel="noopener noreferrer"> {children} </a> )web/oss/src/components/AgentChatSlice/components/AgentMessage.tsx (1)
305-319: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCopy logic is correct and well-structured.
copyTextproperly combines answer and error text, and the early return when empty prevents no-op clipboard writes. The\n\nseparator is consistent with how partial output + error is displayed in the bubble.One minor guideline note: the comment at lines 305-306 spans two lines. As per coding guidelines, in-code comments should be kept to one short line maximum unless a genuinely surprising constraint requires a brief exception. Consider condensing:
📝 Condense comment to one line
- // Copy the answer; append the error on a failed turn (and copy it alone on an answer-less - // failure) so the button isn't a no-op when the agent only returned an error. + // Copy answer + error (error alone on answer-less failure) so the button isn't a no-op. const copyText = [fullText, errorText].filter(Boolean).join("\n\n")Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d9e74c12-04c3-46fa-ade8-e3057e358a9d
📒 Files selected for processing (3)
web/oss/src/components/AgentChatSlice/AgentChatPanel.tsxweb/oss/src/components/AgentChatSlice/assets/markdown.tsxweb/oss/src/components/AgentChatSlice/components/AgentMessage.tsx
… improve inline value wrapping
|
@ashrafchowdury please from now on One issue == One PR! Each PR /write-pr-description please a screenshot. Set your goal to the max number of PRs per week. Think about how to automate this! Use gitbutler, it will help you work on multiple things in parallel |
Context
Three small AgentChat rough edges from QA: