[jules] enhance: Add global Toast notification system for mobile#320
[jules] enhance: Add global Toast notification system for mobile#320
Conversation
- Created ToastContext using react-native-paper Snackbar - Replaced intrusive Alert.alert calls with non-blocking showToast in Auth and Home screens - Updated .Jules tracking files Co-authored-by: Devasy23 <110348311+Devasy23@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for split-but-wiser canceled.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #320 +/- ##
=======================================
Coverage ? 63.55%
=======================================
Files ? 21
Lines ? 2456
Branches ? 254
=======================================
Hits ? 1561
Misses ? 831
Partials ? 64
🚀 New features to boost your workflow:
|
WalkthroughAdded a global toast notification system using React Context and react-native-paper's Snackbar component to replace blocking Alert.alert() calls. Created ToastContext module, wrapped the app navigator with ToastProvider, and updated authentication and home screens to use toast notifications for user feedback. Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mobile/context/ToastContext.js`:
- Line 47: The provider is recreating its value object every render which forces
all useToast() consumers to rerender; inside the component that renders
ToastContext.Provider, memoize the value by creating const value = useMemo(() =>
({ showToast }), [showToast]) and pass value into <ToastContext.Provider
value={value}>; also ensure showToast itself is stable (wrap its definition in
useCallback if it’s recreated each render) so the memo actually prevents
unnecessary rerenders.
- Line 67: The hook useToast currently returns the raw ToastContext value which
leads to obscure runtime errors when used outside a ToastProvider; update
useToast to call useContext(ToastContext), check if the returned value is
undefined/null, and if so throw a clear Error like "useToast must be used within
a ToastProvider" so consumers fail fast; reference the existing symbol useToast
and ToastContext (and the ToastProvider pattern used elsewhere) when making the
change.
In `@mobile/screens/SignupScreen.js`:
- Around line 30-31: The post-signup navigation uses
navigation.navigate('Login') which leaves the SignupScreen in the history and
can allow back navigation to a completed form; change the transition to use
navigation.replace('Login') where the call is made (replace the
navigation.navigate call in the success path after showToast in SignupScreen.js)
so the SignupScreen is removed from the stack and users cannot go back to the
completed signup screen.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 959cad7b-153d-4e50-a287-ba5ff60fa267
📒 Files selected for processing (7)
.Jules/changelog.md.Jules/todo.mdmobile/App.jsmobile/context/ToastContext.jsmobile/screens/HomeScreen.jsmobile/screens/LoginScreen.jsmobile/screens/SignupScreen.js
| }; | ||
|
|
||
| return ( | ||
| <ToastContext.Provider value={{ showToast }}> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Memoize provider value to avoid app-wide rerenders on each toast state change.
At Line 47, value={{ showToast }} creates a new object every render, forcing all useToast() consumers to rerender whenever toast UI state changes.
♻️ Proposed fix
import React, { createContext, useState, useContext, useCallback } from 'react';
+import { useMemo } from 'react';
@@
- return (
- <ToastContext.Provider value={{ showToast }}>
+ const contextValue = useMemo(() => ({ showToast }), [showToast]);
+
+ return (
+ <ToastContext.Provider value={contextValue}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/context/ToastContext.js` at line 47, The provider is recreating its
value object every render which forces all useToast() consumers to rerender;
inside the component that renders ToastContext.Provider, memoize the value by
creating const value = useMemo(() => ({ showToast }), [showToast]) and pass
value into <ToastContext.Provider value={value}>; also ensure showToast itself
is stable (wrap its definition in useCallback if it’s recreated each render) so
the memo actually prevents unnecessary rerenders.
| ); | ||
| }; | ||
|
|
||
| export const useToast = () => useContext(ToastContext); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the mobile/context directory structure
find mobile/context -type f -name "*.js" -o -name "*.ts" -o -name "*.tsx" 2>/dev/null | head -20Repository: Devasy/splitwiser
Length of output: 121
🏁 Script executed:
# Check the exact content of ToastContext.js
cat -n mobile/context/ToastContext.jsRepository: Devasy/splitwiser
Length of output: 2427
🏁 Script executed:
# Search for ToastProvider to understand the context setup
rg "ToastProvider" -A 5 -B 2Repository: Devasy/splitwiser
Length of output: 3168
🏁 Script executed:
# Check if other context hooks in the codebase follow the guard pattern
rg "useContext\(" mobile/context web/contexts 2>/dev/null | head -20Repository: Devasy/splitwiser
Length of output: 445
🏁 Script executed:
# Look for other custom context hooks to see the pattern
ast-grep --pattern 'export const use$_$_ = () => useContext($_);'Repository: Devasy/splitwiser
Length of output: 43
Guard useToast to fail fast when provider is missing.
At Line 67, returning raw context can cause runtime crashes (Cannot destructure property 'showToast') if used outside ToastProvider. Throw a clear error from the hook instead. This pattern is already implemented in the web version (web/contexts/ToastContext.tsx) and should be applied consistently here.
🛡️ Proposed fix
-export const useToast = () => useContext(ToastContext);
+export const useToast = () => {
+ const context = useContext(ToastContext);
+ if (!context) {
+ throw new Error('useToast must be used within a ToastProvider');
+ }
+ return context;
+};📝 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.
| export const useToast = () => useContext(ToastContext); | |
| export const useToast = () => { | |
| const context = useContext(ToastContext); | |
| if (!context) { | |
| throw new Error('useToast must be used within a ToastProvider'); | |
| } | |
| return context; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/context/ToastContext.js` at line 67, The hook useToast currently
returns the raw ToastContext value which leads to obscure runtime errors when
used outside a ToastProvider; update useToast to call useContext(ToastContext),
check if the returned value is undefined/null, and if so throw a clear Error
like "useToast must be used within a ToastProvider" so consumers fail fast;
reference the existing symbol useToast and ToastContext (and the ToastProvider
pattern used elsewhere) when making the change.
| showToast('Account created successfully! Please login.', 'success'); | ||
| navigation.navigate('Login'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer replace('Login') after successful signup to avoid stale back navigation.
At Line 31, navigate('Login') can keep Signup in history; users may return to a completed form via back gesture. replace is usually a better post-signup transition.
♻️ Proposed fix
if (success) {
showToast('Account created successfully! Please login.', 'success');
- navigation.navigate('Login');
+ navigation.replace('Login');
} else {📝 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.
| showToast('Account created successfully! Please login.', 'success'); | |
| navigation.navigate('Login'); | |
| showToast('Account created successfully! Please login.', 'success'); | |
| navigation.replace('Login'); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/screens/SignupScreen.js` around lines 30 - 31, The post-signup
navigation uses navigation.navigate('Login') which leaves the SignupScreen in
the history and can allow back navigation to a completed form; change the
transition to use navigation.replace('Login') where the call is made (replace
the navigation.navigate call in the success path after showToast in
SignupScreen.js) so the SignupScreen is removed from the stack and users cannot
go back to the completed signup screen.
This PR implements a comprehensive global Toast notification system for the Mobile application using React Native Paper's
Snackbar. It replaces intrusiveAlert.alertcalls during form validation and network feedback, significantly improving the UX.The
ToastProvideris seamlessly integrated at the root level withinApp.jsinside thePaperProvider, allowing all screens (e.g.LoginScreen,SignupScreen, andHomeScreen) to present themed, non-blocking feedback messages to users.This task aligns with the project's 'Error Handling & UX Polish' initiatives.
PR created automatically by Jules for task 9659098714573135941 started by @Devasy23
Summary by CodeRabbit