Length of reply message #721
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request primarily modify the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/components/message.tsx (1)
Line range hint
428-432: Enhance reply message interaction featuresWhile the truncation works, the copy functionality should ensure it copies the full message content, not the truncated version.
Consider updating the copy handler:
onClick={() => { - void navigator.clipboard.writeText(message.content); + const fullContent = message.type === "message" && message.replyTo + ? message.replyTo.content + : message.content; + void navigator.clipboard.writeText(fullContent); setIsModalOpen(!isModalOpen); toast.success("Copied to clipboard"); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/message.tsx(1 hunks)
🔇 Additional comments (1)
src/components/message.tsx (1)
50-52: Verify truncation impact across the codebase
Let's ensure the truncation doesn't affect other features or tests.
✅ Verification successful
Let me try with corrected file type flags for ripgrep.
Let me try one more time with simpler commands and without file type flags.
Truncation implementation is isolated and safe
The truncation of reply message content is only used in this single location within src/components/message.tsx for display purposes. There are no other usages of replyTo.content across the codebase, and no test files would be affected by this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of reply message content
echo "Checking for other usages of reply message content..."
rg -l "replyTo.*content" --type typescript --type tsx
# Search for related test files
echo "Checking for related test files..."
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts message
Length of output: 345
Script:
#!/bin/bash
# Search for other usages of reply message content
echo "Checking for other usages of reply message content..."
rg -l "replyTo.*content" -t ts -t tsx
# Search for test files and show their content
echo "Checking for related test files..."
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts message -x cat {}
Length of output: 334
Script:
#!/bin/bash
# Search for other usages of reply message content
echo "Searching for replyTo.content usage..."
rg "replyTo.*content"
# List and show content of message-related test files
echo "Looking for message-related test files..."
find . -type f -name "*message*test*" -o -name "*message*spec*"
Length of output: 495
| {message.replyTo.content.length < 20 | ||
| ? message.replyTo.content | ||
| : `${message.replyTo.content.slice(0, 20)}...`} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance message truncation implementation
The current implementation has several areas for improvement:
- The character limit is hard-coded
- The truncation might not handle multi-byte characters or emojis correctly
- Missing accessibility considerations for screen readers
Consider this improved implementation:
- {message.replyTo.content.length < 20
- ? message.replyTo.content
- : `${message.replyTo.content.slice(0, 20)}...`}
+ <span title={message.replyTo.content}>
+ {message.replyTo.content.length <= MAX_REPLY_LENGTH
+ ? message.replyTo.content
+ : `${message.replyTo.content.slice(0, MAX_REPLY_LENGTH)}…`}
+ </span>Add this constant at the top of the file:
const MAX_REPLY_LENGTH = 20; // Maximum length for reply message previewThe improvements include:
- Using a named constant for better maintainability
- Adding a title attribute for accessibility
- Using proper ellipsis character (…)
- Wrapping in a span for better semantic structure
Summary by CodeRabbit