fix(ndk_flutter): prevent QR code overflow in nostr connect dialog#636
Merged
Conversation
Constrain the QR view with a ConstrainedBox (maxWidth/maxHeight 300) so it no longer overflows on narrow screens nor stretches the dialog on wide ones. Also switch to PrettyQrSmoothSymbol with a standard quiet zone and force a white background for best scannabillity. Wire up NostrConnect (app name + default relays) in the sample app's login popup and widgets demo, and wrap the login popup content in a SingleChildScrollView to avoid overflow when the dialog content grows.
📝 WalkthroughWalkthroughThe PR updates NostrConnect integration across the NDK Flutter package and sample app. The dialog view refines QR code visual presentation with white background and smoother rendering, while the sample app explicitly configures NostrConnect with standardized app name and relay endpoints in both the login popup and widgets demo. ChangesNostrConnect UI and Configuration Updates
🎯 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: 1
🧹 Nitpick comments (1)
packages/sample-app/lib/widgets_demo_page.dart (1)
219-226: ⚡ Quick winExtract duplicated NostrConnect configuration to a shared constant.
The NostrConnect configuration (appName and relays list) is duplicated between this file and
login_popup.dart. Consider extracting these values to a shared constant to ensure consistency and simplify maintenance.♻️ Proposed refactor
Create a new file
lib/config/nostr_config.dart:import 'package:ndk/ndk.dart'; const String kNostrAppName = 'NDK sample app'; const List<String> kNostrRelays = [ "wss://relay.damus.io", "wss://relay.primal.net", "wss://relay.nmail.li", ]; NostrConnect createNostrConnect() { return NostrConnect( appName: kNostrAppName, relays: kNostrRelays, ); }Then use it in both files:
+import 'package:ndk_demo/config/nostr_config.dart'; -nostrConnect: NostrConnect( - appName: 'NDK sample app', - relays: [ - "wss://relay.damus.io", - "wss://relay.primal.net", - "wss://relay.nmail.li", - ], -), +nostrConnect: createNostrConnect(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/sample-app/lib/widgets_demo_page.dart` around lines 219 - 226, Extract the duplicated NostrConnect config into shared constants and a factory: create a new module exposing const String kNostrAppName, const List<String> kNostrRelays and a helper NostrConnect createNostrConnect() that returns NostrConnect(appName: kNostrAppName, relays: kNostrRelays). Replace the inline NostrConnect(...) constructions in the code (the instances you see using NostrConnect with appName and relays) to import the new module and call createNostrConnect() (or use the constants) so both places reference the same kNostrAppName / kNostrRelays values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ndk_flutter/lib/widgets/login/nostr_connect_dialog_view.dart`:
- Around line 18-31: Replace the incorrect constant PrettyQrQuietZone.standard
with the correct PrettyQrQuietZone.standart in the PrettyQrView decoration;
update the usage inside the widget that builds the QR (the PrettyQrView.data /
PrettyQrDecoration block) so it matches the library's constant name (same fix
used elsewhere like in n_wallet_actions.dart).
---
Nitpick comments:
In `@packages/sample-app/lib/widgets_demo_page.dart`:
- Around line 219-226: Extract the duplicated NostrConnect config into shared
constants and a factory: create a new module exposing const String
kNostrAppName, const List<String> kNostrRelays and a helper NostrConnect
createNostrConnect() that returns NostrConnect(appName: kNostrAppName, relays:
kNostrRelays). Replace the inline NostrConnect(...) constructions in the code
(the instances you see using NostrConnect with appName and relays) to import the
new module and call createNostrConnect() (or use the constants) so both places
reference the same kNostrAppName / kNostrRelays values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74541b2e-b322-4a08-accc-6104a820c17f
📒 Files selected for processing (3)
packages/ndk_flutter/lib/widgets/login/nostr_connect_dialog_view.dartpackages/sample-app/lib/login_popup.dartpackages/sample-app/lib/widgets_demo_page.dart
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #636 +/- ##
==========================================
+ Coverage 72.03% 72.09% +0.05%
==========================================
Files 209 209
Lines 10653 10653
==========================================
+ Hits 7674 7680 +6
+ Misses 2979 2973 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
frnandu
approved these changes
May 21, 2026
1-leo
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Constrain the QR view with a ConstrainedBox (maxWidth/maxHeight 300) so it no longer overflows on narrow screens nor stretches the dialog on wide ones. Also switch to PrettyQrSmoothSymbol with a standard quiet zone and force a white background for best scannabillity.
Wire up NostrConnect (app name + default relays) in the sample app's login popup and widgets demo, and wrap the login popup content in a SingleChildScrollView to avoid overflow when the dialog content grows.
Summary by CodeRabbit
Style
Refactor