Skip to content

fix: align paykit auth sheet with figma - #1421

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

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

Conversation

@jvsena42

@jvsena42 jvsena42 commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

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

Description

  • Earn consent: swaps the illustration for the three-coin stack the frame uses and draws it at the frame's scale, drops the extra 16dp side inset so the copy and buttons span the sheet width, and sets the gaps around the illustration to 32dp.
  • 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 32dp between sections, removes the divider under the permissions, uses the Figma copy for the Paykit details and trust warning, and sets both to the 15sp body style.
  • Authorize: pins the trust warning and the profile card to the bottom of the sheet; the sections above scroll when they do not fit.
  • Profile card: replaces the centered 96dp avatar card with the compact row (48dp avatar, truncated key above the name). This applies to every Pubky auth dialog, as the design now uses it for all of them.
  • Authorizing: sets the label to the 15sp button text style and gives it the button's height so the content above no longer shifts when the buttons are replaced.
  • Success: names the requester in the summary sentence, insets the text 32dp 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 16dp and gains the duotone tab from the Figma icon.

Out of Scope

  • strings.xml: the Figma title reads "Authorization Succesful"; the existing correctly spelled "Authorization Successful" is kept.
  • Authorize paykit FaceID frame: the Face ID prompt is the iOS system dialog; Android keeps its own biometric/PIN prompt.
  • Type.kt: body styles keep the app-wide 0.4sp letter spacing where the frames show 0.
  • Translations: the new and changed strings are English only.
  • bitkit-ios: ported in fix: align paykit auth sheet with figma bitkit-ios#880.

Design

Preview

Pixel 9 emulator, dev build, combined paykit-access-v1.watch-only-account-v1 claim.

Screen Before / After / Figma
Earn consent Earn consent
Authorize Authorize
Authorizing Authorizing
Success Success

QA Notes

Journeys

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

Manual Tests

  • Open a pubkyauth://signin_grant 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 Figma frames above — needs a valid auth URL fixture.
  • Open a Pubky auth URL without a Bitkit claim → the Authorize screen shows the compact profile card and no DETAILS section — needs a valid auth URL fixture.

Automated Checks

  • updated PubkyAuthApprovalSheetTest.kt — accent tags in a requester id are removed, including nested ones
  • ran just compile, just test, just lint — pass locally.

@jvsena42 jvsena42 changed the title fix/paykit auth figma parity fix: align paykit auth sheet with figma Oct 5, 2026
@jvsena42 jvsena42 self-assigned this Oct 5, 2026
@jvsena42
jvsena42 marked this pull request as ready for review October 5, 2026 14:39
@jvsena42
jvsena42 requested review from a team, ben-kaufman and coreyphillips and removed request for a team October 5, 2026 14:43
@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Updates the authorization approval sheet UI.

The PR appears safe to merge, with non-blocking feedback about keeping consent readable in short windows.

Findings

  1. P2 Footer crowds out consent ▶

Summary

Updates the Pubky authorization sheet to match the Paykit design.

  • Adds requester names, section headings, compact profile cards, and revised illustrations.
  • Keeps the warning and identity card below the scrolling details.
  • Non-blocking feedback: provide a short-window fallback for the fixed footer.

jvsena42 explicitly deferred translations, app-wide letter-spacing changes, the iOS biometric dialog, and the matching iOS implementation. jvsena42 intentionally retained the correctly spelled success title.

Reviews (1) · Last reviewed commit: "chore: add auth sheet changelog fragment"

Comment thread app/src/main/java/to/bitkit/ui/screens/profile/PubkyAuthApprovalSheet.kt Outdated
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 1dcb6ad (run).

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

@jvsena42
jvsena42 added this pull request to stack #1423 October 5, 2026 15:53
@jvsena42
jvsena42 force-pushed the fix/paykit-auth-figma-parity branch from a2e9569 to 18cbbec Compare October 6, 2026 12:13
Base automatically changed from codex/paykit-shared-runtime-local-20260930 to master October 7, 2026 17:17
@ovitrif
ovitrif force-pushed the fix/paykit-auth-figma-parity branch from 18cbbec to 355a467 Compare October 7, 2026 17:17
@jvsena42
jvsena42 force-pushed the fix/paykit-auth-figma-parity branch 2 times, most recently from 84f5432 to ed9dc03 Compare October 8, 2026 16:04
jvsena42 and others added 7 commits October 9, 2026 08:23
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>
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 ed9dc03 to 1dcb6ad Compare October 9, 2026 11:28

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Full review of the complete pull request diff against merge base 50c9bc34, at 1dcb6ad. No earlier assessment baseline was available.

2 actionable findings — resolve or provide an evidence-backed rebuttal.

The grant, local unlock, and capabilities path are unchanged. Consent copy matches the linked Figma frames and the comparison revision 12e44e9 on bitkit-ios#880. wallet-leg.xml, paykit-only-approval.xml, and paykit-reconnect.xml still match the Paykit details sentence and the existing test tags. An earlier note about a non-scrolling footer is already addressed here: the warning and profile scroll with the details when the column is taller than the sheet (thread).

Validation: Inspected PubkyAuthApprovalSheetTest (accent-tag stripping only). I did not run unit tests or a device. On this commit, GitHub CI build, lint, and the local e2e matrix 37923940835 succeeded. Those e2e jobs do not open this sheet. Device testing: not performed in this review. iOS was comparison-only, not a full review.

Non-blocking consistency suggestions: The Authorize frame stacks Before you continue and the profile directly under the details, with the spare space above the buttons. This sheet, like iOS, expands a spacer so that group sits on the bottom when the content is short. iOS still joins every service name with "and"; Android still shows only the first. The success check is scaled uniformly in a 256dp slot; the Authorized frame places a 274×206 image inside a 256×192 frame.

Suggested additional test cases:

  • Android, dev build, pubkyauth://signin_grant whose cid is longer than the sheet text width and has no spaces: open Authorize and Success. The requester is fully visible or ellipsized, and the permission sentence stays fully visible.
  • Android, same flow, with a cid that contains an encoded newline plus a second sentence, and one that contains a bidi override: the sheet shows one consent sentence in the app's word order, with no extra line.
  • Android earn consent on a phone-width sheet: the coin stays clear of the title, matching the Earn frame.

Findings

  • [MEDIUM] Isolate requester text in the consent sentence — inline at app/src/main/java/to/bitkit/ui/screens/profile/PubkyAuthApprovalSheet.kt:510.
  • [LOW] Clip the earn coin to its illustration frame — inline at app/src/main/java/to/bitkit/ui/screens/profile/PubkyAuthApprovalSheet.kt:298.

} else {
stringResource(R.string.profile__auth_approval_service, service)
}
BodyM(text = text.withAccentBoldBright(), color = Colors.White64)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MEDIUM] Isolate requester text in the consent sentence

Trigger: a sign-in grant whose cid is a long domain, or contains an encoded newline or bidi control. ClientId::new in pubky 0.15.0, which paykit-rs v0.1.0-rc71 uses, accepts any non-empty string up to 253 characters, and parse_client_id stores that value.

Mechanism: DescriptionText and SuccessDescriptionText only strip <accent> tags, then interpolate the requester into a multiline BodyM. A newline becomes another line inside the consent copy. A bidi override can reorder that sentence. A long id has no line-break opportunities, and BodyM clips overflow instead of ellipsizing, so the id is cut off with no marker. The same helper is used for the service name.

Consequence: the lead sentence can show attacker-written lines, a reordered permission statement, or a truncated requester, while Authorize still grants that exact client id. The separate permission rows stay accurate, but the sentence that names the requester does not.

Expected: render the requester and service as single-line literal runs, with bidi isolates, newlines and other controls removed or replaced, and ellipsis when they do not fit, so they cannot add lines, reorder the sentence, or hide the rest of the id.

Evidence basis: source analysis at 1dcb6ad. Parser checked in pubky v0.15.0 (ClientId::new, parse_client_id) as pinned by paykit-rs v0.1.0-rc71, matching paykit-android 0.1.0-rc71. The unit test only covers accent tags. Not executed on a device.

.align(Alignment.CenterHorizontally),
.align(Alignment.CenterHorizontally)
.graphicsLayer {
scaleX = COIN_SCALE

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[LOW] Clip the earn coin to its illustration frame

Trigger: the watch-only earn step on a phone-width sheet.

Mechanism: the coin is laid out at 256dp and then scaled by 329/256 from the center and shifted 7dp down, with clipping left off. The painted height is about 329dp, so roughly 43dp extends below the slot. The spacer under the image is 32dp, and the headline is drawn immediately after that, so the art meets the title. In the Earn frame the raster is 329×247 and sits inside a 256×192 illustration frame in the 256 slot, with the title below that slot. The comparison iOS sheet at 12e44e9 does not apply this scale; SheetIntro uses aspect-fit with a 256pt max height.

Consequence: the coin collides with the earn headline instead of leaving the gap shown in the frame.

Expected: size and clip the asset to that illustration frame so the title stays clear of the art.

Evidence basis: source analysis at 1dcb6ad, compared with Figma node 49203:235173 and iOS SheetIntro at 12e44e9. Not executed on a device.

This branch has not been deployed

No deployments
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.

2 participants