fix: polish payment request ui - #1196
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
# Conflicts: # app/src/androidTest/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreenTest.kt # app/src/main/java/to/bitkit/ui/ContentView.kt # app/src/main/java/to/bitkit/ui/components/Money.kt # app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt # app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt # app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveSheet.kt # app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Greptile SummaryThis PR restyles the payment-request list and creation flow to match existing Bitkit components and the intended visual layout.
Confidence Score: 4/5The PR appears safe to merge, with non-blocking follow-ups for compact-height accessibility, recipient spacing consistency, and debug-fixture isolation. The core payment-request data flow remains intact, but fixed and proportional sizing can clip controls in compact windows, the recipient rows diverge from the intended shared spacing, and the debug fixture can persistently replace real behavior after an external deep link. Files Needing Attention: app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt; app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
|
| Filename | Overview |
|---|---|
| app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt | Rebuilds all payment-request creation steps; recipient spacing and compact-height behavior need attention. |
| app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt | Separates incoming, outgoing, and historical requests while preserving the production data flow. |
| app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt | Adds fixture-backed flows and synthetic creation, but fixture activation persists without a mode guard or reset. |
| app/src/debug/java/to/bitkit/viewmodels/PaymentRequestFixtureRuntime.kt | Provides deterministic debug fixture data for the new visual automation. |
| app/src/release/java/to/bitkit/viewmodels/PaymentRequestFixtureRuntime.kt | Correctly compiles fixture activation out of release builds. |
| app/src/main/java/to/bitkit/ui/components/PubkyContactRow.kt | Aligns shared contact-row typography and default spacing with the canonical Contacts layout. |
| app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveSheet.kt | Tracks whether the recipient picker originated from the invoice editor and preserves system-back navigation. |
| .maestro/payment-request/payment-request.yaml | Covers the seven revised payment-request visual states using deterministic fixture data. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Payments list] --> B[Choose recipient]
B --> C[Enter amount]
C --> D[Review details]
D --> E[Create request]
E --> F[Sent or queued]
G[Debug fixture deep link] --> H[Fixture contacts and requests]
H --> A
H --> E
Reviews (1): Last reviewed commit: "fix: cap keypad height and restore card ..." | Re-trigger Greptile
jvsena42
left a comment
There was a problem hiding this comment.
Ran the Figma comparison — the PR body's "Figma" column links to PNG exports on a gist rather than frames, but PaymentRequestFixtureRuntime.kt:15 names three node ids (48185:303457, 48185:303440, 48185:303376) and docs/screens-map.md:3 gives the file key, so I resolved them directly.
The three available frames largely match. 48185:303376 confirms the INCOMING → 2 glow cards with Dismiss/Pay → OUTGOING structure, with pending replacing the fiat line. get_variable_defs returns only Gray/Gray 6, Gray/Gray 5, Brand/Bitkit and the Alpha/White set — and the code uses Colors.Gray6/Gray5/Brand/White64|16|10 throughout, so no hardcoded colours or type were introduced. 48185:303457 vs 48185:303440 confirm the CONTACTS-header toggle is real design intent, and showContactsHeader matches both.
Not compared: the amount, details and sent steps — no frame for those was referenced anywhere in the PR, so I can't adjudicate F3 or F4 below. If you have those node ids, worth adding them next to the existing comment.
Screens-map rows at head: CreatePaymentRequestScreen.kt → Payment Request (…) › Request payment enter amount; PaymentRequestsScreen.kt → Payment Requests › Payment Requests; SendContactSelectScreen.kt → Send (Contact) (Lightning) › Send to contact. ReceiveSheet.kt isn't a *Screen.kt so it's out of the map's scope.
Also checked clean: Money.kt has no unit or rounding change (secondaryText only swaps which composable fills the existing secondary slot); the details back-arrow removal is safe since SheetTopBar.onBack is nullable and system back still pops; both navigateTo(PaymentRequestRecipient) sites set showRecipientContactsHeader immediately before navigating, so there's no stale-value path; incoming ∪ outgoing is exactly the old active set so history is unchanged; and conventions are right throughout (PascalCase testTags, ImmutableList at the boundary, no trailing comma after modifier =, strings alphabetical, one changelog fragment).
For context on greptile's 24dp thread — the Contact instance in both recipient frames is 96dp tall, closer to the 30dp override than to 24dp. Design data only; not reopening it.
Four LOW notes inline. Nothing blocking — Paykit UI is behind isPaykitEnabled (default off).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The three node ids you resolved were on a scratch page that no longer exists. Everything now points at the |
jvsena42
left a comment
There was a problem hiding this comment.
Clean — no findings this pass, and the Figma gap I left open last time is now closed.
Figma: I compared the five frames I previously couldn't. Your follow-up superseded the three scratch-page ids I'd resolved, so I went through the Bitkit - Handoff v62 frames you linked.
- Details, both variants (
47993:199959fiat caption,48049:273665AMOUNT label) — structurally identical in the file:Amountgroup withHeader 6at y=0 h=18 and the amount at y=34 h=36, text field at y=94, recipient at y=196 h=98, expiry at y=318 h=62. Only theHeader 6text differs between them, which is exactly whatfromInvoiceEditorswitches atCreatePaymentRequestScreen.kt:281-282. Matches. - This settles F3 from my last review, in your favour. The fiat caption is design intent. More to the point, my concern was that a BTC-primary user would see
₿ 14 500and no fiat anywhere with no way to toggle — that's wrong: both frames render the recipient card with both units stacked (₿ 14 500/$ 14.50), andMoneyCell(Money.kt:58-77) renders primary plusprimaryDisplay.not()in exactly that shape. Fiat is present on both variants. Nothing to change. - Amount (
48030:206606) — back arrow, "Amount" title, contact avatar in the action slot, fiat caption above the amount, unit toggle right-aligned below it, divider, keypad, Continue.PaymentRequestAmountContent:161-215matches band for band, including the toggle sitting under the amount rather than beside it. - Sent (
48030:206596) — check, 32dp, Display headline with the second word accented, 8dp, body copy, 24dp, card, then OK.PaymentRequestSentContent:553-597matches. ThedeliveryStatusqueued/sent branch at:571-591is a state the frame doesn't specify — that's an addition, not an omission, so no delta. - No hardcoded colours or type introduced, consistent with the token check from the earlier pass.
Code, traced clean at head:
Money.ktis additive only. The one change issecondaryText: String? = null(:50); non-null fills the existing secondaryCaptionBslot, null runs the oldprimaryDisplay.not()branch unchanged. Primary line andrememberMoneyTextuntouched — no formatting, rounding, unit-switch, sign orULongchange. All threeMoneyCellcall sites are Paykit-only surfaces, so despiteMoney.ktbeing shared, no ungated screen's rendering changes.ReceiveSheet.kt— the ordinary receive path is byte-for-byte unchanged. Exactly two lines changed (:225,:235), both insidePaymentRequestRecipient/PaymentRequestDetails, which areInternalOnlyand reachable only whenpaymentRequestTargets.isNotEmpty()— andPaykitPaymentRequestRepo.kt:333-339empties that wheneverisAvailable()is false. QR / EditInvoice / CJIT amount / address-copy are untouched.skipPaymentRequestAmountis set immediately before bothnavigateTocalls, so neither flag can read stale.- The release fixture split genuinely compiles out.
app/build.gradle.kts:242-257declares onlydebugandrelease, so no third build type misses a source set;release/…/PaymentRequestFixtureRuntime.kt:7returnsnull. The debug variant additionally requiresEnv.network == REGTEST, so mainnet and tnet debug builds reject it too.PaymentRequestFixture.ktstays inmainbut is inert with no writer. - Long/hostile counterparty text can't overlay the amount — name, subtitle and note are all
maxLines = 1with ellipsis, and the amount sits in its own weighted column.Text13Upuses Kotlinuppercase(), which isLocale.ROOT-invariant, so no Turkish-dotted-I on z-base-32'si. - Screens-map rule satisfied — rows exist at
docs/screens-map.md:73and:75; the PR adds and removes no*Screen.kt, soScreensMapTestis not at risk.
Pre-existing, noted not filed: PubkyImage.kt:59-60 does AsyncImage(model = uri) with no host allowlist for contact avatars. Not in this diff — the diff only changes padding and typography around it.
Worth a re-run before merge: CI shows e2e-tests-local - lnurl_transfer, e2e-tests-staging - hardware_wallet and multi_address_2_regtest red. None touch these files and pubky_paykit passes, so they read as unrelated, but I'd rather you confirm than assume.
This PR polishes the Payment Request screens so spacing, contact rows, amount keypad, and success layout match the
Bitkit - Handoff v62frames, using the existing Bitkit components (Contacts rows, Receive amount keypad, backup success) instead of the GPT layout from #1172.Figma is intent. Existing Bitkit components win when Figma disagrees. The screens were captured with a Maestro flow on a Pixel emulator and measured against the handoff frames band by band. Frames: Payment Requests, Payment Request from Receive > Contacts, Contact profile or Payment Requests, Payment Request from Receive > Edit invoice.
Description
Text13Up+BodyMSB) and runs the recipient list to the sheet floor; tapping a row chooses the recipient. Following the two handoff flows, the CONTACTS label shows when Choose Recipient opens from the Receive invoice editor and is hidden from the Payment Requests list, Contacts, and contact profile paths..maestro/payment-request/payment-request.yamlthat seeds the frame fixture through a debug-only deep link on the regtest flavor and captures one screenshot per frame.bitkit://dev-fixture/payment-request?clearremoves the fixture; release builds compile it out.Preview
QA Notes
Manual Tests
pending, Request Payment button under the list.regression:Payments tab → Incoming card → Pay / Dismiss: still works.regression:Send → contact picker: rows still show the truncated key above the name.Automated Checks
CreatePaymentRequestScreenTest.ktcovers the CONTACTS header per entry path and the details fiat caption vs AMOUNT label;PaymentRequestsScreenTest.ktcovers the Incoming section with Pay and Dismiss and the Outgoingpendinglabel.payment-request.yamlseeds the fixture and captures the seven handoff frames on a Pixel emulator; layout measured against the frames with a band-by-band ledger.just compile,just test, andjust lintpass locally on the pushed head.