Skip to content

Feat/qrcode variant 2#1793

Open
remicolin wants to merge 90 commits intodevfrom
feat/qrcode-dsltop-variant-2
Open

Feat/qrcode variant 2#1793
remicolin wants to merge 90 commits intodevfrom
feat/qrcode-dsltop-variant-2

Conversation

@remicolin
Copy link
Copy Markdown
Collaborator

@remicolin remicolin commented Feb 26, 2026

Description

A brief description of the changes, what and how is being changed.

Tested

Explain how the change has been tested (for example by manual testing, unit tests etc) or why it's not necessary (for example version bump).

How to QA

How can the change be tested in a repeatable manner?

Summary by CodeRabbit

  • New Features

    • Added desktop and mobile QR code display variants with responsive layouts
    • Introduced new visual components for improved authentication flows
    • Added comprehensive icon library for UI elements
    • Exported SelfDeepLinkButton component as public API entry point
  • Chores

    • Updated build configuration to support PNG asset handling
    • Enhanced CI/CD workflows for improved change detection and targeted testing

transphorm and others added 30 commits October 10, 2025 15:16
Release to Production - 2025-10-12
Release to Production - 2025-10-26
Release to Production - 2025-11-04
…allowing dynamic switching between iOS and Android during tests. This change improves test isolation and avoids hoisting issues with jest.mock.
Bugfix: Workaround Mobile CI tests flakiness
…nges. Added checks for 'circuits' in circuits.yml and 'contracts' or 'common' in contracts.yml to determine if tests should execute on dev branch. This avoids too wide changelist in trigger filter that is problematic
…in circuits.yml and contracts.yml. This change ensures that the full history is available for subsequent steps in the workflows.
… checks. Added error handling for git diff command in circuits.yml and contracts.yml to ensure robust execution and prevent workflow failures due to diff errors.
…tibility and performance across all CI configurations. This change replaces the previous version v4 in circuits, contracts, and other workflow files.
SELF-1684: Ensure checks are run with pull requests to staging/main
Release to Production - 2025-12-07
transphorm and others added 16 commits March 10, 2026 08:56
Release to Staging v2.9.16 - 2026-03-10
Release to Production v2.9.16 - 2026-03-10
Release to Staging v2.9.16 - 2026-03-13
Release to Production v2.9.16 - 2026-03-16
Release to Staging v2.9.16 - 2026-03-27
Release to Staging v2.9.16 - 2026-03-30
Release to Production v2.9.16 - 2026-03-29
Release to Staging v2.9.16 - 2026-04-03
Release to Production v2.9.16 - 2026-04-06
Release to Staging v2.9.16 - 2026-04-08
Release: staging v2.9.16 — recovery, navigation, security, cleanup
Release to Staging v2.9.17 - 2026-04-09
Release to Staging v2.9.17 - 2026-04-10
Release to Staging v2.9.17 - 2026-04-10
Release to Production v2.9.16 - 2026-04-12
@remicolin remicolin marked this pull request as ready for review April 16, 2026 23:57
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 16, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
self-webview-app Ignored Ignored Preview Apr 30, 2026 10:00am

Request Review

@gitguardian
Copy link
Copy Markdown

gitguardian Bot commented Apr 16, 2026

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
19414827 Triggered Generic Password a9d87bf app/ios/PassportReaderCore.swift View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@remicolin remicolin changed the base branch from main to dev April 17, 2026 00:00
@remicolin remicolin changed the title Feat/qrcode dsltop variant 2 Feat/qrcode variant 2 Apr 17, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 17, 2026

Greptile Summary

This PR introduces two new display variants for the @selfxyz/qrcode component — desktop and mobile — alongside the existing hybrid default. The desktop variant wraps the QR code in a branded card with a header (app logo + Self logo), a QR section, and a reactive footer that shows instruction steps or a live proof-status card. The mobile variant replaces the QR code entirely with a phone mockup image and a deep-link CTA button, targeting users who are already on a mobile device. A new SelfDeepLinkButton standalone component is also exported. The changes are additive and backward-compatible (variant defaults to 'hybrid').

Key findings:

  • proofStep is accepted but never used in MobileQRcode — the component renders a fully static UI regardless of proof state (connecting, verified, failed). Users on mobile get no visual feedback after returning to the page, and the CTA button remains visible even after a successful or failed verification. The analogous desktop component (DesktopFooter) correctly handles state transitions.
  • The "Open Self app" button in MobileQRcode duplicates the markup of the newly exported SelfDeepLinkButton — the component should reuse SelfDeepLinkButton internally.
  • getDesktopDescription is imported but unused in MobileQRcode.tsx.
  • darkMode is forwarded only to the inner QRCode in the desktop variant; the outer card, header, and footer always render with light-mode colours.

Confidence Score: 3/5

Safe to merge only after the proofStep handling in MobileQRcode is addressed — it currently provides no state feedback on the primary mobile user path.

The desktop variant and overall architecture are well-structured and additive. However, MobileQRcode accepts a proofStep prop but ignores it entirely, meaning the mobile UI never reflects verification progress or outcome. This breaks the primary UX of the mobile variant. The fix is isolated to MobileQRcode and should be straightforward, but it needs to happen before ship.

sdk/qrcode/components/MobileQRcode.tsx requires the most attention — the unused proofStep prop is the single blocking issue.

Important Files Changed

Filename Overview
sdk/qrcode/components/SelfQRcode.tsx Adds variant prop ('hybrid'
sdk/qrcode/components/MobileQRcode.tsx New mobile variant component; proofStep prop is accepted but never consumed — the UI is fully static regardless of proof state, and getDesktopDescription is an unused import.
sdk/qrcode/components/DesktopQRcode.tsx New desktop variant wrapper; darkMode is forwarded only to the inner QRCode, leaving the card/header/footer always light-themed.
sdk/qrcode/components/DesktopFooter.tsx Renders instruction steps on initial state and a status card (connecting/verified/failed) on non-initial states — logic mirrors QRcodeSteps correctly.
sdk/qrcode/components/DesktopHeader.tsx New presentational header component showing app logo, dots separator, and Self logo — looks correct.
sdk/qrcode/components/SelfDeepLinkButton.tsx New standalone deep-link button component reusing mobile CTA styles; duplicates button markup already present in MobileQRcode.
sdk/qrcode/components/icons.tsx Adds DownloadIcon, VerifyIcon, ArrowReturnIcon as inline SVGs alongside existing PNG-based icons; consistent interface.
sdk/qrcode/utils/styles.ts Adds desktop and mobile variant style helpers; fixed-width (373px) consistent with existing design intent.
sdk/qrcode/utils/utils.ts Adds getDesktopDescription, getDesktopStatusTitle, and getDesktopStatusSubtitle helpers; subtitle is a static string regardless of step.
sdk/qrcode/index.ts Adds SelfDeepLinkButton export; no other changes.
sdk/qrcode/package.json Version bump to 1.0.21; no dependency changes.
sdk/qrcode/tsup.config.ts Adds PNG loader alongside existing SVG loader so new PNG assets are inlined as data URLs in both ESM and CJS builds.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    SelfQRcode["SelfQRcode\n(variant prop)"]
    SelfQRcode -->|variant = 'hybrid'\ndefault| Hybrid["Hybrid layout\nQRCode + StatusBanner"]
    SelfQRcode -->|variant = 'desktop'| DesktopQRcode["DesktopQRcode"]
    SelfQRcode -->|variant = 'mobile'| MobileQRcode["MobileQRcode"]

    DesktopQRcode --> DesktopHeader["DesktopHeader\n(app logo + Self logo)"]
    DesktopQRcode --> QRCode["QRCode\n(qrcode.react)"]
    DesktopQRcode --> DesktopFooter["DesktopFooter"]

    DesktopFooter -->|initialState| Steps["Instruction steps\n(Scan / Verify / Rescan)"]
    DesktopFooter -->|non-initial| StatusCard["Status card\n(Connecting / Verified / Failed)"]

    MobileQRcode --> DesktopHeader
    MobileQRcode --> PhoneMockup["Phone mockup image"]
    MobileQRcode --> InstructionSteps["Mobile instruction steps\n(Download / Verify / Return)"]
    MobileQRcode --> CTAButton["Deep-link CTA button\n(Open Self app)"]

    style MobileQRcode fill:#FEF3C7,stroke:#F59E0B
    style CTAButton fill:#FEF3C7,stroke:#F59E0B
Loading

Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/dev..." | Re-trigger Greptile

<div style={mobileCardStyle()} role="region" aria-label="Self authentication">
<DesktopHeader appName={selfApp.appName} appLogo={selfApp.logoBase64} />
<div style={mobilePhoneSectionWrapperStyle()}>
<div style={mobilePhoneSectionStyle()}>
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.

P1 proofStep accepted but never used

proofStep is declared in MobileQRcodeProps and destructured, but it is never referenced in the render. The component always shows the same phone mockup + "Open Self app" CTA regardless of whether proof generation is in progress, has succeeded, or has failed. This means mobile users get no visual feedback after they return to the page — the button remains visible even after a successful or failed verification.

The analogous DesktopFooter component shows a status card (connecting, verified, failed) when proofStep is not an initial state. The mobile variant should do the same — either hide the CTA and show a status message, or at minimum disable/hide the button after verification is complete.

} from '../utils/styles.js';
import { getDesktopDescription } from '../utils/utils.js';
import DesktopHeader from './DesktopHeader.js';
import { ArrowReturnIcon, DownloadIcon, VerifyIcon } from './icons.js';
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.

P2 Unused import

getDesktopDescription is imported but never used directly in MobileQRcode. It is used internally by the DesktopHeader component; importing it here is redundant and will trigger lint warnings.

Suggested change
import { ArrowReturnIcon, DownloadIcon, VerifyIcon } from './icons.js';
import DesktopHeader from './DesktopHeader.js';

Comment on lines +57 to +62
</div>
</div>
);
})}
</div>
<div style={mobileCtaSectionStyle()}>
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.

P2 Duplicated deep-link button

The CTA button rendered here (<a href={qrValue} style={mobileCtaButtonStyle()}>…) is nearly identical to the SelfDeepLinkButton component that is already exported from index.ts. Consider reusing SelfDeepLinkButton to avoid divergence between the two implementations — replace the inline <a> block with <SelfDeepLinkButton href={qrValue} />.

Comment on lines +18 to +29
const DesktopQRcode = memo(
({ proofStep, qrValue, size, darkMode, selfApp }: DesktopQRcodeProps) => (
<div style={desktopCardStyle()} role="img" aria-label="Self authentication QR code">
<DesktopHeader appName={selfApp.appName} appLogo={selfApp.logoBase64} />
<div style={desktopQrSectionStyle()}>
<div style={desktopQrWrapperStyle(proofStep)}>
<QRCode value={qrValue} size={size} darkMode={darkMode} proofStep={proofStep} />
</div>
</div>
<DesktopFooter proofStep={proofStep} />
</div>
)
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.

P2 darkMode prop not applied to card/header/footer styles

darkMode is forwarded to the inner QRCode component (which changes the QR dot colour) but none of the wrapping elements — desktopCardStyle(), desktopHeaderStyle(), desktopFooterStyle(), etc. — are aware of it. As a result the card always renders with a white background and black text even when darkMode={true} is passed by the consumer.

If dark mode is intentionally out-of-scope for the desktop variant in this PR, consider removing the darkMode prop from DesktopQRcodeProps to signal that clearly, or document the limitation.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 002eec19-e158-40aa-a486-599303076082

📥 Commits

Reviewing files that changed from the base of the PR and between a825d8f and bcb9331.

⛔ Files ignored due to path filters (9)
  • sdk/qrcode/assets/3dots.png is excluded by !**/*.png
  • sdk/qrcode/assets/error.png is excluded by !**/*.png
  • sdk/qrcode/assets/phone-mockup.png is excluded by !**/*.png
  • sdk/qrcode/assets/qrcode.png is excluded by !**/*.png
  • sdk/qrcode/assets/shieldcheck.png is excluded by !**/*.png
  • sdk/qrcode/assets/shieldcheck2.png is excluded by !**/*.png
  • sdk/qrcode/assets/shieldlightning.png is excluded by !**/*.png
  • sdk/qrcode/assets/squarecheck.png is excluded by !**/*.png
  • sdk/qrcode/assets/warning.png is excluded by !**/*.png
📒 Files selected for processing (16)
  • .github/workflows/circuits.yml
  • .github/workflows/contracts.yml
  • .github/workflows/core-sdk-ci.yml
  • .github/workflows/qrcode-sdk-ci.yml
  • sdk/qrcode/components/DesktopFooter.tsx
  • sdk/qrcode/components/DesktopHeader.tsx
  • sdk/qrcode/components/DesktopQRcode.tsx
  • sdk/qrcode/components/MobileQRcode.tsx
  • sdk/qrcode/components/SelfDeepLinkButton.tsx
  • sdk/qrcode/components/SelfQRcode.tsx
  • sdk/qrcode/components/icons.tsx
  • sdk/qrcode/index.ts
  • sdk/qrcode/tsup.config.ts
  • sdk/qrcode/types/svg.d.ts
  • sdk/qrcode/utils/styles.ts
  • sdk/qrcode/utils/utils.ts

Comment on lines +28 to +60
proofStep: number;
qrValue: string;
selfApp: SelfApp;
}

const MOBILE_STEPS = [
{ icon: DownloadIcon, text: 'Download the Self Mobile app' },
{ icon: VerifyIcon, text: 'Verify your identity' },
{ icon: ArrowReturnIcon, text: 'Return to this application and click on the button below' },
];

const MobileQRcode = memo(({ proofStep, qrValue, selfApp }: MobileQRcodeProps) => (
<div style={mobileCardStyle()} role="region" aria-label="Self authentication">
<DesktopHeader appName={selfApp.appName} appLogo={selfApp.logoBase64} />
<div style={mobilePhoneSectionWrapperStyle()}>
<div style={mobilePhoneSectionStyle()}>
<img src={phoneMockup} alt="Self app preview" style={mobilePhoneImgStyle()} />
</div>
</div>
<div style={mobileFooterStyle()}>
{MOBILE_STEPS.map((step, index) => {
const Icon = step.icon;
return (
<div key={index} style={desktopStepStyle()}>
<div style={desktopStepInnerStyle()}>
<div style={desktopStepIconStyle()}>
<Icon size={18} />
</div>
<span style={desktopStepTextStyle()}>{step.text}</span>
</div>
</div>
);
})}
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.

⚠️ Potential issue | 🟠 Major

proofStep is accepted but never rendered — status feedback missing on mobile.

The component signature takes proofStep, but it's never read anywhere in the JSX (linter confirms: 'proofStep' is defined but never used). The desktop variant uses proofStep to drive border color and a status card; on mobile, users will get no visual feedback as the proof progresses through MOBILE_CONNECTED → PROOF_GENERATION_STARTED → PROOF_VERIFIED / FAILED. Either wire it up (e.g. swap the footer/CTA for a status card like desktop once proofStep > WAITING_FOR_MOBILE) or drop it from the props so callers don't think it has an effect.

🧰 Tools
🪛 GitHub Check: quality-checks

[failure] 39-39:
'proofStep' is defined but never used

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.

5 participants