Conversation
📝 WalkthroughWalkthroughThe changes address a mobile UX issue where QR codes appeared too large and control buttons remained visible. Modifications include reducing QR generation scale, conditionally hiding the "Copy & Display Invoice" button when QR is displayed, and making modal content vertically stacked with scrolling capability. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/pages/bc-send-payment.ts (1)
223-227:⚠️ Potential issue | 🟡 Minor
scale=2shrinks the desktop QR unnecessarily — consider a CSS-constrained approach instead.
renderQR()is shared between the mobile and desktop paths. On desktop, the QR was previously 260×260 CSS px (scale=4); it's now 130×130 CSS px. Since the canvas has no CSS width/height set, the intrinsic size is the rendered size. 130 CSS px is likely too small to scan reliably at typical arm's-length distance from a desktop monitor.A better fix keeps the intrinsic buffer at scale=4 (better sampling for scanner apps) and constrains the rendered size via CSS:
🔧 Proposed fix — CSS-constrained size instead of shrinking the buffer
- const scale = 2; + const scale = 4; canvas.width = moduleCount * scale; canvas.height = moduleCount * scale;Then add a
max-widthto the canvas element so it never overflows the mobile modal while still appearing at full size on desktop:- <canvas id="qr" class="dark:bg-white dark:p-4"></canvas> + <canvas id="qr" class="dark:bg-white dark:p-4 max-w-full" style="max-width: 220px;"></canvas>
max-w-fullensures it shrinks within any constrained container, and the inlinemax-widthacts as the absolute cap (tune the value as needed).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/pages/bc-send-payment.ts` around lines 223 - 227, The QR rendering is using scale=2 which reduces the intrinsic canvas size and makes desktop QR too small; change the scale constant used in qr.renderTo2dContext(ctx, scale) from 2 to 4 to keep a higher-resolution buffer, and instead constrain the displayed size via CSS on the canvas element (e.g., add a responsive utility class like "max-w-full" and an absolute cap such as an inline max-width of "260px" or equivalent styling) so mobile modal layout is respected while desktop renders at full readable size.
🧹 Nitpick comments (1)
dev/vite/index.html (1)
43-47: Optional: clean up the commented-outcommentfield in the dev harness.No runtime impact since it's a dev-only file, but the Lorem Ipsum block is clearly leftover from local testing.
🧹 Proposed cleanup
- const payButtonInvoice = await ln.requestInvoice({ - satoshi: 1, - //comment: - // "Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s", - }); + const payButtonInvoice = await ln.requestInvoice({satoshi: 1});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/vite/index.html` around lines 43 - 47, Remove the leftover commented-out "comment" block inside the ln.requestInvoice call used to create payButtonInvoice in the dev harness; specifically edit the payButtonInvoice = await ln.requestInvoice({...}) invocation to delete the commented Lorem Ipsum lines so only the active fields (e.g., satoshi: 1) remain, keeping the dev UI code clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/pages/bc-send-payment.ts`:
- Around line 223-227: The QR rendering is using scale=2 which reduces the
intrinsic canvas size and makes desktop QR too small; change the scale constant
used in qr.renderTo2dContext(ctx, scale) from 2 to 4 to keep a higher-resolution
buffer, and instead constrain the displayed size via CSS on the canvas element
(e.g., add a responsive utility class like "max-w-full" and an absolute cap such
as an inline max-width of "260px" or equivalent styling) so mobile modal layout
is respected while desktop renders at full readable size.
---
Nitpick comments:
In `@dev/vite/index.html`:
- Around line 43-47: Remove the leftover commented-out "comment" block inside
the ln.requestInvoice call used to create payButtonInvoice in the dev harness;
specifically edit the payButtonInvoice = await ln.requestInvoice({...})
invocation to delete the commented Lorem Ipsum lines so only the active fields
(e.g., satoshi: 1) remain, keeping the dev UI code clean.
1 participant
Fixes #363
Summary by CodeRabbit