Show the payment QR on desktop and wallet copy on mobile

[joshuakrueger-dfx]

EN:
The payment page now shows the large QR on a desktop and wallet copy on a handheld, so displayQr: false no longer leaves a payer reading "scan the QR code" with no code on screen; the merchant setting is renamed to what it now means. Rebased onto current develop, with the Italian wallet noun, the OpenCryptoPay standard fallback, the visual specs' reality declaration and two untested derivations fixed on top. Eight mutations were re-run on the final head and all eight turn tests red. Full-stack E2E reports success without running anything in this repository at the moment — that is an open item, not a claim.

DE:
Die Zahlungsseite zeigt den grossen QR jetzt auf dem Desktop und die Wallet-Auswahl auf dem Handgerät, damit displayQr: false keinen Zahler mehr vor "Scanne den QR-Code" ohne QR-Code stehen lässt; die Händler-Einstellung heisst, was sie bewirkt. Auf aktuelles develop rebased, dazu behoben: das italienische Wort für Wallet, der Rückfall auf den Standard der Zahlung, die Reality Declaration der Visual-Specs und zwei ungetestete Ableitungen. Acht Mutationen wurden auf dem finalen Head neu gefahren, alle acht färben Tests rot. Full-stack E2E meldet derzeit Erfolg, ohne etwas auszuführen — das ist ein offener Punkt, keine Behauptung.

Details

Open — cannot be closed from this branch

• Consume the SDK for the pay-request path instead of raw fetch. Review finding 6.
  The contract belongs in DFXswiss/packages, and packages#201
  (open since 2 Aug, two commits, no review yet) already carries the types — PaymentLinkPayRequest,
  PaymentLinkPayTerminal, PaymentQuote, TransferAmount, hasPaymentQuote.
  What it does not carry is exactly the three calls this repo makes by hand: the unauthenticated
  GET paymentLink/payment, the LNURL callback, and lnurlp/wait.
  Why not from here: it needs a merge plus an npm release of @dfx.swiss/react in another repo
  before services can import anything — building a second contract beside [NO-TASK] Block direct login with address #201 would collide on
  merge. Who: @TaprootFreak owns [NO-TASK] Block direct login with address #201; the three missing calls need an owner there.

• Confirm the device split on real hardware — iOS Safari and Android Chrome with
  "Request Desktop Site" on, plus one desktop browser.
  Why not from here: the split is covered by mocked unit tests and an emulated Playwright device
  profile only; neither is a phone. Who: anyone with a deployed build and a phone.

• Restore the API checkout credential for the Full-stack E2E job. Since 2026-08-18 it
  ends after 10 s with No API checkout credential — not starting the e2e stack on every pull
  request in this repository and still reports success, so the e2e-stack half of this branch is
  currently unproven on CI.
  Why not from here: a repository secret and workflow permissions, not a branch.
  Who: whoever owns the CI secrets.

---

Symptom (verbatim): "Scanne den QR-Code mit einer kompatiblen App, um die Zahlung durchzuführen." — shown on the payment page while no QR code was rendered anywhere on it (screenshot 2026-08-05, CHF 12'333 payment, route "Revoltaris GmbH", default configuration).
Scale: DefaultPaymentLinkConfig.displayQr is false, so this is what every OpenCryptoPay merchant gets who never changed the setting; the first merchant printing invoice QR codes bills more than 100 invoices a month.
Smaller fix considered: Setting displayQr = true on that one route, which needs no code at all — insufficient because on a phone the QR is unusable (the screen showing it is the screen you would have to scan it with) and it pushes the wallet list down, so a fixed setting can only ever serve one of the two devices.

Context

A merchant wants one single static QR code printed on every paper invoice they mail: the customer scans it, types in the invoice number and the amount, and pays in crypto. That is the whole point — no QR generated per invoice, no separate mail per customer, one code on the template and done.

DFX already supports exactly that: app.dfx.swiss/invoice?recipient=<route label> registers the invoice on first scan and hands the customer straight to the payment page, and the invoice number travels with the payment into the merchant's history. What it was never built for is a payer standing in front of it — every screen on that path was written for the merchant creating a link. This PR comes out of walking that path end to end with a merchant who is about to print it on more than 100 invoices a month.

Problem

The payment page always tells the payer to scan a QR code, but only renders one when the route has displayQr enabled — and DefaultPaymentLinkConfig.displayQr is false. On the default configuration the payer reads an instruction pointing at nothing, while the QR sits two levels deep under "Payment details" → "QR Code".

Turning displayQr on per route does not fix it. On a phone the QR is useless — the screen showing it is the screen you would have to scan it with — and it pushes the wallet list down. A fixed setting can only serve one of the two devices.

Change

The page follows the device, inside the OpenCryptoPay section where the large QR lives:

	large QR	collapsed "QR Code" row	sentence
displayQr true	yes (unchanged)	no (unchanged)	scan the QR
default, desktop	yes (new)	no	scan the QR
default, handheld	no (unchanged)	yes (unchanged)	choose your wallet
any device, non-OCP standard	no	yes (fixed, see below)	—

The setting changes meaning, and its label changes with it. displayQr is now a force switch:
"Yes" shows the code on every device, "No" hands the decision to the device. There is no longer a
configuration that hides the large QR on desktop. The dropdown was therefore renamed from
"Display QR code" to "Always show QR code" (payment-routes.screen.tsx:449, 674, 1346, 1416), so it
stops promising something the page no longer delivers. Merchants who deliberately set
displayQr: false will see a changed payer view without a migration and without an opt-out — that
is a deliberate product decision, not an oversight, and the API field keeps its name displayQr.

The device is read from the pointer, not from the user agent alone. isMobile from
react-device-detect is computed once at module import. "Request Desktop Site" — a standard toggle
in iOS Safari and Android Chrome — flips it to false on an actual phone, and the payer then gets
the large QR plus "scan it with a compatible app" on the very device they would have to scan with.
matchMedia('(pointer: coarse)') describes the input hardware instead of the UA string and survives
that toggle. The reactive width from useWindowContext() cannot do this job: window.context.tsx:18
caps it at Math.min(offsetWidth, 768), so a phone requesting the desktop site reports the same
value as any wide desktop.

react-device-detect is now declared in dependencies. Eight files already imported it although it
only arrived transitively; the version matches what the lock file already resolved, and the lock
diff is that single line.

Review round

All six findings from the review are fixed in this PR. Three independent passes over the result
(conformity, logic, security) found one blocker of their own, which is fixed as well.

Finding 1 — an explicit displayQr = false could no longer be honoured on desktop. Resolved by
the product decision above: the switch is a force switch and the label says so.

Finding 2 — "Request desktop site" handed the payer an unscannable QR. Fixed via the coarse
pointer, in a reactive hook (src/hooks/device.hook.ts) that also survives a missing matchMedia
and older Safari's addListener-only API.

Finding 3 — the new German string broke the file's form of address. Fixed, together with the
untranslated "wallet" in fr.json and it.json. The guard against a repeat is a test over the
translation files themselves (src/__tests__/translation-conventions.test.ts), with the
pre-existing exceptions listed by name so a new violation fails instead of joining them silently.
That guard is deliberately repo-wide, not scoped to the four new strings: the finding was a break of
the file's convention, and a test that only looks at the new strings would not have caught it.
The price is one unrelated correction it forced — it.json:1135, "wallet esterno" →
"portafoglio esterno" in the Safe send copy.

Finding 4 — the same predicate spelled out three times. Now derived once as showLargeQr, and
the OpenCryptoPay guard is derived once as showsOcpSection rather than written out twice.

Finding 5 — the no-quote counter branch told the payer to scan a QR that is never rendered.
The sentence now names only what is on screen.

Finding 6 — the pay-request and callback calls bypass the SDK. See the open item at the top.
Not fixed here, because the fix has to land and be released in another repo first.

Blocker found by the review passes, not in the original report: with showLargeQr true on
desktop, the collapsed "QR Code" row disappeared — but the large QR sits behind a second guard that
only renders it for the OpenCryptoPay standard. A payment link whose standard is PayToAddress or
LightningBolt11 therefore showed no QR at all on desktop, from first load. It came in with the
first commit of this PR. showLargeQr now knows that guard. Side effect, taken deliberately: the
pre-existing case displayQr: true on a non-OCP standard, which showed no QR either, is fixed too.

Review round 3 — handbook label, e2e-stack split, coverage numbers

Head b8f959b5, rebased onto current develop (a801ba1d, includes #1270) so e2e-stack/ is on the branch. Three items from
the 2026-08-13 review:

1. Handbook + visual E2E for the routes label. e2e/payment-routes-qr-label.spec.ts opens
   /routes on mocked APIs, expands Default configuration, and snapshots
   "Always show QR code". Entry payment-routes-qr-label in scripts/handbook/metadata.json.
2. e2e-stack device split. e2e-stack/specs/payment-links.spec.ts now opens /pl on desktop
   (scan sentence, no collapsed QR row) and handheld (wallet copy + collapsed QR row). The loc API
   cannot build a Lightning/BTC quote (404 No BTC transfer amount found); those two tests fulfill
   paymentLink/payment themselves. Declared in docs/test-architecture.md (the file Make the collection-IBAN toggle readable and let the QR code follow it #1270 added for this). The /routes mixed test
   also expands Default configuration and asserts the renamed label against the real stack.
3. Per-file coverage. Re-measured on Studio, Node 20, on this head — table below. Still not
   100 % branches on the two screens; the remaining arms are listed there.

Handbook

The payment screen appeared in no handbook entry, and e2e/payment-and-special.spec.ts asserts
nothing about it beyond a visible body. Two baselines now show the split this PR is about — desktop
with the large QR and the scan sentence, handheld with the wallet list — plus a metadata.json
entry. A third baseline shows the payment-routes Default configuration row with the renamed
"Always show QR code" label. The payment APIs are served from fixtures with a fixed quote expiry
and the wait poll held open, so the shot does not depend on a live backend.

playwright.config.ts now pins REACT_APP_API_URL for the web server, defaulting to the same value
scripts/e2e-test.sh already uses, and REACT_APP_PUBLIC_URL to http://localhost:3001 so
Lnurl.prependLnurl can build a URL when the routes baseline renders a payment-link QR. Without
the API pin the payment baselines were not reproducible: the QR encodes Api.url, so regenerating
them with the .env.sample default produced a different matrix and a red test with no code change
at all.

The path filter in handbook-check.yaml is deliberately left as it was. The review asked for it
to trigger on this change, and it does — the new baselines live under e2e/screenshots/**, which the
existing filter already matches. Adding the screen and the hook to that list was tried and reverted:
scripts/handbook/build.js reads committed PNGs only, Playwright runs in no workflow of this repo,
and the twin list in handbook-deploy.yaml would have had to move with it. A screen diff would have
started a 25-minute ARM Docker build that cannot detect a stale baseline.

Review round 4 — rebase onto current develop (2026-08-19)

Rebased onto develop 0bba3597. Three text conflicts, all in documentation, all resolved by keeping the newer develop table and re-adding this branch's row; git range-diff shows commits 1–11 identical, 12 and 14 differing only in that resolution, and the last one — a pure table-width commit — dropped because prettier reformats the merged table anyway.

Two things the rebase surfaced, plus four findings from the review lanes that ran twice over the whole diff:

1. The Italian wallet noun. develop 01a00f83 - Describe adding a wallet address on /account-merge #1404 added two /account-merge strings calling the wallet "wallet" in Italian, while its own fr.json says portefeuille and the other eighteen Italian occurrences say portafoglio. The convention guard this branch adds reads the whole file, so the rebase turned it red. Fixed by following the file, not by widening the exception list.
2. The permission-denied error formula was untested. Replacing it with apiError ?? error left all 58 tests in the three payment-routes suites green. There is now a case for it; under that mutation it is the one test that turns red.
3. An unset form standard counted as OpenCryptoPay (showsOcpSection, finding 1 of the 2026-08-12 review, filed as payment-link: derive OCP section from payRequest.standard when form standard unset #1350). Since showLargeQr is !isHandheld on desktop, that window rendered a scannable OpenCryptoPay QR for a Pay-to-Address payment — and stayed that way when the standard is missing from the list. The derivation now falls back to payRequest.standard. Closes payment-link: derive OCP section from payRequest.standard when form standard unset #1350.
4. The visual specs' fakes were undeclared (Declare visual-e2e payment fakes in docs/test-architecture.md #1381) and one matcher was bound to dev.api.dfx.swiss. Both specs are now in the reality declaration with what a green run does not prove, and the matcher goes by path. Closes Declare visual-e2e payment fakes in docs/test-architecture.md #1381.
5. Leftover debug listeners in payment-qr-device.spec.ts (e2e/payment-qr-device: drop leftover console.log pageerror listeners #1383) removed, and the Italian cashier line moved from formal Lei to tu like its neighbours (it.json: cashier payment string uses formal Lei amid informal tu payment copy #1353). Closes both.
6. Rejected: the deeplink request in payment-link.screen.tsx:161 has no generation guard and no catch. Real, but outside every hunk of this diff — git diff -U0 puts the changes at +142…150 and +268, never at 161. Filed as payment-link.screen: guard the deeplink request against superseded responses #1413 instead of widened into this pull request.

Verification

Measured with Node 20 on the exact revision of this branch (cd3ed6ad), after the rebase:

• npm run lint → empty output, exit 0. npm run format:md:check → clean.

• CI=true npm test, full suite → 112 suites / 1545 tests green, locally and on GitHub Actions.

• Full-stack E2E reports success without executing anything. The job ends after 10 s with No API checkout credential — not starting the e2e stack. The job ran. Not specific to this branch: every pull-request run in this repository does that since 2026-08-18 (checked on feat/app2-preview and fix/tx-uid-guest-actions), while the develop push run of that day still ran for 20 minutes. The e2e-stack device split last really ran on head b8f959b5 (231 passed). Until the credential is back, no branch here can prove full-stack coverage — added as an open item above.

• Playwright payment-qr-device.spec.ts → 2 passed, twice in a row without
  --update-snapshots. Regenerating the baselines after pinning the API URL produced
  byte-identical files (54 900 and 33 342 bytes), so reproducibility is shown, not claimed.

• node scripts/handbook/build.js → no orphan warning, group payment-qr-device present in
  index.html and manifest.json.

• Counter-check against the reviewed revision 397f9b3: production files reset to that state,
  tests kept → 3 suites / 9 tests fail, the suite green again after restoring. Every file was
  compared by blob hash between the machine that ran the tests and the one that made the commits;
  all 13 match.

• Mutations, re-run on the final head cd3ed6ad, each asserting its match count before patching and each verified restored afterwards (working tree clean). All eight turn tests red:

  Mutation	Failing
  isMobile || hasCoarsePointer → &&	6 of 9 device-hook
  displayQr || !isHandheld → &&	4 of 75 payment-link
  showsOcpSection && dropped from showLargeQr	3 of 75 payment-link
  standard fallback removed (unset counts as OCP again)	1 of 75 payment-link
  German Deine → deine (103 values)	2 of 7 translation
  French portefeuille → wallet (21 values)	2 of 7 translation
  Italian portafoglio → wallet (21 values)	2 of 7 translation
  permission-denied precedence removed from the error hint	1 of 59 payment-routes

• Earlier mutations, measured on head b8f959b5 before the rebase and not re-run since:

  • a French value carrying {{wallet}} and the common noun : 1 of 7 fail (this one only after
    dropping {{wallet}} from the exception list — with it there, the guard let the string through)
  • the form label at payment-routes.screen.tsx:1346 reverted to "Display QR code" : 3 of 54
    routes tests fail — the place a rename could have been missed before these tests existed
  • the DONE-summary label at :1416 reverted : 2 fail; deactivate sending ACTIVE : 1 fail;
    the public/multiple toggle inverted : 2 fail; the permission denied filter inverted : 2 fail

Test coverage

The review asked for 100 % on every file this PR touches. Reached on the new file, not on the two
screens, and the numbers are:

file	statements	branches	functions	lines
src/hooks/device.hook.ts (new)	100	100	100	100
src/screens/payment-link.screen.tsx	100 (was 49.67)	97.5 (was 46.30)	100 (was 36.20)	100 (was 48.96)
src/screens/payment-routes.screen.tsx	98.88 (was 0)	92.03 (was 0)	100 (was 0)	100 (was 0)

Re-measured with the full suite on head cd3ed6ad, Node 20 — not carried over from an earlier head. The open branches on payment-link.screen.tsx are now 357, 455, 484, 667, 795, 802.

payment-link.screen.tsx is fully covered on statements, functions and lines. The branches still
open are 357, 454, 459-460, 479, 662, 790, 797 — optional-chaining and nullish-coalescing arms
whose alternative value never occurs in a reachable state.

payment-routes.screen.tsx is fully covered on functions and lines. Three statements stay open:
591 is an onClick guard behind a .filter(item => item.text) that already removed the falsy
case, and 1078/1082 are break arms for RECIPIENT/CONFIG/PAYMENT without a paymentLinkId, a
state in which the Save button is not rendered. The open branches on both screens are ?? '',
|| fallback and optional chains whose alternative arm is unreachable behind an earlier guard;
forcing them would mean writing tests for states the app cannot enter.

Neither screen was restructured to make it cheaper to test, and no istanbul ignore was added —
the production diff is unchanged at 15 lines.

That second block of screen tests is a contribution of its own, not evidence for the QR fix. The QR
fix is carried by the mutation numbers above.

Not verified

• No test on a physical phone, tablet or desktop browser — see the open item at the top.
• The remaining Playwright specs were not run with the newly pinned REACT_APP_API_URL; only
  payment-qr-device.spec.ts was. With reuseExistingServer and a dev server already running, the
  pin does not apply — it takes a fresh web server start.
• The renamed label is now asserted on the list (:449, :674) and inside PaymentLinkForm
  (:1346, :1416) plus a handbook baseline of the expanded default-configuration row.
• Baselines exist for chromium-darwin only, like all 154 already in the repo. No workflow runs
  Playwright, so no CI job compares them.
• The Docker image build and container smoke test from handbook-check.yaml were not run locally,
  only build.js.
• Whether pointer: coarse stays true on every touch device with "Request Desktop Site" across the
  full browser matrix — verified in Chromium via the Playwright device profile and in jsdom, nowhere
  else.

---

Final pass (cd3ed6a):
Coherent: The core is one piece — the payment page decides by device, the setting is renamed to what it now means, the translations follow, and three baselines show the split plus the renamed routes label. The e2e-stack device tests sit next to that because the review required the full-stack spec to open both sides of the split. The commits added on 2026-08-19 stay inside that frame: they repair what the rebase broke and pin the two derivations the QR decision rests on.
Nothing extra: The smallest change that removes the symptom is still ~4 lines. Everything beyond it is named by a review: the routes handbook baseline, the e2e-stack assertions, the stated coverage, the quote-fulfill declaration, and this round's four fixes. Deliberately not built: 100 % branches on the two screens (remaining arms listed above), a real Lightning quote in loc, the SDK consumption (blocked on packages#201), and no generation guard for the deeplink request — that one is outside every hunk of this diff and now sits in #1413.
Sources closed: Review 2026-08-13 (TaprootFreak, CHANGES_REQUESTED on a53798a) — handbook routes label: implemented; e2e-stack device split: implemented, but the job runs nothing today (see Verification); per-file coverage: numbers stated and re-measured on this head, not 100 % branches, remaining arms listed. Review 2026-08-07 (findings 1–6) — 1–5 implemented; 6 open with owner at the top. Mara 2026-08-12 / 2026-08-13 APPROVE — follow-ups #1350, #1353, #1381 and #1383 are now fixed here; #1351, #1352, #1382 and #1384 stay filed. Review lanes 2026-08-19 — four findings implemented, one rejected with the diff ranges as evidence (#1413). Issues — none linked for this work. Commits — each says what its diff does and, where it fixes a mutation gap, which mutation turns red. Plan — product decision on displayQr is in the Change section.

[TaprootFreak]

Review — full run

Two independent review lanes (conformity + logic), every finding re-verified against the source. Measured locally on 397f9b3 with Node 20: ESLint exit 0, 74 suites / 794 tests green. Reverting payment-link.screen.tsx to the pre-PR state turns exactly the two default-config cases red (2 of 4) — the new tests really do hang on the change.

Bugs introduced by this PR

1. An explicit displayQr = false can no longer be honoured on desktop.
payRequest.displayQr || !isMobile (src/screens/payment-link.screen.tsx:612, and the same predicate for the copy at :623) evaluates to true for every desktop visitor, whatever displayQr says. The field is a non-optional boolean (src/dto/payment-link.dto.ts:57) behind a merchant-facing dropdown "Display QR code" (src/screens/payment-routes.screen.tsx:1345), so a deliberate "No" is indistinguishable from the default and is silently ignored. The setting now only has an effect on mobile. The PR describes this as "an explicit displayQr keeps precedence", but that only holds for true. This needs a product decision before merge, and whatever that decision is, the dropdown must stop promising something it no longer delivers.

2. "Request desktop site" hands the payer a QR they cannot scan.
isMobile is computed once from the user agent at module import (react-device-detect 2.2.3, isMobileAndTabletType(device) || getIPad13()). Turning on "Request Desktop Site" — a standard toggle in iOS Safari and Android Chrome — flips it to false on an actual phone. The payer then gets the large QR plus "Scan the QR-Code with a compatible app", on the very device they would have to scan it with, while the "Choose your wallet" variant that this PR added for exactly that situation is suppressed. The same screen already carries a reactive width signal from useWindowContext() and uses it for layout decisions at lines 378, 385, 406 and 438; the new check does not use it.

3. The new German string breaks the file's form of address.
src/translations/languages/de.json:913 reads "Wähle deine Wallet" in lower case, while the file capitalises the address form in 222 of 227 occurrences, including the neighbouring line ("Du hast noch keine Payment Routen"). fr.json:913 and it.json:913 leave "wallet" untranslated where the files otherwise use "portefeuille" (17 occurrences) and "portafoglio"; the only existing exception is a product name.

4. The same predicate is spelled out three times.
Lines 521, 612 and 623 each restate the device rule instead of deriving it once into a named constant. The PR's own mutation testing exists to catch the drift that this invites.

Pre-existing bugs in the files this PR touches

These predate the diff. Reporting them as bugs, not as observations — a bug does not get less severe for being older.

5. The no-quote counter branch tells the payer to scan a QR that is never rendered.
payment-link.screen.tsx:632-637 unconditionally shows "Tell the cashier that you want to pay with crypto and then scan the QR-Code with a compatible app to complete the payment." Both QrBasic sites in the screen (:527, :614) sit behind paymentHasQuote(payRequest), so in this state no QR exists anywhere on the page. That is the same defect class this PR fixes, one branch over. It can be read as a sequence ("tell the cashier, then scan"), but the payer sees an instruction pointing at nothing for as long as the counter has not activated an amount.

6. The pay-request and callback calls bypass the SDK.
src/contexts/payment-link.context.tsx:354,517 and :200-206 build API URLs by hand from Api.url and fire them with a raw fetch (fetchJson, src/util/utils.ts:291-294). Endpoint knowledge — verb, query shape, response type — lives in this repo and will silently go stale. PaymentRoutesInterface (SDK 1.7.0) covers most payment-link operations but has no method for the unauthenticated pay-request path, so this belongs in DFXswiss/packages (packages/react/src/hooks/payment-routes.hook.ts) as an additive, optional-parameter change, released, then consumed here. A useful side effect: the tests would mock the feature hook again instead of useApi.

Hard requirements for this PR

• Self-contained. Every point above is fixed in this PR. No follow-up PR unless it is explicitly granted as an exception. Finding 1 is a gate, not a deferral: the product decision happens before merge.
• Handbook coverage must be 100 %. The handbook renders committed Playwright baselines plus scripts/handbook/metadata.json. e2e/payment-and-special.spec.ts loads the payment link page but contains zero toHaveScreenshot assertions, and none of the 34 metadata entries describes the payment page — the screen this PR changes does not appear in the handbook at all. handbook-check.yaml does not even trigger, because its path filter matches none of the changed files. The device split is the whole point of this change, so both variants need a baseline and a metadata entry.
• Test coverage must be 100 % on every file this PR touches. Currently src/screens/payment-link.screen.tsx sits at 47.71 % statements, 43.24 % branches, 36.20 % functions, 48.25 % lines (--collectCoverageFrom='src/screens/payment-link.screen.tsx'); uncovered lines include 556-699, which contains the branch this PR changes, and 327-379, 398-492, 837-987. The four new cases all fix useWindowContext at width: 1024 and paymentHasQuote at true, so neither finding 2 nor finding 5 is reachable by the current suite.
• Anything that belongs in the SDK goes into the SDK. See finding 6: the fix is a PR in DFXswiss/packages first, then consumption here — not a workaround at the call site.

Checked and dismissed

The claim that react-device-detect misclassifies tablets does not hold — isMobile deliberately includes them (isMobileAndTabletType(device) || getIPad13(), verified in the installed package), so the PR's reasoning is correct. The missing Playwright baseline is not a CONTRIBUTING violation on its own either: the rule says regenerate the screenshots your change affects, and no baseline covers this screen. The gap is the handbook requirement above, which asks for baselines to be created. The package-lock.json diff was verified as minimal and drift-free: react-device-detect was already resolved at 2.2.3.

[joshuakrueger-dfx]

Review round 2 — all six findings addressed, plus one blocker they surfaced

Head a53798ad, nine commits on top of the reviewed 397f9b3. Measured on the branch with Node 20:
ESLint exit 0 with empty output, tsc -p tsconfig.build.json --noEmit exit 0, 80 suites / 910
tests green in 10 s without --forceExit. Resetting the production files to 397f9b3 and keeping
the tests turns 3 suites / 9 tests red; restoring brings the suite back green, and every file
was compared by blob hash between the machine that ran the tests and the one that made the commits.
All six checks are green on this revision.

One of your points I am answering rather than following — the workflow path filter, with the
evidence below. Everything else is done.

1 — the product decision

displayQr becomes a force switch: "Yes" shows the code on every device, "No" hands the
decision to the device. There is no longer a configuration that hides the large QR on desktop, and
merchants who deliberately set false get a changed payer view without a migration and without an
opt-out. That is a deliberate call, made by JK, not an oversight — it is stated as such in the PR
body rather than buried. The dropdown is renamed accordingly, at all four sites
(payment-routes.screen.tsx:449, 674, 1346, 1416), so it stops promising what the page no longer
delivers. The API field keeps its name.

The three-valued alternative (null = auto) was considered and dropped: it needs a DTO change, a
migration and a release in DFXswiss/api, which would make this PR depend on another repo — the
opposite of the self-contained requirement below.

2 — the device check, and why width cannot do it

Fixed via matchMedia('(pointer: coarse)') in a reactive hook (src/hooks/device.hook.ts), which
also survives a missing matchMedia and older Safari's addListener-only API.

The width signal you pointed at cannot separate the two cases. window.context.tsx:18 caps it:

const effectiveWidth = Math.min(el.offsetWidth, 768); // max-w-screen-md

A phone with "Request Desktop Site" gets a layout viewport of ~980 px and is capped to 768 — the
same value any wide desktop reports. The coarse pointer describes the input hardware instead of the
UA string, so it survives the toggle, and unlike width it does not misjudge a narrow desktop window
as a phone. Covered by two unit cases (desktop UA + coarse pointer, with and without the force
switch) and by the handheld Playwright profile, where both signals were asserted, not assumed.

3 — the translations

Deine capitalised, portefeuille and portafoglio in place of the English word. The guard is a
test over the translation files themselves (src/__tests__/translation-conventions.test.ts), with
the pre-existing exceptions listed by name so a new violation fails instead of joining them
silently. It is deliberately repo-wide: the finding was a break of the file's convention, and a
test scoped to the four new strings would not have caught it. That scope forced one unrelated
correction — it.json:1135, "wallet esterno" → "portafoglio esterno" — which is in the diff and
named in the PR body.

4 — the repeated predicate

Derived once as showLargeQr. The OpenCryptoPay guard is now derived once as showsOcpSection too,
instead of being written out a second time — see the blocker below for why that mattered.

5 — the counter branch

The sentence no longer mentions a QR that the branch never renders.

6 — the SDK

Not fixed here, because the fix is already yours:
packages#201 has been open since 2 August and
carries PaymentLinkPayRequest, PaymentLinkPayTerminal, PaymentQuote, TransferAmount and
hasPaymentQuote. What it does not carry is exactly the three calls this repo makes by hand: the
unauthenticated GET paymentLink/payment, the LNURL callback, and lnurlp/wait.

Building a second contract next to yours in @dfx.swiss/react would duplicate your types and
collide on merge, so nothing was built here. This stands as a checkbox at the top of the PR body
with the two things it needs: #201 merged and @dfx.swiss/react released, and an owner for the
three calls it does not yet carry. Neither is reachable from this branch.

One thing worth passing on, since I checked it rather than assumed it: your waitForPayment is
not the same endpoint as the payer path. paymentLink/payment/wait is authenticated and
answers with a PaymentLink; lnurlp/wait/:id is public and answers with a status only. Whoever
adds the missing calls will need both.

The blocker your findings surfaced

Chasing finding 4 turned up something none of us had: with showLargeQr true on desktop, the
collapsed "QR Code" row disappears — but the large QR sits behind a second guard
(payment-link.screen.tsx:615) that renders it only for the OpenCryptoPay standard. A payment link
whose standard is PayToAddress or LightningBolt11 therefore showed no QR at all on desktop,
from first load, on the one screen whose purpose is paying. It came in with the first commit of this
PR. The derivation now knows that guard, and three cases cover it; removing showsOcpSection &&
turns 2 of 67 payment-link tests red.

Taken along deliberately: displayQr: true on a non-OCP standard showed no QR either, and now falls
back to the collapsed row.

Handbook

Two baselines, desktop and handheld, plus a metadata.json entry. The payment APIs come from
fixtures with a fixed quote expiry and the wait poll held open, so the shot does not need a live
backend.

One correction worth naming: the baselines were not reproducible as first written. The QR
encodes Api.url, which comes from REACT_APP_API_URL — the one input the spec did not pin.
Regenerating them with the .env.sample default produced a different matrix and a red test with no
code change. playwright.config.ts now pins it to the value scripts/e2e-test.sh already uses;
after that, regenerating produced byte-identical files (54 900 and 33 342 bytes), and the spec passes
twice in a row without --update-snapshots.

The path filter in handbook-check.yaml is unchanged, and I think that is right. Your point is
met — the check triggers on this PR, because the new baselines live under e2e/screenshots/**,
which the existing filter already matches. Adding the screen and the hook was tried and reverted:
scripts/handbook/build.js reads committed PNGs only, Playwright runs in no workflow of this repo,
and the twin list in handbook-deploy.yaml would have had to move with it. A screen diff would have
started a 25-minute ARM image build that cannot tell a stale baseline from a fresh one. If you want
the filter widened anyway, say so and I will mirror both lists.

Coverage

file	statements	branches	functions	lines
src/hooks/device.hook.ts (new)	100	100	100	100
src/screens/payment-link.screen.tsx	100 (was 49.67)	90.42 (was 46.30)	100 (was 36.20)	100 (was 48.96)
src/screens/payment-routes.screen.tsx	98.88 (was 0)	90.35 (was 0)	100 (was 0)	100 (was 0)

Functions and lines are at 100 % on all three files, statements on two of three. What is left:

• Three statements in payment-routes.screen.tsx. 591 is an onClick guard sitting behind a
  .filter(item => item.text) that has already removed the falsy case. 1078 and 1082 are break
  arms for RECIPIENT/CONFIG/PAYMENT without a paymentLinkId — a state in which the Save button is
  not rendered at all.
• The branch percentages. These are ?? '', || fallback and optional chains whose alternative
  arm is unreachable behind an earlier guard. Forcing them means writing tests for states the app
  cannot enter, which pins nothing.

No istanbul ignore was added and neither screen was restructured to be cheaper to test — the
production diff is still 15 lines. If you want the last three statements covered anyway, say so and
I will do it; I would rather name them than reach 100 % by making the code lie about itself.

Worth flagging: payment-routes.screen.tsx (1526 lines) only entered the coverage scope because
you asked for the dropdown to be renamed. One string change pulled a whole screen into a 100 %
requirement — the tests are worth having, but the rule as written couples the two.

Verification

Mutations, each with the match count asserted before patching, the diff shown, and the file verified
restored by blob hash afterwards:

mutation	result
isMobile || hasCoarsePointer → &&	6 of 9 device-hook tests fail
showLargeQr || → &&	4 of 23 payment-link tests fail
showsOcpSection && removed	2 of 26 payment-link tests fail
German Deine → deine	1 of 7 translation tests fail
French portefeuille → wallet	2 of 7 translation tests fail
a French value with {{wallet}} and the common noun	1 of 7 fail — but only after dropping {{wallet}} from the exception list; with it there the guard let the string through, which is why that entry is gone

Not verified

• No physical phone, tablet or desktop browser. The device split rests on mocked unit tests and an
  emulated Playwright profile. It is a checkbox at the top of the PR body.
• The remaining Playwright specs were not run with the newly pinned REACT_APP_API_URL.
• One fixture detail worth knowing before touching these tests: PosLinkButton calls fetchPosUrl
  from onMount on every render, so with two or more links the effects thrash setIsLoadingPos
  while posUrls is still empty and React aborts with "Maximum update depth exceeded". Multi-link
  cases are therefore split into separate renders. The screen itself is untouched — but that is a
  real re-render loop in the component, not a test artefact.
• Baselines exist for chromium-darwin only, as do all 154 already in the repo, and no workflow
  compares them.
• The Docker build and container smoke test from handbook-check.yaml were not run locally, only
  build.js.

[joshuakrueger-dfx]

@marassteiner please review

This is a PR, not an issue: 20 files against develop, head a53798a, 7/7 checks green.

[marassteiner]

Sorry — I've reached my hourly job limit and can't take this on right now. I'll pick it up automatically once the limit resets.

[joshuakrueger-dfx]

@marassteiner please review

This is a PR, not an issue: 20 files against develop, head a53798ad, 7/7 checks green.

[marassteiner]

Sorry — I've reached my hourly job limit and can't take this on right now. I'll pick it up automatically once the limit resets.

[marassteiner]

EN: Working on this now — job dfxswiss__services__1275__pr-review.
DE: Ich arbeite jetzt daran — Auftrag dfxswiss__services__1275__pr-review.

[marassteiner]

Recommendation: Approve

No merge-blockers. CI is green (7/7). Local run OK (see Gate B). Follow-ups only — filed as issues and linked below.

Merge-blockers

None.

Severity-first findings (follow-up)

Major

1. src/screens/payment-link.screen.tsx:147-149 — OCP assumed while form standard is unset
   showsOcpSection treats !selectedPaymentStandard as OpenCryptoPay. The form value is set only after paymentStandards loads (:214-216); if fetchPaymentStandards fails, it stays unset. After this PR, desktop + displayQr: false yields showLargeQr = true in that window and can render the OCP large QR even when payRequest.standard is non-OCP.
   The unset→OCP section wrapper is pre-existing; this PR amplifies it for the large QR on desktop defaults. Happy path self-corrects once standards hydrate. Not treated as a merge-blocker.
   → #1350

Minor

2. src/screens/payment-link.screen.tsx:633-635 — wallet copy when lists are empty
   Handheld / !showLargeQr always shows “Choose your wallet…”, even if all wallet lists are empty after load. Collapsible QR remains available.
   → #1351

3. playwright.config.ts:39-45 — API URL pin skipped when reusing existing server
   webServer.env.REACT_APP_API_URL does not apply when reuseExistingServer takes over a locally running app; baseline QR matrix can still drift.
   → #1352

4. src/translations/languages/it.json:914 — formal Lei next to informal tu
   Cashier string uses Lei (“Dica… vuole”); neighbouring payment strings use tu (“Scansiona…”, “Scegli il tuo…”). de/fr are consistent within themselves.
   → #1353

Explicit non-findings

• Device split via matchMedia('(pointer: coarse)') + isMobile is coherent with the PR product decision (survives “Request Desktop Site”).
• Label rename “Always show QR code” matches force-switch semantics for displayQr.
• Non-OCP desktop no longer loses all QR access (showLargeQr knows the OCP section guard) — intentional fix.
• Declaring react-device-detect in dependencies matches prior imports.
• Open SDK item (packages#201) correctly deferred in the PR body.

Gate B — local run

Step	Result
npm ci	Failed on review host: npm 11 / Node 24 stricter lockfile (Missing: @esbuild/* from lock file). Environmental — CI on Node 20 is green; PR only adds one lock line for react-device-detect. Used npm install fallback.
npm run lint	pass (exit 0, max-warnings 0)
CI=true npm test -- --watchAll=false	pass — 80 suites / 910 tests
BROWSER=none PORT=3456 npm run start:dev	pass — webpack compiled, TypeScript “No issues found”; curl http://127.0.0.1:3456/ → HTTP 200; process listening on :3456

CI

All required/visible checks SUCCESS at head a53798ad.

Notes

• Real-device confirmation (iOS Safari / Android Chrome + “Request Desktop Site”) remains open as stated in the PR body — not a code merge-blocker.
• No merge from this review.

[TaprootFreak]

EN: Changes required: payment-routes has no visual/handbook baseline for the new QR label, and e2e-stack does not cover the desktop/handheld split.
DE: Änderung zwingend erforderlich: Für payment-routes fehlt die visuelle/Handbook-Baseline zum neuen QR-Label, und e2e-stack deckt den Desktop/Handheld-Split nicht ab.

Details

The /pl device split itself has a Playwright spec, two committed baselines, and a scripts/handbook/metadata.json entry (payment-qr-device). That part is in good shape.

Handbook + visual E2E — incomplete

src/screens/payment-routes.screen.tsx is also changed: the user-visible label Display QR code became Always show QR code in every occurrence. That is a screen/flow change.

CONTRIBUTING.md § Handbook: every changed screen needs a committed baseline for each visual variant and a metadata entry. There is no e2e/ spec and no handbook entry for the payment-routes screen in this PR.

Required: add a Playwright screenshot (or regenerate the existing routes baseline) for the new label, and add/update the matching scripts/handbook/metadata.json entry.

E2E (e2e-stack) — not met

/pl and /routes are claimed by e2e-stack/specs/registry/payment-links.ts. The full-stack spec was not updated to open the payment page on both sides of the device/mode split (desktop QR vs handheld wallet copy) that this PR introduces.

Required: update e2e-stack/specs/payment-links.spec.ts so the new device-aware behaviour is actually exercised.

Unit 100% on payment-link.screen.tsx, payment-routes.screen.tsx and device.hook.ts is not proven by a coverage run. The PR adds large tests including payment-link.coverage.test.tsx; state the per-file numbers. If any touched file is below 100% on all four metrics, that is also blocking.

[joshuakrueger-dfx]

@marassteiner please review

This is a PR, not an issue: 25 files against develop, head b8f959b5, 7/7 checks green on that revision (Build and test 102/1405, Full-stack E2E 231 passed, handbook, review, CodeQL, Analyze).

[marassteiner]

EN: Working on this now — job dfxswiss__services__1275__pr-review__r2.
DE: Ich arbeite jetzt daran — Auftrag dfxswiss__services__1275__pr-review__r2.

[marassteiner]

EN: Approved: the device split, handbook baselines, and e2e-stack coverage meet the last change request; remaining items are follow-ups, not merge-blockers.
DE: Genehmigt: Geräte-Split, Handbook-Baselines und e2e-stack-Abdeckung erfüllen die letzte Änderungsbitte; der Rest sind Follow-ups, keine Merge-Blocker.

Details

Verdict

Approve. No merge-blocker on head b8f959b5.

The 2026-08-13 request (handbook label, e2e-stack device split, stated coverage numbers) is met. The payment page now shows a large QR on desktop and wallet copy on handheld when displayQr is off; the routes label matches the force-switch meaning.

Merge-blockers

None.

Declared deviations — accepted

• Coverage below 100 % on the two screens. CONTRIBUTING.md requires 100 % on touched instrumented files. The PR states the numbers (device.hook.ts 100/100/100/100; payment-link.screen.tsx 100/96.7/100/100; payment-routes.screen.tsx 98.88/92.03/100/100) and lists the remaining arms as unreachable optional-chaining / nullish-coalescing. That is a declared deviation. Accepted: the remaining arms are not production paths this change introduces, and forcing them would test states the app cannot enter.
• Raw fetch on the pay-request path. Declared, blocked on packages#201. No new raw API call in this PR. Accepted.

Follow-up findings (not blocking)

Sev	Finding	Issue
Medium	Visual e2e specs mock /v1//v2 and a synthetic JWT; only the e2e-stack quote fake is declared in docs/test-architecture.md:116	#1381
Medium	Handheld e2e-stack test title claims the collapsed QR row; :513-515 never assert it	#1384
Low	Pre-existing any in the two touched screens (payment-link.screen.tsx:393,511,531; payment-routes.screen.tsx:170,464,602,689,896,993,1023)	#1382
Low	Leftover console.log in e2e/payment-qr-device.spec.ts:139-143	#1383

Still valid from the previous pass, not worsened by this head:

• #1350 — showsOcpSection treats an unset form standard as OCP (payment-link.screen.tsx:144-149)
• #1351 — wallet copy when all wallet lists are empty (:638-640)
• #1352 — reuseExistingServer skips the new REACT_APP_API_URL pin (playwright.config.ts:39-45)
• #1353 — Italian cashier string still uses formal Lei next to informal tu

Round-1 close-out

• Handbook + visual baseline for “Always show QR code”: e2e/payment-routes-qr-label.spec.ts, baseline PNG, scripts/handbook/metadata.json:162.
• e2e-stack desktop/handheld split: e2e-stack/specs/payment-links.spec.ts:425-517 (quote fulfill declared in docs/test-architecture.md:116-122).
• Per-file coverage numbers: in the PR body.

Gate B — local run

Step	Result
npm ci	Failed on this host: npm 11 / Node 24 lockfile (Missing: @esbuild/* from lock file). Environmental — CI on Node 20 is green at this head; the PR only adds one lock line for react-device-detect. Continued with npm install.
npm install	pass (exit 0); lockfile restored afterwards so the tree stayed clean
npm run lint	pass (exit 0, max-warnings 0)
CI=true npm test -- --watchAll=false	pass — 102 suites / 1405 tests
BROWSER=none PORT=3456 npm run start:dev	pass — webpack compiled, TypeScript “No issues found”; curl http://127.0.0.1:3456/ → HTTP 200; node listening on :3456

npm ci is Case 2 (outside the diff): same lockfile mismatch on Node 24 as on develop; CI “Build and test” is SUCCESS on Node 20 for b8f959b5.

CI

All 7 checks SUCCESS on b8f959b5 (Build and test, Full-stack E2E, handbook, review, CodeQL, Analyze).

Notes

• Real-device confirmation (iOS Safari / Android Chrome + “Request Desktop Site”) remains open as stated in the PR body — not a code merge-blocker.
• No merge from this review.

[joshuakrueger-dfx]

EN:
Ready after 5 review passes.
The payment page now shows the large QR on a desktop and the wallet list on a handheld, so a merchant's displayQr: false no longer leaves a payer reading "scan the QR code" with no code on screen.

DE:
Bereit nach 5 Review-Durchläufen.
Die Zahlungsseite zeigt den grossen QR jetzt auf dem Desktop und die Wallet-Liste auf dem Handgerät, damit displayQr: false keinen Zahler mehr vor "Scanne den QR-Code" ohne QR-Code stehen lässt.

Details

Rebase

Rebased onto develop 0bba3597. Three conflicts, all in documentation, resolved by keeping the newer develop table and re-adding this branch's row. git range-diff shows commits 1–11 identical, 12 and 14 differing only in that resolution, and the final table-width commit dropped because prettier reformats the merged table anyway.

What the passes found

Pass 1 — two lanes over the full diff.

• The convention guard this branch adds turned red after the rebase: develop 01a00f83 - Describe adding a wallet address on /account-merge #1404 added two Italian /account-merge strings calling the wallet "wallet", while its own fr.json says portefeuille and the other 18 Italian occurrences say portafoglio. Followed the file rather than widening the exception list.
• The permission-denied precedence in the routes error hint was untested: replacing it with apiError ?? error kept 58 of 58 tests in the three payment-routes suites green. There is a case for it now; under that mutation it is the one test that turns red.
• Debug listeners left in payment-qr-device.spec.ts (e2e/payment-qr-device: drop leftover console.log pageerror listeners #1383) removed; the Italian cashier line moved from formal Lei to tu like its neighbours (it.json: cashier payment string uses formal Lei amid informal tu payment copy #1353).

Pass 2.

• playwright.config.ts swallowed an explicit REACT_APP_API_URL, which is exactly the local call CONTRIBUTING documents (line 138). It now takes E2E_API_URL, then an explicit REACT_APP_API_URL, then the default.
• An unset form standard counted as OpenCryptoPay (payment-link: derive OCP section from payRequest.standard when form standard unset #1350). With showLargeQr being !isHandheld on a desktop, that window rendered a scannable OpenCryptoPay QR for a Pay-to-Address payment — permanently when the standard is missing from the list. The derivation falls back to payRequest.standard; restoring the old expression turns the new case red, 1 of 27 in the screen suite.

Pass 3 — findings caused by pass 2's own fix. Letting a foreign REACT_APP_API_URL through meant the routes-label spec, whose matcher was bound to dev.api.dfx.swiss, would have left its mocks and talked to a real backend; it matches by path now. The fakes in both visual specs were undeclared (#1381) — they are in the reality declaration now, with what a green run does not prove.

Pass 4. The wording of that new declaration named payloads the spec does not fake. Corrected to the endpoints it actually answers.

Pass 5. Conformity lane: 0 findings. Logic lane: the deeplink request in payment-link.screen.tsx:161 has no generation guard and no catch — real, but outside every hunk of this diff (git diff -U0 puts the changes at +142…150 and +268). Filed as #1413 rather than widened into this pull request.

Follow-ups closed here

#1350 (unset standard), #1353 (formal Lei), #1381 (undeclared visual fakes), #1383 (debug listeners). Still filed: #1351, #1352, #1382, #1384.

Evidence

• CI=true npm test, full suite: 112 suites / 1545 tests green, locally under Node 20 and on GitHub Actions.
• Eight mutations re-run on the final head cd3ed6ad, each asserting its match count before patching and each verified restored: device split ||→&& 6 of 9; displayQr || !isHandheld→&& 4 of 75; OCP guard dropped 3 of 75; standard fallback removed 1 of 75; German Deine→deine 2 of 7; French portefeuille→wallet 2 of 7; Italian portafoglio→wallet 2 of 7; error-hint precedence removed 1 of 59.
• Coverage re-measured on this head: device.hook.ts 100/100/100/100, payment-link.screen.tsx 100/97.5/100/100, payment-routes.screen.tsx 98.88/92.03/100/100.

One check is green without running

Full-stack E2E ends after 10 s with No API checkout credential — not starting the e2e stack. The job ran. Every pull-request run in this repository has done that since 2026-08-18 (checked on feat/app2-preview and fix/tx-uid-guest-actions), while the develop push run of that day still ran 20 minutes. The e2e-stack device split last really ran on head b8f959b5, 231 passed. Listed as an open item; it needs a repository secret, not a branch.

Gates

No unresolved review threads. mergeable: MERGEABLE on cd3ed6ad, all 23 commits signed and verified.

[joshuakrueger-dfx]

@marassteiner please review

This is a PR, not an issue: 25 files against develop, head cd3ed6a, 7/7 checks green.

Your last review was on b8f959b, which is no longer the head.

[marassteiner]

EN: Working on this now — job dfxswiss__services__1275__pr-review__r3.
DE: Ich arbeite jetzt daran — Auftrag dfxswiss__services__1275__pr-review__r3.

[marassteiner]

Job dfxswiss__services__1275__pr-review__r3: Der Lauf hat das Zeitlimit von 40 Minuten überschritten und wurde beendet.