Skip to content

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

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

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

Conversation

@ovitrif

@ovitrif ovitrif commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

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)

  • ✅ Same layout as Send's amount screen:
    • 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: on iPhone SE, iPhone 16 Pro and iPhone 16 Pro Max, the controls, pad rows and Continue land in the same places as on Send.
  • ✅ Disabled Continue is grey text only, as in Figma.

Payment Requested (PaymentRequestSentView)

  • ✅ 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 SheetIntro's proportional size (80% of the screen width), and it shrinks to fit on short screens, so the details and OK stay visible.
  • ❌ The "PAYMENT REQUESTED" headline keeps the fixed Display text size, as on the other success sheets. It does not scale with the screen.

Out of Scope

None.

Design

Preview

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

Figma iPhone SE iPhone 16 Pro iPhone 16 Pro Max
After
Send
Before

Payment Requested:

Figma iPhone SE iPhone 16 Pro iPhone 16 Pro Max
After
Before

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

iPhone SE iPhone 16 Pro Max
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

  • 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 — completion shows the RECIPIENT caption, with OK at the bottom and the details directly above it

Manual 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.
  • The images and recordings come from iOS 26.5 simulators: iPhone SE (3rd generation), iPhone 16 Pro and iPhone 16 Pro Max. A local test harness renders each screen full screen. 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.

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:38:57.504132Z e710826 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, ben-kaufman and pwltr and removed request for a team November 2, 2026 20:38
@greptile-apps

greptile-apps Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; no actionable regression was established.

Summary

Pins the request keypad above Continue and groups the sent-screen details above OK.

  • The request amount keypad sits directly above Continue.
  • The request completion sheet places the recipient above OK.

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

@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. 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: SendAmountView also puts CustomButton straight after NumberPad with 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 VStack with 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 removed Spacer(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)

@ovitrif

ovitrif commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 63012817, 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 SheetIntro's 80%-of-width size, and shrinks to fit on short screens.

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

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

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

Code scan at 6301281: no findings.

Checked:

  • PaymentRequestAmountView sits in a ScrollView whose VStack has minHeight = the container height, so Spacer(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, which paykit-amount-layout.xml action 4 still covers. The view is shared with the subscription amount step (testIdentifierPrefix), so the subscription screen gets the same layout; no identifier changed.
  • PaymentRequestSentView is not scrollable. The check is now resizable + 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_recipient exists in en.lproj ("Recipient").
  • Structure against both linked Figma frames (Amount 49107:130091, Payment requested 49111: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).

⚠️ Device gate: not run — the simulator host is not accepting injected input this pass. The PR build (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 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.

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.

@ovitrif

ovitrif commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

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.

@ovitrif
ovitrif merged commit 2ee882f into master Nov 3, 2026
21 checks passed
@ovitrif
ovitrif deleted the fix/paykit-request-layout branch November 3, 2026 10:34
@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

4 participants