Skip to content

feat(settings): add mobile Approve sign in card - #20952

Draft
dschom wants to merge 1 commit into
mainfrom
FXA-14234
Draft

feat(settings): add mobile Approve sign in card#20952
dschom wants to merge 1 commit into
mainfrom
FXA-14234

Conversation

@dschom

@dschom dschom commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Because

  • After scanning the pairing QR code shown on their computer, users on mobile have no screen telling them the sign-in still needs approving on that computer, so the flow looks stalled.
  • Approving a sign-in from a second device is a security decision, so the mobile screen needs to show which device made the request — browser, estimated location, and IP — before the user commits to pairing.
  • FXA-14234 specifies this card as the mobile "Approve sign in" step of the reworked desktop-to-mobile pairing flow (epic FXA-12855).

This pull request

  • Adds packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/index.tsx, a presentational card rendering the Firefox brand logo, a desktop/mobile sync illustration, the heading and instruction copy, the device info block, and a Cancel action. The card surface comes from AppLayout, matching the sibling Pair2 card in feat(settings): add mobile "Connect this device" pairing card #20954.card only paints a backdrop from mobileLandscape up, so on a phone this still reads as the full-bleed mobile design. Device details arrive as a remoteMetadata prop rather than the component reading the pairing channel itself.
  • Switches the wordmark's flat fill="black" to currentColor (the fox glyph is all gradients, so only the lettering needed it) and renders it inlined, so it stays legible on the dark card surface. The device-info box gets dark: variants to match. Same one-line SVG change feat(settings): add mobile "Connect this device" pairing card #20954 made to its copy of the asset.
  • Makes onCancel a required prop so routing this card cannot leave its only available action inert. Per the ticket, no button behaviour is wired up here — the handler lands with the route.
  • Extends the shared components/DeviceInfoBlock rather than reimplementing it. The Figma frame shows "Firefox on Laurels-MacBook-Pro" — the device name folded into the browser line, no heading — which is a third presentation the component did not have, so showDeviceName-style booleans do not cover it. Adds deviceNameDisplay?: 'heading' | 'inline' | 'hidden' (default heading) plus className?: string (default 'mt-8 mb-4'), both defaulted to today's behaviour so none of the four live Pair/* callers change. All three modes fall back to the OS line when the payload carries no device name, which is what pairing-authority-integration produces on the authority pages.
  • Moves the one new translatable string into DeviceInfoBlock/en.ftl as device-info-browser-device, alongside the device-info-browser-os it parallels. Net zero new strings versus the previous approach, but the id is now owned by the component that renders it instead of being borrowed across a package boundary.
  • Puts both Figma exports in the shared components/images collection — graphic_firefox_wordmark.svg and graphic_sync_devices.svg — behind FirefoxWordmarkImage and SyncDevicesImage wrappers, following the directory's existing PreparedImage pattern. FXA-14235 through FXA-14247 render the same lockup and illustration, so their aria-labels are localized once here (firefox-wordmark-image-aria-label, sync-devices-image-aria-label) rather than redefined per page. The card imports the two wrappers and needs no image markup, asset, or useFtlMsgResolver of its own. Both are added to the Components/Images storybook page.
  • Adds en.ftl, index.stories.tsx with four stories (device name, OS fallback, unknown location, long device name), and a component test suite that asserts wiring and checks every message this page owns against the Fluent bundle. The device-name and geo permutations are covered in DeviceInfoBlock's own suite, which gains tests for the two new modes, the no-device-name fallback, and the className override.

Issue that this pull request solves

Closes: FXA-14234

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on: components/DeviceInfoBlock/index.tsx — it is the only touched file with existing callers. The new card's index.tsx should read as pure markup.
  • Suggested review order: DeviceInfoBlock/index.tsx (smallest, shared) → its tests → the card's index.tsxen.ftl → stories/tests.
  • Risky or complex parts: this PR now modifies a component with four live callersPair/AuthAllow, Pair/SuppAllow, Pair/AuthWaitForSupp, Pair/SuppWaitForAuth. Both new props default to current behaviour and no call site changed, but that is the claim worth checking. The new card itself is still unrouted and cannot regress anything.
  • Worth eyeballing in both themes: the currentColor wordmark and the device-info box, via the Storybook theme toolbar.

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

  • The sync illustration needed two fixes after export. Figma padded it asymmetrically (~22px of empty space on the left, none on the right), which pushed the artwork ~10px off-centre, so its viewBox is cropped to the visible content. It also embedded the browser-tab favicon at 1046x1081 for a ~3px render; downscaling that payload took the asset from 946KB to 58KB.
  • /fxa-review-quick returned APPROVE with 0 blocking issues. Folded in: the testAllL10n bundle check, required onCancel, and trimming component tests that duplicated coverage held elsewhere.
  • Adopting AppLayout resolved two things previously deferred here: the <main> landmark now comes from the layout, and the card inherits .card's dark treatment (with dark: variants added for the wordmark and device-info box). Still deferred: the document title, which belongs to whoever wires the route and should be passed as AppLayout's title prop.
  • Open design question, same as feat(settings): add mobile "Connect this device" pairing card #20954: AppLayout renders a Mozilla logo in the page header while the design puts a Firefox lockup inside the card, so both appear. The desktop sibling frames have no in-card logo, which suggests the mobile Pair2 screens are meant to drop the standard header — a layout call for the flow-wiring ticket.
  • An earlier revision of this branch carried a page-local device-info-lines.ts that duplicated DeviceInfoBlock's location branching and borrowed FTL ids it owned, with a TODO to consolidate later. That is gone — the logic lives in the shared component and the duplicate module and its 137-line suite are deleted, so this PR is a net reduction in the pairing code it touches.
  • Merge-order note for feat(settings): add mobile "Connect this device" pairing card #20954: that PR adds showDeviceName?: boolean to DeviceInfoBlock for the same box; this one adds the deviceNameDisplay enum, whose 'hidden' covers the identical case. Two interacting booleans on a shared component read badly, hence the enum — but whichever lands second has to reconcile. For feat(settings): add mobile "Connect this device" pairing card #20954 that is a one-line change from showDeviceName={false} to deviceNameDisplay="hidden".
  • Fixed in passing, and outside the ticket: DeviceInfoBlock/index.stories.tsx spread Partial<RemoteMetadata> as JSX props instead of merging into remoteMetadata. TypeScript skips excess-property checks on spreads, so it compiled while silently rendering nothing — the WithDeviceName story never showed a device name. The new mode stories use that helper, so it was corrected rather than built on.
  • testAllL10n does not work on pages with a labelled PreparedImage, so this page filters rather than calls it. Jest stubs SVG imports with a component whose text content is the file name, so an attribute-only message (.aria-label = Firefox) can never match its rendered text and the check always throws. Every page that hits this resolves it by commenting testAllL10n out wholesale — Pair/Success, ConnectAnotherDevice, SigninTokenCode, Signup/ConfirmSignupCode. Here the shared-image ids are skipped and asserted against the bundle directly instead, so the rest of the page keeps real coverage. Worth a follow-up ticket against the harness; not fixed here.
  • FXA-14233 ("Connect this device") renders the same lockup and illustration, and feat(settings): add mobile "Connect this device" pairing card #20954 currently carries its own copies — including a PNG placeholder for the illustration, since the Figma frame was annotated "Github link to come". It can delete both and import FirefoxWordmarkImage / SyncDevicesImage, which also upgrades its illustration from the PNG to the SVG. The device-info box is likewise shared now via DeviceInfoBlock.

@dschom
dschom force-pushed the FXA-14234 branch 3 times, most recently from 035fa45 to e9640ad Compare August 1, 2026 01:41
Because:
 - After scanning the pairing QR code on their computer, users on mobile need
   a screen that tells them the sign-in still has to be approved on the
   computer, and that shows the requesting device's details so they can
   confirm the request is really theirs before pairing completes.

This commit:
 - Adds `packages/fxa-settings/src/pages/Pair2/Supplicant/ApproveSignIn/index.tsx`,
   a presentational card rendering the Firefox brand logo, a desktop/mobile sync
   illustration, the heading and instruction copy, the device info block, and a
   Cancel action. It supplies its own card surface rather than using `AppLayout`,
   per the Pair2 mobile designs. `onCancel` is a required prop so that routing the
   card cannot leave its only action inert; the flow logic lands with the route.
 - Adds `device-info-lines.ts`, which selects the Fluent message for each line of
   the device info block as a pure function so the device-name/OS fallback and the
   five geo-resolution permutations are unit tested without rendering. The location
   and IP lines reuse the shipped `device-info-block-*` messages, so only the
   browser-on-device string is new.
 - Adds `en.ftl` with the new strings, `index.stories.tsx` with four stories
   covering the device-name, OS-fallback, unknown-location, and long-device-name
   cases, and tests for the component and the message selection. The component
   suite asserts wiring and runs `testAllL10n` against the bundle to catch drift
   between fallback text and the Fluent messages; the permutations live in the
   pure-function suite.
 - Exports `firefox-brand-logo.svg` and `sync-devices.svg` from Figma. The sync
   illustration is cropped to its visible bounds so it centres correctly, and the
   browser-tab favicon it embedded at 1046x1081 for a ~3px render is downscaled,
   taking the asset from 946KB to 58KB.

Closes #FXA-14234
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.

1 participant