Skip to content

fix: pin paykit request keypad and ok button - #1758

Merged
ovitrif merged 4 commits into
masterfrom
fix/paykit-request-layout
Nov 3, 2026
Merged

ovitrif merged 4 commits into
masterfrom
fix/paykit-request-layout

Conversation

@ovitrif

@ovitrif ovitrif commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1705
Fixes #1533

Twin: synonymdev/bitkit-ios#1194

The Paykit request Amount screen and the "Payment Requested" sheet did not follow the layout of Send's amount screen and the other success sheets. This PR makes them match and adapt to the screen height.

The BTC/USD toggle part of #1533 already shipped in #1654; this PR covers that issue's layout part.

Description

Status of each requested fix, at the current head:

Request Amount (PaymentRequestAmountContent)

  • ✅ Same sheet as Send's amount screen: both are the large sheet (sheetHeight()).
  • ✅ Same layout as Send:
    • the amount sits at the top;
    • the spare space sits between the amount and the controls;
    • the BTC/BITCOIN controls sit on the row directly above the divider and the number pad;
    • the pad sits directly above Continue.
  • ✅ Responsive: at 1080×2410 and 1440×3120, the controls, pad rows and Continue land on the same pixels as Send's. At 360×640 the sheet scrolls, as Send's does. There, the request pad sits about 10 dp higher than Send's, because Send's controls row also carries the two-line AVAILABLE balance.

Payment Requested (PaymentRequestSentContent)

  • ✅ OK is pinned to the bottom of the sheet, with the headline, description and recipient card directly above it.
  • ✅ A RECIPIENT caption sits above the recipient card, as in Figma.
  • ✅ The checkmark scales with the screen. It reuses the proportional size of SheetIntro and the "Sent" transaction sheet (SHEET_INTRO_IMAGE_WIDTH_FRACTION, 80% of the sheet width), and it shrinks to fit on short screens, so the details and OK stay visible.
  • ✅ With large text on a short screen, the checkmark gives way and the body scrolls, so nothing overlaps the headline.
  • ❌ The "PAYMENT REQUESTED" headline keeps the fixed Display text size, as on the other success sheets. It does not scale with the screen.

Disabled Continue

Out of Scope

Design

Preview

Request Amount, Figma and the app at three screen sizes. Send's amount screen is the reference row:

Figma 360×640 1080×2410 (Pixel 10 Pro) 1440×3120
After
Send
Before

Payment Requested:

Figma 360×640 1080×2410 (Pixel 10 Pro) 1440×3120
After
Before

Payment Requested at 360×640 with 1×, 1.5× and 2× text, then at 1080×2410 and 1440×3120:

Emulator recordings of the three fixed screens (request Amount, Payment Requested and Send amount) at each size, at 2x:

360×640 1080×2410 (Pixel 10 Pro) 1440×3120
and-small-rec.mp4
and-pixel10pro-rec.mp4
and-tall-rec.mp4

QA Notes

Setup

Two Bitkit instances on regtest with Paykit enabled, each with a Pubky identity, saved as each other's contacts and linked, as in journeys/payment-requests/README.md. Check on a short phone and on a tall phone, such as a Pixel 10 Pro.

Journeys

  • J1: updated paykit-amount-layout.xml — the request Amount layout matches Send's amount screen: spare space under the amount, then the controls row, divider, keypad and Continue in the same places
  • J2: updated request-summary.xml — the Sent receipt shows the RECIPIENT caption, with OK at the bottom and the details directly above it

Manual Tests

N/A

Automated Checks

  • updated PaymentRequestAmountLocaleTest.kt — the controls row sits at least 12 dp below the amount, with the spare space between them, as on Send

Evidence

  • 561c5610f: just compile, just lint and just test pass. PaymentRequestAmountLocaleTest.kt passes on an Android 16 emulator.
  • The images and recordings come from the same emulator (recordings at 71f5cf808, which differs only at large text sizes), at 360×640 (320 dpi), 1080×2410 (420 dpi) and 1440×3120 (560 dpi). A local test harness renders each screen at full sheet size. The Before images are built from master.

Gaps

  • J1 and J2 were not driven end to end: sending a request needs two linked Pubky identities. The images and recordings show the rendered screens, not the full request flow.
  • CreatePaymentRequestScreenTest.amountShowsNumberPadAndContinue fails on the emulator. It also fails with master's screen code, so this PR did not cause it.

Models used

  • Planning/scoping: claude-opus-5-5 (reasoning: medium)
  • Implementation: claude-opus-5-5 (reasoning: medium)
  • Review: Not performed

@ovitrif ovitrif added this to the 3.2.0 milestone Nov 2, 2026
@ovitrif ovitrif self-assigned this Nov 2, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T20:16:18.697791Z dcab2f0 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ovitrif
ovitrif requested review from a team, coreyphillips and pwltr and removed request for a team November 2, 2026 20:16
@greptile-apps

greptile-apps Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Updates the payment-request amount and sent screens:

  • The request keypad sits above Continue, with spare room above it.
  • The Payment Requested receipt keeps its details above a bottom-pinned OK button.

No actionable issues were found.

Reviews (1) · Last reviewed commit: "fix: pin paykit request keypad and ok bu..." · Reviewed by Greptile

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from b28b672 (run).

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

coreyphillips
coreyphillips previously approved these changes Nov 2, 2026

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

Two independent reviews.

the reviewers disagree, your call

  • Journey prose now differs from iOS with no twin carrying it (journeys/paykit-amount-layout.xml:5)

Comment thread journeys/paykit-amount-layout.xml Resolved

@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. Moves the Paykit request number pad to Send's placement (sized from the available height, directly above Continue, spare space between the divider and the pad) and pins OK to the bottom of the Payment Requested sheet with a scrolling body and the RECIPIENT caption. Reviewed dcab2f0f8, full tier, static: on the Bitkit track an auto review does not build an Android branch, so this was reasoned from the code and from CI, not run.

What I checked, and 4 candidates I ruled out

Read in full: CreatePaymentRequestScreen.kt (both composables), NumberPad.kt (the availableHeight sizing), Spacers.kt (FillHeight(min)), SendAmountScreen.kt (the reference layout).
Call sites traced: PaymentRequestAmountContent is also the Amount step of CreateSubscriptionScreen.kt, so the subscription amount screen gets the same pad placement; J1 already covers it. PaymentRequestSentContent serves the create flow and Receive > Request payment, as the body says.
CI: build, lint, detekt, build-local and Greptile green; the e2e matrix was still pending when this was posted.

Ruled out

  • Weights inside a vertical scroll: FillHeight sits in a verticalScroll column, but heightIn(min = maxHeight) gives the column a bounded minimum to distribute, the same construction the screen used before and SendAmountScreen uses for USDT.
  • Pad taller than the space on a short screen: NumberPad takes maxHeight / ROWS clamped between the min and ideal key height, identical to Send's call; past that the column scrolls, and the short-screen step of paykit-amount-layout.xml covers reaching every key and Continue.
  • Sent sheet width: the inner Column has no fillMaxWidth, but BodyM fills the width, so the checkmark's CenterHorizontally still centres on the sheet.
  • New string: wallet__payment_request_recipient is an existing resource on master; nothing new to translate.

Not verified: J1 and J2 on a device, and no Compose test asserts positions (the existing CreatePaymentRequestScreenTest.kt checks only that the nodes are displayed).

Merge confidence: 4/5, no findings, but the layout change has no test that asserts placement and I did not build it.

Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the run metadata available to this review does not report it)

@ovitrif
ovitrif dismissed coreyphillips’s stale review November 2, 2026 20:45

The keypad and OK button placement is not fixed as requested yet; re-review after the follow-up push.

@ovitrif

ovitrif commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 71f5cf808, which reworks the layout to match Send and the other success sheets:

  • Request Amount now uses Send's amount-screen layout. The spare space sits under the amount, and the BTC/BITCOIN controls sit on the row directly above the divider and the keypad.
  • The Payment Requested checkmark scales with the screen, using the same 80%-of-width size as SheetIntro, and shrinks to fit on short screens.
  • The disabled primary button style is shared by every screen, so it is tracked separately in bug: disabled primary button shows a filled pill #1759.

The description lists each fix with its status, plus Figma side-by-sides at 360×640, 1080×2410 and 1440×3120 and emulator recordings.

@ovitrif
ovitrif requested a review from coreyphillips November 2, 2026 21:12

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

Rechecked after 1 new commit (71f5cf8). Nothing blocking remains.

still open

  • Journey prose now differs from iOS with no twin carrying it (journeys/paykit-amount-layout.xml:5). This PR edits the action prose of two shared journeys, but no bitkit-ios PR carries the same edits. On iOS master, journeys/paykit-amount-layout.xml has blob 4053d110e, the same blob Android had before this PR (index 4053d110e..ab80d3521). So this change starts the drift. request-summary.xml also gains a RECIPIENT caption and OK-at-bottom assertion. CLAUDE.md asks for identical <action> prose on both platforms, carried by the matching iOS PR. bitkit-ios#1138 tracks the Sent sheet fix, but the PR body has no Twin: line, and nothing tracks the new request-amount keypad assertion on iOS. I did not check whether iOS already places its request keypad directly above Continue. Codex still sees it: The supplied body now links synonymdev/bitkit-ios#1194, matching your response, but its files were inaccessible. I could not compare its action prose against journeys/paykit-amount-layout.xml:5, so the matching port remains unverified.

new since the last review

worth doing, does not block

  • Sent checkmark overflows onto the headline when no spare height is left (app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt:742-754)
  • Checkmark overflows its box when no height remains (app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt:751-752)

@ovitrif

ovitrif commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Journey prose now differs from iOS with no twin carrying it (journeys/paykit-amount-layout.xml:5)

@coreyphillips The twin synonymdev/bitkit-ios#1194 carries it. On both PR branches, journeys/paykit-amount-layout.xml is byte-identical, including the new action. request-summary.xml gets the same RECIPIENT/OK assertion on both sides, and its other wording already differed between the platforms before this PR.

Pushed 561c561: the Payment Requested checkmark now drops out when its slot has no height, instead of overflowing onto the headline at large text sizes.

@ovitrif
ovitrif requested a review from coreyphillips November 2, 2026 21:36

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

Rechecked after 1 new commit (561c561). Nothing blocking remains.

addressed

  • Sent checkmark overflows onto the headline when no spare height is left (app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt:742-754). Fixed: CreatePaymentRequestScreen.kt:751-752 now uses fillMaxWidth(0.8f).fillMaxHeight() with ContentScale.Fit. The weighted Box passes max height h, the remaining slot, so the Image is exactly 0.8W x h. Fit scales the check to min(0.8W, h), and at h = 0 it draws nothing, so it no longer overflows onto the headline. I checked the modifier chain and the Box/Column constraints by reading the code; I did not run it on a device.
  • Checkmark overflows its box when no height remains (app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt:751-752). Fixed: This is the same fix as point 2, at CreatePaymentRequestScreen.kt:752. With aspectRatio gone, nothing falls back to a width-sized square. The image is bounded by its slot height and scaled with Fit, the same way SheetIntro bounds its artwork without aspectRatio.

still open

  • Journey prose now differs from iOS with no twin carrying it (journeys/paykit-amount-layout.xml:5). This PR edits the action prose of two shared journeys, but no bitkit-ios PR carries the same edits. On iOS master, journeys/paykit-amount-layout.xml has blob 4053d110e, the same blob Android had before this PR (index 4053d110e..ab80d3521). So this change starts the drift. request-summary.xml also gains a RECIPIENT caption and OK-at-bottom assertion. CLAUDE.md asks for identical <action> prose on both platforms, carried by the matching iOS PR. bitkit-ios#1138 tracks the Sent sheet fix, but the PR body has no Twin: line, and nothing tracks the new request-amount keypad assertion on iOS. I did not check whether iOS already places its request keypad directly above Continue. Codex still sees it: I could not verify the response's byte-identical claim for journeys/paykit-amount-layout.xml:5 against synonymdev/bitkit-ios#1194. GitHub was unreachable, and the twin's head is absent from local refs.

@ovitrif

ovitrif commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Journey prose now differs from iOS with no twin carrying it (journeys/paykit-amount-layout.xml:5)

@coreyphillips For a check that doesn't need the twin checked out: journeys/paykit-amount-layout.xml is blob df74405ae on this PR's head and on synonymdev/bitkit-ios#1194's head, so the file is identical on both.

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

Rechecked after replies, with no new commit since 561c561. Nothing blocking remains.

still open

  • Journey prose now differs from iOS with no twin carrying it (journeys/paykit-amount-layout.xml:5). This PR edits the action prose of two shared journeys, but no bitkit-ios PR carries the same edits. On iOS master, journeys/paykit-amount-layout.xml has blob 4053d110e, the same blob Android had before this PR (index 4053d110e..ab80d3521). So this change starts the drift. request-summary.xml also gains a RECIPIENT caption and OK-at-bottom assertion. CLAUDE.md asks for identical <action> prose on both platforms, carried by the matching iOS PR. bitkit-ios#1138 tracks the Sent sheet fix, but the PR body has no Twin: line, and nothing tracks the new request-amount keypad assertion on iOS. I did not check whether iOS already places its request keypad directly above Continue. Codex still sees it: journeys/paykit-amount-layout.xml:5 belongs to blob df74405ae, matching the response, and the description now links the twin. API and browser access failed, so I could not verify that bitkit-ios#1194 carries the identical file.

@ovitrif

ovitrif commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Merging: approved by @coreyphillips on the current head, the screenshots in the description were approved, on-device recordings and Figma side-by-sides are posted, no open threads, CI green.

@ovitrif
ovitrif merged commit 1415556 into master Nov 3, 2026
23 checks passed
@ovitrif
ovitrif deleted the fix/paykit-request-layout branch November 3, 2026 08:26
@ovitrif ovitrif removed this from the 3.2.0 milestone Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants