fix: prevent text overlapping in invoice PDF layout #2021
Conversation
- Add explicit width constraints and word-wrap to all sections - Set fixed table column widths to prevent NAME/DATE overlap - Increase address container width and add text wrapping
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughReplaced responsive width classes with explicit inline percentage widths and added word-wrap/overflow-wrap rules across the invoice PDF template to prevent text overlap in header, address blocks, middle metadata sections, and the line items table. No data or logic changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/views/pdfs/invoices.html.erb (4)
61-63: Remove unused CSS class.The
.w-48class is defined but never used in the template, contributing to stylesheet bloat.Apply this diff to remove the unused class:
- .w-48 { - width: 12rem; - }
172-188: Simplify by removing redundantmax-widthdeclarations.Lines 172 and 186 set both
widthandmax-widthto identical values (60% and 38% respectively). In this fixed layout context,max-widthis redundant since the element won't naturally exceed the specified width.Apply this diff:
- <div class="flex" style="width: 60%; max-width: 60%;"> + <div class="flex" style="width: 60%;">- <div class="mt-2 font-normal text-base text-right text-miru-dark-purple-1000" style="width: 38%; max-width: 38%;"> + <div class="mt-2 font-normal text-base text-right text-miru-dark-purple-1000" style="width: 38%;">
195-219: Standardize word-wrap property usage for consistency.The word-wrap implementation is inconsistent:
- Lines 195, 215 use only
word-wrap: break-word- Lines 198, 219 use both
word-wrap: break-word; overflow-wrap: break-wordFor maximum PDF generator compatibility and consistency, apply both properties uniformly to all text that might overflow.
Apply this diff:
- <p class="font-bold text-base text-miru-dark-purple-1000" style="word-wrap: break-word;"> + <p class="font-bold text-base text-miru-dark-purple-1000" style="word-wrap: break-word; overflow-wrap: break-word;"> <%=client.name%> </p>- <p class="font-normal text-base text-miru-dark-purple-1000" style="word-wrap: break-word;"> + <p class="font-normal text-base text-miru-dark-purple-1000" style="word-wrap: break-word; overflow-wrap: break-word;"> <%=invoice["invoice_number"]%> </p>
245-247: Consider addingoverflow-wrapfor consistency andword-breakfor extreme cases.The NAME and DESCRIPTION cells use only
word-wrap: break-word, which is inconsistent with some address fields that use bothword-wrapandoverflow-wrap. Additionally, for extremely long unbreakable strings (e.g., URLs, long email addresses), consider addingword-break: break-wordto force breaking within words when necessary.Apply this diff for improved text overflow handling:
- <td class="border-b-2 border-miru-gray-200 px-1 py-3 font-normal text-base text-miru-dark-purple-1000 text-left" style="width: 18%; word-wrap: break-word;"><%=item[:name]%></td> + <td class="border-b-2 border-miru-gray-200 px-1 py-3 font-normal text-base text-miru-dark-purple-1000 text-left" style="width: 18%; word-wrap: break-word; overflow-wrap: break-word; word-break: break-word;"><%=item[:name]%></td>- <td class="border-b-2 border-miru-gray-200 px-1 pl-2 py-3 font-normal text-base text-miru-dark-purple-1000 text-left" style="width: 30%; word-wrap: break-word;"> + <td class="border-b-2 border-miru-gray-200 px-1 pl-2 py-3 font-normal text-base text-miru-dark-purple-1000 text-left" style="width: 30%; word-wrap: break-word; overflow-wrap: break-word; word-break: break-word;">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/views/pdfs/invoices.html.erb(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Running tests
🔇 Additional comments (4)
app/views/pdfs/invoices.html.erb (4)
192-228: Width distribution correctly sums to 100%.The middle section layout changes effectively partition the available space (28% + 20% + 28% + 24% = 100%) and add appropriate word-wrap properties to prevent overflow.
234-239: Table header width distribution correctly addresses NAME/DATE overlap.The explicit column widths (totaling 100%) combined with
table-layout: fixedeffectively prevent the NAME/DATE overlapping issue described in #2020. The width allocation appropriately prioritizes DESCRIPTION (30%) while providing adequate space for other columns.
245-258: Table body cell widths correctly match header widths.The inline width specifications maintain perfect alignment with the table headers, and word-wrap is appropriately applied only to NAME and DESCRIPTION columns where long text is expected.
172-188: Layout approach is sound; verify PDF rendering with long company names.The word-wrap property has been renamed to overflow-wrap, with word-wrap being an alias. Using both properties (as in line 187) provides defensive cross-browser coverage and is acceptable; the single
word-wrapon line 178 also works fine.The flex layout pattern you've used is correct:
- Setting explicit width on a flex item throws intrinsic sizing out the window, making the layout easier to reason about.
flex-shrink: 0prevents logo shrinking;flex: 1; min-width: 0allows text to shrink while wrapping is enabled- Combined width constraint (60% + 38% = 98%) prevents overlaps
Before merging, manually test the PDF output with extreme cases: very long company names without spaces (e.g., email-style addresses or URLs in the name field) to confirm wrapping/truncation behavior matches your design intent.
- Remove unused .w-48 CSS class - Remove redundant max-width declarations - Standardize word-wrap with overflow-wrap for consistency - Add word-break to table cells for extreme overflow cases - Use Tailwind utility classes (w-1/4, w-1/5) instead of inline percentages
…df-layout---text-overlapping-issues
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/pdfs/invoices.html.erb (1)
188-225:⚠️ Potential issue | 🟠 MajorAllow the metadata row to expand with wrapped fields.
Line 188 still enforces a fixed height (
h-36) while client/invoice fields can now wrap. That can clip or overlap content when long values are present. Consider using a min-height instead.🛠️ Suggested adjustment
- <div class="flex justify-between border-b-2 border-miru-gray-400 px-10 py-5 h-36"> + <div class="flex justify-between border-b-2 border-miru-gray-400 px-10 py-5" style="min-height: 9rem;">
🤖 Fix all issues with AI agents
In `@app/views/pdfs/invoices.html.erb`:
- Around line 168-185: The header container uses a fixed height class h-50 which
causes wrapped company/address text (invoice.company.name,
invoice.company.formatted_address) to clip; change the layout so the outer div
(the one with class "flex justify-between border-b-2 ...") uses a min-height
instead of a fixed height (remove or replace h-50 with an appropriate min-h
class or inline min-height) and ensure child blocks keep word-wrap/overflow-wrap
so the header can grow vertically when content wraps.
- Change h-50 to min-height: 12.5rem for header section - Change h-36 to min-height: 9rem for metadata section - Allows sections to expand when content wraps, preventing overflow
3 participants
Before Changes:
After Changes:
Fixes #2020
Summary by CodeRabbit