Skip to content

fix: align paykit auth sheet with figma - #880

Open
jvsena42 wants to merge 4 commits into
masterfrom
fix/paykit-auth-figma-parity
Open

jvsena42 wants to merge 4 commits into
masterfrom
fix/paykit-auth-figma-parity

Conversation

@jvsena42

@jvsena42 jvsena42 commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

iOS port of synonymdev/bitkit-android#1421.

This PR aligns the Paykit authorization sheet with the Auth (paykit) flow in Figma.

Description

  • Earn consent: uses the three-coin illustration from the frame (coin-stack-4).
  • Authorize: names the requester in the lead sentence (app.paykit.server is requesting permission…) and removes the separate Requester ID line. The old sentence stays as the fallback when a request carries no client id.
  • Authorize: groups the content under REQUESTED PERMISSIONS, DETAILS and BEFORE YOU CONTINUE headings with 32pt between sections, removes the divider under the permissions, and uses the Figma copy for the Paykit details and trust warning.
  • Profile card: replaces the centered 96pt avatar card with the compact row (48pt avatar, truncated key above the name). This applies to every Pubky auth dialog. Its placeholder avatar, shown when the profile has no picture, is grey like the app's other avatar placeholders; it was Pubky green.
  • Authorizing: the label uses the 15pt button text style.
  • Success: names the requester in the summary sentence, insets the text 32pt from the sheet edges and draws the check illustration at the frame's scale. The content scrolls when a long requester id does not fit, so the OK button stays on screen.
  • The requester id and service name are shown literally: <accent> tags in them are removed before the sentence is styled, so a request cannot inject its own emphasis.
  • Permission row: folder icon is 16pt.

Out of Scope

  • PubkyAuthApprovalSheet.swift: the AUTHORIZATION RELAY section and the relay line on the Earn screen are iOS additions that the Figma frames do not show; they are kept.
  • SheetIntro.swift: the Earn screen keeps the shared intro component's sizing and side insets instead of the frame's.
  • Localizable.strings: the Figma title reads "Authorization Succesful"; the correctly spelled title is kept. New and changed strings are English only.
  • Authorize paykit FaceID frame: Face ID is the system prompt and is unchanged.

Design

Preview

iPhone 17 simulator, fresh wallet and Bitkit-created Pubky profile, combined paykit-access-v1.watch-only-account-v1 claim.

iOS and Figma

QA Notes

Journeys

N/A — no journey added or updated. The route and identifiers are unchanged, so wallet-leg.xml and paykit-only-approval.xml still drive this sheet as written.

Manual Tests

  • Open a bitkit://pubky-auth/setup URL with caps=/pub/paykit/:rw, a cid and x-bitkit-claim=paykit-access-v1.watch-only-account-v1 → the Earn, Authorize, Authorizing and Success screens match the image above — needs a valid auth URL fixture.

Automated Checks

  • updated PubkyAuthApprovalSheetTests.swift — accent tags in a requester id are removed, including nested ones
  • ran PubkyAuthApprovalSheetTests, swiftformat --lint and node scripts/validate-translations.js — pass locally; a 253-character and a tag-wrapped cid were checked on the iPhone 17 simulator.

Base automatically changed from codex/paykit-shared-runtime-local-20260930 to master October 7, 2026 17:01
@jvsena42
jvsena42 force-pushed the fix/paykit-auth-figma-parity branch from 685a89e to a06a143 Compare October 8, 2026 09:11
@jvsena42
jvsena42 marked this pull request as ready for review October 8, 2026 09:37
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Updates text and layout of an authorization dialog.

The PR appears safe to merge; no actionable issues were established.

What we checked:

  • New text still appears: LocalizationHelper.getString uses the English resource when the selected language lacks a key. The new keys are present in English.

Summary

Updates the Pubky authorization sheet’s copy and layout to match the supplied design.

  • Names the requester in approval and success text.
  • Adds section headings, adjusts spacing and illustrations, and uses a compact profile card.
  • No actionable issues were established.
  • jvsena42 explicitly keeps English-only strings, the authorization relay sections, shared intro sizing, the correctly spelled success title, and the existing Face ID prompt.

Reviews (1) · Last reviewed commit: "fix: align paykit auth sheet with figma" · 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. Aligns the Pubky auth sheet with the Figma frames: names the requester in the lead and success sentences, adds the DETAILS and BEFORE YOU CONTINUE headings, and switches to the compact profile row. Reviewed a06a143, full tier (an approval sheet is a security path), reasoned from the code and CI: iOS does not build on this box.

What I checked, and 3 candidates I ruled out

Read in full: PubkyAuthApprovalSheet.swift; traced config.request.clientID back to Paykit.parsePubkyAuthUrl in PubkyAuthRequest.swift; t(_:variables:) in LocalizeHelpers.swift
CI: validate (translations), Greptile and change detection green; Run Tests, Run Integration Tests and build-local were still pending at review time

Ruled out

  • Requester id lost from the screen: the separate Requester ID line goes, but the id now appears in the lead sentence whenever it is non-empty, and the fallback sentence covers the empty case, so a user still sees who is asking before approving.
  • Removed string keys still referenced: no Swift reference to pubky_auth__requester or pubky_auth__paykit_access_title remains, and the other localizations carry no copy of them.
  • Missing illustration: coin-stack-4.imageset exists in Assets.xcassets/Illustrations.

Merge confidence: 4/5, no findings and copy or layout a unit test would not apply to, but the test and build jobs were still pending and the branch was not built here.

@jvsena42 jvsena42 self-assigned this Oct 8, 2026
@jvsena42
jvsena42 added this pull request to stack #895 October 8, 2026 10:33
@jvsena42
jvsena42 requested review from a team, ben-kaufman and coreyphillips and removed request for a team October 8, 2026 11:52
@coreyphillips

Copy link
Copy Markdown
Contributor

Two independent reviews.

worth doing, does not block

  • Success screen now renders an unbounded, requester-supplied client id (Bitkit/Views/Sheets/PubkyAuthApproval/PubkyAuthApprovalSheet.swift:331). The success sentence now interpolates config.request.clientID through pubky_auth__success_named, but successContent is not in a ScrollView. It stacks the text, a fixed 256pt check scaled to about 274pt, and the OK button. The old requester line was lineLimit(1) with tail truncation. The new text wraps without limit, and the cid comes straight from the auth URL. A long cid could push the OK button (PubkyAuthOK) off screen. I did not find a length cap on cid in PubkyAuthRequest.parse, but I did not check whether Paykit's parser enforces one, so this is unconfirmed. The authorize screen is scrollable, so it is not affected.
  • Requester IDs are parsed as accent markup (Bitkit/Views/Sheets/PubkyAuthApproval/PubkyAuthApprovalSheet.swift:303). Requester IDs can inject <accent> tags into the authorization copy. The pinned ClientId validation accepts any nonempty value up to 253 bytes, while AccentedText interprets these tags instead of displaying the ID literally. This affects both authorization and success text. Escape markup tokens or compose emphasized spans without parsing request data.
  • Changelog fragment omits the pull request reference (changelog.d/next/paykit-auth-figma-parity.changed.md:1). The fragment filename does not follow the required <issue-or-pr>.<category>.md convention. Rename it to 880.changed.md so the collected release entry retains its pull request reference.

nits

  • Changelog fragment is not named after the PR number (changelog.d/next/paykit-auth-figma-parity.changed.md). The fragment is changelog.d/next/paykit-auth-figma-parity.changed.md, but the convention in .agents/commands/pr.md (section 8b) and AGENTS.md is <issue-or-pr>.<category>.md. Every other fragment in the directory follows it. It should be 880.changed.md.

the reviewers disagree, your call

  • Requester is no longer shown when a request has no permissions (Bitkit/Views/Sheets/PubkyAuthApproval/PubkyAuthApprovalSheet.swift:240). Objection: Paykit rejects empty or invalid capability lists before this view is shown, while direct signup requests have an empty client ID. The claimed production state, a nonempty client ID with no parsed permissions, is not reachable.

coreyphillips
coreyphillips previously approved these changes Oct 8, 2026
@jvsena42

jvsena42 commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Went through the review.

  • Unbounded client id on the success screen: fixed in 508f705. The success content is in a scroll view with the OK button outside it. Checked on the iPhone 17 simulator with a 253-character cid: the text scrolls and PubkyAuthOK stays on screen.
  • Requester ids parsed as accent markup: fixed in 508f705. pubkyAuthLiteralText removes <accent> tags from the client id and service name, repeatedly so nested tags cannot rebuild one. Covered in PubkyAuthApprovalSheetTests.swift. Android got the same fix in fix: align paykit auth sheet with figma bitkit-android#1421.
  • Changelog fragment name: fixed in 3d58d12, now 880.changed.md.
  • Requester not shown with no permissions: not changed. As the objection says, a nonempty client id with no parsed permissions is rejected before the sheet opens.

jvsena42 and others added 4 commits October 9, 2026 08:24
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42
jvsena42 force-pushed the fix/paykit-auth-figma-parity branch from 3d58d12 to 12e44e9 Compare October 9, 2026 11:26
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.

3 participants