Skip to content

Update QrCodeGenerator for PDF use#3781

Open
andreia-ferreira wants to merge 2 commits into
masterfrom
andreia/3739/qr-code-update
Open

Update QrCodeGenerator for PDF use#3781
andreia-ferreira wants to merge 2 commits into
masterfrom
andreia/3739/qr-code-update

Conversation

@andreia-ferreira

Copy link
Copy Markdown
Collaborator

Towards #3715

(Split from #3774)
This PR expands the QrCodeGenerator in order to provide a bitmap with a centered logo. This will be used to embed a QR code in PDF exports.

The shared constants were also moved from GroundQrCode to QrCodeGenerator and unit tests were added.

@shobhitagarwal1612 PTAL?

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.33%. Comparing base (b8ccfab) to head (dba6a17).

Files with missing lines Patch % Lines
...roundplatform/ui/components/qrcode/GroundQrCode.kt 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3781      +/-   ##
============================================
+ Coverage     66.14%   66.33%   +0.18%     
  Complexity     1660     1660              
============================================
  Files           386      387       +1     
  Lines         10039    10057      +18     
  Branches       1288     1290       +2     
============================================
+ Hits           6640     6671      +31     
+ Misses         2722     2709      -13     
  Partials        677      677              
Files with missing lines Coverage Δ
...rm/ui/components/qrcode/QrCodeGenerator.android.kt 100.00% <ø> (+100.00%) ⬆️
...ndplatform/ui/components/qrcode/QrCodeGenerator.kt 100.00% <100.00%> (ø)
...roundplatform/ui/components/qrcode/GroundQrCode.kt 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shobhitagarwal1612 shobhitagarwal1612 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great! Especially the test coverage!

val qrBitmap by
produceState<ImageBitmap?>(initialValue = null, key1 = content, key2 = showLogo) {
value = withContext(Dispatchers.Default) { generateQrBitmap(content, showLogo) }
value = withContext(Dispatchers.Default) { encodeQrBitmap(content, showLogo) }

@shobhitagarwal1612 shobhitagarwal1612 Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: We can add the named argument useHighEcc to help with making the intent clearer.

): ImageBitmap =
if (logo == null || !fitsLogoCapacity(content)) {
encodeQrBitmap(content, useHighEcc = false)
} else encodeQrBitmap(content, useHighEcc = true).withCenteredLogo(logo, logoSizeFraction)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Please consider adding braces around the else branch for consistency.

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.

2 participants