feat: New onboarding #1510
feat: New onboarding #1510
Conversation
WalkthroughThis pull request introduces several changes across the design system and onboarding features. The modifications include updating text size styles in the design system, adding new onboarding-related components like Changes
Possibly related PRs
Suggested Reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| import { AnimatedText, IAnimatedTextProps } from "@/design-system/Text"; | ||
| import { memo } from "react"; | ||
|
|
||
| export const OnboardingSubtitle = memo(function OnboardingSubtitle( | ||
| props: IAnimatedTextProps | ||
| ) { | ||
| const { style, ...rest } = props; | ||
| return ( | ||
| <AnimatedText | ||
| size="sm" | ||
| style={[{ textAlign: "center" }, style]} | ||
| {...rest} | ||
| /> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The filename onboarding-subtitlte.tsx contains a typo. Please rename it to onboarding-subtitle.tsx and update all corresponding imports. This will maintain consistency and prevent potential issues with file references.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
|
|
||
| return ( | ||
| <Screen | ||
| key={getRandomId()} |
There was a problem hiding this comment.
Using getRandomId() as a key prop will force a re-render on every render cycle since it generates a new value each time. This can hurt performance, especially for complex component trees. Either remove the key prop if it's not needed for list reconciliation, or use a stable identifier that persists across renders.
Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
design-system/Text/Text.styles.ts (1)
7-7: Use consistent object property syntax for size keys.The array syntax
["3xl"]differs from the object property syntax used for other sizes. Consider using consistent object property notation:- ["3xl"]: { fontSize: 48, lineHeight: 60 } satisfies TextStyle, + "3xl": { fontSize: 48, lineHeight: 60 } satisfies TextStyle,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
design-system/Text/Text.styles.ts(1 hunks)features/conversation/conversation-message/conversation-message-content-types/conversation-message-simple-text.tsx(1 hunks)features/onboarding/components/onboarding-subtitlte.tsx(1 hunks)features/onboarding/components/onboarding-title.tsx(1 hunks)features/onboarding/onboarding-get-started.screen.tsx(1 hunks)screens/Navigation/Navigation.tsx(2 hunks)
🧰 Additional context used
🪛 GitHub Check: tsc
features/onboarding/onboarding-get-started.screen.tsx
[failure] 120-120:
Object literal may only specify known properties, and 'color' does not exist in type 'ViewStyle | RecursiveArray<Falsy | ViewStyle | RegisteredStyle>'.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: performance-test
- GitHub Check: test
🔇 Additional comments (6)
features/onboarding/components/onboarding-subtitlte.tsx (1)
4-15: LGTM! Well-structured component with performance optimization.The component is well-implemented with proper memoization and props handling. The centered text alignment is appropriate for onboarding UI.
features/onboarding/components/onboarding-title.tsx (1)
4-16: LGTM! Consistent implementation with OnboardingSubtitle.The component follows the same pattern as OnboardingSubtitle with appropriate styling for a title component.
screens/Navigation/Navigation.tsx (1)
198-203: LGTM! Proper navigation setup for the onboarding screen.The OnboardingGetStartedScreen is correctly integrated into the SignedOutNavigation stack with appropriate header configuration.
design-system/Text/Text.styles.ts (2)
10-11: Verify design specifications with Andrew.The comments indicate that some text sizes need confirmation. Since this PR introduces new onboarding UI, it's important to ensure all text sizes are finalized.
7-8: Review line height ratios for visual consistency.The line height to font size ratios are inconsistent between the new sizes:
- "3xl": 60/48 = 1.25
- "xxl": 44/40 = 1.1
Consider maintaining a consistent ratio for better visual rhythm across text sizes.
Run this script to analyze line height ratios across all text sizes:
features/conversation/conversation-message/conversation-message-content-types/conversation-message-simple-text.tsx (1)
36-36: Verify the intentional size increase for emoji rendering.The change from
xxl(40px) to3xl(48px) represents a 20% size increase for emoji rendering. While this might improve visibility, it could affect layout and spacing.Please confirm if this size increase was intentional for the onboarding feature or if it's an unrelated enhancement.
Run this script to find other potential emoji size references:
| import { AnimatedText, IAnimatedTextProps } from "@/design-system/Text"; | ||
| import { memo } from "react"; | ||
|
|
||
| export const OnboardingSubtitle = memo(function OnboardingSubtitle( | ||
| props: IAnimatedTextProps | ||
| ) { | ||
| const { style, ...rest } = props; | ||
| return ( | ||
| <AnimatedText | ||
| size="sm" | ||
| style={[{ textAlign: "center" }, style]} | ||
| {...rest} | ||
| /> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Fix typo in filename: "subtitlte" → "subtitle"
The filename contains a typo which should be corrected to maintain consistency and prevent confusion. This will require updating import statements in other files as well.
Run this script to verify all imports that need to be updated:
#!/bin/bash
# Find all files importing the misspelled component
rg -l "onboarding-subtitlte"| <Icon | ||
| style={{ | ||
| color: "#A1F293", | ||
| }} | ||
| icon="checkmark" | ||
| size={40} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use theme color instead of hard-coded value
Replace the hard-coded color "#A1F293" with a theme color to maintain consistency and support different themes.
<Icon
style={{
- color: "#A1F293",
+ tintColor: theme.colors.success.primary, // or appropriate theme color
}}
icon="checkmark"
size={40}
/>📝 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.
| <Icon | |
| style={{ | |
| color: "#A1F293", | |
| }} | |
| icon="checkmark" | |
| size={40} | |
| /> | |
| <Icon | |
| style={{ | |
| tintColor: theme.colors.success.primary, | |
| }} | |
| icon="checkmark" | |
| size={40} | |
| /> |
🧰 Tools
🪛 GitHub Check: tsc
[failure] 120-120:
Object literal may only specify known properties, and 'color' does not exist in type 'ViewStyle | RecursiveArray<Falsy | ViewStyle | RegisteredStyle>'.
|
|
||
| return ( | ||
| <Screen | ||
| key={getRandomId()} |
There was a problem hiding this comment.
Remove getRandomId() from Screen key prop
Using a random key can cause unnecessary remounts of the entire screen component and its children. Remove this key prop as it's not needed here.
- <Screen
- key={getRandomId()}
+ <ScreenCommittable suggestion skipped: line range outside the PR's diff.
| <Pressable> | ||
| <AnimatedVStack | ||
| entering={FadeIn.springify().delay(600).duration(3000)} | ||
| style={{ | ||
| paddingBottom: insets.bottom + theme.spacing.sm, | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| rowGap: theme.spacing.xxs, | ||
| }} | ||
| > | ||
| <PressableScale> | ||
| <AnimatedCenter | ||
| style={{ | ||
| borderRadius: 999, | ||
| width: theme.avatarSize.lg, | ||
| height: theme.avatarSize.lg, | ||
| backgroundColor: theme.colors.fill.primary, | ||
| justifyContent: "center", | ||
| alignItems: "center", | ||
| }} | ||
| > | ||
| <Icon | ||
| style={{ | ||
| color: "#A1F293", | ||
| }} | ||
| icon="checkmark" | ||
| size={40} | ||
| /> | ||
| </AnimatedCenter> | ||
| </PressableScale> | ||
| <AnimatedText size="xs">Create a Contact Card</AnimatedText> | ||
| </AnimatedVStack> | ||
| </Pressable> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify nested Pressable components
The component has nested Pressable and PressableScale which might lead to conflicting touch handling. Consider combining them into a single PressableScale.
- <Pressable>
- <AnimatedVStack
+ <PressableScale>
+ <AnimatedVStack
entering={FadeIn.springify().delay(600).duration(3000)}
style={{
paddingBottom: insets.bottom + theme.spacing.sm,
justifyContent: "center",
alignItems: "center",
rowGap: theme.spacing.xxs,
}}
>
- <PressableScale>
<AnimatedCenter
style={{
borderRadius: 999,
width: theme.avatarSize.lg,
height: theme.avatarSize.lg,
backgroundColor: theme.colors.fill.primary,
justifyContent: "center",
alignItems: "center",
}}
>
<Icon
style={{
color: "#A1F293",
}}
icon="checkmark"
size={40}
/>
</AnimatedCenter>
- </PressableScale>
<AnimatedText size="xs">Create a Contact Card</AnimatedText>
</AnimatedVStack>
- </Pressable>
+ </PressableScale>📝 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.
| <Pressable> | |
| <AnimatedVStack | |
| entering={FadeIn.springify().delay(600).duration(3000)} | |
| style={{ | |
| paddingBottom: insets.bottom + theme.spacing.sm, | |
| justifyContent: "center", | |
| alignItems: "center", | |
| rowGap: theme.spacing.xxs, | |
| }} | |
| > | |
| <PressableScale> | |
| <AnimatedCenter | |
| style={{ | |
| borderRadius: 999, | |
| width: theme.avatarSize.lg, | |
| height: theme.avatarSize.lg, | |
| backgroundColor: theme.colors.fill.primary, | |
| justifyContent: "center", | |
| alignItems: "center", | |
| }} | |
| > | |
| <Icon | |
| style={{ | |
| color: "#A1F293", | |
| }} | |
| icon="checkmark" | |
| size={40} | |
| /> | |
| </AnimatedCenter> | |
| </PressableScale> | |
| <AnimatedText size="xs">Create a Contact Card</AnimatedText> | |
| </AnimatedVStack> | |
| </Pressable> | |
| <PressableScale> | |
| <AnimatedVStack | |
| entering={FadeIn.springify().delay(600).duration(3000)} | |
| style={{ | |
| paddingBottom: insets.bottom + theme.spacing.sm, | |
| justifyContent: "center", | |
| alignItems: "center", | |
| rowGap: theme.spacing.xxs, | |
| }} | |
| > | |
| <AnimatedCenter | |
| style={{ | |
| borderRadius: 999, | |
| width: theme.avatarSize.lg, | |
| height: theme.avatarSize.lg, | |
| backgroundColor: theme.colors.fill.primary, | |
| justifyContent: "center", | |
| alignItems: "center", | |
| }} | |
| > | |
| <Icon | |
| style={{ | |
| color: "#A1F293", | |
| }} | |
| icon="checkmark" | |
| size={40} | |
| /> | |
| </AnimatedCenter> | |
| <AnimatedText size="xs">Create a Contact Card</AnimatedText> | |
| </AnimatedVStack> | |
| </PressableScale> |
🧰 Tools
🪛 GitHub Check: tsc
[failure] 120-120:
Object literal may only specify known properties, and 'color' does not exist in type 'ViewStyle | RecursiveArray<Falsy | ViewStyle | RegisteredStyle>'.
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
Summary by CodeRabbit
Release Notes
New Features
OnboardingGetStartedScreenwith engaging user experienceDesign System Updates
3xltext size for improved typographyNavigation