Skip to content

fix: usdt amount keypad sits above continue - #1061

Merged
ovitrif merged 2 commits into
masterfrom
fix/amount-keypad-layout
Oct 21, 2026
Merged

ovitrif merged 2 commits into
masterfrom
fix/amount-keypad-layout

Conversation

@ovitrif

@ovitrif ovitrif commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1056
Twin: synonymdev/bitkit-android#1636

This PR moves the USDT Amount step's keypad down so it sits right above Continue, as on the Bitcoin amount screen. The amount stays at the top, the free space sits between the amount and the controls bar, and the keypad keeps its 16 pt side insets.

Description

  • Removes the flexible frame around the keypad that centred it in the free space and left a gap above Continue.
  • Puts a flexible spacer under the amount, as the Bitcoin branch of SendAmountView does, so the controls bar and keypad sit at the bottom.
  • Applies this to both USDT amount screens: UsdtSendView and the Paykit request Amount step in SendAmountView when it pays in USDT.
  • Updates usdt-send-review-and-details.xml, which asserted the old 32-point gap.

Out of Scope

  • SendAmountView.swift bitcoin branch and the other numpad screens: unchanged.
  • A redesign of the amount layout (the amount centred and an edge-to-edge keypad, as in the USDT Figma frame): it would change all three amount flows together, not USDT alone. Tracked in feat: amount screens share the centred, edge-to-edge layout #1083.

Design

Bitkit - Refactor v63 › Send (USDT) › Amount USDT. This PR follows the Bitcoin amount screen's layout rather than the frame's centred amount and edge-to-edge keypad, so the three amount screens stay consistent.

Preview

Before, 3.0.1 (balance masked) After, regtest

QA Notes

Journeys

  • updated usdt-send-review-and-details.xml — the amount sits at the top, the controls sit right above the keypad and Continue sits right below it, as on the Bitcoin Amount screen

Manual Tests

  • Open a Paykit request that pays in USDT and reach Amount → same layout as the USDT wallet's Amount — the fixture issuer in journeys/payment-requests/README.md publishes Bitcoin endpoints only; a request paid in USDT on Arbitrum needs an issuer that publishes a USDT endpoint

Automated Checks

N/A

@ovitrif ovitrif self-assigned this Oct 20, 2026
@ovitrif ovitrif changed the title fix: place the usdt amount keypad right above continue fix: usdt amount keypad sits above continue Oct 20, 2026
@ovitrif
ovitrif marked this pull request as ready for review October 20, 2026 23:52
@ovitrif
ovitrif requested review from a team, coreyphillips and pwltr and removed request for a team October 20, 2026 23:52
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 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-10-20T23:54:24.643317Z 7b281b8 Draft marked ready
ℹ️ 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 commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator Author

Superseded: 5040d45 changed the layout to match the Bitcoin amount screen (see the description's Preview); the checks below covered the earlier layout.

QA on an iPhone 17 Pro simulator (iOS 27.0), regtest debug build of this branch:

  • USDT Send → Amount: the ₿ line and the dollar amount sit in the free space above Available; the controls bar is right above the divider and keypad; the key columns centre on the thirds of the full sheet width; Continue sits right under the keypad.
  • The layout needs no more height than before: it only drops the fixed 32 pt gap under the amount, and the keypad keeps its own height.

Not driven: the Paykit request Amount step paid in USDT (see Manual Tests); it uses the same layout code.

@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Moves the extra space above the USDT keypad into the amount header on both amount screens.

  • Both USDT amount screens place the amount above a full-width keypad and Continue.

Reviewed the source only; iOS visual checks were not run on Linux.

Reviews (1) · Last reviewed commit: "fix: place the usdt amount keypad right ..." · Reviewed by Greptile

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No findings. Puts the USDT amount keypad edge to edge directly above Continue and centres the amount in the free space above it, on both the Send sheet amount step and USDT Send. Reviewed 7b281b8, light tier.

What I checked, and 3 candidates I ruled out

Read in full: SendAmountView.swift, UsdtSendView.swift (amountContent), Components/NumberPad.swift (fixed key height)
CI: SwiftFormat, validate, build-local green; tests and e2e still running
Build: Bitkit track: the branch was not built here (Android branches are not built on this box, iOS does not build on Linux), so this review is reasoned from the code and from CI.

Ruled out

  • Bitcoin amount step changed: every edit is behind app.paykitUsesUsdt, and the non-USDT keypad padding is 0.
  • Keypad collapses without maxHeight: NumberPad sizes itself from a fixed buttonHeight * 4, so the free space now goes to the amount header instead.
  • Negative padding clips: both parents pad 16 horizontally, so -16 reaches the sheet edge and no further.

Merge confidence: 4/5, CI had not finished; a layout change no unit test applies to, covered by the updated journey.

@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 7b281b8: no issues found.

  • Layout: the amount is centred between flexible spacers. The keypad's key height comes from the available height, clamped to 50–75, and Continue stays below it.
  • Bitcoin path unchanged: on Android the scroll fallback applies only when the Paykit amount step is paid in USDT.
  • Edge-to-edge keypad: the 16 dp bleed is not clipped, and the moved modifier body is unchanged.
  • Untouched: Back, insets and nav-bar padding.
  • Parity: Android adds a scroll fallback for short screens with large text. iOS keeps its fixed layout, which is 32 pt shorter than before, so the SE plus largest Dynamic Type case is no worse than master. I'll raise it only if the device run shows Continue clipped. See synonymdev/bitkit-android#1636.
  • Journeys: usdt-send-review-and-details.xml updated, identical across both repos.

Device gate (partial): 7b281b8 — not run yet. Pending:

  • keys 1 and 3 reach the screen edges, and the outer 16 dp of key 1 takes taps;
  • the gap from the keypad to Continue;
  • a short screen with large text, scrolled to Continue;
  • the same on the Paykit Amount step paid in USDT.

Device results will follow in a separate review.

@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 results, part 1, on the USDT fixture (iPhone 17 simulator). PR 7b281b8 compared with master 5ce560f, on the USDT wallet's Send → Amount screen:

master PR
Keys 1 / 3 (default text) x 16–139 / 263–386 x 0–134 / 268–402, edge to edge
Gap from the last key row to Continue 56 pt 0 pt; Continue sits right below, as on the Bitcoin screen
Largest text (AX5): Continue bottom 20 pt off screen, label cut fully on screen, reachable
  • Edge taps: taps at x 2 and x 6 on key 1, and at x 398 on key 3, each registered.
  • Largest text: typed 1, tapped Continue and the review opened. The no-scroll concern from my code scan doesn't show up. This PR is better than master here.
  • Bitcoin amount screen: unchanged, with the same frames on master and the PR.
  • Journey: the changed action in usdt-send-review-and-details.xml passed.

Not from this PR, same on both builds: at AX5 the sheet content is wider than the screen, so the amount header, AVAILABLE and the outer keypad columns are clipped at the sides. No iPhone SE simulator exists, so the large-text checks ran on iPhone 17.

Device gate (partial): 7b281b8 — 5 checks passed, 0 failed. Pending: the Paykit Amount step paid in USDT (the changed paykitUsesUsdt branch). The run needs a fresh USDT request from the Android device.

@pwltr

pwltr commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

I recommend matching the existing on-chain and Lightning send amount layout rather than introducing a separate layout for USDT. This is the same recommendation as on the Android twin: synonymdev/bitkit-android#1636 (comment)

Fixing the floating keypad makes sense, but centring the amount vertically and making the keypad edge-to-edge are not necessary to fix that issue. Please keep the amount placement and 16 pt keypad side insets consistent with the Bitcoin screens. The shared keypad sizing and spacing above Continue can remain as they are on iOS.

USDT’s extra controls can wrap where needed without changing the basic layout. If we want to redesign the amount layout, that should be a consistent change across all three flows rather than USDT alone.

@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Please keep the amount placement and 16 pt keypad side insets consistent with the Bitcoin screens. The shared keypad sizing and spacing above Continue can remain as they are on iOS.

@pwltr done in 5040d45. Both USDT amount screens (UsdtSendView and the Paykit request Amount in SendAmountView when it pays in USDT) now keep the amount at the top with a flexible spacer below it, as the Bitcoin branch does, and the keypad keeps its 16 pt side insets. The keypad sizing and the spacing above Continue are unchanged. The description, journey line and changelog now describe this layout, with a fresh screenshot.

A redesign toward the Figma frame (the amount centred, an edge-to-edge keypad) is listed under Out of Scope as a change for all three amount flows. @jvsena42 your device results here covered the earlier layout: the edge-to-edge keys and the Continue gap now match the Bitcoin screen instead.

@ovitrif
ovitrif enabled auto-merge October 21, 2026 13:33

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

worth doing, does not block

  • paykit-quoted-usdt.xml still asserts the old USDT amount layout (journeys/paykit-quoted-usdt.xml:19). journeys/paykit-quoted-usdt.xml:19 still says "the balance stays close below the amount and the keypad uses the remaining space without clipping Continue". This PR makes both halves of that false on the route the journey drives. The journey opens a USDT-only contact and reaches Amount. That is the app.paykitUsesUsdt branch of SendAmountView. After this PR, that branch puts Spacer() (SendAmountView.swift:168) between the amount and the Available row. It also drops .frame(maxHeight: app.paykitUsesUsdt ? .infinity : nil) from the keypad. So the free space now sits between the amount and the balance, and the keypad keeps its fixed NumberPad.contentHeight. Anyone who drives this journey will get a failure. AGENTS.md says a PR updates any journey whose route it changes and lists it under #### Journeys. This journey also covers the Paykit USDT Amount step that the PR's Manual Tests says it could not drive. I confirmed this by reading the code and the journey text. I did not drive it on a simulator. The merged Android twin left the same line in bitkit-android/journeys/paykit-quoted-usdt.xml:19. Make the same edit there so the two files stay identical, for example: "the balance, chips and keypad sit at the bottom above Continue, as on direct USDT Send".

the reviewers disagree, your call

  • Closes #1056 but the deferred redesign has no tracking issue (general). Objection: The repository guidance requires linked issues for deferred work, but the PR body and #1056 were inaccessible. I cannot independently confirm the claimed tracking omission from the code diff. Answer: gh pr view 1061 shows Closes #1056, and the Out of Scope bullet "A redesign of the amount layout (the amount centred and an edge-to-edge keypad...)" carries no issue link, though .agents/commands/pr.md:112 requires one for real work. #1056's Expected behavior items 1 (amount centred) and 3 (no side padding) are exactly what the PR leaves out, it keeps the 16 pt insets, and no open issue tracks them; #974 covers the request-creation amount screen, not the send flows.

@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips thanks.

journeys/paykit-quoted-usdt.xml:19 still says "the balance stays close below the amount and the keypad uses the remaining space without clipping Continue".

Right. To keep this PR's approvals, the line changes in #1082 (fbe1d64), and the same words go into the Android file in synonymdev/bitkit-android#1637 (fc46b472b): "the balance, chips and keypad sit at the bottom above Continue, as on direct USDT Send."

the deferred redesign has no tracking issue

Filed #1083 for a shared centred, edge-to-edge layout across all three amount flows, and linked it from the Out of Scope bullet here and on synonymdev/bitkit-android#1636.

@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 5040d45. Nothing blocking remains.

addressed

  • Closes #1056 but the deferred redesign has no tracking issue. Fixed: gh issue view 1083 shows an open issue in the 3.2.0 milestone covering the centred amount and edge-to-edge keypad across all three amount flows. This PR's Out of Scope bullet now ends "Tracked in #1083", and the body of Android #1636 links it too.

still open

  • paykit-quoted-usdt.xml still asserts the old USDT amount layout (journeys/paykit-quoted-usdt.xml:19). journeys/paykit-quoted-usdt.xml:19 still says "the balance stays close below the amount and the keypad uses the remaining space without clipping Continue". This PR makes both halves of that false on the route the journey drives. The journey opens a USDT-only contact and reaches Amount. That is the app.paykitUsesUsdt branch of SendAmountView. After this PR, that branch puts Spacer() (SendAmountView.swift:168) between the amount and the Available row. It also drops .frame(maxHeight: app.paykitUsesUsdt ? .infinity : nil) from the keypad. So the free space now sits between the amount and the balance, and the keypad keeps its fixed NumberPad.contentHeight. Anyone who drives this journey will get a failure. AGENTS.md says a PR updates any journey whose route it changes and lists it under #### Journeys. This journey also covers the Paykit USDT Amount step that the PR's Manual Tests says it could not drive. I confirmed this by reading the code and the journey text. I did not drive it on a simulator. The merged Android twin left the same line in bitkit-android/journeys/paykit-quoted-usdt.xml:19. Make the same edit there so the two files stay identical, for example: "the balance, chips and keypad sit at the bottom above Continue, as on direct USDT Send". journeys/paykit-quoted-usdt.xml:19 is unchanged on this branch and on origin/master. #1082 (fbe1d64) has the right wording, but gh pr view 1082 shows it is still OPEN, and this PR's body does not mention it. Merge #1082 first, or link it from this PR's Journeys or Out of Scope section, so whoever drives journeys after this merges is not checking a stale assertion.

@ovitrif
ovitrif merged commit 50f2e3e into master Oct 21, 2026
27 of 31 checks passed
@ovitrif
ovitrif deleted the fix/amount-keypad-layout branch October 21, 2026 17:47
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.

bug: amount keypad floats mid-sheet instead of sitting above continue

5 participants