Repository navigation
fix: share received-payment artwork layout - #1664
Conversation
[Low impact] The PR appears safe to merge; no actionable regression was found. SummaryUSDT received-payment artwork now uses the same width-based sizing and bottom placement as Bitcoin and Lightning.
Reviews (1) · Last reviewed commit: "fix: share received-payment artwork layo..." · Reviewed by Greptile |
|
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
left a comment
There was a problem hiding this comment.
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:
offsetandsizehave 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.
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
There was a problem hiding this comment.
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.
- Twin: bitkit-ios#1092 fix: reuse received payment artwork layout matches the artwork-only scope and journey changes; iOS was reviewed in source, not device-tested here.
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
jvsena42
left a comment
There was a problem hiding this comment.
No findings.
Checked and clean:
NewTransactionSheet.kt:113-125: USDT now uses the same bottom-anchored,FillWidthimage as Bitcoin and Lightning; only the drawable changes (:116). The removed imports have no other use, and thetransaction_received_imagetest tag is unchanged, soNewTransactionSheetViewTeststill 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.
|
Merged: approved and green at 5db05a6. |
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
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 buildwith 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 byjourneys/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
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 at5db05a6, including confirmation, dismissal and no repeated payment-received notification.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
NewTransactionSheetViewTest.kt— all three existing received/sent sheet tests passed.Evidence
The external device review at
5db05a6approved 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) andjust lint(zero Detekt and Android Lint findings). Changed journeys parse andgit diff --checkpasses.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
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)