feat: long-press QR code to share as image#689
Open
dannym-arx wants to merge 10 commits into
Open
Conversation
dannym-arx
requested review from
cypherpinkdev,
erskingardner,
jgmontoya,
mubarakcoded and
pepina-dev
as code owners
May 20, 2026 19:27
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you:
Chapters generated by Stage for commit 7d69244 on May 22, 2026 9:13am UTC. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds long-press-to-share for the profile QR with animated hold feedback, captures the QR as PNG and shares via SharePlus, updates SharePlus test mocking, extends widget tests for long-press flows, and adds localized hold/share strings across multiple ARB locale files. ChangesQR Code Long-Press Sharing
Sequence Diagram(s)sequenceDiagram
participant User
participant GestureDetector
participant AnimationController
participant AnimatedBuilder
participant QrImageView
participant RepaintBoundary
participant SharePlus
User->>GestureDetector: long-press start
GestureDetector->>AnimationController: start color/dot animation
AnimationController->>AnimatedBuilder: frame tick
AnimatedBuilder->>QrImageView: update eye/data color
User->>GestureDetector: release after hold
GestureDetector->>RepaintBoundary: request toImage(pixelRatio:3)
RepaintBoundary->>RepaintBoundary: toByteData(PNG)
RepaintBoundary->>SharePlus: pass image bytes
SharePlus->>User: show native share sheet
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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)
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: 2
🤖 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 `@lib/screens/share_profile_screen.dart`:
- Around line 60-82: In captureAndShareQr, wrap the image capture
(boundary.toImage / img.toByteData) and the SharePlus.instance.share call in a
try/catch so failures are surfaced; on error, call the existing showErrorNotice
callback with a user-facing message and optionally log the error (include the
caught exception), and ensure you return early if capture returns null or is
cancelled; reference qrRepaintKey/currentContext/findRenderObject, toImage,
toByteData, and SharePlus.instance.share when making the changes.
- Around line 60-82: The async method captureAndShareQr updates dotCount.value,
reads isHolding.value and calls qrColorAnim.forward() across awaits and can run
after the widget is disposed; fix it by adding a mounted guard (e.g., a
useRef<bool> or mounted boolean) that you set true on init and false on dispose,
then check that guard immediately after each await (before modifying
dotCount.value, before calling qrColorAnim.forward or reading isHolding.value,
and before invoking SharePlus) and abort/return if not mounted; also avoid
calling qrColorAnim.forward() if the animation controller is disposed by
checking the guard first so no post-disposal access occurs.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: febfc7a8-adbd-4855-8209-c1d1d02be524
⛔ Files ignored due to path filters (10)
lib/l10n/generated/app_localizations.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_de.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_en.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_es.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_fr.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_it.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_pt.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_ru.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_tr.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_zh.dartis excluded by!**/generated/**,!lib/l10n/generated/**
📒 Files selected for processing (11)
lib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_pt.arblib/l10n/app_ru.arblib/l10n/app_tr.arblib/l10n/app_zh.arblib/l10n/app_zh_Hant.arblib/screens/share_profile_screen.dart
📜 Review details
⏰ 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). (4)
- GitHub Check: Test (4/4)
- GitHub Check: Test (3/4)
- GitHub Check: Test (2/4)
- GitHub Check: Test (1/4)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.arb
⚙️ CodeRabbit configuration file
**/*.arb: These are localization files. Check for:
- Consistent key naming across all locale files
- Proper ICU message format for plurals/gender
- No hardcoded strings that should be localized
Files:
lib/l10n/app_zh.arblib/l10n/app_zh_Hant.arblib/l10n/app_it.arblib/l10n/app_ru.arblib/l10n/app_es.arblib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_fr.arblib/l10n/app_pt.arblib/l10n/app_tr.arb
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
lib/**/*.dart: Use single quotes for strings in Dart/Flutter
Enable and followprefer_const_constructorslinting rule in Dart
Enable and followprefer_final_localslinting rule in Dart
Maintain line width of 100 characters in Dart/Flutter code
Preserve trailing commas in Dart/Flutter code
Do not add comments except for code that is really complex or hard to understand
Useflutter_screenutilfor all size values to ensure responsive layouts: use.wfor width,.hfor height,.spfor font size/letter spacing,.rfor radius
Avoid StatefulWidget; prefer providers (shared app-wide state) or hooks (widget-local state) instead
Avoid using// coverage:ignore,// coverage:ignore-line,// coverage:ignore-start, or// coverage:ignore-endto bypass coverage requirements; write tests instead
Files:
lib/screens/share_profile_screen.dart
lib/screens/**
⚙️ CodeRabbit configuration file
lib/screens/**: This is a Flutter screen (full-page widget).
Architecture rules:
- Screens should WATCH Riverpod providers for shared state
- Use flutter_hooks for ephemeral/local state (NOT StatefulWidget)
- Pass data to hooks, not refs
- Use flutter_screenutil (.w, .h, .sp, .r) for all size values
- Widgets should use const constructors where possible
- No comments except for truly complex logic
- When a widget is extracted from a screen and only used in that one
screen, it should be prefixed with the screen name (e.g.
ChatListTile for a widget only used in the chat list screen).
These are screen-scoped widgets and do NOT use the Wn prefix.
Files:
lib/screens/share_profile_screen.dart
🧠 Learnings (21)
📚 Learning: 2026-02-02T10:05:10.264Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 128
File: lib/l10n/app_it.arb:53-53
Timestamp: 2026-02-02T10:05:10.264Z
Learning: For the Sloth app localization (lib/l10n/app_it.arb), ensure the Italian translations for the keys 'cancel' and 'discard' are both set to 'Annulla'. This should be consistently used across UI labels, dialogs, and tooltips. If there are multiple occurrences of these keys, verify all use the same translation to avoid inconsistencies.
Applied to files:
lib/l10n/app_it.arb
📚 Learning: 2026-03-04T16:40:26.588Z
Learnt from: josefinalliende
Repo: marmot-protocol/whitenoise PR: 408
File: lib/l10n/app_ru.arb:107-107
Timestamp: 2026-03-04T16:40:26.588Z
Learning: In the whitenoise Flutter project lib/l10n/app_ru.arb, the invitedYouToChatSuffix uses masculine past-tense (пригласил) as the default form. Since Nostr identities have no gender metadata, ICU gender-select is not feasible. Do not flag masculine default as a missing gender-agreement issue for Russian or other locales in this project.
Applied to files:
lib/l10n/app_ru.arb
📚 Learning: 2026-01-05T20:05:32.918Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: lib/screens/settings_screen.dart:76-88
Timestamp: 2026-01-05T20:05:32.918Z
Learning: In the Sloth project, tooltips should not be used for UI elements. During code reviews, verify that UI components do not include Tooltip widgets and replace them with accessible alternatives (e.g., long-press hints, labels, or interactive guidance). Apply this guidance across Dart UI files under lib (not just settings_screen.dart) to maintain a consistent design approach.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-09T13:25:18.531Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 33
File: lib/services/message_service.dart:14-26
Timestamp: 2026-01-09T13:25:18.531Z
Learning: When errors are presented to users in the Sloth codebase (UI layers such as snackbars, dialogs, toasts), show friendly, user-facing messages instead of raw Rust error messages. Implement a mapping from internal error types or codes to clear, non-technical messages, and surface actionable guidance for end users where appropriate. Avoid exposing internal stack traces or language runtime details; centralize common user-facing error wording in a dedicated utility or service and translate backend errors into concise, helpful UI messages.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:40:41.044Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 552
File: lib/screens/profile_keys_screen.dart:71-71
Timestamp: 2026-04-06T09:40:41.044Z
Learning: In this Flutter/Dart codebase, when using flutter_screenutil (e.g., ScreenUtil or ScreenUtil-based sizing like `w`, `h`), do not require scaled units for literal zero values. Specifically, in EdgeInsets (and similar sizing/padding/margin/gap APIs), bare numeric `0` should be allowed (e.g., `EdgeInsets.all(0)`, `EdgeInsets.symmetric(vertical: 0)`, `SizedBox(width: 0)`), because scaling `0` remains `0`. Only flag ScreenUtil violations when a non-zero literal needs to be expressed via `0.w`/`0.h` equivalents; do not flag bare `0`.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-06T01:08:41.552Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: lib/widgets/wn_warning_box.dart:28-79
Timestamp: 2026-01-06T01:08:41.552Z
Learning: In reviews for the marmot-protocol/sloth repository, avoid suggesting accessibility, design, or general improvements unless they are strictly relevant to the PR description and its stated goals. Focus feedback on the specific PR objectives and requirements; ignore unrelated stylistic or broad improvement suggestions.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-15T14:42:54.111Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 39
File: lib/hooks/use_user_search.dart:47-53
Timestamp: 2026-01-15T14:42:54.111Z
Learning: When using npubFromHex(String hexPubkey) from lib/utils/formatting.dart, it already handles errors internally and returns null on failure. Do not wrap calls to this function in try/catch blocks. Instead, check for a null return and handle accordingly (e.g., treat as invalid hex). This guideline applies to all Dart files in the repository that may call this function.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-16T17:35:32.431Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 43
File: lib/hooks/use_network_relays.dart:155-184
Timestamp: 2026-01-16T17:35:32.431Z
Learning: In Dart/Flutter code, prefer void as the return type for fire-and-forget async functions that are intended to run in the background without blocking the caller. Using Future or Future<void> would allow callers to accidentally await, which could block for long periods. This pattern makes the call fire-and-forget by design. Apply to functions that intentionally should not be awaited; document the intent where appropriate. (Example context: pollRelayStatus in lib/hooks/use_network_relays.dart uses void to prevent callers from awaiting a potentially long-running background task.)
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-19T15:50:56.684Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 52
File: lib/screens/app_settings_screen.dart:30-40
Timestamp: 2026-01-19T15:50:56.684Z
Learning: Target the Flutter SDK version >= 3.27 across the repo. Since features like Column.spacing and Row.spacing were added in Flutter 3.27, you can safely use them in Dart files (e.g., lib/screens/app_settings_screen.dart) as long as the pubspec.yaml environment specifies Flutter >= 3.27. If needed, enforce this by validating the environment constraint (e.g., flutter, sdk: flutter) in pubspec.yaml.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-29T03:02:34.290Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 100
File: lib/widgets/wn_dropdown_selector.dart:126-139
Timestamp: 2026-01-29T03:02:34.290Z
Learning: Global font-family usage guideline: Since Manrope is configured in lib/theme.dart for both light and dark themes, individual TextStyle declarations should not specify fontFamily: 'Manrope'. During reviews, verify that no TextStyle instances override fontFamily unnecessarily; rely on theme inheritance instead. If a TextStyle must specify a font for a specific case, ensure it is clearly justified and localized, and document why the override is needed. This guideline applies across Dart files in the project (e.g., lib/widgets/wn_dropdown_selector.dart and related components).
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-29T16:02:52.588Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 129
File: test/widgets/wn_overlay_test.dart:84-91
Timestamp: 2026-01-29T16:02:52.588Z
Learning: In Dart/Flutter code, when constructing widgets inside an already-const context (for example, within a const Scaffold with a child that is a collection of widgets), avoid adding explicit const keywords to the children. The Dart analyzer treats these as implicitly const, and adding explicit const can trigger the unnecessary_const lint. Review test files and general Dart files to ensure you do not redundantly prefix children with const in const contexts; rely on the implicit const behavior instead.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-07T03:58:22.587Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 193
File: lib/services/android_signer_service.dart:13-13
Timestamp: 2026-02-07T03:58:22.587Z
Learning: In Dart files across the repository, retain NIP (Nostr Implementation Possibilities) and MIP (Marmot Implementation Possibilities) protocol reference comments that link code to their specifications. While general guidance discourages extraneous comments, these references improve traceability and maintainability by documenting the protocol context for related implementation details. Keep them in Dart sources (e.g., lib/services/android_signer_service.dart) and ensure they are kept up-to-date.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-11T17:29:43.985Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 225
File: lib/screens/app_settings_screen.dart:64-69
Timestamp: 2026-02-11T17:29:43.985Z
Learning: In the whitenoise Flutter app, after deleting all data and resetting auth, use Routes.goToHome(context) instead of Routes.goToLogin(context) because the home screen is the app entry point and will manage authentication routing. Apply this change to navigation calls that should land on the home screen after a data reset or auth reset. Update lib/screens/app_settings_screen.dart and any similar navigation points accordingly.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-18T18:36:13.394Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 315
File: lib/screens/edit_group_screen.dart:45-47
Timestamp: 2026-02-18T18:36:13.394Z
Learning: Whitenoise uses automated code formatting through the precommit workflow (just precommit). Do not tweak line wrapping or formatting manually; follow the formatter's output. Run the precommit formatter locally and ensure the code matches its styling decisions, and only deviate if the formatter cannot express the intended style (in which case adjust the code to satisfy the formatter).
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:36:06.726Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 553
File: lib/widgets/wn_copyable_field.dart:38-40
Timestamp: 2026-04-06T09:36:06.726Z
Learning: In Dart, `String.operator*` is a valid built-in operator for repeating strings (e.g., `'⬤' * 16`). In code reviews of Dart (`.dart`) files, do not treat expressions like `'text' * n` as invalid syntax and do not recommend replacing them with alternatives such as `List.filled(...).join()`; the operator is supported by `dart:core`.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-05-13T13:46:32.612Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/hooks/use_block_actions.dart:3-3
Timestamp: 2026-05-13T13:46:32.612Z
Learning: In marmot-protocol/whitenoise code reviews, do not flag imports that reference FRB-generated bindings under `package:rust_lib_whitenoise/src/rust/...` as private-internals violations. In `rust_lib_whitenoise`, Dart bindings are intentionally generated into `lib/src/rust/`, and `analysis_options.yaml` sets `implementation_imports: false` to suppress the related Dart lint—so `package:rust_lib_whitenoise/src/rust/...` imports are the correct/only way to consume that FRB-generated API.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-05-13T13:46:39.250Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/services/foreground_service.dart:13-16
Timestamp: 2026-05-13T13:46:39.250Z
Learning: In this repo (marmot-protocol/whitenoise), `rust_lib_whitenoise` is an FRB-generated Flutter package where the generated Dart bindings are intentionally exposed under `lib/src/rust/` as the public API surface. Do not treat imports like `package:rust_lib_whitenoise/src/rust/...` as breaking package API boundaries in code reviews (the project also sets `implementation_imports: false` in `analysis_options.yaml` to opt into this). Never raise concerns about `src/` imports from `rust_lib_whitenoise`.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-15T15:00:17.356Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 39
File: lib/screens/user_search_screen.dart:50-75
Timestamp: 2026-01-15T15:00:17.356Z
Learning: In lib/screens/user_search_screen.dart and similar screen widgets in the sloth repository, prefer using inline ternary expressions for conditional rendering (e.g., isLoading → empty state → list) over extracting into separate widget classes when the conditional UI is simple. This improves readability by keeping the render logic in a single place. Reserve extracting to a separate widget only when the conditional UI becomes sufficiently complex or is reused across multiple screens.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-10T04:57:31.475Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 208
File: lib/screens/signup_screen.dart:66-70
Timestamp: 2026-02-10T04:57:31.475Z
Learning: In Dart/Flutter using flutter_hooks, a ValueNotifier (from useState) is disposed when the widget unmounts. Async callbacks (e.g., .then() on animations) may still run after disposal and throw 'A ValueNotifier was used after being disposed'. Reviewers should ensure that state updates in async callbacks are guarded by a mounted check (e.g., if (mounted) { notifier.value = ... } or cancel/avoid updating after disposal). This guideline applies to screen widgets under lib/screens and similar Dart files using hooks that manage widget lifecycle. Update code patterns to prevent post-disposal state updates, and document the rationale in comments where necessary.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-19T10:33:16.889Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 315
File: lib/screens/group_info_screen.dart:146-149
Timestamp: 2026-02-19T10:33:16.889Z
Learning: Pubkey validation (length/format) is performed by the Rust crate before data reaches Dart. Do not add defensive substring guards in Dart for pubkeys in lib/screens/**/*.dart or other Flutter code consuming group member pubkeys, as this would duplicate validation and be unnecessary. If any validation is needed in Dart, keep it to lightweight sanity checks only after data enters Dart and ensure it aligns with the Rust validation rules.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:40:51.965Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 552
File: lib/screens/sign_out_screen.dart:86-86
Timestamp: 2026-04-06T09:40:51.965Z
Learning: When reviewing Flutter/Dart code that uses ScreenUtil (e.g., ScreenUtil `.h`/`.w`/`.sp`), do not require converting a literal `0` used for padding/margin/sizing to a ScreenUtil expression. In sizing contexts like `EdgeInsets` (or other `EdgeInsets`/layout parameters), raw `0` is mathematically invariant (`0 * scale = 0`) and should not be flagged as a ScreenUtil violation. Only non-zero literal size values should be checked for ScreenUtil usage.
Applied to files:
lib/screens/share_profile_screen.dart
🔇 Additional comments (15)
lib/l10n/app_de.arb (1)
97-97: LGTM!lib/l10n/app_en.arb (1)
402-405: LGTM!lib/l10n/app_es.arb (1)
97-97: LGTM!lib/l10n/app_fr.arb (1)
97-97: LGTM!lib/l10n/app_it.arb (1)
97-97: LGTM!lib/l10n/app_pt.arb (1)
97-97: LGTM!lib/l10n/app_ru.arb (1)
97-97: LGTM!lib/l10n/app_tr.arb (1)
97-97: LGTM!lib/l10n/app_zh.arb (1)
97-97: LGTM!lib/l10n/app_zh_Hant.arb (1)
97-97: LGTM!lib/screens/share_profile_screen.dart (5)
1-1: LGTM!Also applies to: 4-4, 10-10
49-58: LGTM!
135-149: LGTM!
150-179: LGTM!
184-193: LGTM!
✅ Coverage: 99.03% → 99.03% (0.00%)History
|
dannym-arx
marked this pull request as draft
May 20, 2026 19:37
Contributor
Author
|
@coderabbitai one more full review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainThe PR description image shows a delightful pixel art illustration of two marmots in a mountain meadow — one holding a smartphone displaying a QR code — with the text "SHARE! <3" at the top. Very on-brand! 🐾 Let me pull up the full source to do a thorough review. 🏁 Script executed: Length of output: 274552
Review of PR
|
| Severity | Issue |
|---|---|
| 🔴 High | XFile.fromData missing name: 'qr_code.png' — affects share sheet on iOS & some Android targets |
| 🟡 Medium | No concurrent-invocation guard on captureAndShareQr() |
| 🟡 Medium | catch block doesn't reset dot/animation state |
| 🟡 Medium | "Hold to share" label semantics during active hold |
The core logic is solid and the UX idea is great — just the XFile filename is a must-fix before shipping. 🐾
Contributor
Author
|
@coderabbitai one more, mr rabbit |
|
(ˆ(oo)ˆ) 🐇 ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/screens/share_profile_screen.dart (1)
68-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap the full capture pipeline in the existing error/finally guard.
isCapturingflips totrueat Line 70, but thetry/finallydoes not start until Line 97. IfqrColorAnim.forward()orboundary.toImage(...)throws before that, the method exits withisCapturingstilltrue, and every later long-press is ignored until rebuild. ThebyteData == nullbranch also still fails silently instead of using the localized share error path.🛡️ Minimal fix
Future<void> captureAndShareQr() async { if (isCapturing.value) return; isCapturing.value = true; - final boundary = qrRepaintKey.currentContext?.findRenderObject() as RenderRepaintBoundary?; - if (boundary == null) { - isCapturing.value = false; - return; - } final errorMessage = context.l10n.shareQrCodeError; - // Start dots and animation immediately; capture the last painted frame - // in parallel — it's still full-color since no new frame has rendered yet. - dotCount.value = 1; - qrColorAnim.forward(); - final captureFuture = boundary - .toImage(pixelRatio: 3) - .then((img) => img.toByteData(format: ui.ImageByteFormat.png)); - // onLongPressStart fires at 500ms total; dots step every 500ms after that - await Future.delayed(const Duration(milliseconds: 500)); - if (!isMounted.value || !isHolding.value) { - isCapturing.value = false; - return; - } - dotCount.value = 2; - await Future.delayed(const Duration(milliseconds: 500)); - if (!isMounted.value || !isHolding.value) { - isCapturing.value = false; - return; - } - dotCount.value = 3; try { + final boundary = + qrRepaintKey.currentContext?.findRenderObject() as RenderRepaintBoundary?; + if (boundary == null) return; + dotCount.value = 1; + qrColorAnim.forward(); + final captureFuture = boundary + .toImage(pixelRatio: 3) + .then((img) => img.toByteData(format: ui.ImageByteFormat.png)); + await Future.delayed(const Duration(milliseconds: 500)); + if (!isMounted.value || !isHolding.value) return; + dotCount.value = 2; + await Future.delayed(const Duration(milliseconds: 500)); + if (!isMounted.value || !isHolding.value) return; + dotCount.value = 3; final byteData = await captureFuture; - if (!isMounted.value || byteData == null || !isHolding.value) { - isCapturing.value = false; - return; - } + if (!isMounted.value || !isHolding.value) return; + if (byteData == null) throw StateError('QR capture returned null bytes'); await SharePlus.instance.share( ShareParams( files: [ @@ } catch (e) { if (isMounted.value) { dotCount.value = 0; qrColorAnim.reverse(); showErrorNotice(errorMessage); } } finally { isCapturing.value = false; } }🤖 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 `@lib/screens/share_profile_screen.dart` around lines 68 - 123, The captureAndShareQr method currently sets isCapturing.value = true before the try/finally block, so exceptions thrown by qrColorAnim.forward() or boundary.toImage(...) can leave isCapturing stuck true; wrap the entire capture pipeline (starting immediately after setting isCapturing.value = true) inside a try { ... } catch { ... } finally { isCapturing.value = false; } block so any thrown error resets isCapturing; also move the existing catch body (dotCount.value = 0; qrColorAnim.reverse(); showErrorNotice(errorMessage);) into that catch and ensure the branch where byteData == null triggers the same showErrorNotice(errorMessage) path instead of silent return, referencing captureAndShareQr, isCapturing, qrColorAnim.forward/reverse, boundary.toImage, byteData, and showErrorNotice to locate the changes.
🤖 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 `@lib/l10n/app_zh_Hant.arb`:
- Around line 97-98: Update the two localization entries "holdToShareQrCode" and
"shareQrCodeError" to match the existing Traditional Chinese style by using "QR
碼" (with a space) and the consistent failure phrasing "請再試一次"; specifically
replace the value for holdToShareQrCode with the variant using "QR 碼" and
replace shareQrCodeError's value to use "請再試一次" so both keys align with the
locale's established wording.
---
Duplicate comments:
In `@lib/screens/share_profile_screen.dart`:
- Around line 68-123: The captureAndShareQr method currently sets
isCapturing.value = true before the try/finally block, so exceptions thrown by
qrColorAnim.forward() or boundary.toImage(...) can leave isCapturing stuck true;
wrap the entire capture pipeline (starting immediately after setting
isCapturing.value = true) inside a try { ... } catch { ... } finally {
isCapturing.value = false; } block so any thrown error resets isCapturing; also
move the existing catch body (dotCount.value = 0; qrColorAnim.reverse();
showErrorNotice(errorMessage);) into that catch and ensure the branch where
byteData == null triggers the same showErrorNotice(errorMessage) path instead of
silent return, referencing captureAndShareQr, isCapturing,
qrColorAnim.forward/reverse, boundary.toImage, byteData, and showErrorNotice to
locate the changes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 208663bf-4b0d-484c-bfd3-7054e656ad2e
⛔ Files ignored due to path filters (10)
lib/l10n/generated/app_localizations.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_de.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_en.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_es.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_fr.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_it.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_pt.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_ru.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_tr.dartis excluded by!**/generated/**,!lib/l10n/generated/**lib/l10n/generated/app_localizations_zh.dartis excluded by!**/generated/**,!lib/l10n/generated/**
📒 Files selected for processing (11)
lib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_pt.arblib/l10n/app_ru.arblib/l10n/app_tr.arblib/l10n/app_zh.arblib/l10n/app_zh_Hant.arblib/screens/share_profile_screen.dart
📜 Review details
⏰ 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). (4)
- GitHub Check: Test (4/4)
- GitHub Check: Test (1/4)
- GitHub Check: Test (3/4)
- GitHub Check: Test (2/4)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.arb
⚙️ CodeRabbit configuration file
**/*.arb: These are localization files. Check for:
- Consistent key naming across all locale files
- Proper ICU message format for plurals/gender
- No hardcoded strings that should be localized
Files:
lib/l10n/app_fr.arblib/l10n/app_de.arblib/l10n/app_tr.arblib/l10n/app_it.arblib/l10n/app_pt.arblib/l10n/app_ru.arblib/l10n/app_zh_Hant.arblib/l10n/app_en.arblib/l10n/app_zh.arblib/l10n/app_es.arb
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
lib/**/*.dart: Use single quotes for strings in Dart/Flutter
Enable and followprefer_const_constructorslinting rule in Dart
Enable and followprefer_final_localslinting rule in Dart
Maintain line width of 100 characters in Dart/Flutter code
Preserve trailing commas in Dart/Flutter code
Do not add comments except for code that is really complex or hard to understand
Useflutter_screenutilfor all size values to ensure responsive layouts: use.wfor width,.hfor height,.spfor font size/letter spacing,.rfor radius
Avoid StatefulWidget; prefer providers (shared app-wide state) or hooks (widget-local state) instead
Avoid using// coverage:ignore,// coverage:ignore-line,// coverage:ignore-start, or// coverage:ignore-endto bypass coverage requirements; write tests instead
Files:
lib/screens/share_profile_screen.dart
lib/screens/**
⚙️ CodeRabbit configuration file
lib/screens/**: This is a Flutter screen (full-page widget).
Architecture rules:
- Screens should WATCH Riverpod providers for shared state
- Use flutter_hooks for ephemeral/local state (NOT StatefulWidget)
- Pass data to hooks, not refs
- Use flutter_screenutil (.w, .h, .sp, .r) for all size values
- Widgets should use const constructors where possible
- No comments except for truly complex logic
- When a widget is extracted from a screen and only used in that one
screen, it should be prefixed with the screen name (e.g.
ChatListTile for a widget only used in the chat list screen).
These are screen-scoped widgets and do NOT use the Wn prefix.
Files:
lib/screens/share_profile_screen.dart
🧠 Learnings (21)
📚 Learning: 2026-02-02T10:05:10.264Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 128
File: lib/l10n/app_it.arb:53-53
Timestamp: 2026-02-02T10:05:10.264Z
Learning: For the Sloth app localization (lib/l10n/app_it.arb), ensure the Italian translations for the keys 'cancel' and 'discard' are both set to 'Annulla'. This should be consistently used across UI labels, dialogs, and tooltips. If there are multiple occurrences of these keys, verify all use the same translation to avoid inconsistencies.
Applied to files:
lib/l10n/app_it.arb
📚 Learning: 2026-03-04T16:40:26.588Z
Learnt from: josefinalliende
Repo: marmot-protocol/whitenoise PR: 408
File: lib/l10n/app_ru.arb:107-107
Timestamp: 2026-03-04T16:40:26.588Z
Learning: In the whitenoise Flutter project lib/l10n/app_ru.arb, the invitedYouToChatSuffix uses masculine past-tense (пригласил) as the default form. Since Nostr identities have no gender metadata, ICU gender-select is not feasible. Do not flag masculine default as a missing gender-agreement issue for Russian or other locales in this project.
Applied to files:
lib/l10n/app_ru.arb
📚 Learning: 2026-01-05T20:05:32.918Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: lib/screens/settings_screen.dart:76-88
Timestamp: 2026-01-05T20:05:32.918Z
Learning: In the Sloth project, tooltips should not be used for UI elements. During code reviews, verify that UI components do not include Tooltip widgets and replace them with accessible alternatives (e.g., long-press hints, labels, or interactive guidance). Apply this guidance across Dart UI files under lib (not just settings_screen.dart) to maintain a consistent design approach.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-09T13:25:18.531Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 33
File: lib/services/message_service.dart:14-26
Timestamp: 2026-01-09T13:25:18.531Z
Learning: When errors are presented to users in the Sloth codebase (UI layers such as snackbars, dialogs, toasts), show friendly, user-facing messages instead of raw Rust error messages. Implement a mapping from internal error types or codes to clear, non-technical messages, and surface actionable guidance for end users where appropriate. Avoid exposing internal stack traces or language runtime details; centralize common user-facing error wording in a dedicated utility or service and translate backend errors into concise, helpful UI messages.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:40:41.044Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 552
File: lib/screens/profile_keys_screen.dart:71-71
Timestamp: 2026-04-06T09:40:41.044Z
Learning: In this Flutter/Dart codebase, when using flutter_screenutil (e.g., ScreenUtil or ScreenUtil-based sizing like `w`, `h`), do not require scaled units for literal zero values. Specifically, in EdgeInsets (and similar sizing/padding/margin/gap APIs), bare numeric `0` should be allowed (e.g., `EdgeInsets.all(0)`, `EdgeInsets.symmetric(vertical: 0)`, `SizedBox(width: 0)`), because scaling `0` remains `0`. Only flag ScreenUtil violations when a non-zero literal needs to be expressed via `0.w`/`0.h` equivalents; do not flag bare `0`.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-06T01:08:41.552Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: lib/widgets/wn_warning_box.dart:28-79
Timestamp: 2026-01-06T01:08:41.552Z
Learning: In reviews for the marmot-protocol/sloth repository, avoid suggesting accessibility, design, or general improvements unless they are strictly relevant to the PR description and its stated goals. Focus feedback on the specific PR objectives and requirements; ignore unrelated stylistic or broad improvement suggestions.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-15T14:42:54.111Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 39
File: lib/hooks/use_user_search.dart:47-53
Timestamp: 2026-01-15T14:42:54.111Z
Learning: When using npubFromHex(String hexPubkey) from lib/utils/formatting.dart, it already handles errors internally and returns null on failure. Do not wrap calls to this function in try/catch blocks. Instead, check for a null return and handle accordingly (e.g., treat as invalid hex). This guideline applies to all Dart files in the repository that may call this function.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-16T17:35:32.431Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 43
File: lib/hooks/use_network_relays.dart:155-184
Timestamp: 2026-01-16T17:35:32.431Z
Learning: In Dart/Flutter code, prefer void as the return type for fire-and-forget async functions that are intended to run in the background without blocking the caller. Using Future or Future<void> would allow callers to accidentally await, which could block for long periods. This pattern makes the call fire-and-forget by design. Apply to functions that intentionally should not be awaited; document the intent where appropriate. (Example context: pollRelayStatus in lib/hooks/use_network_relays.dart uses void to prevent callers from awaiting a potentially long-running background task.)
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-19T15:50:56.684Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 52
File: lib/screens/app_settings_screen.dart:30-40
Timestamp: 2026-01-19T15:50:56.684Z
Learning: Target the Flutter SDK version >= 3.27 across the repo. Since features like Column.spacing and Row.spacing were added in Flutter 3.27, you can safely use them in Dart files (e.g., lib/screens/app_settings_screen.dart) as long as the pubspec.yaml environment specifies Flutter >= 3.27. If needed, enforce this by validating the environment constraint (e.g., flutter, sdk: flutter) in pubspec.yaml.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-29T03:02:34.290Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 100
File: lib/widgets/wn_dropdown_selector.dart:126-139
Timestamp: 2026-01-29T03:02:34.290Z
Learning: Global font-family usage guideline: Since Manrope is configured in lib/theme.dart for both light and dark themes, individual TextStyle declarations should not specify fontFamily: 'Manrope'. During reviews, verify that no TextStyle instances override fontFamily unnecessarily; rely on theme inheritance instead. If a TextStyle must specify a font for a specific case, ensure it is clearly justified and localized, and document why the override is needed. This guideline applies across Dart files in the project (e.g., lib/widgets/wn_dropdown_selector.dart and related components).
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-29T16:02:52.588Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 129
File: test/widgets/wn_overlay_test.dart:84-91
Timestamp: 2026-01-29T16:02:52.588Z
Learning: In Dart/Flutter code, when constructing widgets inside an already-const context (for example, within a const Scaffold with a child that is a collection of widgets), avoid adding explicit const keywords to the children. The Dart analyzer treats these as implicitly const, and adding explicit const can trigger the unnecessary_const lint. Review test files and general Dart files to ensure you do not redundantly prefix children with const in const contexts; rely on the implicit const behavior instead.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-07T03:58:22.587Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 193
File: lib/services/android_signer_service.dart:13-13
Timestamp: 2026-02-07T03:58:22.587Z
Learning: In Dart files across the repository, retain NIP (Nostr Implementation Possibilities) and MIP (Marmot Implementation Possibilities) protocol reference comments that link code to their specifications. While general guidance discourages extraneous comments, these references improve traceability and maintainability by documenting the protocol context for related implementation details. Keep them in Dart sources (e.g., lib/services/android_signer_service.dart) and ensure they are kept up-to-date.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-11T17:29:43.985Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 225
File: lib/screens/app_settings_screen.dart:64-69
Timestamp: 2026-02-11T17:29:43.985Z
Learning: In the whitenoise Flutter app, after deleting all data and resetting auth, use Routes.goToHome(context) instead of Routes.goToLogin(context) because the home screen is the app entry point and will manage authentication routing. Apply this change to navigation calls that should land on the home screen after a data reset or auth reset. Update lib/screens/app_settings_screen.dart and any similar navigation points accordingly.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-18T18:36:13.394Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 315
File: lib/screens/edit_group_screen.dart:45-47
Timestamp: 2026-02-18T18:36:13.394Z
Learning: Whitenoise uses automated code formatting through the precommit workflow (just precommit). Do not tweak line wrapping or formatting manually; follow the formatter's output. Run the precommit formatter locally and ensure the code matches its styling decisions, and only deviate if the formatter cannot express the intended style (in which case adjust the code to satisfy the formatter).
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:36:06.726Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 553
File: lib/widgets/wn_copyable_field.dart:38-40
Timestamp: 2026-04-06T09:36:06.726Z
Learning: In Dart, `String.operator*` is a valid built-in operator for repeating strings (e.g., `'⬤' * 16`). In code reviews of Dart (`.dart`) files, do not treat expressions like `'text' * n` as invalid syntax and do not recommend replacing them with alternatives such as `List.filled(...).join()`; the operator is supported by `dart:core`.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-05-13T13:46:32.612Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/hooks/use_block_actions.dart:3-3
Timestamp: 2026-05-13T13:46:32.612Z
Learning: In marmot-protocol/whitenoise code reviews, do not flag imports that reference FRB-generated bindings under `package:rust_lib_whitenoise/src/rust/...` as private-internals violations. In `rust_lib_whitenoise`, Dart bindings are intentionally generated into `lib/src/rust/`, and `analysis_options.yaml` sets `implementation_imports: false` to suppress the related Dart lint—so `package:rust_lib_whitenoise/src/rust/...` imports are the correct/only way to consume that FRB-generated API.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-05-13T13:46:39.250Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/services/foreground_service.dart:13-16
Timestamp: 2026-05-13T13:46:39.250Z
Learning: In this repo (marmot-protocol/whitenoise), `rust_lib_whitenoise` is an FRB-generated Flutter package where the generated Dart bindings are intentionally exposed under `lib/src/rust/` as the public API surface. Do not treat imports like `package:rust_lib_whitenoise/src/rust/...` as breaking package API boundaries in code reviews (the project also sets `implementation_imports: false` in `analysis_options.yaml` to opt into this). Never raise concerns about `src/` imports from `rust_lib_whitenoise`.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-15T15:00:17.356Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 39
File: lib/screens/user_search_screen.dart:50-75
Timestamp: 2026-01-15T15:00:17.356Z
Learning: In lib/screens/user_search_screen.dart and similar screen widgets in the sloth repository, prefer using inline ternary expressions for conditional rendering (e.g., isLoading → empty state → list) over extracting into separate widget classes when the conditional UI is simple. This improves readability by keeping the render logic in a single place. Reserve extracting to a separate widget only when the conditional UI becomes sufficiently complex or is reused across multiple screens.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-10T04:57:31.475Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 208
File: lib/screens/signup_screen.dart:66-70
Timestamp: 2026-02-10T04:57:31.475Z
Learning: In Dart/Flutter using flutter_hooks, a ValueNotifier (from useState) is disposed when the widget unmounts. Async callbacks (e.g., .then() on animations) may still run after disposal and throw 'A ValueNotifier was used after being disposed'. Reviewers should ensure that state updates in async callbacks are guarded by a mounted check (e.g., if (mounted) { notifier.value = ... } or cancel/avoid updating after disposal). This guideline applies to screen widgets under lib/screens and similar Dart files using hooks that manage widget lifecycle. Update code patterns to prevent post-disposal state updates, and document the rationale in comments where necessary.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-19T10:33:16.889Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 315
File: lib/screens/group_info_screen.dart:146-149
Timestamp: 2026-02-19T10:33:16.889Z
Learning: Pubkey validation (length/format) is performed by the Rust crate before data reaches Dart. Do not add defensive substring guards in Dart for pubkeys in lib/screens/**/*.dart or other Flutter code consuming group member pubkeys, as this would duplicate validation and be unnecessary. If any validation is needed in Dart, keep it to lightweight sanity checks only after data enters Dart and ensure it aligns with the Rust validation rules.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:40:51.965Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 552
File: lib/screens/sign_out_screen.dart:86-86
Timestamp: 2026-04-06T09:40:51.965Z
Learning: When reviewing Flutter/Dart code that uses ScreenUtil (e.g., ScreenUtil `.h`/`.w`/`.sp`), do not require converting a literal `0` used for padding/margin/sizing to a ScreenUtil expression. In sizing contexts like `EdgeInsets` (or other `EdgeInsets`/layout parameters), raw `0` is mathematically invariant (`0 * scale = 0`) and should not be flagged as a ScreenUtil violation. Only non-zero literal size values should be checked for ScreenUtil usage.
Applied to files:
lib/screens/share_profile_screen.dart
🔇 Additional comments (8)
lib/l10n/app_de.arb (1)
97-98: LGTM!lib/l10n/app_en.arb (1)
402-409: LGTM!lib/l10n/app_es.arb (1)
97-98: LGTM!lib/l10n/app_fr.arb (1)
97-98: LGTM!lib/l10n/app_it.arb (1)
97-98: LGTM!lib/l10n/app_pt.arb (1)
97-98: LGTM!lib/l10n/app_ru.arb (1)
97-98: LGTM!lib/l10n/app_tr.arb (1)
97-98: LGTM!
dannym-arx
marked this pull request as ready for review
May 20, 2026 20:22
dannym-arx
marked this pull request as draft
May 20, 2026 20:23
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/screens/share_profile_screen.dart (1)
81-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap the synchronous capture path in the existing
try/finally.At Line 82,
toImageSync(...)can throw before entering yourtryblock (Line 97). That leavesisCapturing.valuestucktrue, so subsequent long-press shares are permanently blocked until rebuild.💡 Minimal fix
Future<void> captureAndShareQr() async { if (isCapturing.value) return; isCapturing.value = true; - final boundary = qrRepaintKey.currentContext?.findRenderObject() as RenderRepaintBoundary?; - if (boundary == null) { - isCapturing.value = false; - return; - } - final errorMessage = context.l10n.shareQrCodeError; - dotCount.value = 1; - qrColorAnim.forward(); - final captureFuture = boundary - .toImageSync(pixelRatio: 3) - .toByteData(format: ui.ImageByteFormat.png); - await Future.delayed(const Duration(milliseconds: 500)); - if (!isMounted.value || !isHolding.value) { - isCapturing.value = false; - return; - } - dotCount.value = 2; - await Future.delayed(const Duration(milliseconds: 500)); - if (!isMounted.value || !isHolding.value) { - isCapturing.value = false; - return; - } - dotCount.value = 3; try { + final boundary = qrRepaintKey.currentContext?.findRenderObject() as RenderRepaintBoundary?; + if (boundary == null) return; + final errorMessage = context.l10n.shareQrCodeError; + dotCount.value = 1; + qrColorAnim.forward(); + final captureFuture = boundary + .toImageSync(pixelRatio: 3) + .toByteData(format: ui.ImageByteFormat.png); + await Future.delayed(const Duration(milliseconds: 500)); + if (!isMounted.value || !isHolding.value) return; + dotCount.value = 2; + await Future.delayed(const Duration(milliseconds: 500)); + if (!isMounted.value || !isHolding.value) return; + dotCount.value = 3; final byteData = await captureFuture; if (!isMounted.value || byteData == null || !isHolding.value) return; await SharePlus.instance.share( ShareParams( files: [ @@ } finally { isCapturing.value = false; } }Also applies to: 97-122
🤖 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 `@lib/screens/share_profile_screen.dart` around lines 81 - 83, The synchronous image capture call using boundary.toImageSync(...) can throw before your existing try/finally that clears isCapturing.value, so move or wrap the toImageSync(...) invocation (the captureFuture creation currently assigned to captureFuture) inside the same try that sets isCapturing.value and ensure the finally block always resets isCapturing.value; in practice locate the code around boundary, toImageSync, captureFuture and the try/finally that clears isCapturing.value and wrap the sync call there (or set/reset isCapturing.value with an outer try/finally) so no exception can leave isCapturing.value stuck true.
🤖 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.
Duplicate comments:
In `@lib/screens/share_profile_screen.dart`:
- Around line 81-83: The synchronous image capture call using
boundary.toImageSync(...) can throw before your existing try/finally that clears
isCapturing.value, so move or wrap the toImageSync(...) invocation (the
captureFuture creation currently assigned to captureFuture) inside the same try
that sets isCapturing.value and ensure the finally block always resets
isCapturing.value; in practice locate the code around boundary, toImageSync,
captureFuture and the try/finally that clears isCapturing.value and wrap the
sync call there (or set/reset isCapturing.value with an outer try/finally) so no
exception can leave isCapturing.value stuck true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 001582a3-31b4-44e0-988a-cf95289edf3f
📒 Files selected for processing (4)
lib/screens/share_profile_screen.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.darttest/screens/start_chat_screen_test.dart
📜 Review details
⏰ 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). (4)
- GitHub Check: Test (4/4)
- GitHub Check: Test (1/4)
- GitHub Check: Test (3/4)
- GitHub Check: Test (2/4)
🧰 Additional context used
📓 Path-based instructions (5)
test/**/*_test.dart
📄 CodeRabbit inference engine (AGENTS.md)
test/**/*_test.dart: Test files must mirror source structure with_test.dartsuffix and maintain minimum 99% code coverage
UsesetUpTestView(tester)test helper to configure test view dimensions
UsemountTestApp(tester, overrides)test helper to mount full app with provider overrides
UsemountHook(tester, useHook)test helper to test individual hooks
UsemountWidget(child, tester)test helper to mount single widget
UsemountStackedWidget(child, tester)test helper to mount widget in Stack
Mock Rust API usingRustLib.initMock(api: mockApi)in tests
Preferfind.byKey()overfind.byIcon()in tests; add keys to icons in widgets and usefind.byKey(const Key('icon_name'))in tests
Use valid 64-character hex strings for pubkeys in tests (seetest_helpers.dartfor examples), not dummy values like'abc'or'test-pubkey'
Files:
test/screens/start_chat_screen_test.darttest/screens/share_profile_screen_test.dart
test/**
⚙️ CodeRabbit configuration file
test/**: IMPORTANT: CI enforces coverage regression (coverage must never decrease). It does not enforce a fixed 95% minimum threshold.
Rules:
- Test files mirror source structure with _test.dart suffix
- Use helpers from test/test_helpers.dart (setUpTestView, mountTestApp, etc.)
- Mock Rust API using RustLib.initMock(api: mockApi)
- Always extend MockWnApi from test/mocks/mock_wn_api.dart
- Prefer find.byKey() over find.byIcon() for widget testing
- Use valid 64-char hex strings for pubkeys, not dummy values like 'abc'
- Tests must be deterministic — no external service dependencies
Files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dart
test/mocks/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Always extend
MockWnApifromtest/mocks/mock_wn_api.dartinstead of implementingRustLibApidirectly to ensure consistent mock behavior
Files:
test/mocks/mock_share_plus.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
lib/**/*.dart: Use single quotes for strings in Dart/Flutter
Enable and followprefer_const_constructorslinting rule in Dart
Enable and followprefer_final_localslinting rule in Dart
Maintain line width of 100 characters in Dart/Flutter code
Preserve trailing commas in Dart/Flutter code
Do not add comments except for code that is really complex or hard to understand
Useflutter_screenutilfor all size values to ensure responsive layouts: use.wfor width,.hfor height,.spfor font size/letter spacing,.rfor radius
Avoid StatefulWidget; prefer providers (shared app-wide state) or hooks (widget-local state) instead
Avoid using// coverage:ignore,// coverage:ignore-line,// coverage:ignore-start, or// coverage:ignore-endto bypass coverage requirements; write tests instead
Files:
lib/screens/share_profile_screen.dart
lib/screens/**
⚙️ CodeRabbit configuration file
lib/screens/**: This is a Flutter screen (full-page widget).
Architecture rules:
- Screens should WATCH Riverpod providers for shared state
- Use flutter_hooks for ephemeral/local state (NOT StatefulWidget)
- Pass data to hooks, not refs
- Use flutter_screenutil (.w, .h, .sp, .r) for all size values
- Widgets should use const constructors where possible
- No comments except for truly complex logic
- When a widget is extracted from a screen and only used in that one
screen, it should be prefixed with the screen name (e.g.
ChatListTile for a widget only used in the chat list screen).
These are screen-scoped widgets and do NOT use the Wn prefix.
Files:
lib/screens/share_profile_screen.dart
🧠 Learnings (27)
📚 Learning: 2025-12-23T14:37:11.241Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 9
File: test/screens/error_screen_test.dart:60-71
Timestamp: 2025-12-23T14:37:11.241Z
Learning: In tests for screen widgets that are not wired through the app's Routes.build (e.g., ErrorScreen under lib/screens/error_screen.dart), avoid mounting the full app with mountTestApp. instead, configure a minimal test harness with a custom router or navigator setup (e.g., a MaterialApp with a mock router) to render the screen in isolation. This ensures the test focuses on the screen's behavior without depending on the app's global routing configuration.
Applied to files:
test/screens/start_chat_screen_test.darttest/screens/share_profile_screen_test.dart
📚 Learning: 2026-01-05T21:28:05.652Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: test/screens/profile_keys_screen_test.dart:14-23
Timestamp: 2026-01-05T21:28:05.652Z
Learning: In test mocks for the marmot-protocol/sloth project, allow skipping bounds checking for substring operations when test inputs are controlled and have proven sufficient length (e.g., 'test_pubkey' is always 12 characters). Favor simpler, deterministic mock implementations in such controlled test contexts, but avoid relaxing bounds checks in production code or tests with variable inputs. Apply this guideline primarily to test files under the test/ directory.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dart
📚 Learning: 2026-01-07T16:49:18.694Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 33
File: test/screens/chat_screen_test.dart:245-282
Timestamp: 2026-01-07T16:49:18.694Z
Learning: In the marmot-protocol/sloth repository, it is acceptable to use force-unwrapping (!) in Dart test files as an implicit assertion: if a value is unexpectedly null, the test will fail. This guideline applies only to test code under the test/ directory and should not be used in production code.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dart
📚 Learning: 2026-01-11T22:43:09.610Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 36
File: test/screens/chat_list_screen_test.dart:74-76
Timestamp: 2026-01-11T22:43:09.610Z
Learning: In Dart tests (e.g., test/screens/chat_list_screen_test.dart and related files), use setUp() to call _api.reset() to clean up StreamController resources. Do not suggest or rely on tearDownAll for this cleanup.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dart
📚 Learning: 2026-04-14T03:52:37.716Z
Learnt from: josefinalliende
Repo: marmot-protocol/whitenoise PR: 565
File: test/providers/offline_provider_test.dart:63-177
Timestamp: 2026-04-14T03:52:37.716Z
Learning: In this repository, it’s acceptable to use fixed-time waits for test timing using `Future.delayed` (e.g., `await Future.delayed(const Duration(milliseconds: 10))`) inside test files. During code reviews, don’t recommend replacing these fixed sleeps with event-based or poll-based synchronization when the change is localized to Dart test files under the `test/` directory.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dart
📚 Learning: 2026-01-06T01:08:41.552Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: lib/widgets/wn_warning_box.dart:28-79
Timestamp: 2026-01-06T01:08:41.552Z
Learning: In reviews for the marmot-protocol/sloth repository, avoid suggesting accessibility, design, or general improvements unless they are strictly relevant to the PR description and its stated goals. Focus feedback on the specific PR objectives and requirements; ignore unrelated stylistic or broad improvement suggestions.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-01-15T14:42:54.111Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 39
File: lib/hooks/use_user_search.dart:47-53
Timestamp: 2026-01-15T14:42:54.111Z
Learning: When using npubFromHex(String hexPubkey) from lib/utils/formatting.dart, it already handles errors internally and returns null on failure. Do not wrap calls to this function in try/catch blocks. Instead, check for a null return and handle accordingly (e.g., treat as invalid hex). This guideline applies to all Dart files in the repository that may call this function.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-01-16T17:35:32.431Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 43
File: lib/hooks/use_network_relays.dart:155-184
Timestamp: 2026-01-16T17:35:32.431Z
Learning: In Dart/Flutter code, prefer void as the return type for fire-and-forget async functions that are intended to run in the background without blocking the caller. Using Future or Future<void> would allow callers to accidentally await, which could block for long periods. This pattern makes the call fire-and-forget by design. Apply to functions that intentionally should not be awaited; document the intent where appropriate. (Example context: pollRelayStatus in lib/hooks/use_network_relays.dart uses void to prevent callers from awaiting a potentially long-running background task.)
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-01-19T15:50:56.684Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 52
File: lib/screens/app_settings_screen.dart:30-40
Timestamp: 2026-01-19T15:50:56.684Z
Learning: Target the Flutter SDK version >= 3.27 across the repo. Since features like Column.spacing and Row.spacing were added in Flutter 3.27, you can safely use them in Dart files (e.g., lib/screens/app_settings_screen.dart) as long as the pubspec.yaml environment specifies Flutter >= 3.27. If needed, enforce this by validating the environment constraint (e.g., flutter, sdk: flutter) in pubspec.yaml.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-01-29T03:02:34.290Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 100
File: lib/widgets/wn_dropdown_selector.dart:126-139
Timestamp: 2026-01-29T03:02:34.290Z
Learning: Global font-family usage guideline: Since Manrope is configured in lib/theme.dart for both light and dark themes, individual TextStyle declarations should not specify fontFamily: 'Manrope'. During reviews, verify that no TextStyle instances override fontFamily unnecessarily; rely on theme inheritance instead. If a TextStyle must specify a font for a specific case, ensure it is clearly justified and localized, and document why the override is needed. This guideline applies across Dart files in the project (e.g., lib/widgets/wn_dropdown_selector.dart and related components).
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-01-29T16:02:52.588Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 129
File: test/widgets/wn_overlay_test.dart:84-91
Timestamp: 2026-01-29T16:02:52.588Z
Learning: In Dart/Flutter code, when constructing widgets inside an already-const context (for example, within a const Scaffold with a child that is a collection of widgets), avoid adding explicit const keywords to the children. The Dart analyzer treats these as implicitly const, and adding explicit const can trigger the unnecessary_const lint. Review test files and general Dart files to ensure you do not redundantly prefix children with const in const contexts; rely on the implicit const behavior instead.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-02-07T03:58:22.587Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 193
File: lib/services/android_signer_service.dart:13-13
Timestamp: 2026-02-07T03:58:22.587Z
Learning: In Dart files across the repository, retain NIP (Nostr Implementation Possibilities) and MIP (Marmot Implementation Possibilities) protocol reference comments that link code to their specifications. While general guidance discourages extraneous comments, these references improve traceability and maintainability by documenting the protocol context for related implementation details. Keep them in Dart sources (e.g., lib/services/android_signer_service.dart) and ensure they are kept up-to-date.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-02-11T17:29:43.985Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 225
File: lib/screens/app_settings_screen.dart:64-69
Timestamp: 2026-02-11T17:29:43.985Z
Learning: In the whitenoise Flutter app, after deleting all data and resetting auth, use Routes.goToHome(context) instead of Routes.goToLogin(context) because the home screen is the app entry point and will manage authentication routing. Apply this change to navigation calls that should land on the home screen after a data reset or auth reset. Update lib/screens/app_settings_screen.dart and any similar navigation points accordingly.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-02-18T18:36:13.394Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 315
File: lib/screens/edit_group_screen.dart:45-47
Timestamp: 2026-02-18T18:36:13.394Z
Learning: Whitenoise uses automated code formatting through the precommit workflow (just precommit). Do not tweak line wrapping or formatting manually; follow the formatter's output. Run the precommit formatter locally and ensure the code matches its styling decisions, and only deviate if the formatter cannot express the intended style (in which case adjust the code to satisfy the formatter).
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:36:06.726Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 553
File: lib/widgets/wn_copyable_field.dart:38-40
Timestamp: 2026-04-06T09:36:06.726Z
Learning: In Dart, `String.operator*` is a valid built-in operator for repeating strings (e.g., `'⬤' * 16`). In code reviews of Dart (`.dart`) files, do not treat expressions like `'text' * n` as invalid syntax and do not recommend replacing them with alternatives such as `List.filled(...).join()`; the operator is supported by `dart:core`.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-05-13T13:46:32.612Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/hooks/use_block_actions.dart:3-3
Timestamp: 2026-05-13T13:46:32.612Z
Learning: In marmot-protocol/whitenoise code reviews, do not flag imports that reference FRB-generated bindings under `package:rust_lib_whitenoise/src/rust/...` as private-internals violations. In `rust_lib_whitenoise`, Dart bindings are intentionally generated into `lib/src/rust/`, and `analysis_options.yaml` sets `implementation_imports: false` to suppress the related Dart lint—so `package:rust_lib_whitenoise/src/rust/...` imports are the correct/only way to consume that FRB-generated API.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-05-13T13:46:39.250Z
Learnt from: dannym-arx
Repo: marmot-protocol/whitenoise PR: 656
File: lib/services/foreground_service.dart:13-16
Timestamp: 2026-05-13T13:46:39.250Z
Learning: In this repo (marmot-protocol/whitenoise), `rust_lib_whitenoise` is an FRB-generated Flutter package where the generated Dart bindings are intentionally exposed under `lib/src/rust/` as the public API surface. Do not treat imports like `package:rust_lib_whitenoise/src/rust/...` as breaking package API boundaries in code reviews (the project also sets `implementation_imports: false` in `analysis_options.yaml` to opt into this). Never raise concerns about `src/` imports from `rust_lib_whitenoise`.
Applied to files:
test/screens/start_chat_screen_test.darttest/mocks/mock_share_plus.darttest/screens/share_profile_screen_test.dartlib/screens/share_profile_screen.dart
📚 Learning: 2026-02-05T11:59:09.400Z
Learnt from: erskingardner
Repo: marmot-protocol/sloth PR: 189
File: test/widgets/wn_filter_chip_test.dart:1-3
Timestamp: 2026-02-05T11:59:09.400Z
Learning: In Dart/Flutter test files, Offset is exported by package:flutter_test/flutter_test.dart. If you already import flutter_test, you do not need to import dart:ui for Offset. Ensure there are no conflicting Offset symbols when both imports occur.
Applied to files:
test/screens/start_chat_screen_test.darttest/screens/share_profile_screen_test.dart
📚 Learning: 2026-04-28T17:39:39.941Z
Learnt from: pepina-dev
Repo: marmot-protocol/whitenoise PR: 600
File: test/hooks/use_leave_group_test.dart:67-78
Timestamp: 2026-04-28T17:39:39.941Z
Learning: In this repo’s Dart test files under `test/` (e.g., `test/hooks/use_leave_group_test.dart`), it’s acceptable to avoid calling `mockApi.reset()` at the start of `setUp()` when the test suite already manually resets all relevant individual mock fields in `setUp()`. Reviewers should not flag the absence of a `reset()` call as long as every field that could leak state between tests is explicitly reinitialized and the tests are passing.
Applied to files:
test/screens/start_chat_screen_test.darttest/screens/share_profile_screen_test.dart
📚 Learning: 2026-02-03T10:47:03.596Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 164
File: test/mocks/mock_scanner_service.dart:7-60
Timestamp: 2026-02-03T10:47:03.596Z
Learning: In Dart/Flutter tests that involve widgets with platform channel interactions, avoid fully mocking complex controllers. Use a hybrid approach: use the real MobileScannerController for widget compatibility and mock only the internal streams (e.g., barcodeStream) to control test scenarios without triggering platform calls. Apply this pattern to Dart test files under test/mocks (e.g., test/mocks/mock_scanner_service.dart) and similar mock implementations.
Applied to files:
test/mocks/mock_share_plus.dart
📚 Learning: 2026-01-05T20:05:32.918Z
Learnt from: untreu2
Repo: marmot-protocol/sloth PR: 22
File: lib/screens/settings_screen.dart:76-88
Timestamp: 2026-01-05T20:05:32.918Z
Learning: In the Sloth project, tooltips should not be used for UI elements. During code reviews, verify that UI components do not include Tooltip widgets and replace them with accessible alternatives (e.g., long-press hints, labels, or interactive guidance). Apply this guidance across Dart UI files under lib (not just settings_screen.dart) to maintain a consistent design approach.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-09T13:25:18.531Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 33
File: lib/services/message_service.dart:14-26
Timestamp: 2026-01-09T13:25:18.531Z
Learning: When errors are presented to users in the Sloth codebase (UI layers such as snackbars, dialogs, toasts), show friendly, user-facing messages instead of raw Rust error messages. Implement a mapping from internal error types or codes to clear, non-technical messages, and surface actionable guidance for end users where appropriate. Avoid exposing internal stack traces or language runtime details; centralize common user-facing error wording in a dedicated utility or service and translate backend errors into concise, helpful UI messages.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:40:41.044Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 552
File: lib/screens/profile_keys_screen.dart:71-71
Timestamp: 2026-04-06T09:40:41.044Z
Learning: In this Flutter/Dart codebase, when using flutter_screenutil (e.g., ScreenUtil or ScreenUtil-based sizing like `w`, `h`), do not require scaled units for literal zero values. Specifically, in EdgeInsets (and similar sizing/padding/margin/gap APIs), bare numeric `0` should be allowed (e.g., `EdgeInsets.all(0)`, `EdgeInsets.symmetric(vertical: 0)`, `SizedBox(width: 0)`), because scaling `0` remains `0`. Only flag ScreenUtil violations when a non-zero literal needs to be expressed via `0.w`/`0.h` equivalents; do not flag bare `0`.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-01-15T15:00:17.356Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 39
File: lib/screens/user_search_screen.dart:50-75
Timestamp: 2026-01-15T15:00:17.356Z
Learning: In lib/screens/user_search_screen.dart and similar screen widgets in the sloth repository, prefer using inline ternary expressions for conditional rendering (e.g., isLoading → empty state → list) over extracting into separate widget classes when the conditional UI is simple. This improves readability by keeping the render logic in a single place. Reserve extracting to a separate widget only when the conditional UI becomes sufficiently complex or is reused across multiple screens.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-10T04:57:31.475Z
Learnt from: josefinalliende
Repo: marmot-protocol/sloth PR: 208
File: lib/screens/signup_screen.dart:66-70
Timestamp: 2026-02-10T04:57:31.475Z
Learning: In Dart/Flutter using flutter_hooks, a ValueNotifier (from useState) is disposed when the widget unmounts. Async callbacks (e.g., .then() on animations) may still run after disposal and throw 'A ValueNotifier was used after being disposed'. Reviewers should ensure that state updates in async callbacks are guarded by a mounted check (e.g., if (mounted) { notifier.value = ... } or cancel/avoid updating after disposal). This guideline applies to screen widgets under lib/screens and similar Dart files using hooks that manage widget lifecycle. Update code patterns to prevent post-disposal state updates, and document the rationale in comments where necessary.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-02-19T10:33:16.889Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 315
File: lib/screens/group_info_screen.dart:146-149
Timestamp: 2026-02-19T10:33:16.889Z
Learning: Pubkey validation (length/format) is performed by the Rust crate before data reaches Dart. Do not add defensive substring guards in Dart for pubkeys in lib/screens/**/*.dart or other Flutter code consuming group member pubkeys, as this would duplicate validation and be unnecessary. If any validation is needed in Dart, keep it to lightweight sanity checks only after data enters Dart and ensure it aligns with the Rust validation rules.
Applied to files:
lib/screens/share_profile_screen.dart
📚 Learning: 2026-04-06T09:40:51.965Z
Learnt from: untreu2
Repo: marmot-protocol/whitenoise PR: 552
File: lib/screens/sign_out_screen.dart:86-86
Timestamp: 2026-04-06T09:40:51.965Z
Learning: When reviewing Flutter/Dart code that uses ScreenUtil (e.g., ScreenUtil `.h`/`.w`/`.sp`), do not require converting a literal `0` used for padding/margin/sizing to a ScreenUtil expression. In sizing contexts like `EdgeInsets` (or other `EdgeInsets`/layout parameters), raw `0` is mathematically invariant (`0 * scale = 0`) and should not be flagged as a ScreenUtil violation. Only non-zero literal size values should be checked for ScreenUtil usage.
Applied to files:
lib/screens/share_profile_screen.dart
🔇 Additional comments (3)
test/mocks/mock_share_plus.dart (1)
2-45: LGTM!test/screens/share_profile_screen_test.dart (1)
19-19: LGTM!Also applies to: 31-31, 243-350
test/screens/start_chat_screen_test.dart (1)
518-518: LGTM!
dannym-arx
marked this pull request as ready for review
May 21, 2026 10:18
pepina-dev
reviewed
May 21, 2026
pepina-dev
left a comment
pepina-dev
left a comment
Collaborator
There was a problem hiding this comment.
I tried it and it works fine for me 👍🏻 but my agent left a review with request changes conclusions. We are about to get in the plane soon so I'm not able to review it more deeply but I leave the comments of my agent and let you decide if there are hallucinations or not:
🤖 ### 🔴 must — block ship until fixed
M1.
ui.Imagereturned bytoImageSyncis never disposed — native-memory leak per long-press attempt.lib/screens/share_profile_screen.dart:81-83final captureFuture = boundary .toImageSync(pixelRatio: 3) .toByteData(format: ui.ImageByteFormat.png);Per Flutter docs (
RenderRepaintBoundary.toImageSync): "The returned image must be disposed by the caller, see Image.dispose." The intermediateui.Imagehere is anonymous, so nothing ever callsdispose()on it. AtpixelRatio: 3on a 256-logical-pixel square, each capture allocates ~2.4 MB of Skia-owned native memory. In debug mode this fires Flutter'sImage.dispose()warning the first time the user long-presses — and on iOS, repeated long-presses on a memory-constrained device can pressure the app group container (notification service extension shares memory budget).
How to address:final image = boundary.toImageSync(pixelRatio: 3); try { final byteData = await image.toByteData(format: ui.ImageByteFormat.png); // ...use byteData... } finally { image.dispose(); }The
image.dispose()must run on every exit path, including the existing early-return branches (!isMounted.value,!isHolding.value,byteData == null) and thecatch.M2. Captured PNG includes the
ClipRRect's transparent corners — the shared image renders with transparent rounded corners.lib/screens/share_profile_screen.dart:191-217
TheRepaintBoundary(key: qrRepaintKey)wrapsConstrainedBox > AspectRatio > ClipRRect > AnimatedBuilder > QrImageView.toImageSynccaptures the boundary's rendered output, which means the four rounded-corner regions are encoded as fully-transparent pixels in the PNG. When the recipient opens the file in (a) iMessage's dark-mode bubble, (b) any messenger that renders attachments on a non-white background, or (c) Gallery's dark theme, the QR will appear to have four small dark-blob corners.
This is a real UX bug for the "I want my friend to scan this" use case — and worse, on some scanners the transparent corners are alpha-blended against an arbitrary background and the corner finder patterns of the QR (which intentionally sit near the corners) can come out with halos that hurt scan reliability.
How to address: either capture from a wrapper widget that paints an opaque background behind the QR (move theRepaintBoundaryinside theClipRRect, or wrap the QR in aContainer(color: colors.backgroundSecondary)before the boundary), or — preferably — drop the rounded clip for the captured image by capturing a separate non-clipped offstage widget. Add a golden test that asserts the four corners of the PNG are opaque.M3.
boundary == nullis a silent failure — the user is left holding the QR with no feedback.lib/screens/share_profile_screen.dart:71-75final boundary = qrRepaintKey.currentContext?.findRenderObject() as RenderRepaintBoundary?; if (boundary == null) { isCapturing.value = false; return; }In this path
isHolding.valueis alreadytrue(set byonLongPressStartbeforecaptureAndShareQrruns) anddotCount.valueis still0, so the user sees "Hold to share QR code" with no dot progression until they release. NoshowErrorNoticeis shown, no log is emitted. This should never happen in practice — but if a future refactor breaks the key wiring (e.g., wrapping the boundary in a conditionalBuilder), this branch becomes a guaranteed bug that's hard to diagnose because the failure is invisible.
How to address: onboundary == null, callshowErrorNotice(context.l10n.shareQrCodeError)and log at warning level. Add a unit test that fakes theqrRepaintKey.currentContextto null (or refactors capture into a hook so it can be tested in isolation) and asserts the error notice is shown.M4.
start_chat_screen_test.dartinvite-share test no longer asserts the file payload, only.text— coverage regression on the only other call site ofSharePlus.instance.share.test/screens/start_chat_screen_test.dart:512-519
Diff trades aMethodCall.arguments['text']check forshareCalls[0].text. That's fine, butShareParamsalso acceptsfiles: …. If a future regression adds a strayfiles: [XFile.fromData(…)]to the invite-share call (copy-paste from the new QR share code), this test passes silently because it never assertsshareCalls[0].files.
How to address: addexpect(shareCalls[0].files, isNull, reason: 'invite share is text-only'). Cheap and prevents this exact category of cross-contamination since both share call sites now share the same fixture.
- Switch captureAndShareQr to toImageSync() so the full-color frame is captured synchronously before the fade animation advances - Rewrite mock_share_plus to replace SharePlatform.instance with a single shared mutable mock; SharePlus.instance is a static final that captures the platform at first access, so swapping the platform later has no effect — mutating shouldFail on the already-captured instance does - Restructure the two runAsync tests: start gesture + pump(600ms) outside runAsync so the long-press timer and Future.delayed timers stay on the fake clock; enter runAsync only to give toByteData(png) real time - Update start_chat_screen_test for new ShareParams-based mock return type
CI is slower than local — toByteData(png) takes more than 100ms there. Both runAsync tests now poll every 100ms (up to 3s) until the expected state appears, so they pass regardless of machine speed.
Lint rule depend_on_referenced_packages requires it since mock_share_plus.dart imports SharePlatform directly.
M1: Hold ui.Image reference and dispose it in finally — toImageSync returns a caller-owned image (~2.4MB at pixelRatio: 3) that was leaking on every long-press. M2: Move RepaintBoundary inside ClipRRect so the captured PNG contains the unclipped rectangular QR. Previously the rounded-corner regions were encoded as transparent pixels, which renders as dark blobs on dark backgrounds and can halo the corner finder patterns when alpha-blended. M4: Assert shareCalls[0].files is null in the invite-share test to catch accidental file-attachment regressions in the text-only path.
Contributor
Author
|
I think this one can be merged, right? or is there another blocker? |
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
2 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.
Press and hold the QR code on the Share & Connect screen for 1.5s to share it as a PNG via the OS share sheet.
The QR fades from black to gray during the hold (150ms ease-in). A dot counter below steps from "." to ".." to "..." at 500ms intervals, giving the marmot holding the phone a clear countdown. Release early and nothing happens.
Closes #345
screenrecording-2026-05-20_21-27-53.mp4
Summary by CodeRabbit
New Features
Bug Fixes
Localization
Tests