-
Notifications
You must be signed in to change notification settings - Fork 852
[IP-988]: template layout redesign #1408
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
base: prep/v171
Are you sure you want to change the base?
Conversation
* Template mPDF Templates Update * remove custom sections * Update InvoicePlane.php missing notes section, added condition to discount, (only visible if discount given) * Update InvoicePlane_Web.php same changes as mPDF template * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update .github/workflows/README.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Delete .github/workflows/quickstart.yml --------- Co-authored-by: ZERO <32537564+YasirRWebDesigner@users.noreply.github.com> Co-authored-by: Niels Drost <47660417+nielsdrost7@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
nielsdrost7
left a comment
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.
@copilot this was a template layout redesign
Lint the living crap out of this PR, I had some merge conflicts.
It's just a template redesign, see if you can improve stuff, based on the new incoming source from the last 3 years (that's how old the PR was)
|
@nielsdrost7 I've opened a new pull request, #1409, to work on those changes. Once the pull request is ready, I'll request review from you. |
nielsdrost7
left a comment
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.
In application/views/invoice_templates/pdf/InvoicePlane - overdue.php around
lines 148 to 187, the item name and description are rendered into the wrong
table cells (description printed under "Item" and name under "Description"); fix
by swapping the two echo calls so the Item column outputs the escaped item name
(use the existing _htmlsc($item->item_name) call) and the Description column
outputs the escaped, nl2br-formatted item description (use
nl2br(htmlsc($item->item_description))); preserve existing classes, escaping and
formatting for both cells.
In application/views/invoice_templates/pdf/InvoicePlane - paid.php around lines
78 to 84, the "Invoiced To" string is hardcoded on line 79; replace it with the
translation helper call (e.g., use _trans('bill_to') instead of ('Invoiced To'))
so the label uses the existing localization system — update the echo/print to
output _trans('bill_to') within the
element and keep surrounding markup
unchanged.
In application/views/invoice_templates/pdf/InvoicePlane - paid.php around lines
132 to 167, the item name and description are placed under the wrong headers
(description is under "item" and name is under "description"); swap the cell
outputs so the "item" column shows the item name and the "description" column
shows the item description — specifically, render the item name in the first
cell using proper escaping (e.g. _htmlsc($item->item_name)) and render the
description in the second cell using nl2br(htmlsc($item->item_description));
keep existing formatting, line breaks and escaping functions but reverse their
positions so columns match their headers.
In application/views/invoice_templates/pdf/InvoicePlane - paid.php around lines
164 to 166, the invoice row currently echoes $item->item_total (which includes
item-level tax) but the invoice summary is computed from subtotals and tax
breakdowns; change the template to output $item->item_subtotal for the row total
so each line matches the subtotal-based summary math and avoids double-counting
tax.
In application/views/invoice_templates/pdf/InvoicePlane.php around lines 99 to
104, the header text "Invoiced To" is hardcoded which bypasses localization;
replace the literal string with the translation helper (for example call
_trans('bill_to') or the existing translation key used elsewhere) and echo that
value instead, making sure to use the same escaping pattern as other outputs
(e.g., wrap with _htmlsc if needed) so the PDF header is translatable and safe.
In application/views/invoice_templates/pdf/InvoicePlane.php around lines 152 to
168, the item and description cells are inverted: the code currently prints
item_description under the "item" header and item_name under the "description"
header. Fix by swapping those two contents so the cell under the "item"
column echoes the item name (escaped, e.g., htmlsc($item->item_name)) and the
cell under the "description" column echoes the description (escaped and
formatted, e.g., nl2br(htmlsc($item->item_description))). Ensure you keep the
existing CSS classes and escaping functions when moving the values.
In application/views/invoice_templates/pdf/InvoicePlane.php around lines
184–186, the code prints $item->item_total which includes item-level taxes;
replace this with $item->item_subtotal so the row total excludes item taxes and
matches the invoice summary calculations; keep the existing format_currency()
wrapper (i.e., call format_currency($item->item_subtotal)) so formatting remains
consistent.
In application/views/invoice_templates/public/InvoicePlane_Web.php around lines
124-133, fix the typo "Inovoiced To" and make it localizable: replace the
hard-coded, misspelled string with the properly spelled "Invoiced To" and emit
it via the localization helper (use _trans(...) for the displayed label) so the
public view is translated for non-English users.
In application/views/quote_templates/public/InvoicePlane_Web.php around lines 89
to 97, there's an unmatched closing
the template by deleting that stray closing paragraph tag so all remaining tags
are balanced and the conditional phone/mobile blocks remain unchanged.
In application/views/quote_templates/public/InvoicePlane_Web.php around lines
257-303, the quote notes are being rendered twice (once in the column block at
~257-261 and again in the table block at ~287-301); remove the duplicate output
by keeping only one rendering path — either delete the later table block that
outputs $quote->notes or change the second conditional to an else (or else-if)
so notes are rendered only once, ensuring the remaining rendering preserves the
intended HTML sanitization (nl2br/htmlsc or strip_tags) you prefer.
|
@nielsdrost7 I've opened a new pull request, #1410, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Fix merge conflict duplicates in all template files Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com> * Fix quote web template: remove merge conflicts and duplicates Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 |
* Initial plan * Address review feedback: fix swapped columns, localize strings, remove duplicates Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com> * Fix item_total to item_subtotal in overdue template for consistency Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
nielsdrost7
left a comment
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.
@copilot
Lint the living crap out of this PR, I had some merge conflicts.
It's just a template redesign, see if you can improve stuff, based on the new incoming source from the last 3 years (that's how old the PR was)
I had some merge conflicts again
|
@nielsdrost7 I've opened a new pull request, #1420, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Standardize translation functions and fix template inconsistencies Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com> * Remove duplicate incomplete HTML structure from public invoice template Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
No description provided.