-
Notifications
You must be signed in to change notification settings - Fork 211
Feat/add logo confirmation screen #1666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughIntroduces a new LogoConfirmation screen component in the document onboarding flow. Adds corresponding navigation routes, TypeScript type definitions, SDK-level logo confirmation screen, and new SDK events (LOGO_CONFIRMED, LOGO_NOT_FOUND). Updates navigation logic to route through logo confirmation before document onboarding. Changes
Sequence DiagramsequenceDiagram
actor User
participant AppNav as App Navigation
participant LogoConfirm as LogoConfirmationScreen<br/>(App)
participant SDKComponent as LogoConfirmationScreen<br/>(SDK)
participant EventSystem as SDK Event<br/>System
participant DocOnboard as Document<br/>Onboarding
User->>AppNav: Selects document (type 'i')
AppNav->>LogoConfirm: Navigate with documentType,<br/>countryCode
LogoConfirm->>SDKComponent: Render with logo & callbacks
SDKComponent->>User: Display logo & Yes/No prompt
User->>SDKComponent: Click Yes or No
SDKComponent->>EventSystem: Emit LOGO_CONFIRMED<br/>or LOGO_NOT_FOUND event
alt Confirmed
EventSystem->>LogoConfirm: onConfirm triggered
LogoConfirm->>DocOnboard: Navigate to onboarding
else Not Found
EventSystem->>LogoConfirm: onNotFound triggered
LogoConfirm->>User: Show "Document not supported"<br/>modal
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/src/screens/documents/selection/LogoConfirmationScreen.tsx`:
- Around line 5-13: Reorder or autofix the imports in LogoConfirmationScreen.tsx
to satisfy simple-import-sort/imports: group and sort external library imports
first (react, react-native-safe-area-context, `@react-navigation/native`), then
third-party package imports from `@selfxyz/`* (components, constants, and
SDKLogoConfirmationScreen), and finally any local imports; you can run the
project's import-sort autofix or manually reorder the import statements
referencing symbols useCallback, useSafeAreaInsets, RouteProp, useRoute, YStack,
slate100, and LogoConfirmationScreen (SDKLogoConfirmationScreen) to resolve the
CI failure.
In `@packages/mobile-sdk-alpha/src/flows/onboarding/logo-confirmation-screen.tsx`:
- Around line 55-61: Prettier is complaining about the multiline JSX for the
title and logo block in the LogoConfirmationScreen component: collapse the
multiline BodyText and the View that renders the logo into single-line JSX to
satisfy formatting rules — specifically update the BodyText element that
currently contains "Does your document have this symbol?" to a single-line JSX
element and similarly render the logo container (the View using
styles.logoContainer that contains the logo variable) on one line; keep the same
props, children (logo), and styles (styles.titleText, styles.logoContainer) but
reformat them to single-line form.
| import React, { useCallback } from 'react'; | ||
| import { useSafeAreaInsets } from 'react-native-safe-area-context'; | ||
| import type { RouteProp } from '@react-navigation/native'; | ||
| import { useRoute } from '@react-navigation/native'; | ||
|
|
||
| import { YStack } from '@selfxyz/mobile-sdk-alpha/components'; | ||
| import { slate100 } from '@selfxyz/mobile-sdk-alpha/constants/colors'; | ||
| import { LogoConfirmationScreen as SDKLogoConfirmationScreen } from '@selfxyz/mobile-sdk-alpha'; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is failing due to import order—please run autofix.
simple-import-sort/imports is failing in CI, which blocks merge. Please run the project’s import sort autofix or reorder these imports to satisfy the rule.
🧰 Tools
🪛 GitHub Actions: Mobile CI
[error] 5-5: Run autofix to sort these imports! simple-import-sort/imports
🪛 GitHub Actions: Workspace CI
[error] 5-5: Run autofix to sort these imports! simple-import-sort/imports
🪛 GitHub Check: build-deps
[failure] 5-5:
Run autofix to sort these imports!
🪛 GitHub Check: workspace-lint
[failure] 5-5:
Run autofix to sort these imports!
🤖 Prompt for AI Agents
In `@app/src/screens/documents/selection/LogoConfirmationScreen.tsx` around lines
5 - 13, Reorder or autofix the imports in LogoConfirmationScreen.tsx to satisfy
simple-import-sort/imports: group and sort external library imports first
(react, react-native-safe-area-context, `@react-navigation/native`), then
third-party package imports from `@selfxyz/`* (components, constants, and
SDKLogoConfirmationScreen), and finally any local imports; you can run the
project's import-sort autofix or manually reorder the import statements
referencing symbols useCallback, useSafeAreaInsets, RouteProp, useRoute, YStack,
slate100, and LogoConfirmationScreen (SDKLogoConfirmationScreen) to resolve the
CI failure.
| <BodyText style={styles.titleText}> | ||
| Does your document have this symbol? | ||
| </BodyText> | ||
|
|
||
| <View style={styles.logoContainer}> | ||
| {logo} | ||
| </View> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix Prettier warnings to unblock CI.
Workspace CI reports Prettier violations for the multiline text node and logo block; please reformat to the single-line form.
✅ Suggested formatting fix
- <BodyText style={styles.titleText}>
- Does your document have this symbol?
- </BodyText>
+ <BodyText style={styles.titleText}>Does your document have this symbol?</BodyText>
- <View style={styles.logoContainer}>
- {logo}
- </View>
+ <View style={styles.logoContainer}>{logo}</View>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <BodyText style={styles.titleText}> | |
| Does your document have this symbol? | |
| </BodyText> | |
| <View style={styles.logoContainer}> | |
| {logo} | |
| </View> | |
| <BodyText style={styles.titleText}>Does your document have this symbol?</BodyText> | |
| <View style={styles.logoContainer}>{logo}</View> |
🧰 Tools
🪛 GitHub Actions: Workspace CI
[warning] 55-55: prettier/prettier: Replace ⏎··········Does·your·document·have·this·symbol?⏎········ with Does·your·document·have·this·symbol?
[warning] 59-59: prettier/prettier: Replace ⏎··········{logo}⏎········ with {logo}
🪛 GitHub Check: lint
[warning] 59-59:
Replace ⏎··········{logo}⏎········ with {logo}
[warning] 55-55:
Replace ⏎··········Does·your·document·have·this·symbol?⏎········ with Does·your·document·have·this·symbol?
🪛 GitHub Check: workspace-lint
[warning] 59-59:
Replace ⏎··········{logo}⏎········ with {logo}
[warning] 55-55:
Replace ⏎··········Does·your·document·have·this·symbol?⏎········ with Does·your·document·have·this·symbol?
🤖 Prompt for AI Agents
In `@packages/mobile-sdk-alpha/src/flows/onboarding/logo-confirmation-screen.tsx`
around lines 55 - 61, Prettier is complaining about the multiline JSX for the
title and logo block in the LogoConfirmationScreen component: collapse the
multiline BodyText and the View that renders the logo into single-line JSX to
satisfy formatting rules — specifically update the BodyText element that
currently contains "Does your document have this symbol?" to a single-line JSX
element and similarly render the logo container (the View using
styles.logoContainer that contains the logo variable) on one line; keep the same
props, children (logo), and styles (styles.titleText, styles.logoContainer) but
reformat them to single-line form.
Description
For the passport and ID cards, prompt the user to check if their document has a biometric logo. On yes, continue flow. On no, display error popup.
Tested
Built local mobile app with this, worked as intended.
How to QA
Build the mobile up and try to add a new passport or ID card.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.