fix: userflow #161
fix: userflow #161
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces state management enhancements in the Ideology Test component to manage answer selection and processing delays. It also removes dependencies on notification permissions in the registration component. The changes include new state variables and modifications to the answer handling logic in the Ideology Test, as well as commenting out unused code and simplifying permission requests in the registration page. Additionally, it enhances user data fetching in the Welcome component and modifies modal management across various components. Changes
Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 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 (
|
✅ Deploy Preview for lucent-florentine-971919 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/src/app/register/page.tsx (2)
55-79: Streamlined permission handling improves user flow.The notification permission request has been simplified to return a resolved promise rather than requesting actual permissions from the user. This change:
- Removes a potential friction point in the registration process
- Simplifies the user flow by eliminating permission requests
- Maintains the function interface while making it a no-op
Consider fully removing the commented code for better maintainability rather than keeping it commented out. If needed in the future, the code can be retrieved from version control.
- // Function to request notification permission - commented out const requestPermission = async () => { - // Notification permission request is now commented out - // This allows registration to proceed without requiring notification permissions - /* - const requestPermissionPayload: RequestPermissionPayload = { - permission: Permission.Notifications, - }; - try { - const payload = await MiniKit.commandsAsync.requestPermission( - requestPermissionPayload, - ); - return payload; - } catch (error) { - console.error("Permission request failed:", error); - if (error instanceof Error) { - setErrorCode(error.message as NotificationErrorCode); - } - return null; - } - */ - - // Return a resolved promise to maintain function signature return Promise.resolve(null); };
172-173: Consider updating the comment to reflect the changed behavior.Since the
requestPermissionfunction no longer actually requests notification permissions, the comment should be updated to reflect the current behavior.- // Request notification permission after successful registration + // Previously requested notification permission, now simplified to improve user flow await requestPermission();apps/web/src/app/ideology-test/page.tsx (1)
279-287: Good implementation of delayed navigation with visual feedback.Adding a delay before moving to the next question provides valuable visual feedback to users, showing them which answer they've selected before proceeding.
Consider extracting the delay duration to a named constant at the top of the file for better maintainability:
// Add near the top of the file with other constants +const ANSWER_FEEDBACK_DELAY_MS = 800; // Then in the setTimeout - }, 800); // 800ms delay to show the selected answer + }, ANSWER_FEEDBACK_DELAY_MS);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/app/ideology-test/page.tsx(8 hunks)apps/web/src/app/register/page.tsx(3 hunks)
🔇 Additional comments (9)
apps/web/src/app/register/page.tsx (1)
8-9: Simplified import declarations improve code clarity.Commenting out these imports is appropriate since the associated functionality is no longer being used.
apps/web/src/app/ideology-test/page.tsx (8)
32-33: Good addition of state variables for improved UX.Adding these state variables improves the user experience by:
- Tracking the currently selected answer for visual feedback
- Preventing multiple submissions while an answer is being processed
240-246: Enhanced answer handling with visual feedback.The improved
handleAnswerfunction now provides better user feedback by:
- Preventing multiple clicks/answers while processing
- Visually indicating the selected answer before proceeding
288-291: Good error handling with state reset.The error handling logic now properly resets both the processing state and selected answer state, ensuring the UI remains responsive even if an error occurs.
409-412: Improved button selection logic.The enhanced selection logic now considers both current and saved selections, providing appropriate visual feedback to users.
423-425: Appropriate button disabling during processing.Disabling the answer buttons while processing prevents users from submitting multiple answers quickly, which improves both UX and data integrity.
438-442: Consistent UI state management for navigation buttons.The Previous button is now correctly disabled while an answer is being processed, maintaining UI consistency.
454-458: Proper End Test button state management.The End Test button is appropriately disabled during answer processing, preventing accidental submissions.
467-471: Consistent Next button state handling.The Next button is properly disabled during answer processing, ensuring a smooth user experience.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/web/src/middleware.ts (1)
17-17: Note potential redundancy in welcome page handling.The welcome page is now both in the
publicPathsarray and has a special handling case (lines 73-80). While this works, it creates some redundancy in the code. Consider removing it frompublicPathsif the special case is meant to be the only way to access this page.apps/web/src/app/insights/page.tsx (1)
175-178: Consider refactoring for better modal transition management.Using setTimeout for UI transitions can lead to inconsistent behavior across different devices with varying performance. Consider using a state machine or a dedicated library for managing modal transitions.
- setTimeout(() => { - setIsModalOpen(false); - }, 50); + // Using a more declarative approach + const transitionToShareModal = () => { + setIsShareModalOpen(true); + // Use requestAnimationFrame for smoother transitions + requestAnimationFrame(() => { + requestAnimationFrame(() => { + setIsModalOpen(false); + }); + }); + }; + + transitionToShareModal();apps/web/src/app/api/user/by-wallet/route.ts (1)
54-58: Provide more context in error responses.Returning a generic "Failed to fetch user data" message is fine for production, but consider logging or including additional troubleshooting details when in development mode, to expedite debugging.
apps/web/src/app/welcome/page.tsx (3)
21-24: Limit excessive console logging.Repeated logs can clutter the console, especially in production. If you need detailed logs, consider a debug mode or a logging library that can be filtered by severity level.
50-74: Consolidate repeated fetch logic.You perform a second fetch from
/api/auth/sessionif the first call fails to retrieve a name. Consider extracting both fetch attempts into a small helper function or a single consolidated endpoint to reduce repetition and improve maintainability.
84-86: Refine fallback logic.Your multi-stage fallback to
sessionStorageand finally "User" makes sense, but you might want to provide a user notice or an error state if no name is found. Else, they may be labeled "User" unexpectedly.Also applies to: 88-98
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/src/app/(home)/page.tsx(2 hunks)apps/web/src/app/api/user/by-wallet/route.ts(1 hunks)apps/web/src/app/insights/page.tsx(2 hunks)apps/web/src/app/register/page.tsx(4 hunks)apps/web/src/app/settings/page.tsx(3 hunks)apps/web/src/app/welcome/page.tsx(2 hunks)apps/web/src/components/layout/LayoutContent.tsx(1 hunks)apps/web/src/components/ui/navigation/BannerTop.tsx(1 hunks)apps/web/src/components/ui/overlays/VerifyModal.tsx(1 hunks)apps/web/src/hooks/useAuth.ts(1 hunks)apps/web/src/middleware.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/register/page.tsx
🔇 Additional comments (13)
apps/web/src/components/ui/navigation/BannerTop.tsx (1)
7-7:❓ Verification inconclusive
Error handling has been removed from the verification process.
You've removed the
errorvariable from the destructured return value ofuseVerification(). This means the component no longer has a way to inform users if verification fails. Consider whether error handling should be implemented elsewhere or if users should still be notified of verification failures.
🏁 Script executed:
#!/bin/bash # Check if error handling is implemented elsewhere in the codebase # Look for references to verification error handling in other components echo "Searching for verification error handling in other components..." rg -l "error.*useVerification|useVerification.*error" --type=tsx # Check the implementation of useVerification hook to see if errors are handled elsewhere echo "Checking the implementation of useVerification hook..." rg -l "function useVerification" --type=tsLength of output: 435
Attention: Missing User-Facing Error Handling in Verification
It appears that the
errorfield has been removed from the destructured return ofuseVerification()in theBannerTop.tsxcomponent, which means that any failures during the verification process won’t be communicated to the user. The search for error-specific handling in the codebase (and the hook’s implementation) hasn’t revealed any alternative error notification mechanism.
- Location to review:
apps/web/src/components/ui/navigation/BannerTop.tsx– currently only destructuresisVerifying,isVerified,isLoading, andhandleVerify.apps/web/src/hooks/useVerification.ts– verify if it still handles error logic internally that might be propagated elsewhere.Recommendation:
Please confirm whether error handling is addressed at a higher level (for example, by a global error boundary or separate notification system). If not, consider reintroducing error handling in this component (or via a dedicated mechanism) to ensure users are informed of verification failures as needed.apps/web/src/components/layout/LayoutContent.tsx (1)
73-82: Good UX improvement to prevent navigation flickering during modal transitions.Adding the delay before hiding the navigation when closing the share modal is a useful improvement that prevents flickering during transitions. The code is well-commented and clearly explains the purpose of the timeout.
apps/web/src/middleware.ts (2)
11-12: LGTM: New public API endpoint added.Adding the
/api/user/by-walletpath to the public paths is appropriate if you need to retrieve user information based on wallet address without authentication.
73-80: Good implementation of registration flow.The special case for the welcome page correctly handles users who have just completed registration by checking for the
registration_statuscookie. This implementation works well with the changes in theuseAuthhook to create a smooth registration flow.apps/web/src/app/settings/page.tsx (3)
12-12: Clean approach to commented-out imports.The comment clearly explains why the import is commented out rather than removed completely, which is good practice for temporary changes.
30-54: Good implementation of placeholder toggle component.The DummyToggleSwitch component correctly implements the visual aspects of a toggle with proper accessibility attributes while making it clear it's a placeholder without actual theme-changing functionality.
Some positive aspects:
- Includes proper ARIA attributes (role="switch", aria-checked)
- Maintains internal state for UI interactions
- Uses conditional classes for visual feedback
- Clear naming that indicates its temporary nature
251-252: Clear usage with explanatory comment.The comment clearly explains this is a placeholder for the dark theme toggle functionality, which is helpful for other developers.
apps/web/src/app/insights/page.tsx (1)
59-66: Effective consolidation of modal state management.The combined effect is a good approach to ensure the bottom navigation stays hidden when either modal is open, reducing redundancy and potential inconsistencies.
apps/web/src/components/ui/overlays/VerifyModal.tsx (1)
42-48: Improved user guidance for verification process.The updated text provides clearer instructions that focus on using the World ID app rather than finding a physical Orb, making the verification process more accessible to users. The step-by-step guidance is also more direct and actionable.
apps/web/src/app/(home)/page.tsx (2)
28-29: Good code maintenance with explanatory comments.Keeping the function with a clear explanation of why it's retained despite not being needed for the current implementation shows good code maintenance practices.
72-76: Simplified verification modal display logic.The removal of the sessionStorage check means the modal will appear consistently for unverified users. This is a more assertive approach to user verification.
The comment correctly explains the change, which helps other developers understand the intent behind the modification.
apps/web/src/app/api/user/by-wallet/route.ts (2)
5-17: Well-defined response interface – minimal improvements needed.The
UserResponseinterface is consistent with the returned data structure. If you're certain the database fields are always strings, you can streamline by removing unnecessary.toString()calls. Otherwise, this looks good.
21-29: Consider validating or sanitizing wallet addresses.Currently, there's no validation for
walletAddress. Depending on your application's needs, you might want to ensure it's properly formatted or sanitized to avoid potential edge cases or database attacks.
| // Check if registration was just completed | ||
| const registrationComplete = sessionStorage.getItem("registration_complete"); | ||
| if (registrationComplete && pathname === "/welcome") { | ||
| // If we're on the welcome page and registration was just completed, | ||
| // skip auth check to prevent redirection | ||
| setAuthState((prev) => ({ ...prev, loading: false })); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure session storage key is cleared after use.
This new code properly handles users who have just completed registration by allowing them to access the welcome page. However, there's no code to clear the registration_complete item from sessionStorage after it's been used, which might lead to unwanted behavior if the user navigates back to the welcome page later.
// Check if registration was just completed
const registrationComplete = sessionStorage.getItem("registration_complete");
if (registrationComplete && pathname === "/welcome") {
// If we're on the welcome page and registration was just completed,
// skip auth check to prevent redirection
setAuthState((prev) => ({ ...prev, loading: false }));
+ // Clear the flag to prevent future bypassing of auth checks
+ sessionStorage.removeItem("registration_complete");
return;
}📝 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.
| // Check if registration was just completed | |
| const registrationComplete = sessionStorage.getItem("registration_complete"); | |
| if (registrationComplete && pathname === "/welcome") { | |
| // If we're on the welcome page and registration was just completed, | |
| // skip auth check to prevent redirection | |
| setAuthState((prev) => ({ ...prev, loading: false })); | |
| return; | |
| } | |
| // Check if registration was just completed | |
| const registrationComplete = sessionStorage.getItem("registration_complete"); | |
| if (registrationComplete && pathname === "/welcome") { | |
| // If we're on the welcome page and registration was just completed, | |
| // skip auth check to prevent redirection | |
| setAuthState((prev) => ({ ...prev, loading: false })); | |
| // Clear the flag to prevent future bypassing of auth checks | |
| sessionStorage.removeItem("registration_complete"); | |
| return; | |
| } |
Summary by CodeRabbit
New Features
Bug Fixes
Chores