fix: adjust Onboarding screens ui feedback - part 1#1250
Open
lealobanov wants to merge 9 commits into
Open
Conversation
PR SummaryAdjusted UI feedback for multiple onboarding screens to improve layout, spacing, and accessibility support. Key changes include moving buttons inside scroll views, making animations responsive to screen height, fixing icon colors, updating Changes
autogenerated by presubmit.ai |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
![github-actions[bot]](https://avatars.github.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Files Processed (1)
- packages/ui/src/components/onboarding/OnboardingHeader.tsx (1 hunk)
Actionable Comments (0)
Skipped Comments (2)
-
packages/ui/src/components/onboarding/OnboardingHeader.tsx [31-36]
maintainability: "Hardcoded line height values instead of design tokens."
-
packages/ui/src/components/onboarding/OnboardingHeader.tsx [31-31]
possible issue: "Horizontal padding removed from title text."
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Files Processed (2)
- packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx (3 hunks)
Actionable Comments (0)
Skipped Comments (3)
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [56-56]
maintainability: "Inconsistent styling approach mixing inline styles with Tamagui props."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [182-182]
maintainability: "Inconsistent styling approach mixing inline styles with Tamagui props."
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [27-27]
maintainability: "Magic number used for max animation height."
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (2)
- 5bf8b91: fix: adjust spacing
Closes #111
- 2d15012: fix: adjust spacing
Closes #111
Files Processed (5)
- packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/RecoveryPhraseScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx (3 hunks)
- packages/ui/src/components/onboarding/AccountCreationLoadingState.tsx (1 hunk)
- packages/ui/src/components/onboarding/RecoveryPhraseQuestion.tsx (3 hunks)
Actionable Comments (0)
Skipped Comments (3)
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [56-56]
maintainability: "Inconsistent styling approach mixing inline styles with Tamagui props."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [182-182]
maintainability: "Inconsistent styling approach mixing inline styles with Tamagui props."
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [27-27]
maintainability: "Magic number used for maximum animation height."
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Files Processed (3)
- apps/react-native/src/navigation/AppNavigator.tsx (1 hunk)
- packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportProfileScreen.query.tsx (4 hunks)
Actionable Comments (0)
Skipped Comments (1)
-
packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx [198-198]
possible issue: "Line height may be too small relative to font size."
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Files Processed (3)
- packages/screens/src/onboarding/ConfirmRecoveryPhraseScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx (4 hunks)
Actionable Comments (1)
-
packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx [238-239]
possible bug: "Verify JSX closing tags are properly balanced."
Skipped Comments (1)
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [170-231]
possible issue: "Removing ScrollView may cause content overflow on smaller screens."
Comment on lines
238
to
+239
| </YStack> | ||
|
|
||
| {/* Maybe later button - Ghost style, centered like secure enclave profile */} | ||
| <XStack justify="center" pb="$8"> | ||
| <Button variant="ghost" onPress={handleMaybeLater}> | ||
| <Text fontSize="$4" fontWeight="600" color="$textSecondary"> | ||
| {t('onboarding.notificationPreferences.maybeLater')} | ||
| </Text> | ||
| </Button> | ||
| </XStack> | ||
| </YStack> | ||
| </ScrollView> |
There was a problem hiding this comment.
There appears to be a missing closing tag for </ScrollView>. The ScrollView is opened at line 186 but the closing tag structure seems incomplete based on the diff. Verify that the JSX structure is properly balanced.
Contributor
Author
There was a problem hiding this comment.
I have manually checked this comment, it is not valid
lealobanov
force-pushed
the
111-onboarding-ui-feedback
branch
from
ec89927 to
c441d59
Compare
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
changed the title
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (7)
- c441d59: fix: ui feedback
Closes #111
- 203f6c2: fix: adjust spacing
Closes #111
- 2c41308: fix: adjust spacing
Closes #111
- 1d7f9be: fix: adjust spacing
Closes #111
- e4467d6: fix: adjust spacing
Closes #111
- f200ffe: fix: adjust spacing
Closes #111
- 467e49a: fix: adjust spacing
Closes #111
Files Processed (11)
- apps/react-native/src/navigation/AppNavigator.tsx (1 hunk)
- packages/screens/src/onboarding/ConfirmRecoveryPhraseScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/GetStartedScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/RecoveryPhraseScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportProfileScreen.query.tsx (4 hunks)
- packages/ui/src/components/onboarding/AccountCreationLoadingState.tsx (1 hunk)
- packages/ui/src/components/onboarding/OnboardingHeader.tsx (1 hunk)
- packages/ui/src/components/onboarding/RecoveryPhraseQuestion.tsx (3 hunks)
Actionable Comments (0)
Skipped Comments (4)
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [56-57]
maintainability: "Inconsistent use of magic number for max height constraint."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [182-183]
maintainability: "Inconsistent use of magic number for max height constraint."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [50-51]
possible bug: "Navigation options are not restored when loading state ends."
-
packages/screens/src/onboarding/ConfirmRecoveryPhraseScreen.query.tsx [107-108]
possible bug: "Navigation options are not restored when account creation ends."
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
force-pushed
the
111-onboarding-ui-feedback
branch
from
bf9cd42 to
6c6173b
Compare
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (9)
- 6c6173b: fix: recovery options ui
Closes #111
- 3e94c37: fix: icon colors
Closes #111
- 463c138: fix: ui feedback
Closes #111
- b874125: fix: adjust spacing
Closes #111
- e2083e7: fix: adjust spacing
Closes #111
- 9bfacdd: fix: adjust spacing
Closes #111
- 611cea1: fix: adjust spacing
Closes #111
- 0a2a16e: fix: adjust spacing
Closes #111
- c924968: fix: adjust spacing
Closes #111
Files Processed (14)
- apps/react-native/src/navigation/AppNavigator.tsx (1 hunk)
- packages/icons/src/react-native/UploadCloud.generated.tsx (1 hunk)
- packages/screens/src/onboarding/ConfirmRecoveryPhraseScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/GetStartedScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/RecoveryPhraseScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportOtherMethodsScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportProfileScreen.query.tsx (4 hunks)
- packages/ui/src/components/onboarding/AccountCreationLoadingState.tsx (1 hunk)
- packages/ui/src/components/onboarding/ImportOptionCard.tsx (4 hunks)
- packages/ui/src/components/onboarding/OnboardingHeader.tsx (1 hunk)
- packages/ui/src/components/onboarding/RecoveryPhraseQuestion.tsx (3 hunks)
Actionable Comments (0)
Skipped Comments (3)
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [56-57]
maintainability: "Inconsistent use of magic number for max height constraint."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [182-183]
maintainability: "Duplicated magic number for animation container max height."
-
packages/ui/src/components/onboarding/ImportOptionCard.tsx [90-98]
possible issue: "Missing gap between icon and text in vertical layout."
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
force-pushed
the
111-onboarding-ui-feedback
branch
from
6c6173b to
8fbfd62
Compare
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (9)
- 8fbfd62: fix: recovery options ui
Closes #111
- 108a0c5: fix: icon colors
Closes #111
- 473efd0: fix: ui feedback
Closes #111
- 102af57: fix: adjust spacing
Closes #111
- cf4be39: fix: adjust spacing
Closes #111
- a9e0e28: fix: adjust spacing
Closes #111
- 44b3e3f: fix: adjust spacing
Closes #111
- 6642713: fix: adjust spacing
Closes #111
- 4848842: fix: adjust spacing
Closes #111
Files Processed (14)
- apps/react-native/src/navigation/AppNavigator.tsx (1 hunk)
- packages/icons/src/react-native/UploadCloud.generated.tsx (1 hunk)
- packages/screens/src/onboarding/ConfirmRecoveryPhraseScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/GetStartedScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/RecoveryPhraseScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportOtherMethodsScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportProfileScreen.query.tsx (4 hunks)
- packages/ui/src/components/onboarding/AccountCreationLoadingState.tsx (1 hunk)
- packages/ui/src/components/onboarding/ImportOptionCard.tsx (4 hunks)
- packages/ui/src/components/onboarding/OnboardingHeader.tsx (1 hunk)
- packages/ui/src/components/onboarding/RecoveryPhraseQuestion.tsx (3 hunks)
Actionable Comments (0)
Skipped Comments (4)
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [56-57]
possible issue: "Inconsistent height calculation between container and animation."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [182-183]
possible issue: "Inconsistent height calculation between container and animation."
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [26-27]
maintainability: "Magic numbers could be extracted to named constants."
-
packages/ui/src/components/onboarding/ImportOptionCard.tsx [90-98]
maintainability: "Inconsistent lineHeight values between vertical and horizontal layouts."
lealobanov
temporarily deployed
to
development
GitHub Actions
Inactive
lealobanov
force-pushed
the
111-onboarding-ui-feedback
branch
from
8fbfd62 to
55effb4
Compare
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (9)
- 55effb4: fix: recovery options ui
Closes #111
- 8129f3f: fix: icon colors
Closes #111
- a8c5291: fix: ui feedback
Closes #111
- 1e130b0: fix: adjust spacing
Closes #111
- 95d7c54: fix: adjust spacing
Closes #111
- 8a25a6e: fix: adjust spacing
Closes #111
- ee38655: fix: adjust spacing
Closes #111
- ec0fe57: fix: adjust spacing
Closes #111
- afa7d82: fix: adjust spacing
Closes #111
Files Processed (0)
Actionable Comments (0)
Skipped Comments (0)
lealobanov
force-pushed
the
111-onboarding-ui-feedback
branch
from
55effb4 to
fbaad14
Compare
There was a problem hiding this comment.
✅ LGTM!
Review Summary
Commits Considered (9)
- fbaad14: fix: recovery options ui
Closes #111
- 8eaa30d: fix: icon colors
Closes #111
- c3bc21f: fix: ui feedback
Closes #111
- 5b8f664: fix: adjust spacing
Closes #111
- e558f8a: fix: adjust spacing
Closes #111
- 70caf1e: fix: adjust spacing
Closes #111
- 3fbc15e: fix: adjust spacing
Closes #111
- 5d13a94: fix: adjust spacing
Closes #111
- f601613: fix: adjust spacing
Closes #111
Files Processed (14)
- apps/react-native/src/navigation/AppNavigator.tsx (1 hunk)
- packages/icons/src/react-native/UploadCloud.generated.tsx (1 hunk)
- packages/screens/src/onboarding/ConfirmRecoveryPhraseScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/GetStartedScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/RecoveryPhraseScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportOtherMethodsScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportProfileScreen.query.tsx (4 hunks)
- packages/ui/src/components/onboarding/AccountCreationLoadingState.tsx (1 hunk)
- packages/ui/src/components/onboarding/ImportOptionCard.tsx (4 hunks)
- packages/ui/src/components/onboarding/OnboardingHeader.tsx (1 hunk)
- packages/ui/src/components/onboarding/RecoveryPhraseQuestion.tsx (3 hunks)
Actionable Comments (0)
Skipped Comments (4)
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [38-88]
possible issue: "Removing ScrollView may cause content overflow on small screens."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [170-231]
possible issue: "Removing ScrollView may cause content overflow on small screens."
-
packages/ui/src/components/onboarding/ImportOptionCard.tsx [32-32]
possible bug: "Default layout change may break existing component usages."
-
packages/screens/src/recovery/ImportProfileScreen.query.tsx [134-138]
possible issue: "Subtitle removed from import option card."
lealobanov
force-pushed
the
111-onboarding-ui-feedback
branch
from
fbaad14 to
16b591a
Compare
There was a problem hiding this comment.
🚨 Pull request needs attention.
Review Summary
Commits Considered (9)
- 16b591a: fix: recovery options ui
Closes #111
- 32dd674: fix: icon colors
Closes #111
- 88dc6d2: fix: ui feedback
Closes #111
- ad47b10: fix: adjust spacing
Closes #111
- 4fe8019: fix: adjust spacing
Closes #111
- 4bc8154: fix: adjust spacing
Closes #111
- d5762e0: fix: adjust spacing
Closes #111
- 0cdb52b: fix: adjust spacing
Closes #111
- 26163ed: fix: adjust spacing
Closes #111
Files Processed (14)
- apps/react-native/src/navigation/AppNavigator.tsx (1 hunk)
- packages/icons/src/react-native/UploadCloud.generated.tsx (1 hunk)
- packages/screens/src/onboarding/ConfirmRecoveryPhraseScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/GetStartedScreen.query.tsx (1 hunk)
- packages/screens/src/onboarding/NotificationPreferencesScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/RecoveryPhraseScreen.query.tsx (3 hunks)
- packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportOtherMethodsScreen.query.tsx (4 hunks)
- packages/screens/src/recovery/ImportProfileScreen.query.tsx (1 hunk)
- packages/ui/src/components/onboarding/AccountCreationLoadingState.tsx (1 hunk)
- packages/ui/src/components/onboarding/ImportOptionCard.tsx (4 hunks)
- packages/ui/src/components/onboarding/OnboardingHeader.tsx (1 hunk)
- packages/ui/src/components/onboarding/RecoveryPhraseQuestion.tsx (3 hunks)
Actionable Comments (1)
-
packages/ui/src/components/onboarding/ImportOptionCard.tsx [32-32]
possible bug: "Default layout change may break existing component usages."
Skipped Comments (4)
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [26-27]
performance: "Consider memoizing the calculated animation height."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [37-38]
performance: "Consider memoizing the calculated animation height."
-
packages/screens/src/onboarding/SecureEnclaveScreen.query.tsx [170-231]
possible issue: "Removed ScrollView may cause content overflow on smaller screens."
-
packages/screens/src/onboarding/ProfileTypeSelectionScreen.query.tsx [38-88]
possible issue: "Removed ScrollView may cause content overflow on smaller screens."
| subtitle, | ||
| badge, | ||
| layout = 'horizontal', | ||
| layout = 'vertical', |
There was a problem hiding this comment.
Changing the default layout from 'horizontal' to 'vertical' is a breaking change that could affect existing usages of ImportOptionCard that rely on the previous default. Ensure all existing usages have been updated or explicitly pass the layout prop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #111
With accessibility text scaling on:
🔗 Related Issues
Linked automatically from the branch name. If incorrect, edit:
Self-Checklist
📝 Description
📸 Screenshots/Videos