Fix modal interaction for email verification flow #4878
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request involve updates to various language files to include new tooltip entries related to email verification in Catalan, English, and Spanish. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AccountRoute
participant UserProfileView
participant EditUserModal
participant ProjectGIVbackToast
User->>AccountRoute: Access Account Page
AccountRoute-->>User: Render Account Page (without VerifyEmailBanner)
User->>UserProfileView: Access User Profile
UserProfileView-->>User: Render User Profile (with VerifyEmailBanner if email not verified)
User->>EditUserModal: Update User Information
EditUserModal-->>User: Reset Query Parameters and Show Success Message
User->>ProjectGIVbackToast: Hover for Tooltip
ProjectGIVbackToast-->>User: Show Tooltip with Email Verification Info
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![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
🧹 Outside diff range and nitpick comments (4)
src/components/views/userProfile/VerifyEmailBanner.tsx (1)
23-37: Consider improving the routing logic implementationA few suggestions to enhance the code:
- Use a boolean comparison instead of string
- Extract the navigation logic
- Add error handling for router.push
Consider this refactoring:
+const VERIFY_EMAIL_QUERY = 'opencheck'; + +const navigateToEmailVerification = (router: NextRouter) => { + return router.push( + { + pathname: Routes.MyAccount, + query: { [VERIFY_EMAIL_QUERY]: true }, + }, + undefined, + { shallow: true }, + ).catch(error => { + console.error('Failed to navigate:', error); + }); +}; + onClick={() => { if ( router.pathname === Routes.MyAccount && - router.query.opencheck === 'true' + router.query[VERIFY_EMAIL_QUERY] === 'true' ) { setShowModal?.(true); } else { - router.push( - { - pathname: Routes.MyAccount, - query: { opencheck: 'true' }, - }, - undefined, - { shallow: true }, - ); + navigateToEmailVerification(router); } }}src/components/views/project/ProjectGIVbackToast.tsx (3)
Line range hint
356-392: Consider enhancing tooltip accessibility and mobile UXThe tooltip implementation could be improved in the following areas:
- Accessibility: Add ARIA attributes for screen readers
- Mobile UX: Add touch support since hover doesn't work well on mobile devices
Consider applying these improvements:
<ContentWrapper> <Wrapper $isverified={isAdminEmailVerified}> {/* ... */} </Wrapper> {!isAdminEmailVerified && ( - <TooltipWrapper> + <TooltipWrapper + role="tooltip" + aria-label={formatMessage({ id: 'label.email_tooltip' })} + tabIndex={0} + > {formatMessage({ id: 'label.email_tooltip' })} </TooltipWrapper> )} </ContentWrapper>
438-462: Align tooltip styles with design systemThe tooltip styling could be improved for better maintainability and consistency:
- Use design system color tokens instead of hardcoded values
- Add z-index to prevent overlay issues
Consider these styling improvements:
const TooltipWrapper = styled.div` position: absolute; bottom: 100%; left: 50%; transform: translateX(-50%); - background: #1a1a1a; - color: #fff; + background: ${neutralColors.gray[900]}; + color: ${neutralColors.gray[100]}; padding: 8px 12px; border-radius: 4px; font-size: 12px; white-space: nowrap; opacity: 0; visibility: hidden; + z-index: 1; transition: opacity 0.2s ease-in-out, visibility 0.2s ease-in-out; `;
476-477: Follow React boolean prop naming conventionThe
$isverifiedprop doesn't follow React's convention for boolean props. Consider usingisVerified(without$prefix) to align with React's naming patterns.-const Wrapper = styled(Flex)<{ $isverified: boolean }>` +const Wrapper = styled(Flex)<{ isVerified: boolean }>` /* ... */ - pointer-events: ${({ $isverified }) => ($isverified ? 'auto' : 'none')}; - opacity: ${({ $isverified }) => ($isverified ? '1' : '0.75')}; + pointer-events: ${({ isVerified }) => (isVerified ? 'auto' : 'none')}; + opacity: ${({ isVerified }) => (isVerified ? '1' : '0.75')}; `;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
lang/ca.json(1 hunks)lang/en.json(1 hunks)lang/es.json(1 hunks)pages/account.tsx(0 hunks)src/components/modals/EditUserModal.tsx(3 hunks)src/components/views/project/ProjectGIVbackToast.tsx(4 hunks)src/components/views/userProfile/UserProfile.view.tsx(2 hunks)src/components/views/userProfile/VerifyEmailBanner.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- pages/account.tsx
✅ Files skipped from review due to trivial changes (2)
- lang/ca.json
- lang/es.json
🔇 Additional comments (10)
src/components/views/userProfile/VerifyEmailBanner.tsx (3)
7-11: LGTM: Clean prop interface definition
The TypeScript interface for the optional setShowModal prop is well-defined and follows best practices.
7-11: Verify integration with UserProfileView
According to the PR summary, this component is integrated into UserProfileView. Let's ensure the integration is properly implemented.
✅ Verification successful
Integration with UserProfileView is properly implemented
The verification shows that:
- VerifyEmailBanner is correctly imported in UserProfileView
- The component is used with the
setShowModalprop being passed from UserProfileView's state - The integration matches the component's interface
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the UserProfileView integration
ast-grep --pattern 'import { VerifyEmailBanner } from $_'
ast-grep --pattern 'const UserProfileView = $_'
Length of output: 109
Script:
#!/bin/bash
# Let's try a broader search to find the integration
rg -l "VerifyEmailBanner" --type tsx
rg "UserProfileView" --type tsx -A 5
Length of output: 129
Script:
#!/bin/bash
# Search without file type restriction since tsx isn't recognized
rg -l "VerifyEmailBanner"
rg "UserProfileView" -A 5
Length of output: 13251
Script:
#!/bin/bash
# Let's check how VerifyEmailBanner is used in UserProfile.view.tsx
rg -A 10 "VerifyEmailBanner" src/components/views/userProfile/UserProfile.view.tsx
Length of output: 794
23-37: Verify the impact of shallow routing
The use of shallow routing might affect SEO and analytics tracking. Please ensure this aligns with your requirements.
src/components/modals/EditUserModal.tsx (3)
5-5: LGTM: Clean import addition
The Next.js router import is properly placed and follows standard patterns.
47-47: LGTM: Proper hook initialization
The router hook is correctly initialized at the component level, following React hooks rules.
111-120: 🛠️ Refactor suggestion
Verify impact on query parameters and error handling
While the routing logic is correctly implemented, there are a few concerns to address:
- Consider moving the router.push inside the try-catch block to handle potential routing errors
- Verify that clearing all query parameters doesn't affect other features
Let's check for other query parameter usage in the codebase:
Consider wrapping the router.push in the try-catch and preserving necessary query parameters:
if (data.updateUser) {
address && dispatch(fetchUserByAddress(address));
gToast('Profile information updated.', {
type: ToastType.SUCCESS,
title: 'Success',
});
- closeModal();
-
- // Reset router query
- router.push(
- {
- pathname: router.pathname,
- query: {},
- },
- undefined,
- { shallow: true },
- );
+ try {
+ await closeModal();
+ // Reset only specific query parameters if needed
+ const { verifyEmail, ...restQuery } = router.query;
+ await router.push(
+ {
+ pathname: router.pathname,
+ query: restQuery,
+ },
+ undefined,
+ { shallow: true },
+ );
+ } catch (error) {
+ console.error('Navigation error:', error);
+ }
} else {
throw 'Update User Failed.';
}✅ Verification successful
Let me gather more information about the router usage in EditUserModal.tsx and its surrounding context.
Let me check the VerifyEmailModal and related components to understand if there's any connection with EditUserModal's query parameters.
Based on the gathered information, I can now provide the final response:
Router query clearing in EditUserModal is safe to proceed
The current implementation of clearing query parameters in EditUserModal is safe because:
- The modal operates independently of email verification flow (no direct connection found between EditUserModal and VerifyEmailModal)
- Other modals manage their own query parameters independently
- The shallow routing ensures minimal impact on the application state
The suggested try-catch block would be a good practice, but it's not critical as the router operation is simple and contained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for router.query usage to understand potential impacts
rg "router\.(query|push)" -A 3 --type typescript
# Search for email verification related query parameters
ast-grep --pattern 'const { $_ } = router.query'
Length of output: 1177
Script:
#!/bin/bash
# Check EditUserModal.tsx content for router usage and query parameters
rg -A 5 "router\." src/components/modals/EditUserModal.tsx
# Look for any email verification related code in EditUserModal
rg "verifyEmail|email.*verif" src/components/modals/EditUserModal.tsx
# Check if there are any other modal components that handle query parameters
fd -e tsx -e ts modal --exec rg -l "router\.(query|push)"
Length of output: 796
Script:
#!/bin/bash
# Check VerifyEmailModal implementation
rg -A 10 "router\." src/components/modals/VerifyEmailModal.tsx
# Look for any connections between EditUserModal and email verification
rg -A 5 "EditUserModal" src/components/modals/VerifyEmailModal.tsx
rg -A 5 "VerifyEmailModal" src/components/modals/EditUserModal.tsx
# Check if there are any shared query parameters between these modals
ast-grep --pattern 'router.push({
$$$
query: $_,
$$$
})'
Length of output: 3950
src/components/views/userProfile/UserProfile.view.tsx (2)
46-46: Verify the VerifyEmailBanner component exists
The import path suggests the component is in the same directory. Let's verify its existence and implementation.
✅ Verification successful
Import path for VerifyEmailBanner component is correct
The component exists in the expected location and is properly implemented as a React functional component. It accepts the required setShowModal prop and includes the email verification banner functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if VerifyEmailBanner component exists and verify its implementation
fd "VerifyEmailBanner.tsx" src/components/views/userProfile/
Length of output: 115
Script:
#!/bin/bash
# Let's verify the implementation of VerifyEmailBanner component
ast-grep --pattern $'const VerifyEmailBanner = ({ $$ }) => {
$$$
}'
Length of output: 70
Script:
#!/bin/bash
# Let's check the content of the VerifyEmailBanner component file
cat src/components/views/userProfile/VerifyEmailBanner.tsx
Length of output: 1588
127-129: LGTM! Verify the complete email verification flow
The integration looks correct. The banner is properly conditionally rendered and connected to the modal system via setShowModal.
Let's verify the complete email verification flow:
✅ Verification successful
LGTM! Email verification flow is properly implemented
The verification confirms that:
VerifyEmailBanneris correctly rendered based on!user?.isEmailVerifiedcondition- The banner passes
setShowModalto openEditUserModalfor email verification EditUserModalproperly tracks email verification state and handles the verification process- The integration between components maintains proper state management for email verification
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the email verification flow implementation
# Check VerifyEmailBanner's usage of setShowModal
ast-grep --pattern 'const VerifyEmailBanner = ({ setShowModal }: { setShowModal: $_}) => {
$$$
}'
# Check EditUserModal's email verification handling
ast-grep --pattern 'const EditUserModal = ({ setShowModal, user, $$ }: { setShowModal: $_; user: $_; $$ }) => {
$$$
}'
Length of output: 224
Script:
#!/bin/bash
# Let's try a different approach to verify the email verification flow
# Search for VerifyEmailBanner component
rg -A 10 "const VerifyEmailBanner" --type typescript
# Search for EditUserModal component
rg -A 10 "const EditUserModal" --type typescript
# Search for email verification related state or handlers
rg "isEmailVerified|setShowModal|verifyEmail" --type typescript
Length of output: 276
Script:
#!/bin/bash
# Let's try without specifying the file type
# Search for VerifyEmailBanner component
rg -A 10 "const VerifyEmailBanner"
# Search for EditUserModal component
rg -A 10 "const EditUserModal"
# Search for email verification related state or handlers
rg "isEmailVerified|setShowModal|verifyEmail"
# Also search for the files containing these components
fd "VerifyEmailBanner|EditUserModal"
Length of output: 33635
src/components/views/project/ProjectGIVbackToast.tsx (1)
Line range hint 356-477: Implementation aligns well with PR objectives
The changes effectively address the email verification flow by:
- Adding clear visual feedback about verification status
- Preventing interactions when email is not verified
- Providing user guidance through tooltips
lang/en.json (1)
403-403: LGTM! The new email tooltip text is clear and properly formatted.
The added tooltip provides clear instructions for email verification while maintaining consistent style with other tooltips in the file.
Summary by CodeRabbit
Release Notes
New Features
VerifyEmailBannerto prompt users to verify their email addresses in the user profile view.Improvements
Bug Fixes