Repository navigation
fix: pin paykit request keypad and ok button - #1194
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
[Low impact] The PR appears safe to merge; no actionable regression was established. SummaryPins the request keypad above Continue and groups the sent-screen details above OK.
Reviews (1) · Last reviewed commit: "fix: pin paykit request keypad and ok bu..." · Reviewed by Greptile |
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Removes the flexible spacer under the request Amount number pad so it sits on Continue, and pins OK at the bottom of the Payment Requested sheet with a new RECIPIENT caption above the card. Reviewed e710826, light tier (10 changed Swift lines).
What I checked, 2 candidates I ruled out and 1 left open
Read in full: PaymentRequestAmountView and PaymentRequestSentView in CreatePaymentRequestView.swift, SendAmountView.swift for the reference layout, the ReceiveSheet call site, and the twin synonymdev/bitkit-android#1758 journey edits
Build: not built here. This is a Bitkit iOS review, reasoned from the code and from CI, because iOS does not build on Linux
CI: SwiftFormat, validate and Greptile pass. Run Tests, build-local and Run Integration Tests were still pending at review time
Ruled out, and open
- Continue flush against the pad:
SendAmountViewalso putsCustomButtonstraight afterNumberPadwith no spacer, so this matches the screen the PR aligns to. - Spare space no longer distributed in the ScrollView: the remaining
Spacer(minLength: 16)after the divider still expands under.frame(minHeight: geometry.size.height, alignment: .top), which is the same mechanism the removed spacer relied on. - Payment Requested on a short screen: the sheet is a plain
VStackwith a fixed 256 pt image, and its minimum height grows by about 26 pt (description padding +16, caption and its padding +26, card padding +16, the removedSpacer(minLength: 32)-32). I could not render it on a small device here, so whether it still fits on an iPhone SE is unverified, not ruled out.
Merge confidence: 4/5, the short-screen fit of Payment Requested is unverified and the unit and build jobs were pending at review time.
Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (the metadata this run exposes does not record it)
|
Pushed
The description lists each fix with its status, plus Figma side-by-sides on iPhone SE, iPhone 16 Pro and iPhone 16 Pro Max and simulator recordings. |
pwltr
left a comment
There was a problem hiding this comment.
Reviewed commit 63012817dae8676fa8b9f769417ed0858cfe2339 through an independent general source review of the amount-entry layout, keypad and Continue-button placement, shared subscription usage, and request-completion presentation and dismissal.
No actionable correctness findings were identified.
Reviewer model: GPT-6.1-sol
Reasoning effort: xhigh
There was a problem hiding this comment.
Code scan at 6301281: no findings.
Checked:
PaymentRequestAmountViewsits in aScrollViewwhoseVStackhasminHeight= the container height, soSpacer(minLength: 12)above the controls takes the spare space and the pad lands on Continue, as on Send. On a short screen the spacer collapses to 12 pt and the content scrolls, whichpaykit-amount-layout.xmlaction 4 still covers. The view is shared with the subscription amount step (testIdentifierPrefix), so the subscription screen gets the same layout; no identifier changed.PaymentRequestSentViewis not scrollable. The check is nowresizable+aspectRatio(.fit)with only max frames, so it is the one flexible child and shrinks before the headline, description, caption, card or OK get clipped.wallet__payment_request_recipientexists inen.lproj("Recipient").- Structure against both linked Figma frames (Amount
49107:130091, Payment requested49111:130848): same order of elements, RECIPIENT caption present, OK pinned at the bottom. - Twin synonymdev/bitkit-android#1758 is merged with the same journey edits.
perf: not relevant (static layout only).
6301281) built and launched on iPhone 17e, but element taps, direct axe coordinate taps and the hardware Home button all report success and change nothing; the app's main thread is idle, and a freshly booted iPhone 17 ignores taps on the springboard too. paykit-amount-layout.xml, the requester side of payment-requests/request-summary.xml, the Figma screenshot comparison and the l10n check of the RECIPIENT caption (11 locales, longest "DESTINATÁRIO") are still to run; I'll retry on the next pass. Not approving until then.
jvsena42
left a comment
There was a problem hiding this comment.
Device gate result. No findings from the code scan or the device run; approving.
Device gate: 6301281 — iPhone 17e (requester, profile Gate1143) and iPhone 17 (payer, Gate1194B), both on the PR build, saved and linked as Paykit contacts on staging. perf: not relevant (layout only). Figma: 2 screens compared, 0 deltas. l10n: no string changed; the RECIPIENT caption reuses an existing key (longest translation "DESTINATÁRIO", single caption line), not checked per locale.
paykit-amount-layout.xml:
- 1–2: pass. Amount at the top, spare space under it, then the BTC/BITCOIN controls, divider, keypad and Continue, as in the Amount frame. Disabled Continue is grey text. In USD with 1.50 entered, the divider stays at the same height as in BTC.
- 3: pass. Toggling BTC↔USD keeps the controls, divider and keypad in place; the Bitcoin unit control shows only in BTC.
- 4: pass at accessibility-extra-large text. Every key and Continue stay on screen without scrolling; Continue opens details with $ 1.50.
⚠️ 5 (subscription amount): not run. The view is shared, so the layout is the same code path.
payment-requests/request-summary.xml:
- 1–2: pass. A 21,000-sat request with "Lunch last week" reached "Sent" / "You have sent a payment request" in 22 s on staging. The RECIPIENT caption sits above the request card, and OK is at the bottom with the headline, description and card directly above it, matching the Payment requested frame.
- 3–5: pass. The payer's confirmation opened with 21,000 sats, From Gate1143, For "Lunch last week".
⚠️ 6 onward (no-note request, USDT history): not run. They need the local USDT fixture, and this PR does not touch those screens.
|
Merging: approved by @jvsena42 on the current head after a two-device staging run (both journeys pass, Figma matches), the screenshots in the description were approved, no open threads, CI green. Twin of synonymdev/bitkit-android#1758, merged. |
Fixes #1138
Fixes #974
Twin: synonymdev/bitkit-android#1758
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 #974 already shipped in #1078; this PR covers that issue's layout part.
Description
Status of each requested fix, at the current head:
Request Amount (
PaymentRequestAmountView)Payment Requested (
PaymentRequestSentView)RECIPIENTcaption sits above the recipient card, as in Figma.SheetIntro's proportional size (80% of the screen width), and it shrinks to fit on short screens, so the details and OK stay visible.Out of Scope
None.
Design
Preview
Request Amount, Figma and the app on three simulators. Send's amount screen is the reference row:
Payment Requested:
Simulator recordings of the three fixed screens (request Amount, Payment Requested and Send amount), at 2x:
ios-small-rec.mp4
ios-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 device such as iPhone SE and on a tall one such as iPhone 16 Pro Max.Journeys
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 placesrequest-summary.xml— completion shows the RECIPIENT caption, with OK at the bottom and the details directly above itManual Tests
N/A
Automated Checks
N/A
Evidence
63012817: the unit suite with CI's test selection passes, and SwiftFormat lint is clean on the changed file.master.Gaps
Models used
claude-opus-5-5(reasoning:medium)claude-opus-5-5(reasoning:medium)