-
-
Notifications
You must be signed in to change notification settings - Fork 7
Ensure text wrapping for long sentences across UI components #473
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
Ensure text wrapping for long sentences across UI components #473
Conversation
- Updated `EmptyScreen` example buttons to use `whitespace-normal`, `text-left`, and `items-start` for better multi-line support. - Added `break-words` to `BotMessage` container to prevent horizontal overflow from long strings. - Improved `SuggestionsDropdown` and `SearchRelated` buttons with wrapping and top-alignment. - Replaced filename truncation with `break-all` wrapping in `ChatPanel` file attachments. - Ensured consistent icon alignment for multi-line buttons. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
WalkthroughThis PR contains styling and layout adjustments across five React components to improve text wrapping, alignment, and spacing. All changes are visual/presentational modifications to flex layouts, whitespace handling, and container sizing with no functional or control flow changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
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.
Text-wrapping improvements are generally solid, but break-all on filenames may reduce readability and is better replaced with break-words plus min-w-0. The BotMessage container still uses overflow-x-auto, which may preserve horizontal scrollbars even after adding wrapping; consider scoping overflow to pre blocks instead. Also, EmptyScreen’s list key remains potentially non-unique and could cause subtle rendering issues if messages repeat.
Additional notes (1)
- Maintainability |
components/message.tsx:21-24
overflow-x-autocombined withbreak-wordsis a bit contradictory: withbreak-words, long tokens should wrap, so horizontal scrolling may no longer be desirable and can still appear due to code blocks/tables. If the goal is strictly "no horizontal scrollbar", consider removingoverflow-x-auto(or scoping overflow handling to specific Markdown elements likepre).
As-is, this may still show scrollbars in cases where wrapping would be preferred (depending on browser and nested content).
Summary of changes
What changed
This PR adjusts Tailwind utility classes across multiple UI components to ensure long text wraps instead of overflowing/truncating.
Key updates
components/chat-panel.tsx: Attachment pill now top-aligns content (items-start), allows filename wrapping (break-all), and prevents the clear button from shrinking (shrink-0).components/empty-screen.tsx: Example message buttons now support multi-line text (whitespace-normal,text-left), align icon to the top (items-start,mt-1), and wrap heading in a flex container.components/message.tsx: Bot message container addsbreak-wordswhile keepingoverflow-x-auto.components/search-related.tsxandcomponents/suggestions-dropdown.tsx: Related/suggestion items now support multi-line layout (h-auto,items-start), top-aligned arrow icons, and wrap text in a flex child.
| <div className="flex items-start justify-between p-2 bg-muted rounded-lg"> | ||
| <span className="text-sm text-muted-foreground break-all mr-2"> | ||
| {selectedFile.name} | ||
| </span> | ||
| <Button variant="ghost" size="icon" onClick={clearAttachment} data-testid="clear-attachment-button"> | ||
| <Button variant="ghost" size="icon" className="shrink-0" onClick={clearAttachment} data-testid="clear-attachment-button"> | ||
| <X size={16} /> |
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.
break-all will split filenames at arbitrary characters, which can noticeably hurt readability (e.g., splitting extensions or UUIDs mid-token). For filenames, break-words (or overflow-wrap:anywhere via Tailwind break-words) is usually the better default, optionally combined with min-w-0 on the text container to allow flex shrinking.
Given the intent is to avoid horizontal overflow, consider switching to break-words (or break-all only when needed, e.g., for extremely long unbroken strings).
Suggestion
Consider swapping break-all for break-words and ensuring the flex child can shrink:
<div className="flex items-start justify-between p-2 bg-muted rounded-lg">
<span className="min-w-0 mr-2 text-sm text-muted-foreground break-words">
{selectedFile.name}
</span>
<Button
variant="ghost"
size="icon"
className="shrink-0"
onClick={clearAttachment}
data-testid="clear-attachment-button"
>
<X size={16} />
</Button>
</div>Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
| <Button | ||
| key={item.message} // Use a unique property as the key. | ||
| variant="link" | ||
| className="h-auto p-0 text-base flex items-center" | ||
| className="h-auto p-0 text-base flex items-start whitespace-normal text-left" | ||
| name={item.message} | ||
| onClick={async () => { | ||
| submitMessage(item.message); | ||
| }} | ||
| > | ||
| <Icon size={16} className="mr-2 text-muted-foreground" /> | ||
| {item.heading} | ||
| <Icon size={16} className="mr-2 mt-1 text-muted-foreground shrink-0" /> | ||
| <span className="flex-1">{item.heading}</span> | ||
| </Button> |
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.
The key for these items is item.message. If two example entries ever share the same message, React will treat them as the same element and can produce subtle UI bugs (wrong item updating/being focused). Since this is a user-facing list, it’s worth ensuring the key is truly unique (e.g., a stable id field), especially now that layout is more complex.
This is not introduced by your wrapping changes, but it’s in the modified block and is easy to harden while here.
Suggestion
If exampleMessages can be extended, prefer a guaranteed-unique key (e.g., item.id) or combine multiple fields:
key={`${item.message}-${item.heading}`}(Or add an id to the data model and use that.) Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@components/chat-panel.tsx`:
- Around line 295-302: Replace the span's CSS utility that forces
character-level breaks: in the JSX containing selectedFile.name (the span
adjacent to the clearAttachment Button in the chat panel), change the className
token "break-all" to "break-words" so filenames prefer natural break points
while still wrapping long strings; ensure the class update is applied to the
span rendering selectedFile.name and verify visual behavior near the Button with
data-testid "clear-attachment-button".
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
components/chat-panel.tsxcomponents/empty-screen.tsxcomponents/message.tsxcomponents/search-related.tsxcomponents/suggestions-dropdown.tsx
🔇 Additional comments (4)
components/message.tsx (1)
21-21: LGTM!Adding
break-wordsalongsideoverflow-x-autois a good approach for markdown content. It will break long words/URLs to prevent overflow while preserving horizontal scroll as a fallback for content that genuinely needs it (like wide code blocks or tables).components/search-related.tsx (1)
63-73: LGTM!The styling changes correctly enable multi-line text wrapping:
items-starton both the container and button ensures top-alignment when text wrapsh-autowithwhitespace-normal text-leftallows proper text flow- Icon's
mt-1 flex-shrink-0maintains alignment with the first line of textThis is consistent with the similar patterns applied in
empty-screen.tsxandsuggestions-dropdown.tsx.components/suggestions-dropdown.tsx (1)
98-110: LGTM!The multi-line text wrapping implementation is well-structured:
- Button uses
items-start h-auto whitespace-normal text-leftfor proper wrapping behavior- Icon has
mt-1 flex-shrink-0for consistent first-line alignment- Wrapping the query text in
<span className="flex-1">ensures proper flex distributionThis maintains visual consistency with
search-related.tsxandempty-screen.tsx.components/empty-screen.tsx (1)
41-52: LGTM!The button styling changes correctly implement multi-line text support:
items-start whitespace-normal text-leftenables proper text wrapping and alignment- Icon's
mt-1 shrink-0maintains consistent alignment with the first line- Wrapping the heading in
<span className="flex-1">ensures proper flex behaviorThe implementation is consistent with the patterns established in
search-related.tsxandsuggestions-dropdown.tsx.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <div className="flex items-start justify-between p-2 bg-muted rounded-lg"> | ||
| <span className="text-sm text-muted-foreground break-all mr-2"> | ||
| {selectedFile.name} | ||
| </span> | ||
| <Button variant="ghost" size="icon" onClick={clearAttachment} data-testid="clear-attachment-button"> | ||
| <Button variant="ghost" size="icon" className="shrink-0" onClick={clearAttachment} data-testid="clear-attachment-button"> | ||
| <X size={16} /> | ||
| </Button> | ||
| </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.
🧹 Nitpick | 🔵 Trivial
Consider break-words instead of break-all for filenames.
The break-all class breaks text at any character boundary, which can split words mid-character (e.g., document → docu-ment). For filenames, break-words (used in message.tsx) might provide more readable breaks by preferring natural break points while still handling long continuous strings.
That said, since filenames often lack natural word boundaries (e.g., very_long_filename_with_underscores.pdf), break-all may be intentional here. If you've tested and prefer the current behavior, this is fine to keep.
💡 Alternative using break-words
- <span className="text-sm text-muted-foreground break-all mr-2">
+ <span className="text-sm text-muted-foreground break-words mr-2">📝 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 className="flex items-start justify-between p-2 bg-muted rounded-lg"> | |
| <span className="text-sm text-muted-foreground break-all mr-2"> | |
| {selectedFile.name} | |
| </span> | |
| <Button variant="ghost" size="icon" onClick={clearAttachment} data-testid="clear-attachment-button"> | |
| <Button variant="ghost" size="icon" className="shrink-0" onClick={clearAttachment} data-testid="clear-attachment-button"> | |
| <X size={16} /> | |
| </Button> | |
| </div> | |
| <div className="flex items-start justify-between p-2 bg-muted rounded-lg"> | |
| <span className="text-sm text-muted-foreground break-words mr-2"> | |
| {selectedFile.name} | |
| </span> | |
| <Button variant="ghost" size="icon" className="shrink-0" onClick={clearAttachment} data-testid="clear-attachment-button"> | |
| <X size={16} /> | |
| </Button> | |
| </div> |
🤖 Prompt for AI Agents
In `@components/chat-panel.tsx` around lines 295 - 302, Replace the span's CSS
utility that forces character-level breaks: in the JSX containing
selectedFile.name (the span adjacent to the clearAttachment Button in the chat
panel), change the className token "break-all" to "break-words" so filenames
prefer natural break points while still wrapping long strings; ensure the class
update is applied to the span rendering selectedFile.name and verify visual
behavior near the Button with data-testid "clear-attachment-button".
This commit integrates several key features and fixes from the requested PRs: - Resolution Search: Added drawnFeatures support, fixed streaming logic, and improved vision model selection. - Mobile UI: Optimized chat input padding, fixed filename wrapping, and added mobile search portal. - Text Wrapping: Applied whitespace-normal and break-words to various components for better responsiveness. - Metadata: Updated LICENSE and Stripe checkout links. - Consistency: Maintained existing usage and history components as requested. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
This commit provides a final consolidated update of the features requested from PRs #466, #469, #470, #471, and #473: - Resolution Search: Finalized streaming logic and spatial context integration. - UI: Consolidated Tailwind-based layout fixes for mobile responsiveness and text wrapping. - Stability: Removed initial values from StreamableValue instances in app/actions.tsx to ensure hydration consistency. - Metadata: Verified license and copyright information. - Compliance: Ensured Usage and History components remain intact as requested. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
User description
This change addresses text overflow issues across several UI components by ensuring that long sentences and strings wrap to the next line instead of being truncated or causing horizontal scrollbars. Specifically:
EmptyScreenexample messages now wrap properly on mobile and desktop.BotMessagecontent handles long words/strings withbreak-words.SuggestionsDropdownandSearchRelatedqueries now support multi-line display.Verified visually with Playwright screenshots.
PR created automatically by Jules for task 4739911856121500176 started by @ngoiyaeric
PR Type
Enhancement
Description
Implement text wrapping for long sentences across multiple UI components
Replace text truncation with word-breaking in file attachments
Add proper alignment and spacing for multi-line button content
Ensure consistent icon positioning in wrapped text scenarios
Diagram Walkthrough
File Walkthrough
chat-panel.tsx
File attachment wrapping and alignment improvementscomponents/chat-panel.tsx
items-centertoitems-startfortop alignment
break-allwrapping and added rightmargin
shrink-0class to clear button to prevent it from shrinkingempty-screen.tsx
Example message buttons with multi-line text supportcomponents/empty-screen.tsx
whitespace-normalandtext-leftclasses to example messagebuttons
items-centertoitems-startflex-1for proper flex layoutmt-1andshrink-0to icon for better multi-line alignmentmessage.tsx
BotMessage word breaking for long contentcomponents/message.tsx
break-wordsclass to BotMessage container to prevent horizontaloverflow
search-related.tsx
SearchRelated button alignment for wrapped textcomponents/search-related.tsx
h-fittoh-autofor consistent heightitems-startclass for top alignment of multi-line contentsuggestions-dropdown.tsx
Suggestions dropdown wrapping and alignment fixescomponents/suggestions-dropdown.tsx
h-fittoh-autofor better multi-linesupport
items-startclass for top alignment of suggestion textflex-1for proper flex layoutmt-1to arrow icon for alignment with wrapped textSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.