Skip to content

fix: share received-payment artwork layout - #1664

Merged
ovitrif merged 2 commits into
masterfrom
fix/received-payment-artwork
Oct 22, 2026
Merged

ovitrif merged 2 commits into
masterfrom
fix/received-payment-artwork

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Twin: synonymdev/bitkit-ios#1092

This PR reuses the received-payment artwork layout for USDT, removing the fixed-size bottom placement that left a large gap on tall sheets and clipped the composition on compact screens.

Description

  • Shares responsive artwork sizing and placement across Bitcoin, Lightning and USDT while preserving each designed illustration and confetti color.
  • Keeps the existing amount, Details and dismissal controls and updates the received-sheet journey expectations.

Out of Scope

Design

USDT received and Bitcoin/Lightning received, Handoff v62. Coin overlap behind the footer is intentional; responsive sizing preserves the assets rather than forcing the fixed design-frame dimensions onto every sheet.

Preview

Native before/after captures for all three payment types at 375×700, 420×748 and 320×568, plus 320×568 with enlarged text, are saved in the author's local review artifact: /private/tmp/received-artwork-evidence-20261021/preview/review.html. This is a local artifact, not a remotely accessible screenshot upload.

QA Notes

Setup

Use JAVA_HOME=$(/usr/libexec/java_home -v 17) just build with the branch's pinned dependencies and existing private-registry credentials, then install the matching devDebug APK on an owned emulator. The visual fixtures host the production NewTransactionSheetView with an inert 7 USDT or 8,281 sat receipt; they do not send funds. The captured emulator used API 36, arm64, and fixture density 2 with font scales 1.0 and 1.3. Full J1 needs the existing regtest LSP setup documented by journeys/onchain-receive/README.md. Full J2 needs the isolated bridge/provider fixture described in its journey; do not use real funds or reset a wallet.

Journeys

  • J1: updated mempool-then-confirmed-single-sheet.xml — received Bitcoin artwork fits the sheet and keeps the amount and dismissal action readable. Passed in the external exact-head device review at 5db05a6, including confirmation, dismissal and no repeated payment-received notification.
  • J2: updated usdt-bridge-details.xml — received USDT artwork adapts to compact/tall screens and enlarged text while both footer actions remain usable.

Manual Tests

N/A

Automated Checks

  • ran NewTransactionSheetViewTest.kt — all three existing received/sent sheet tests passed.
  • ran temporary native received-sheet render harness — captured all three payment types in four configurations before and after and asserted each footer callback; the harness is not shipped.

Evidence

The external device review at 5db05a6 approved the change with no findings. J1 used a live 14,797-sat regtest receipt. J2’s changed receipt step used separate 7-USDT and 1-USDT transfers on an isolated Arbitrum fork: Details matched the full transaction ID, reopening Receive did not repeat the first receipt, and the second transfer produced a new celebration. A separate debug-triggered production-sheet matrix exercised Bitcoin, Lightning and USDT at 320×568, 375×700 and 420×748 dp plus compact 130% text, with every dismissal action and USDT Details usable. The latter matrix validates rendering/interactions, not Lightning payment execution. This is attributed reviewer evidence, not a new author-run test.

The final source passed just compile, just test (4,405 tests, zero failures) and just lint (zero Detekt and Android Lint findings). Changed journeys parse and git diff --check passes.

Native captures and button checks passed for all twelve candidate configurations, as did the three existing instrumented receipt tests. Both full baseline and candidate runs passed after fixing a temporary fixture that reused a debounced button between distinct receipts. USDT's tall-screen gap is reduced and compact-width margins restored. Bitcoin and Lightning keep their existing width-scaled placement. The final candidate directly reuses it for USDT, keeping bright coin edges away from the compact Details label. Independent source/visual review of the final candidate found no actionable issue; the intermediate compact-label contrast finding is resolved.

Gaps

The author’s before/after captures remain production-view fixtures. The later external review completed J1 and J2’s changed live-receipt step, but did not rerun J2’s unchanged bridge, refund and support steps; full J2 remains unchecked. Lightning payment execution and exact Figma comparison were not performed in that review because its linked frames were unavailable. No provider-settlement or full bridge-journey pass is claimed. No mainnet payment was made.

Models used

  • Planning/scoping: gpt-6-astra (reasoning: xhigh)
  • Implementation: gpt-6-astra (reasoning: xhigh)
  • Review: gpt-6-astra (reasoning: xhigh)

@greptile-apps

greptile-apps Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable regression was found.

Summary

USDT received-payment artwork now uses the same width-based sizing and bottom placement as Bitcoin and Lightning.

  • Received-payment sheets fit their coin artwork to the sheet width.

Reviews (1) · Last reviewed commit: "fix: share received-payment artwork layo..." · Reviewed by Greptile

@ovitrif ovitrif added this to the 3.1.0 milestone Oct 21, 2026
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

Milestone 3.1.0, matching its iOS twin synonymdev/bitkit-ios#1092: a USDT received-sheet layout fix; it closes no issue, so the PR carries the milestone.

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No findings. Makes the received USDT artwork reuse the full-width, FillWidth, bottom-anchored Image that Bitcoin and Lightning already use, choosing only the drawable by usdtAmount. Reviewed 5db05a6, full tier, read from the code and CI (no Android build on this side).

What I checked, and 3 candidates I ruled out

Read in full: NewTransactionSheet.kt at the PR head, the two changed journeys, NewTransactionSheetViewTest.kt (its transaction_received_image assertions still match the kept test tag).
Assets: usdt_coins_received.png (nodpi, 1046x1560) and coin_stack_5.webp (1012x1476 at xxhdpi) have near-identical aspect ratios, so the same width-driven sizing gives USDT about the same height as the Bitcoin stack.
CI: still running when this review was posted (build, lint, build-local pending; detect-changes and e2e determine passed).

Ruled out

  • Unused import left behind: offset and size have no other use in the file after the change, and both imports are removed, so detekt has nothing to flag.
  • USDT composition now off-centre: the old offset(x = 13.5.dp) made up for artwork sitting outside its design slot. The PNG's opaque bounds are about 27 px left of centre out of 1046, so full width now shifts the coins by roughly 2.6% of the sheet. The PR says the before and after captures were checked against the frame, so I am not calling this a defect.
  • Artwork running into the amount on short sheets: height now scales with width and not with sheet height, but Bitcoin and Lightning already behave this way and the author checked 320x568 with larger text. This PR does not introduce it.

Merge confidence: 4/5, CI had not finished and the layout is checked only by the author's captures, with no automated test of it.

@github-actions

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 5db05a6 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: ✅ Approve

Findings: None.

QA for 5db05a6: Android API 36, arm64, dark mode. Videos play at 2×.

Test 1: J1 — Bitcoin receipt and confirmation ✅ The 14,797-sat receipt kept its amount and dismissal action readable and stayed dismissed after confirmation, checked against device events.

I verified the mempool and confirmed events for the same transaction, waited over 30 seconds after confirmation, and checked that the notification dump contained no Payment Received notification.

J1.2x.mp4

Staging regtest receipt, dismissal and confirmation wait.

Test 2: J2 — USDT received celebration ✅ A new 7 USDT receipt opened the correct Details, stayed dismissed on revisit, and a distinct 1 USDT transfer produced another celebration.

The transfers executed on an isolated Arbitrum fork. I matched Details to the funding transaction's complete hash, reopened Receive without repeating that receipt, then received and dismissed the separate 1 USDT transfer. This drives J2's changed receipt step.

J2.2x.mp4

Live receipt, transaction Details, revisit and a different receipt.

Test 3: R1 — responsive artwork ✅ Bitcoin, Lightning and USDT kept readable amounts and usable footer actions across the four recorded configurations.

Each recording exercises all three rails. I inspected the native captures and tapped every dismissal action; USDT Details opened the existing 7 USDT receipt in each configuration. Long randomized dismissal text wraps at compact enlarged text but remains visible and tappable.

320×568 dp, text 1.0 375×700 dp, text 1.0
R1-compact.2x.mp4
R1-standard.2x.mp4
420×748 dp, text 1.0 320×568 dp, text 1.3
R1-tall.2x.mp4
R1-compact-large.2x.mp4

Native modal layouts and footer interactions, using a debug-only receipt trigger with the production sheet unchanged. This matrix does not exercise Lightning payment execution.

Full review of d5526d4..5db05a6, including the changed journeys and reached sheet code. J1 and J2 used unmodified production code. J2's unchanged bridge, refund and support steps were not rerun. The linked Figma frames were unavailable, so exact design fidelity was not checked.

Reviewer model: gpt-6.1-sol
Reasoning effort: high


Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utAck

@jvsena42 jvsena42 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.

No findings.

Checked and clean:

  • NewTransactionSheet.kt:113-125: USDT now uses the same bottom-anchored, FillWidth image as Bitcoin and Lightning; only the drawable changes (:116). The removed imports have no other use, and the transaction_received_image test tag is unchanged, so NewTransactionSheetViewTest still matches.
  • The USDT and Bitcoin artwork have nearly the same aspect ratio, so the shared layout doesn't crop either.
  • Figma: the USDT received frame (node 49920-669650) shows the coins anchored to the bottom, behind the buttons, at close to full width. That matches the new layout.
  • Twin synonymdev/bitkit-ios#1092 makes the same change; each platform keeps its existing Bitcoin/Lightning placement.

Device gate: no run by me. Skipped, already driven this round: J1 mempool-then-confirmed-single-sheet.xml and J2's USDT received-celebration step (Android, by ovi-reviewer at 5db05a6, passed: #1664 (review)). Covered by CI at 5db05a6: unit tests, lint, detekt, e2e (all green). perf: not relevant; l10n: no UI text changed.

@ovitrif
ovitrif merged commit d75110f into master Oct 22, 2026
22 checks passed
@ovitrif
ovitrif deleted the fix/received-payment-artwork branch October 22, 2026 22:41
@ovitrif

ovitrif commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Merged: approved and green at 5db05a6.

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.

4 participants