Conversation
context menu
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis set of changes refactors state management for mail composer modes (reply, reply-all, forward) across multiple components by replacing individual boolean flags with a single Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ThreadContextMenu
participant useQueryState
participant ThreadDisplay
participant ReplyCompose
User->>ThreadContextMenu: Clicks Reply/Reply All/Forward
ThreadContextMenu->>useQueryState: setMode('reply'|'replyAll'|'forward')
useQueryState-->>ThreadDisplay: mode updated
ThreadDisplay->>ReplyCompose: Render with current mode
ReplyCompose->>useQueryState: Read mode to determine composer state
User->>ReplyCompose: Closes composer
ReplyCompose->>useQueryState: setMode(null)
sequenceDiagram
participant User
participant ErrorPage
participant dexieStorageProvider
participant signOut
participant Browser
User->>ErrorPage: Clicks "Try Again"
ErrorPage->>dexieStorageProvider: clear()
ErrorPage->>signOut: signOut()
ErrorPage->>Browser: Redirect to /login
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
apps/mail/components/mail/reply-composer.tsx (2)
146-163:⚠️ Potential issueUnused
const [mail, setMail] = useMail();(line 151) is no longer referenced after the refactor.
Leaving unused hooks will trigger ESLint/TS errors and causes an unnecessary re‑render when the global
796-805:⚠️ Potential issue
onBluralways writes to the “To” field – breaks Cc/Bcc editingInside
RecipientInputthe blur‑handler hard‑codes'to'instead of using the currenttype, so blurring a Cc/Bcc input pushes the address into the wrong array.- onBlur={(e) => handleAddEmail('to', e.currentTarget.value)} + onBlur={(e) => handleAddEmail(type, e.currentTarget.value)}Without this fix users lose Cc/Bcc addresses and see duplicates in “To”.
@@ - onBlur={(e) => handleAddEmail('to', e.currentTarget.value)} + onBlur={(e) => handleAddEmail(type, e.currentTarget.value)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/mail/app/error.tsx(2 hunks)apps/mail/components/context/thread-context.tsx(6 hunks)apps/mail/components/create/prosemirror.css(2 hunks)apps/mail/components/mail/mail-list.tsx(0 hunks)apps/mail/components/mail/reply-composer.tsx(7 hunks)apps/mail/components/mail/thread-display.tsx(5 hunks)apps/mail/components/providers/hotkey-provider-wrapper.tsx(2 hunks)apps/mail/hooks/use-stats.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/mail/components/mail/mail-list.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/mail/components/mail/thread-display.tsx (2)
apps/mail/components/mail/reply-composer.tsx (1)
ReplyCompose(146-1249)apps/mail/lib/utils.ts (1)
cn(51-51)
apps/mail/components/mail/reply-composer.tsx (2)
apps/mail/hooks/use-threads.ts (1)
useThread(136-149)apps/mail/components/mail/use-mail.ts (1)
useMail(23-25)
🔇 Additional comments (7)
apps/mail/components/providers/hotkey-provider-wrapper.tsx (1)
18-18:Details
❓ Verification inconclusive
Verify the intended behavior of commented out MailListHotkeys
The
MailListHotkeyscomponent has been commented out. This appears to be part of the state management refactoring for mail composer modes. Please confirm if this is intentional or if it should be uncommented once the refactoring is complete.
🏁 Script executed:
#!/bin/bash # Check if MailListHotkeys is used elsewhere or if there are any TODOs related to this rg -A 3 -B 3 "MailListHotkeys" --type tsxLength of output: 69
🏁 Script executed:
#!/bin/bash # Retry searching for occurrences of MailListHotkeys without restricting by file type rg "MailListHotkeys" -n .Length of output: 338
Verify intentional removal of MailListHotkeys
I found that
MailListHotkeysis still defined and exported here:• apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:13
and imported (but commented out) here:
• apps/mail/components/providers/hotkey-provider-wrapper.tsx:4, line 18
No other references or TODOs were located. Please confirm whether commenting out
<MailListHotkeys />was deliberate—if it should remain disabled, or if you plan to re‑enable it once the mail‑composer state refactor is complete.apps/mail/components/create/prosemirror.css (3)
11-11: LGTM: Standardized quote usage in placeholder textChanged double quotes to single quotes for consistency.
170-170: LGTM: Standardized heading size commentsAdded clearer comments to indicate the text size utility class equivalents.
Also applies to: 177-177, 184-184
192-192: LGTM: Improved comment formattingStandardized comment spacing for better readability.
apps/mail/hooks/use-stats.ts (1)
21-21: Good defensive programming with null coalescing operatorUsing
data ?? []ensures an empty array is always returned even whendatais null or undefined, preventing potential runtime errors when consuming code tries to iterate over the data array.apps/mail/app/error.tsx (2)
4-5: LGTM: Added necessary imports for error recoveryAdded imports for the sign-out and storage clearing functionality needed for the updated error recovery process.
24-34: Verify if this aggressive error recovery approach is appropriateThe error handling strategy has been significantly changed. Instead of simply calling the
reset()function to retry the operation, the code now:
- Clears all IndexedDB storage
- Signs out the user
- Redirects to the login page
While this approach ensures a clean slate, it could be frustrating for users encountering minor recoverable errors, as they'll lose their session and need to log in again.
Consider if there are certain error types that could use a less drastic recovery approach:
- onClick={async () => { - await dexieStorageProvider().clear(); - await signOut(); - window.location.href = '/login'; - }} + onClick={async () => { + // For critical errors, reset everything + if (error.message.includes("critical") || error.name === "AuthError") { + await dexieStorageProvider().clear(); + await signOut(); + window.location.href = '/login'; + } else { + // For non-critical errors, try the built-in reset + reset(); + } + }}
| className={cn(mode === 'reply' && 'bg-primary/10')} | ||
| onClick={() => { | ||
| setMail((prev) => ({ | ||
| ...prev, | ||
| replyComposerOpen: true, | ||
| replyAllComposerOpen: false, | ||
| forwardComposerOpen: false, | ||
| })); | ||
| setMode('reply'); | ||
| }} | ||
| /> | ||
| {hasMultipleParticipants && ( | ||
| <ThreadActionButton | ||
| icon={ReplyAll} | ||
| label={t('common.threadDisplay.replyAll')} | ||
| disabled={!emailData} | ||
| className={cn(mail.replyAllComposerOpen && 'bg-primary/10')} | ||
| className={cn(mode === 'replyAll' && 'bg-primary/10')} | ||
| onClick={() => { | ||
| setMail((prev) => ({ | ||
| ...prev, | ||
| replyComposerOpen: false, | ||
| replyAllComposerOpen: true, | ||
| forwardComposerOpen: false, | ||
| })); | ||
| setMode('replyAll'); | ||
| }} | ||
| /> | ||
| )} | ||
| <ThreadActionButton | ||
| icon={Forward} | ||
| label={t('common.threadDisplay.forward')} | ||
| disabled={!emailData} | ||
| className={cn(mail.forwardComposerOpen && 'bg-primary/10')} | ||
| className={cn(mode === 'forward' && 'bg-primary/10')} | ||
| onClick={() => { | ||
| setMail((prev) => ({ | ||
| ...prev, | ||
| replyComposerOpen: false, | ||
| replyAllComposerOpen: false, | ||
| forwardComposerOpen: true, | ||
| })); | ||
| setMode('forward'); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure threadId is always present before opening the composer
All three click‑handlers set mode but never (re‑)set threadId.
If the current URL was entered directly (e.g. /mail/inbox/123) and therefore lacks the ?threadId= query param, ReplyCompose will read threadId === null and silently fail to fetch the thread – the composer opens with no data.
onClick={() => {
- setMode('reply');
+ setThreadId(id); // keep the current thread in the query string
+ setMode('reply');
}}Consider doing the same for replyAll and forward, or wrapping this into a small helper to avoid repetition.
📝 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.
| className={cn(mode === 'reply' && 'bg-primary/10')} | |
| onClick={() => { | |
| setMail((prev) => ({ | |
| ...prev, | |
| replyComposerOpen: true, | |
| replyAllComposerOpen: false, | |
| forwardComposerOpen: false, | |
| })); | |
| setMode('reply'); | |
| }} | |
| /> | |
| {hasMultipleParticipants && ( | |
| <ThreadActionButton | |
| icon={ReplyAll} | |
| label={t('common.threadDisplay.replyAll')} | |
| disabled={!emailData} | |
| className={cn(mail.replyAllComposerOpen && 'bg-primary/10')} | |
| className={cn(mode === 'replyAll' && 'bg-primary/10')} | |
| onClick={() => { | |
| setMail((prev) => ({ | |
| ...prev, | |
| replyComposerOpen: false, | |
| replyAllComposerOpen: true, | |
| forwardComposerOpen: false, | |
| })); | |
| setMode('replyAll'); | |
| }} | |
| /> | |
| )} | |
| <ThreadActionButton | |
| icon={Forward} | |
| label={t('common.threadDisplay.forward')} | |
| disabled={!emailData} | |
| className={cn(mail.forwardComposerOpen && 'bg-primary/10')} | |
| className={cn(mode === 'forward' && 'bg-primary/10')} | |
| onClick={() => { | |
| setMail((prev) => ({ | |
| ...prev, | |
| replyComposerOpen: false, | |
| replyAllComposerOpen: false, | |
| forwardComposerOpen: true, | |
| })); | |
| setMode('forward'); | |
| }} | |
| /> | |
| className={cn(mode === 'reply' && 'bg-primary/10')} | |
| onClick={() => { | |
| - setMode('reply'); | |
| + setThreadId(id); // keep the current thread in the query string | |
| + setMode('reply'); | |
| }} | |
| /> |
| const { | ||
| data: { threads }, | ||
| mutate, | ||
| isLoading, | ||
| isValidating, | ||
| } = useThreads(); | ||
| const currentFolder = folder ?? ''; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
useThreads() destructuring is unsafe – may throw when data is undefined
const {
data: { threads },
...
} = useThreads();If useThreads() returns { data: undefined, ... } during the initial fetch, the destructuring of threads will raise “Cannot destructure property ‘threads’ of undefined”.
Safer pattern:
-const { data: { threads }, mutate, isLoading, isValidating } = useThreads();
+const { data, mutate, isLoading, isValidating } = useThreads();
+const threads = data?.threads ?? [];Update the downstream selectedThreads/isUnread/isStarred calculations accordingly.
| import { markAsRead, markAsUnread, toggleStar } from '@/actions/mail'; | ||
| import { useThread, useThreads } from '@/hooks/use-threads'; | ||
| import { useSearchValue } from '@/hooks/use-search-value'; | ||
| import { useThreads } from '@/hooks/use-threads'; | ||
| import { useParams, useRouter } from 'next/navigation'; | ||
| import { modifyLabels } from '@/actions/mail'; | ||
| import { LABELS, FOLDERS } from '@/lib/utils'; | ||
| import { useStats } from '@/hooks/use-stats'; | ||
| import { useParams } from 'next/navigation'; | ||
| import { useMail } from '../mail/use-mail'; | ||
| import { useTranslations } from 'use-intl'; | ||
| import { type ReactNode } from 'react'; | ||
| import { toast } from 'sonner'; | ||
| import { modifyLabels } from '@/actions/mail'; | ||
| import { markAsRead, markAsUnread, toggleStar } from '@/actions/mail'; | ||
| import { useQueryState } from 'nuqs'; | ||
| import { useMemo } from 'react'; | ||
| import { toast } from 'sonner'; | ||
|
|
There was a problem hiding this comment.
Incorrect import for useTranslations causes compile‑time crash
useTranslations is exported from next-intl, not use-intl. The wrong module path breaks the build.
-import { useTranslations } from 'use-intl';
+import { useTranslations } from 'next-intl';📝 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.
| import { markAsRead, markAsUnread, toggleStar } from '@/actions/mail'; | |
| import { useThread, useThreads } from '@/hooks/use-threads'; | |
| import { useSearchValue } from '@/hooks/use-search-value'; | |
| import { useThreads } from '@/hooks/use-threads'; | |
| import { useParams, useRouter } from 'next/navigation'; | |
| import { modifyLabels } from '@/actions/mail'; | |
| import { LABELS, FOLDERS } from '@/lib/utils'; | |
| import { useStats } from '@/hooks/use-stats'; | |
| import { useParams } from 'next/navigation'; | |
| import { useMail } from '../mail/use-mail'; | |
| import { useTranslations } from 'use-intl'; | |
| import { type ReactNode } from 'react'; | |
| import { toast } from 'sonner'; | |
| import { modifyLabels } from '@/actions/mail'; | |
| import { markAsRead, markAsUnread, toggleStar } from '@/actions/mail'; | |
| import { useQueryState } from 'nuqs'; | |
| import { useMemo } from 'react'; | |
| import { toast } from 'sonner'; | |
| import { markAsRead, markAsUnread, toggleStar } from '@/actions/mail'; | |
| import { useThread, useThreads } from '@/hooks/use-threads'; | |
| import { useSearchValue } from '@/hooks/use-search-value'; | |
| import { useParams, useRouter } from 'next/navigation'; | |
| import { modifyLabels } from '@/actions/mail'; | |
| import { LABELS, FOLDERS } from '@/lib/utils'; | |
| import { useStats } from '@/hooks/use-stats'; | |
| import { useMail } from '../mail/use-mail'; | |
| import { useTranslations } from 'next-intl'; | |
| import { type ReactNode } from 'react'; | |
| import { useQueryState } from 'nuqs'; | |
| import { useMemo } from 'react'; | |
| import { toast } from 'sonner'; |
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style
Chores