Skip to content

fix: polish payment request ui - #1196

Draft
ovitrif wants to merge 13 commits into
masterfrom
ovi/polish-payment-request-ui
Draft

fix: polish payment request ui#1196
ovitrif wants to merge 13 commits into
masterfrom
ovi/polish-payment-request-ui

Conversation

@ovitrif

@ovitrif ovitrif commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

This PR polishes the Payment Request screens so spacing, contact rows, amount keypad, and success layout match the Bitkit - Handoff v62 frames, 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

  • Splits the Payments tab list into Incoming and Outgoing, with the contact name as row title, the note as subtitle, glow cards on incoming and a pending label on outgoing amounts.
  • Aligns contact rows with Contacts (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.
  • Rebuilds the amount step on the Receive amount layout: 352dp keypad as in the amount frame, unit toggle under the amount, leftover space split 4:1 around it, keypad shrinking on short windows so Continue stays on screen.
  • Details step drops the header back arrow (system back still pops) and uses the frame spacing between amount, note, recipient, and expiry. Reached from the amount step it shows the fiat value above the amount; reached from the invoice editor it shows the AMOUNT label, as the two details frames specify.
  • Pins the sent layout to check, PAYMENT REQUESTED, card, then OK, with the illustration capped by the space left for the content under it.
  • Adds a Maestro flow at .maestro/payment-request/payment-request.yaml that 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?clear removes the fixture; release builds compile it out.

Preview

Screen Figma (Handoff v62) Before After
List
Recipient
Recipient (from invoice)
Amount empty
Amount filled
Details
Sent

QA Notes

Manual Tests

  • 1. Home → menu → Subscriptions → Payments: list shows Incoming cards with Dismiss and Pay, then Outgoing rows with contact name, note, amount and pending, Request Payment button under the list.
  • 2. Payments → Request Payment: Choose Recipient in-sheet shows the RECIPIENT field and the contact rows to the sheet floor, no CONTACTS label, no Send button.
  • 3. Choose Recipient → tap a contact: Amount opens with the contact avatar in the header, keypad under the amount, Continue disabled at 0.
  • 4. Amount → enter 50 000 → Continue: Payment Request details show the fiat value above the amount, note, recipient card, expiry tabs; header has no back arrow; system back returns to Amount.
  • 5. Details → Send Request: Sent shows check, PAYMENT REQUESTED, the recipient card, OK; OK returns to the Payments tab.
  • 6. Receive → invoice editor → contact picker: Choose Recipient opens with the CONTACTS label; picking a contact lands on details with the AMOUNT label and the pencil.
  • 7. regression: Payments tab → Incoming card → Pay / Dismiss: still works.
  • 8. regression: Send → contact picker: rows still show the truncated key above the name.

Automated Checks

  • Instrumented tests modified: CreatePaymentRequestScreenTest.kt covers the CONTACTS header per entry path and the details fiat caption vs AMOUNT label; PaymentRequestsScreenTest.kt covers the Incoming section with Pay and Dismiss and the Outgoing pending label.
  • Maestro flow added: payment-request.yaml seeds 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, and just lint pass locally on the pushed head.
  • CI: standard compile, unit test, and detekt checks run by the PR bot.

ovitrif and others added 12 commits August 30, 2026 02:12
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>
@ovitrif
ovitrif marked this pull request as ready for review September 9, 2026 21:10
@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR restyles the payment-request list and creation flow to match existing Bitkit components and the intended visual layout.

  • Splits active requests into Incoming and Outgoing sections and updates row titles, subtitles, amount labels, and card effects.
  • Reworks recipient, amount, details, and success layouts and adds UI coverage for the revised states.
  • Adds a debug payment-request fixture and a Maestro flow for deterministic visual captures.
  • The primary follow-ups are compact-height resilience, canonical recipient spacing, and stronger isolation of the debug fixture.

Confidence Score: 4/5

The 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

Important Files Changed

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
Loading

Reviews (1): Last reviewed commit: "fix: cap keypad height and restore card ..." | Re-trigger Greptile

greptile-apps[bot]

This comment was marked as resolved.

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

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.ktPayment Request (…) › Request payment enter amount; PaymentRequestsScreen.ktPayment Requests › Payment Requests; SendContactSelectScreen.ktSend (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).

Comment thread app/src/main/java/to/bitkit/ui/components/PubkyContactRow.kt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ovitrif

ovitrif commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

jvsena42: Not compared: the amount, details and sent steps — no frame for those was referenced anywhere in the PR.

The three node ids you resolved were on a scratch page that no longer exists. Everything now points at the Bitkit - Handoff v62 page: the Preview table links each frame, and the fixture KDoc lists them. The two flows there differ, and this pass follows them: from Receive > Contacts, Contact profile or Payment Requests has no CONTACTS header, the amount steps (empty, populated), details with the fiat caption and sent; from Receive > Edit invoice has the CONTACTS header and details with the AMOUNT label. The list frame is unchanged.

@ovitrif
ovitrif requested a review from jvsena42 September 9, 2026 23:37

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

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:199959 fiat caption, 48049:273665 AMOUNT label) — structurally identical in the file: Amount group with Header 6 at 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 the Header 6 text differs between them, which is exactly what fromInvoiceEditor switches at CreatePaymentRequestScreen.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 500 and 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), and MoneyCell (Money.kt:58-77) renders primary plus primaryDisplay.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-215 matches 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-597 matches. The deliveryStatus queued/sent branch at :571-591 is 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.kt is additive only. The one change is secondaryText: String? = null (:50); non-null fills the existing secondary CaptionB slot, null runs the old primaryDisplay.not() branch unchanged. Primary line and rememberMoneyText untouched — no formatting, rounding, unit-switch, sign or ULong change. All three MoneyCell call sites are Paykit-only surfaces, so despite Money.kt being 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 inside PaymentRequestRecipient/PaymentRequestDetails, which are InternalOnly and reachable only when paymentRequestTargets.isNotEmpty() — and PaykitPaymentRequestRepo.kt:333-339 empties that whenever isAvailable() is false. QR / EditInvoice / CJIT amount / address-copy are untouched. skipPaymentRequestAmount is set immediately before both navigateTo calls, so neither flag can read stale.
  • The release fixture split genuinely compiles out. app/build.gradle.kts:242-257 declares only debug and release, so no third build type misses a source set; release/…/PaymentRequestFixtureRuntime.kt:7 returns null. The debug variant additionally requires Env.network == REGTEST, so mainnet and tnet debug builds reject it too. PaymentRequestFixture.kt stays in main but is inert with no writer.
  • Long/hostile counterparty text can't overlay the amount — name, subtitle and note are all maxLines = 1 with ellipsis, and the amount sits in its own weighted column. Text13Up uses Kotlin uppercase(), which is Locale.ROOT-invariant, so no Turkish-dotted-I on z-base-32's i.
  • Screens-map rule satisfied — rows exist at docs/screens-map.md:73 and :75; the PR adds and removes no *Screen.kt, so ScreensMapTest is 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.

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

On first row of preview, the "Subscriptions" title is not matching "Payment Requests" Figma. Also, the badge is breaks the tab indicator UI

@ovitrif
ovitrif marked this pull request as draft September 10, 2026 14:05
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