chg(tasks): change order of task name, fix(chat): attachment alignment#2908
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR updates UserMessage component styling by removing vertical spacing from the outer wrapper and restructuring the attachments container layout, while refactoring branch name generation logic and adding error handling for clipboard operations. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/core/component/AI/component/message/UserMessage.tsx`:
- Line 90: Change the attachment container in UserMessage.tsx from using the
column layout to a row layout so attachments wrap into new rows rather than new
columns: locate the div with class "flex flex-col flex-wrap justify-end
items-end gap-1 w-full px-1 mb-2" and replace the "flex-col" utility with
"flex-row" (keep the other classes like flex-wrap, justify-end, items-end,
gap-1, w-full, px-1, mb-2) to match Attachment.tsx behavior and ensure
right-aligned chips wrap into additional rows; visually verify with one image,
multiple images, and mixed image/item-preview messages.
In `@js/app/packages/core/util/branchName.ts`:
- Around line 42-48: The try/catch should only wrap the clipboard operation so
failures in downstream side-effects don't mislead the user: await
navigator.clipboard.writeText(branchName) should be inside the try, and on
success (after the await completes) call
analytics.track('task_copy_branch_name') and toast.success('Branch name copied
to clipboard'); keep toast.failure('Could not copy branch name') in the catch.
Update the code around navigator.clipboard.writeText, analytics.track,
toast.success, and toast.failure so only the writeText call is guarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2cfabcda-3551-429b-886e-30e4980efea0
📒 Files selected for processing (2)
js/app/packages/core/component/AI/component/message/UserMessage.tsxjs/app/packages/core/util/branchName.ts
| </Show> | ||
| <Show when={props.message.attachments.length > 0}> | ||
| <div class="flex flex-col space-y-2 w-full px-1 pb-1 mb-2"> | ||
| <div class="flex flex-col flex-wrap justify-end items-end gap-1 w-full px-1 mb-2"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the UserMessage.tsx file around line 90
cat -n js/app/packages/core/component/AI/component/message/UserMessage.tsx | sed -n '75,105p'Repository: macro-inc/macro
Length of output: 1457
🏁 Script executed:
# Also check if there's any git context about this file to understand the PR intent
git log --oneline --all -- js/app/packages/core/component/AI/component/message/UserMessage.tsx | head -5Repository: macro-inc/macro
Length of output: 427
🏁 Script executed:
# Check for recent commits or PR info mentioning attachment alignment
git log --oneline --all | grep -i "attachment\|alignment" | head -10Repository: macro-inc/macro
Length of output: 805
🏁 Script executed:
# Get the full diff for the attachment alignment commit
git show 2e98a0aa --statRepository: macro-inc/macro
Length of output: 50372
🏁 Script executed:
# Check if there are test files for UserMessage
fd -type f "UserMessage" js/app/packages/core/component/AI/component/message/Repository: macro-inc/macro
Length of output: 230
🏁 Script executed:
# Look for any related test or snapshot files
fd -type f -name "*UserMessage*" js/app/Repository: macro-inc/macro
Length of output: 230
🏁 Script executed:
# Also check the broader context of the div - what comes before and after
cat -n js/app/packages/core/component/AI/component/message/UserMessage.tsx | sed -n '88,120p'Repository: macro-inc/macro
Length of output: 1366
🏁 Script executed:
# Look for test or story files related to UserMessage
find js/app -name "*UserMessage*" -o -name "*Attachment*" | grep -E "\.(test|spec|stories)\.(tsx?|jsx?|mdx)$"Repository: macro-inc/macro
Length of output: 41
🏁 Script executed:
# Check for Storybook stories related to chat or messages
find js/app -path "*stories*" -name "*[Cc]hat*" -o -path "*stories*" -name "*[Mm]essage*"Repository: macro-inc/macro
Length of output: 41
🏁 Script executed:
# Look at other chat/message components to see layout patterns
rg -l "flex-wrap" js/app/packages/core/component/AI/component/ -g "*.tsx" -g "*.ts"Repository: macro-inc/macro
Length of output: 255
🏁 Script executed:
# Check the git commit to see what actually changed
git diff 2e98a0aa~1 2e98a0aa -- js/app/packages/core/component/AI/component/message/UserMessage.tsx | head -100Repository: macro-inc/macro
Length of output: 92
🏁 Script executed:
# Check the Attachment component to see how it's structured
cat -n js/app/packages/core/component/AI/component/input/Attachment.tsxRepository: macro-inc/macro
Length of output: 5398
🏁 Script executed:
# Check SendEmail.tsx for comparison of flex-wrap usage
cat -n js/app/packages/core/component/AI/component/tool/SendEmail.tsx | grep -A 10 -B 5 "flex-wrap"Repository: macro-inc/macro
Length of output: 1580
🏁 Script executed:
# Look at the entire UserMessage component to understand the parent container
cat -n js/app/packages/core/component/AI/component/message/UserMessage.tsx | head -120Repository: macro-inc/macro
Length of output: 5041
Switch flex-col to flex-row for attachment wrapping behavior.
With flex-col, items stack vertically and flex-wrap causes them to wrap into adjacent columns. This creates a multi-column layout, which doesn't match typical chat attachment display. The input attachment component (Attachment.tsx) uses flex-row with flex-wrap, which wraps items into new rows instead.
For right-aligned attachment chips that wrap to new rows as intended, use:
♻️ Proposed fix
- <div class="flex flex-col flex-wrap justify-end items-end gap-1 w-full px-1 mb-2">
+ <div class="flex flex-row flex-wrap justify-end items-end gap-1 w-full px-1 mb-2">Verify visually with messages containing (a) one image, (b) multiple images, and (c) mixed image and item-preview attachments to confirm the layout matches design intent.
📝 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.
| <div class="flex flex-col flex-wrap justify-end items-end gap-1 w-full px-1 mb-2"> | |
| <div class="flex flex-row flex-wrap justify-end items-end gap-1 w-full px-1 mb-2"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/core/component/AI/component/message/UserMessage.tsx` at line
90, Change the attachment container in UserMessage.tsx from using the column
layout to a row layout so attachments wrap into new rows rather than new
columns: locate the div with class "flex flex-col flex-wrap justify-end
items-end gap-1 w-full px-1 mb-2" and replace the "flex-col" utility with
"flex-row" (keep the other classes like flex-wrap, justify-end, items-end,
gap-1, w-full, px-1, mb-2) to match Attachment.tsx behavior and ensure
right-aligned chips wrap into additional rows; visually verify with one image,
multiple images, and mixed image/item-preview messages.
| try { | ||
| await navigator.clipboard.writeText(branchName); | ||
| analytics.track('task_copy_branch_name'); | ||
| toast.success('Branch name copied to clipboard'); | ||
| } catch { | ||
| toast.failure('Could not copy branch name'); | ||
| } |
There was a problem hiding this comment.
Narrow the try to the clipboard call only.
analytics.track and toast.success are inside the try, so any throw from them after a successful clipboard write will surface "Could not copy branch name" to the user even though the copy actually succeeded. Scope the try to writeText and run the success-side effects after it resolves.
♻️ Proposed fix
- try {
- await navigator.clipboard.writeText(branchName);
- analytics.track('task_copy_branch_name');
- toast.success('Branch name copied to clipboard');
- } catch {
- toast.failure('Could not copy branch name');
- }
+ try {
+ await navigator.clipboard.writeText(branchName);
+ } catch {
+ toast.failure('Could not copy branch name');
+ return;
+ }
+ analytics.track('task_copy_branch_name');
+ toast.success('Branch name copied to clipboard');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/core/util/branchName.ts` around lines 42 - 48, The try/catch
should only wrap the clipboard operation so failures in downstream side-effects
don't mislead the user: await navigator.clipboard.writeText(branchName) should
be inside the try, and on success (after the await completes) call
analytics.track('task_copy_branch_name') and toast.success('Branch name copied
to clipboard'); keep toast.failure('Could not copy branch name') in the catch.
Update the code around navigator.clipboard.writeText, analytics.track,
toast.success, and toast.failure so only the writeText call is guarded.
1 participant
No description provided.