updated-invoice-dowload-as-pdf-and-updated-ui #45
Conversation
|
Warning Rate limit exceeded@ajey35 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 17 minutes and 47 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplaces html2canvas-based PNG export with a new jsPDF-based PDF generation utility (generateInvoicePDF), adds a reusable InvoicePreview component, and updates SentInvoice/ReceivedInvoice to use PDF generation with invoice-existence guards, user toasts, zero-padded filenames, and revamped drawer headers with larger logo and status chips. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Invoice Page (Sent/Received)
participant PDF as generateInvoicePDF
participant FS as Browser File API
User->>UI: Click "Print / Download"
UI->>UI: validate invoice present
alt no invoice
UI-->>User: show error toast
else invoice present
UI->>UI: set pdfGenerating = true (toast)
UI->>PDF: generateInvoicePDF(invoice, fee)
Note right of PDF: load logo (DOM/fetch/Image)\ncompose header, items, totals\nhandle pagination & footers
PDF-->>UI: return PDF blob/uint8array
UI->>FS: saveAs("invoice-000123.pdf")
FS-->>User: download completes
UI->>UI: set pdfGenerating = false (success toast)
end
alt generation error
PDF-->>UI: error
UI-->>User: show failure toast, log error
UI->>UI: set pdfGenerating = false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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: 1
🧹 Nitpick comments (7)
frontend/src/page/ReceivedInvoice.jsx (7)
843-850: Consider keeping the dialog open during PDF generation.Currently, the confirmation dialog closes immediately before PDF generation starts. If generation fails or takes time, the user loses visual feedback from the dialog context. Consider either:
- Keeping the dialog open with a loading state until PDF generation completes, or
- Moving
setDownloadConfirmOpen(false)inside thefinallyblock ofgeneratePDFThis would provide better user feedback for slower PDF generation or error scenarios.
882-930: Simplify logo loading logic and handle potential DOM access issues.The logo loading logic has several concerns:
DOM access assumption: Line 884 assumes
document.getElementById("invoice-print")exists, but if the drawer isn't fully rendered or the element doesn't exist, this will silently fail.Complex nested fallbacks: Three different logo loading approaches in nested try-catch blocks make this hard to maintain and debug. Consider extracting logo loading to a separate async function.
CORS potential issue: Canvas-based image conversion (lines 889-894) may fail with CORS errors if the logo is served from a different origin without proper headers.
Error handling: The outer catch at line 922 catches all errors from all logo attempts, making it unclear which approach failed.
🔎 Suggested refactor to extract logo loading
Extract logo loading to a separate function:
const loadLogoForPDF = async () => { try { // Try fetching directly first (simpler approach) const response = await fetch('/logo.png'); if (response.ok) { const blob = await response.blob(); return await new Promise((resolve) => { const reader = new FileReader(); reader.onloadend = () => resolve(reader.result); reader.onerror = () => resolve(null); reader.readAsDataURL(blob); }); } } catch (error) { console.warn('Logo loading failed:', error); } return null; // Fallback will be handled by caller };Then in
generatePDF:// Try to add logo const logoDataUrl = await loadLogoForPDF(); if (logoDataUrl) { pdf.addImage(logoDataUrl, 'PNG', 22, yPos + 1, 8, 10); } else { // Fallback: Green box with "CV" pdf.setFillColor(...greenColor); pdf.rect(20, yPos, 12, 12, "F"); pdf.setTextColor(255, 255, 255); pdf.setFontSize(8); pdf.setFont("helvetica", "bold"); pdf.text("CV", 26, yPos + 8, { align: "center" }); }
864-870: Extract color palette to module-level constants.The color palette is defined inline within the
generatePDFfunction. Extracting these to constants at the module level would improve maintainability and allow reuse if you add more PDF generation features.🔎 Proposed refactor
Add constants at the top of the file (after imports):
// PDF Color Palette const PDF_COLORS = { darkGray: [17, 24, 39], mediumGray: [75, 85, 99], lightGray: [156, 163, 175], bgGray: [249, 250, 251], borderGray: [229, 231, 235], greenColor: [34, 197, 94], };Then reference them in the PDF generation:
pdf.setFillColor(...PDF_COLORS.bgGray);
1010-1011: Handle empty location strings more gracefully.Lines 1010 and 1028 construct location strings by concatenating city, country, and postalcode. If all three fields are empty, this results in ", , " which trim() won't fully clean. Consider adding a filter to remove empty values.
🔎 Proposed fix
-const fromLocation = `${invoice.user?.city || ""}, ${invoice.user?.country || ""}, ${invoice.user?.postalcode || ""}`.trim() || "N/A"; +const fromLocation = [invoice.user?.city, invoice.user?.country, invoice.user?.postalcode] + .filter(Boolean) + .join(", ") || "N/A"; -const toLocation = `${invoice.client?.city || ""}, ${invoice.client?.country || ""}, ${invoice.client?.postalcode || ""}`.trim() || "N/A"; +const toLocation = [invoice.client?.city, invoice.client?.country, invoice.client?.postalcode] + .filter(Boolean) + .join(", ") || "N/A";Also applies to: 1028-1029
1091-1216: Table rendering logic is well-structured.The table rendering with pagination support is well-implemented. The unit price symbol checking (lines 1174-1178) prevents duplicate symbols, and the alternating row backgrounds improve readability.
Optional improvement: Consider extracting magic numbers like the page break threshold (250 at line 1145) and column positions (lines 1107-1114) to named constants for better maintainability.
2020-2033: Simplify logo fallback implementation.The logo fallback logic uses
querySelector('.logo-fallback')which assumes a specific DOM structure and could be fragile. Consider using a more React-friendly approach with conditional rendering.🔎 Proposed alternative
const [logoError, setLogoError] = useState(false); // In the render: <div className="bg-white p-2 rounded-lg border border-gray-200 shadow-sm flex-shrink-0"> {!logoError ? ( <img src="/logo.png" alt="Chainvoice" className="h-8 w-8 sm:h-10 sm:w-10 object-contain" onError={() => setLogoError(true)} /> ) : ( <div className="h-8 w-8 sm:h-10 sm:w-10 bg-gradient-to-br from-green-500 to-emerald-600 rounded-lg flex items-center justify-center"> <span className="text-white font-bold text-sm sm:text-base">CV</span> </div> )} </div>
2060-2095: Consider extracting status chip styling for consistency.Status chip styles are defined inline here and similar colors are used in the PDF generation. Consider extracting these to a shared constant or theme to ensure consistency between the UI preview and generated PDF.
The colors should match the PDF_COLORS suggested earlier (lines 864-870).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/page/ReceivedInvoice.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/page/ReceivedInvoice.jsx (2)
frontend/src/page/Treasure.jsx (1)
fee(21-21)frontend/src/components/ui/dialog.jsx (3)
Dialog(7-7)DialogTitle(67-72)DialogContent(26-44)
🔇 Additional comments (4)
frontend/src/page/ReceivedInvoice.jsx (4)
107-109: LGTM!State declarations for PDF download confirmation dialog and loading state are appropriate and well-named.
1244-1245: Verify fee variable availability.The network fee calculation uses the
feevariable from the component scope (line 84). Ensure this value is properly initialized before PDF generation, especially if the user attempts to download before the invoice data is fully loaded.The current code fetches the fee at line 816 during invoice loading, but if a user somehow triggers download before this completes,
feemight be 0 or undefined.Consider adding a guard:
const networkFee = fee ? ethers.formatUnits(fee) : "0";
1288-1293: LGTM!Error handling is comprehensive with proper logging, user feedback via toast, and guaranteed state cleanup in the finally block.
2442-2518: LGTM!The confirmation dialog is well-implemented with clear messaging, invoice details preview, and proper action buttons. The user experience is intuitive and professional.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/page/ReceivedInvoice.jsx (1)
873-929: Logo loading is functional but could be simplified.The current implementation tries three strategies (DOM element → fetch → fallback), which is comprehensive. However, the DOM-based approach relies on finding the element with
id="invoice-print"and assumes the logo img element exists within it. While this works when the drawer is open, it's somewhat fragile.Consider preloading the logo or using a direct import for more reliable access. However, the current implementation with multiple fallbacks ensures PDF generation never fails, which is good defensive programming.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/page/ReceivedInvoice.jsx
🔇 Additional comments (5)
frontend/src/page/ReceivedInvoice.jsx (5)
16-16: LGTM! Clean imports and state setup.The jsPDF library import and Dialog component imports are correctly added to support the new PDF download feature. State management for the confirmation dialog and PDF generation progress is properly initialized.
Also applies to: 45-49, 106-108
842-849: LGTM! Download handlers are clean.The two-step download flow (confirmation → generation) is well implemented with proper async handling.
1173-1177: Good symbol duplication prevention logic.The check to avoid displaying the token symbol twice in the unit price is well thought out. Using
String.includes()works for typical token symbols. For even more precision, you could use a regex with word boundaries, but the current approach is pragmatic and sufficient for standard use cases.
1282-1293: LGTM! Proper finalization and error handling.The PDF save operation, success notification, and error handling are correctly implemented. The
finallyblock ensures thepdfGeneratingstate is always reset, preventing UI lock-up on errors.[approve_code_challenges]
1997-2005: LGTM! Excellent UI improvements and user flow.The responsive drawer sizing, enhanced invoice preview styling, and download confirmation dialog create a professional user experience. The symbol duplication handling in both the UI table and PDF generation is consistent, and the loading states during PDF generation provide clear feedback to users.
The confirmation dialog with invoice summary is a nice UX touch that helps users verify what they're downloading before proceeding.
Also applies to: 2300-2306, 2402-2434, 2441-2517
|
Have you notice the calculation problem in your invoice : 
Is it working fine at your at end? |
|
Also can work more on the UI part of the pdf such as logo and all. |
|
hey @kumawatkaran523 calculation part i didnot changed anything , it is working fine ,, In that Image i hard-coded on dummy invoice in state variable that's why it is mis-calculated |
Have you worked on this? |
yes @kumawatkaran523 Iam |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
frontend/src/page/ReceivedInvoice.jsx (4)
909-957: Consider decoupling PDF logo loading from DOM.The logo loading logic queries the DOM element with ID
invoice-printwhich creates a tight coupling between PDF generation and the drawer's rendering state. This approach may fail intermittently if the drawer isn't fully rendered when PDF generation starts, potentially leading to inconsistent PDF outputs.While the fallback strategy (fetch → green box) is good, consider passing the logo data directly to
generatePDFrather than querying the DOM.🔎 Suggested approach
Option 1: Load logo once when component mounts and store in state/ref:
+ const logoDataRef = useRef(null); + + useEffect(() => { + // Load logo once on mount + fetch('/logo.png') + .then(res => res.blob()) + .then(blob => { + const reader = new FileReader(); + reader.onloadend = () => { + logoDataRef.current = reader.result; + }; + reader.readAsDataURL(blob); + }) + .catch(() => { + logoDataRef.current = null; // Will use fallback + }); + }, []);Then in
generatePDF, uselogoDataRef.currentinstead of querying the DOM.
1271-1272: Specify decimals explicitly in formatUnits for clarity.While
ethers.formatUnits(fee)defaults to 18 decimals (correct for ETH), explicitly specifying the decimals parameter improves code clarity and prevents potential issues if the context changes.🔎 Proposed change
- const networkFee = ethers.formatUnits(fee); + const networkFee = ethers.formatUnits(fee, 18);
879-1321: Consider breaking down the large generatePDF function.The
generatePDFfunction is approximately 440 lines long and handles multiple responsibilities (PDF setup, header, logo, sections, tables, footer, saving). This makes the code harder to understand, test, and maintain.Consider extracting logical sections into helper functions like
addPDFHeader(),addPDFFromBillToSection(),addPDFItemsTable(),addPDFFooter(), etc. This would improve readability and make it easier to test and modify individual sections.
1202-1205: Extract duplicate unitPrice formatting logic.The logic for checking whether
unitPricealready contains the token symbol is duplicated in both the PDF generation (lines 1202-1205) and the drawer UI (lines 2290-2295). Consider extracting this into a reusable helper function.🔎 Proposed helper function
const formatUnitPrice = (unitPrice, symbol) => { const unitPriceStr = String(unitPrice || ""); return unitPriceStr.includes(symbol) ? unitPriceStr : `${unitPriceStr} ${symbol}`; };Then use it in both places:
// In PDF generation const unitPriceDisplay = formatUnitPrice(item.unitPrice, tokenSymbol); // In drawer UI const unitPriceDisplay = formatUnitPrice(item.unitPrice, symbol);Also applies to: 2290-2295
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/page/ReceivedInvoice.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/page/ReceivedInvoice.jsx (2)
frontend/src/page/SentInvoice.jsx (2)
drawerState(344-347)toggleDrawer(349-361)frontend/src/components/ui/dialog.jsx (3)
Dialog(7-7)DialogTitle(67-72)DialogContent(26-44)
🔇 Additional comments (5)
frontend/src/page/ReceivedInvoice.jsx (5)
16-16: LGTM: Clean imports for PDF generation and confirmation dialog.The jsPDF library import and MUI Dialog components are correctly added to support the new PDF download feature. The previous html2canvas dependency has been successfully removed as noted in past reviews.
Also applies to: 45-49
106-108: LGTM: Appropriate state management for PDF download flow.The state variables for dialog control and PDF generation progress are well-named and properly initialized.
870-877: LGTM: Clean separation of concerns in download flow.The handlers properly separate confirmation dialog control from PDF generation, providing a good user experience with an explicit confirmation step.
1987-2427: LGTM: Well-structured drawer UI with responsive design.The invoice drawer UI is thoughtfully implemented with:
- Proper responsive breakpoints for different screen sizes
- Consistent styling that matches the PDF output
- Semantic HTML table structure with colgroup for precise column alignment
- Good fallback handling for missing logo
- Loading state properly displayed during PDF generation
The layout provides a professional invoice preview that accurately represents what users will receive in the PDF.
2432-2507: LGTM: User-friendly confirmation dialog.The PDF download confirmation dialog follows good UX practices by:
- Displaying key invoice details before download
- Providing context about the PDF format
- Offering clear cancel/confirm actions
- Using consistent styling with the rest of the application
This explicit confirmation step prevents accidental downloads and sets proper user expectations.
|
hello @kumawatkaran523 see now is there any changes required ? |
ajey35
force-pushed
the
feat/download-invoice-as-pdf
branch
from
2e2c266 to
d41b17e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
frontend/src/utils/generateInvoicePDF.js (2)
10-12: Verify DOM element availability across all usage contexts.The function queries
document.getElementById("invoice-print")which assumes this element exists in the DOM. However, when generating PDFs programmatically (not from a visible drawer), this element may not be present, causing the first fallback to fail silently.Since you have multiple fallbacks, this may not be critical, but consider documenting this assumption or making the element ID configurable.
[scratchpad_end] -->
🔎 Possible improvement
-const loadLogoImage = async () => { +/** + * Load logo image with multiple fallback methods + * @param {string} elementId - Optional DOM element ID to search for logo (default: "invoice-print") + * @returns {Promise<string|null>} Base64 data URL or null if all methods fail + */ +const loadLogoImage = async (elementId = "invoice-print") => { try { - const invoiceElement = document.getElementById("invoice-print"); + const invoiceElement = document.getElementById(elementId);
520-521: Clarify fee units and ensure correct formatting.Line 520 calls
ethers.formatUnits(fee)without specifying decimals, which defaults to 18 (ETH units). Based on the relevant code snippets showing fee is stored asBigIntand the PR context indicating it's a network fee, this assumption appears correct. However, the function signature doesn't document this assumption.Consider adding JSDoc to clarify that
feeis expected in wei (ETH units).🔎 Documentation improvement
/** * Generate PDF for invoice * @param {Object} invoice - Invoice object - * @param {string|BigInt} fee - Network fee (wei) + * @param {string|BigInt} fee - Network fee in wei (ETH units, will be formatted using 18 decimals) * @returns {Promise<jsPDF>} Generated PDF document */frontend/src/components/InvoicePreview.jsx (2)
16-22: Consider logging errors in formatFee for debugging.The
formatFeehelper function silently catches all errors and returns "0". While this provides a safe fallback, it could hide issues with invalid fee values during development.Consider adding console.warn or console.error in the catch block for debugging purposes.
🔎 Proposed improvement
const formatFee = (feeValue) => { try { return ethers.formatUnits(feeValue); - } catch { + } catch (error) { + console.warn('Failed to format fee value:', feeValue, error); return "0"; } };
321-355: Consider extracting duplicate unit price formatting logic.The logic for appending currency symbols to unit prices (lines 323-327) is duplicated in
generateInvoicePDF.js(lines 458-461). While the duplication is minor and both implementations work correctly, consider extracting this to a shared utility function if you find yourself adding similar formatting logic elsewhere.This is a low-priority refactoring opportunity that could improve maintainability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/InvoicePreview.jsxfrontend/src/page/ReceivedInvoice.jsxfrontend/src/page/SentInvoice.jsxfrontend/src/utils/generateInvoicePDF.js
🧰 Additional context used
🧬 Code graph analysis (4)
frontend/src/components/InvoicePreview.jsx (3)
frontend/src/utils/generateInvoicePDF.js (1)
networkFee(520-520)frontend/src/page/ReceivedInvoice.jsx (1)
fee(78-78)frontend/src/page/SentInvoice.jsx (1)
fee(68-68)
frontend/src/utils/generateInvoicePDF.js (3)
frontend/src/page/ReceivedInvoice.jsx (1)
fee(78-78)frontend/src/page/SentInvoice.jsx (1)
fee(68-68)frontend/src/components/InvoicePreview.jsx (1)
networkFee(24-24)
frontend/src/page/ReceivedInvoice.jsx (4)
frontend/src/page/SentInvoice.jsx (2)
drawerState(344-347)fee(68-68)frontend/src/page/BatchPayment.jsx (2)
drawerState(56-59)fee(45-45)frontend/src/utils/generateInvoicePDF.js (3)
generateInvoicePDF(158-589)generateInvoicePDF(158-589)frontend/src/page/Treasure.jsx (1)
fee(21-21)
frontend/src/page/SentInvoice.jsx (6)
frontend/src/page/ReceivedInvoice.jsx (3)
drawerState(96-99)fee(78-78)error(79-79)frontend/src/page/BatchPayment.jsx (3)
drawerState(56-59)fee(45-45)error(46-46)frontend/src/utils/generateInvoicePDF.js (2)
generateInvoicePDF(158-589)generateInvoicePDF(158-589)frontend/src/page/Treasure.jsx (1)
fee(21-21)frontend/src/hooks/useTokenList.js (1)
error(21-21)frontend/src/hooks/useTokenSearch.js (1)
error(47-47)
🔇 Additional comments (3)
frontend/src/utils/generateInvoicePDF.js (1)
537-540: Review total calculation logic for non-ETH payment tokens.The total calculation at lines 537-540 has different logic based on whether
tokenSymbol === "ETH". When the payment token is ETH, it sumsamountDue + networkFee. However, when the payment token is not ETH, it displays them separately (e.g., "100 USDC + 0.001 ETH").This logic is correct, but ensure it aligns with user expectations and the total displayed in the UI components (ReceivedInvoice.jsx lines 993-1000 and SentInvoice.jsx have similar logic). The consistency is good.
frontend/src/page/SentInvoice.jsx (1)
363-379: LGTM! PDF generation implementation is clean and user-friendly.The refactored
handlePrintfunction correctly:
- Validates invoice selection before proceeding
- Provides user feedback during PDF generation
- Uses consistent filename formatting (zero-padded invoice ID)
- Handles errors gracefully with appropriate user notifications
frontend/src/page/ReceivedInvoice.jsx (1)
861-877: LGTM! Consistent PDF generation implementation.The
handlePrintfunction implementation matches the pattern used in SentInvoice.jsx, providing consistent user experience across both sent and received invoice views.
|
hey @kumawatkaran523 what i thought is that ,, in this PR i just need to fix the issue and bug , for ui implementation ,, |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/components/InvoicePreview.jsx (1)
233-235: Consider validating address length before masking.The address masking logic assumes a standard 42-character Ethereum address (0x + 40 hex characters). While
invoice.paymentToken?.addressshould always be valid, adding a length check would make the code more defensive against malformed data.🔎 Proposed refinement
<p className="text-xs text-gray-500 font-mono mt-0.5"> {invoice.paymentToken?.address - ? `${invoice.paymentToken.address.substring(0, 10)}......${invoice.paymentToken.address.substring(33)}` + ? invoice.paymentToken.address.length >= 42 + ? `${invoice.paymentToken.address.substring(0, 10)}......${invoice.paymentToken.address.substring(invoice.paymentToken.address.length - 9)}` + : invoice.paymentToken.address : "Native Currency"} </p>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/InvoicePreview.jsx
🔇 Additional comments (1)
frontend/src/components/InvoicePreview.jsx (1)
56-60: Past review comment addressed.The company name inconsistency flagged in the previous review has been fixed. Line 57 now correctly renders "Cha" instead of "Chain", producing the consistent "Chainvoice" branding across the preview, PDF, and drawer headers.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
frontend/src/utils/generateInvoicePDF.js (4)
213-222: Consider extracting magic number for text positioning.The vertical offset of
6on Line 221 appears to work for the current font size (16), but extracting it as a named constant would improve maintainability if font sizes change in the future.🔎 Suggested refactor
+const FALLBACK_TEXT_VERTICAL_OFFSET = 6; + if (!logoAdded) { pdf.setFillColor(...greenColor); pdf.rect(logoX, logoY, logoSize, logoSize, "F"); pdf.setDrawColor(...greenColor); pdf.rect(logoX, logoY, logoSize, logoSize, "D"); pdf.setTextColor(255, 255, 255); pdf.setFontSize(16); pdf.setFont("helvetica", "bold"); - pdf.text("CV", logoX + logoSize/2, logoY + logoSize/2 + 6, { align: "center" }); + pdf.text("CV", logoX + logoSize/2, logoY + logoSize/2 + FALLBACK_TEXT_VERTICAL_OFFSET, { align: "center" }); }
347-361: Add length validation before address truncation.Line 349 performs substring operations assuming the address is at least 18 characters long. While Ethereum addresses are standardized at 42 characters (0x + 40 hex), defensive validation would prevent potential runtime errors if malformed data is present.
🔎 Proposed defensive check
if (invoice.paymentToken?.address) { const contractAddr = invoice.paymentToken.address; - const shortAddr = `${contractAddr.substring(0, 10)}......${contractAddr.substring(contractAddr.length - 8)}`; + const shortAddr = contractAddr.length >= 18 + ? `${contractAddr.substring(0, 10)}......${contractAddr.substring(contractAddr.length - 8)}` + : contractAddr; pdf.text(shortAddr, 25, yPos + 19);
432-456: Extract page break threshold as a named constant.The magic number
250on Line 432 represents the page break threshold. Extracting this as a named constant would improve readability and make it easier to adjust pagination behavior.🔎 Suggested refactor
+const PAGE_BREAK_THRESHOLD_Y = 250; // mm from top before adding new page + if (invoice.items && invoice.items.length > 0) { invoice.items.forEach((item, index) => { - if (yPos > 250) { + if (yPos > PAGE_BREAK_THRESHOLD_Y) {
159-593: Consider refactoring for improved maintainability.The
generateInvoicePDFfunction is quite large (~430 lines) and handles multiple concerns (layout, rendering, pagination). While the implementation is functional, consider these optional improvements:
- Extract color constants to module level
- Extract column position definitions as constants
- Break down into smaller helper functions (e.g.,
renderHeader,renderFromBillTo,renderItemsTable,renderTotals,renderFooter)- Move magic numbers (colors, positions, sizes) to named constants
These changes would improve testability and make future modifications easier.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/utils/generateInvoicePDF.js
🔇 Additional comments (3)
frontend/src/utils/generateInvoicePDF.js (3)
9-151: LGTM! Robust logo loading with multiple fallbacks.The three-tiered fallback approach (DOM → Fetch → Image object) ensures reliable logo loading across different scenarios. The error handling, timeouts, and validation checks are appropriate.
296-308: Fix name concatenation fallback logic.Line 296's fallback logic works correctly - when both
fnameandlnameare falsy, the trimmed empty string properly triggers the|| "N/A"fallback. However, verify that this pattern is consistent with the actual data model expectations.
549-593: LGTM! Clean footer implementation with proper pagination.The footer rendering correctly iterates through all pages and adds consistent branding, page numbers, and generation date. The centered alignment and date formatting are appropriate.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/InvoicePreview.jsx (1)
244-245: Hard-coded chain name displays incorrect information.The "Sepolia Testnet" label is hard-coded and will show incorrect network information for invoices created on other chains (mainnet, other testnets, L2s, etc.).
This issue was previously flagged but remains unaddressed. Consider implementing the suggested dynamic chain name resolution based on
invoice.paymentToken.chainIdor similar.
🧹 Nitpick comments (2)
frontend/src/components/InvoicePreview.jsx (2)
16-22: Consider documenting the formatFee assumption.The function calls
ethers.formatUnits(feeValue)without a second parameter, which defaults to 18 decimals (wei to ETH). While this is correct for network fees on Ethereum, the assumption is implicit.🔎 Optional: Add clarity with explicit units parameter or JSDoc
+ /** + * Formats fee value from wei (18 decimals) to ETH + * @param {BigNumberish} feeValue - Fee in wei + * @returns {string} Formatted fee in ETH + */ const formatFee = (feeValue) => { try { - return ethers.formatUnits(feeValue); + return ethers.formatUnits(feeValue, 18); } catch { return "0"; } };
322-333: Minor spacing inconsistency in unit price display.When the symbol is already present (line 331), the code returns the original
unitPriceStrwithout trimming, but when appending the symbol (line 329), it uses the trimmed version. This could result in inconsistent spacing if the original value has trailing whitespace.🔎 Proposed fix for consistent spacing
const unitPriceStr = String(item.unitPrice || ""); const symbol = invoice.paymentToken?.symbol || ""; let unitPriceDisplay = unitPriceStr; if (symbol) { const trimmed = unitPriceStr.trim(); if (!trimmed.endsWith(symbol)) { unitPriceDisplay = `${trimmed} ${symbol}`; } else { - unitPriceDisplay = unitPriceStr; + unitPriceDisplay = trimmed; } }
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/components/InvoicePreview.jsx (1)
429-433: Duplicate: Total calculation still has NaN risk.This issue was previously flagged but remains unaddressed. The calculation uses
parseFloat()oninvoice.amountDueandnetworkFeewithout validation. If either value contains non-numeric characters or is malformed,parseFloatwill returnNaN, and the displayed total will show "NaN ETH" or similar.Original suggestion from previous review
Validate numeric inputs before parsing:
- Ensure
invoice.amountDueandnetworkFeeare stored as numeric types or properly validated strings- Handle undefined/null by falling back to 0 or rendering a placeholder
- Avoid parsing already-formatted display strings
Consider this approach:
<span className="text-base sm:text-lg font-bold text-gray-900"> {invoice.paymentToken?.symbol === "ETH" - ? `${( - parseFloat(invoice.amountDue) + parseFloat(networkFee) - ).toFixed(6)} ETH` + ? (() => { + const amount = parseFloat(invoice.amountDue); + const fee = parseFloat(networkFee); + if (isNaN(amount) || isNaN(fee)) { + return `${invoice.amountDue} ETH + ${networkFee} ETH`; + } + return `${(amount + fee).toFixed(6)} ETH`; + })() : `${invoice.amountDue} ${invoice.paymentToken?.symbol} + ${networkFee} ETH`} </span>
🧹 Nitpick comments (1)
frontend/src/components/InvoicePreview.jsx (1)
1-41: Consider moving imports to the top of the file.The helper map and function are defined before the imports (lines 1-33), which is unconventional. Standard JavaScript/React convention places all imports at the top of the file before any other code.
🔎 Suggested structure
+import React from "react"; +import { ethers } from "ethers"; +import { Chip, Typography } from "@mui/material"; +import PaidIcon from "@mui/icons-material/CheckCircle"; +import UnpaidIcon from "@mui/icons-material/Pending"; +import CancelIcon from "@mui/icons-material/Cancel"; +import CurrencyExchangeIcon from "@mui/icons-material/CurrencyExchange"; + // Helper to map chainId to human-friendly network name const CHAIN_ID_TO_NAME = { ... }; function getNetworkName(chainId) { ... } - -import React from "react"; -import { ethers } from "ethers"; -...
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/InvoicePreview.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/InvoicePreview.jsx (3)
frontend/src/utils/generateInvoicePDF.js (2)
chainId(351-351)networkFee(524-524)frontend/src/page/SentInvoice.jsx (1)
fee(68-68)frontend/src/page/ReceivedInvoice.jsx (1)
fee(78-78)
🔇 Additional comments (2)
frontend/src/components/InvoicePreview.jsx (2)
277-280: Excellent fix for dynamic chain display.The chain name is now dynamically resolved using
getNetworkName()with a fallback chain ID lookup (invoice.paymentToken?.chainId || invoice.chainId), which correctly addresses the previous concern about hard-coded "Sepolia Testnet" text.
357-369: Symbol detection is now precise and correct.The unit price symbol check now uses a regex pattern that tests whether the trimmed price ends with the symbol, with proper escaping of regex special characters. This correctly addresses the previous concern about
includes()producing false positives.
What i meant to say that you dont require to make any figma design and all you just need to replicate the pdf as it is there in chainvoice. |
@kumawatkaran523 well i already did that! |
see this is the pdf dowloaded |
here why did i say figma design bcz i though invoice should be more attractive and neat structured and well organized in ui so that i told like that , well currently it is as you wish to be |
|
@kumawatkaran523 see here both |
2 participants
Download Invoice as PDF and Updated UI and Fixed #42
Invoice PDF generated :
invoice-999999-5.pdf
Summary
Adds the ability to download received invoices as professionally formatted PDF files.
What's New
feat/download-invoice-as-pdf
Technical Details
jsPDFlibrary for PDF generationHow to Test
Screencast.From.2025-12-31.11-19-48.webm
Summary by CodeRabbit
New Features
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.