-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update header titles for DemoFeedScreen and DemoCaptureScreen Add demo mode UI changes with SkipDemoButton and demoMode prop #408
Update header titles for DemoFeedScreen and DemoCaptureScreen Add demo mode UI changes with SkipDemoButton and demoMode prop #408
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
WalkthroughThis pull request introduces several updates focused on a new demo mode. A new text prompt is added to the demo feed screen, and a new Changes
Sequence Diagram(s)sequenceDiagram
participant DemoIntroScreen
participant SkipDemoButton
participant AppStore
participant RevenueCat
participant Router
participant Toast
DemoIntroScreen->>SkipDemoButton: Render SkipDemoButton
SkipDemoButton->>SkipDemoButton: onPress triggered
SkipDemoButton->>AppStore: setHasCompletedOnboarding()
AppStore-->>SkipDemoButton: Confirmation received
SkipDemoButton->>RevenueCat: showProPaywallIfNeeded()
RevenueCat-->>SkipDemoButton: (Response from RevenueCat)
SkipDemoButton->>Router: navigate("/feed")
alt Error Occurred
SkipDemoButton->>Toast: Display error message
end
sequenceDiagram
participant User
participant EventMenu
participant Toast
User->>EventMenu: Click menu option
alt demoMode is active
EventMenu->>Toast: Show "action disabled" notification
Note over EventMenu: Exit without further processing
else
EventMenu->>EventMenu: Process menu selection normally
end
Possibly Related PRs
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
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: 1
🧹 Nitpick comments (5)
apps/expo/src/components/SkipDemoButton.tsx (3)
14-24
: Consider wrapping with error boundary for better error handling.While the hooks usage is appropriate, consider wrapping the component with an error boundary to gracefully handle potential hook failures, especially for
useRevenueCat
which might involve external API calls.+import { ErrorBoundary } from "~/components/ErrorBoundary"; export function SkipDemoButton({ className = "", textClassName = "text-sm text-neutral-2", }: SkipDemoButtonProps) { + return ( + <ErrorBoundary fallback={<Text>Unable to load skip button</Text>}> + <SkipDemoButtonContent className={className} textClassName={textClassName} /> + </ErrorBoundary> + ); +} + +function SkipDemoButtonContent({ + className = "", + textClassName = "text-sm text-neutral-2", +}: SkipDemoButtonProps) {
25-40
: Enhance error handling with specific error types.The error handling could be more specific to provide better user feedback based on the type of error.
try { setHasCompletedOnboarding(true); await showProPaywallIfNeeded(); router.push("/feed"); } catch (error) { + const errorMessage = error instanceof Error ? error.message : "Something went wrong"; + const description = error instanceof Error && error.cause + ? String(error.cause) + : "Please try again"; toast.error("Something went wrong", { - description: "Please try again", + description, }); + // Log error for monitoring + console.error("Skip demo failed:", errorMessage); } finally {
42-53
: Enhance accessibility for better user experience.Add accessibility props to improve the experience for screen reader users.
<Pressable onPress={handlePress} disabled={isLoading} className={`py-2 ${className}`} + accessibilityRole="button" + accessibilityState={{ disabled: isLoading }} + accessibilityLabel="Skip demo" + accessibilityHint="Skips the demo and proceeds to the main feed" >apps/expo/src/app/(onboarding)/onboarding/demo-intro.tsx (2)
148-154
: Consider RTL layout support.The layout uses left positioning and transform which might need adjustment for RTL languages.
-<View className="absolute bottom-48 left-1/2 flex -translate-x-1/2 flex-col items-center space-y-2"> +<View className="absolute bottom-48 start-1/2 flex -translate-x-1/2 flex-col items-center space-y-2">
116-167
: Consider performance optimization for animations.The bottom section uses multiple layers (BlurView, LinearGradient) and animations which could impact performance on lower-end devices.
Consider:
- Using
useCallback
for animation setup- Memoizing gradient styles
- Using lighter blur intensity
+const gradientStyle = { + position: "absolute" as const, + height: "100%", + width: "100%", +}; +const purpleGradient = { + ...gradientStyle, + opacity: 0.3, +}; +const lightGradient = { + ...gradientStyle, + opacity: 0.1, +}; <BlurView - intensity={10} + intensity={5} className="h-full w-full opacity-20" tint="light" /> // ... <LinearGradient colors={["transparent", "#5A32FB"]} locations={[0, 1]} - style={{ - position: "absolute", - height: "100%", - width: "100%", - opacity: 0.3, - }} + style={purpleGradient} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/expo/src/app/(onboarding)/onboarding/demo-feed.tsx
(3 hunks)apps/expo/src/app/(onboarding)/onboarding/demo-intro.tsx
(4 hunks)apps/expo/src/components/EventMenu.tsx
(3 hunks)apps/expo/src/components/SkipDemoButton.tsx
(1 hunks)apps/expo/src/components/UserEventsList.tsx
(6 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
apps/expo/src/components/UserEventsList.tsx
[failure] 346-346:
'demoMode' is assigned a value but never used. Allowed unused vars must match /^_/u
🔇 Additional comments (8)
apps/expo/src/app/(onboarding)/onboarding/demo-feed.tsx (2)
163-167
: LGTM! Well-positioned tooltip for user guidance.The tooltip is appropriately positioned and styled, providing clear instructions for the long-press interaction.
169-178
: LGTM! Proper integration of demo mode.The
demoMode
prop is correctly passed to theUserEventsList
component, enabling demo mode functionality.apps/expo/src/components/EventMenu.tsx (2)
58-58
: LGTM! Clean prop interface update.The
demoMode
prop is properly added with a sensible default value.Also applies to: 85-85
282-285
: LGTM! Proper demo mode handling.The implementation correctly prevents any actions from being executed in demo mode and provides appropriate user feedback.
apps/expo/src/components/UserEventsList.tsx (2)
63-63
: LGTM! Clean prop interface update.The
demoMode
prop is properly added to theUserEventListItem
component.Also applies to: 72-72
130-136
: LGTM! Proper prop passing.The
demoMode
prop is correctly passed down to theEventMenu
component.apps/expo/src/components/SkipDemoButton.tsx (1)
1-12
: LGTM! Clean imports and well-defined interface.The imports are appropriate and the interface is well-defined with optional styling props.
apps/expo/src/app/(onboarding)/onboarding/demo-intro.tsx (1)
18-18
: Verify the removal of async from handlePress.The
handlePress
function is no longer async but still handles async operations (router.push). While this works, verify that this change doesn't affect error handling or navigation behavior.Also applies to: 45-58
✅ Verification successful
Let's run an updated shell script to inspect the usage of
router.push
and the definition of thehandlePress
function in the demo-intro file. We'll use explicit glob patterns to handle.tsx
files correctly.
Verified removal of async from handlePress does not affect navigation behavior.
- The
handlePress
function is declared synchronously (line 45) and callsrouter.push("/onboarding/demo-capture")
.- No evidence of additional error handling was required or expected, and similar navigation methods in the codebase follow this pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other router.push usages to verify consistency rg "router\.push" --type ts --type tsxLength of output: 68
Script:
#!/bin/bash echo "Searching for router.push usage in demo-intro.tsx:" rg "router\.push" --glob="*.tsx" apps/expo/src/app/\(onboarding\)/onboarding/demo-intro.tsx echo "" echo "Searching for handlePress definition in demo-intro.tsx:" rg -n "handlePress" --glob="*.tsx" apps/expo/src/app/\(onboarding\)/onboarding/demo-intro.tsxLength of output: 546
Script:
#!/bin/bash echo "Displaying context around handlePress definition in demo-intro.tsx:" rg -C 10 "const handlePress = \(\) =>" apps/expo/src/app/(onboarding)/onboarding/demo-intro.tsxLength of output: 423
Summary by CodeRabbit