Skip to content

Fix large QR export layout - #868

Merged
praveenperera merged 2 commits into
masterfrom
fix-qr-export
Aug 13, 2026
Merged

Fix large QR export layout#868
praveenperera merged 2 commits into
masterfrom
fix-qr-export

Conversation

@praveenperera

Copy link
Copy Markdown
Member

Summary

  • keep animated QR frame indicators within the available width on iOS and Android
  • keep the iOS QR image square and within its parent width
  • add an iOS regression test with 250 animated QR frames

Root cause

Large transactions create many animated QR frames. The old indicator added a fixed gap for every frame. With enough frames, the gaps made the indicator wider than the screen and expanded the QR layout with it.

User impact

Transaction export QR codes stay fully visible and scannable when the user selects many UTXOs.

Validation

  • just fmt
  • just clippy
  • just lint-swift
  • just lint-android
  • just compile-android
  • just build-ios
  • xcodebuild -scheme Cove -sdk iphonesimulator -arch arm64 build-for-testing

The iOS test bundle compiled successfully. No simulator device was installed, so the focused iOS test did not run locally.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 86925bac-c07c-4753-a2df-2910b2e7c494

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@praveenperera
praveenperera marked this pull request as ready for review August 10, 2026 20:50
@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown

Greptile Summary

The PR keeps animated QR exports within the available width on both mobile platforms and adds platform-specific regression coverage.

  • Replaces fixed-gap frame indicators with width-aware Canvas rendering on Android and iOS.
  • Constrains the iOS QR image to a square aspect ratio within its parent.
  • Adds large-frame-count layout tests for Android and iOS.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
android/app/src/main/java/org/bitcoinppl/cove/views/QrExportView.kt Replaces the weighted box row with a Canvas indicator that dynamically reduces spacing to fit the available width.
android/app/src/androidTest/java/org/bitcoinppl/cove/views/QrExportProgressIndicatorTest.kt Adds the Android large-frame layout and rendering regression test requested by the previous review.
ios/Cove/Views/QrExportContentViews.swift Uses width-aware Canvas rendering for the iOS frame indicator and removes the QR view’s vertical fixed-size constraint.
ios/Cove/QrCodeView.swift Keeps generated QR images square while fitting them within the proposed layout.
ios/CoveTests/QrExportLayoutTests.swift Adds an iOS regression test confirming that a 250-frame indicator fits the available dimensions.
ios/Cove.xcodeproj/project.pbxproj Registers the new QR export layout test in the Cove test target.

Reviews (2): Last reviewed commit: fffd318 | Re-trigger Greptile

Comment on lines +292 to +297
@Composable
private fun QrExportProgressIndicator(
qrCount: Int,
currentIndex: Int,
modifier: Modifier = Modifier,
) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Cover Android Canvas measurement

The new indicator relies on Compose-specific Canvas measurement, density conversion, and pixel rendering for hundreds of frames, but the regression test covers only the SwiftUI implementation. Add an Android layout test using the large frame count so later changes cannot reintroduce overflow, clipping, or collapsed segments on Android.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@praveenperera
praveenperera force-pushed the fix-qr-export branch 2 times, most recently from fffd318 to e12e649 Compare August 13, 2026 20:15
Keep animated QR frame indicators within the available width when large transactions create many frames. Apply the same bounded layout on iOS and Android.
Adds an Android Compose instrumentation test that verifies the QR export progress indicator scales to available width for many frames (250), keeps expected dimensions, and renders visible/different colors at both ends. Updates `QrExportProgressIndicator` visibility from `private` to `internal` so the test can access it.
@praveenperera
praveenperera enabled auto-merge (squash) August 13, 2026 20:23
@praveenperera
praveenperera merged commit 0bb681f into master Aug 13, 2026
9 checks passed
@praveenperera
praveenperera deleted the fix-qr-export branch August 13, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant