feat: implement smooth infinite email deletion experience#1609
feat: implement smooth infinite email deletion experience#1609retrogtx wants to merge 1 commit intoMail-0:stagingfrom
Conversation
- Filter deleted threads at list level to prevent UI gaps - Fix navigation to jump to immediate next email (not skip by 2) - Add aggressive prefetching to maintain thread buffer - Ensure seamless deletion flow with no blank screens
WalkthroughThis update introduces optimistic UI improvements for thread actions in the mail app. It adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant useOptimisticActions
participant useAdjustedStats
participant Server
User->>UI: Move or delete thread
UI->>useOptimisticActions: Create optimistic action (with source)
useOptimisticActions-->>UI: Update optimistic state
UI->>useAdjustedStats: Read stats + optimistic actions
useAdjustedStats-->>UI: Return adjusted stats
UI->>Server: Perform actual move/delete
Server-->>UI: Confirm or reject
UI->>useOptimisticActions: Reconcile optimistic state
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Implements smooth infinite email deletion with optimistic updates across the UI, featuring intelligent prefetching and real-time count adjustments for folders and labels.
- Added
useAdjustedStatshook inapps/mail/hooks/use-stats.tsto handle optimistic updates for folder counts before server confirmation - Enhanced thread prefetching in
apps/mail/hooks/use-threads.tswith multiple strategies including threshold-based and upfront page loading - Improved navigation handling in
apps/mail/hooks/use-mail-navigation.tswith thread ID tracking and focus management during deletions - Integrated optimistic updates throughout sidebar components for immediate UI feedback during bulk operations
- Replaced direct mutations with optimistic actions in hotkeys for consistent deletion behavior
11 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile
| const globalShortcuts = React.useMemo( | ||
| () => keyboardShortcuts.filter((shortcut) => shortcut.scope === scope), | ||
| [], | ||
| ); |
There was a problem hiding this comment.
logic: globalShortcuts memoization has empty deps array but uses keyboardShortcuts from outside the component. Should add keyboardShortcuts to dependency array if it can change.
| const source = action.source.toLowerCase(); | ||
| const destination = action.destination?.toLowerCase(); |
There was a problem hiding this comment.
logic: source is accessed without null check which could crash if action.source is undefined
| const source = action.source.toLowerCase(); | |
| const destination = action.destination?.toLowerCase(); | |
| const source = action.source?.toLowerCase(); | |
| const destination = action.destination?.toLowerCase(); |
| const optimisticlyRemovedCount = Object.values(optimisticActions).reduce((count, action) => { | ||
| if (action.type === 'MOVE' && action.source === folder) { | ||
| return count + action.threadIds.length; | ||
| } | ||
| return count; | ||
| }, 0); |
There was a problem hiding this comment.
logic: This count calculation may over-count if there are multiple MOVE actions for the same threadId. Use a Set to deduplicate threadIds first
| // Update the tracked focused thread ID | ||
| prevFocusedThreadId.current = threadId; | ||
|
|
||
| const currentThreadId = window.location.search.includes('threadId='); |
There was a problem hiding this comment.
logic: This check may return true for any query param containing 'threadId='. Use URLSearchParams to properly parse the query string
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| prevFocusedThreadId.current = null; | ||
| return null; | ||
| } else if (currentItem) { | ||
| // Current item existts and wasnt deleted, then navigate to the next one |
There was a problem hiding this comment.
There's a typo in this comment: existts should be exists and wasnt should be wasn't. The corrected comment would be: "Current item exists and wasn't deleted, then navigate to the next one"
| // Current item existts and wasnt deleted, then navigate to the next one | |
| // Current item exists and wasn't deleted, then navigate to the next one |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![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
🧹 Nitpick comments (3)
apps/mail/hooks/use-stats.ts (1)
22-66: Solid implementation of optimistic stats adjustment.The logic correctly:
- Calculates deltas for source/destination folders based on MOVE actions
- Prevents negative counts with
Math.max(0, ...)- Uses memoization for performance optimization
- Maintains the same interface as the original
useStatshookConsider adding null safety for the label property:
- const delta = deltas[stat.label?.toLowerCase() ?? ''] ?? 0; + const labelKey = stat.label?.toLowerCase(); + const delta = labelKey ? (deltas[labelKey] ?? 0) : 0;This makes the intent clearer when stats don't have labels.
apps/mail/hooks/use-mail-navigation.ts (2)
102-206: Consider refactoring navigateNext for improved maintainabilityThe
navigateNextfunction has become quite complex with multiple nested conditions. Consider extracting helper functions to improve readability:
handleOutOfBoundsIndex(prevIndex, items)- for lines 119-133handleDeletedThread(prevIndex, items, prevThreadId)- for lines 135-169handleNormalNavigation(prevIndex, items)- for lines 170-204This would make the main logic flow clearer and easier to test.
279-280: Apply consistent formatting for hotkey configurations- useHotkeys('Enter', handleEnter, { preventDefault: true,enabled: !isCommandPaletteOpen }); - useHotkeys('Escape', handleEscape, { preventDefault: true,enabled: !isCommandPaletteOpen }); + useHotkeys('Enter', handleEnter, { preventDefault: true, enabled: !isCommandPaletteOpen }); + useHotkeys('Escape', handleEscape, { preventDefault: true, enabled: !isCommandPaletteOpen });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/mail/components/mail/mail-list.tsx(1 hunks)apps/mail/components/ui/app-sidebar.tsx(2 hunks)apps/mail/components/ui/nav-main.tsx(4 hunks)apps/mail/components/ui/sidebar-labels.tsx(1 hunks)apps/mail/hooks/use-mail-navigation.ts(6 hunks)apps/mail/hooks/use-optimistic-actions.ts(2 hunks)apps/mail/hooks/use-stats.ts(2 hunks)apps/mail/hooks/use-threads.ts(2 hunks)apps/mail/lib/hotkeys/navigation-hotkeys.tsx(1 hunks)apps/mail/lib/hotkeys/thread-display-hotkeys.tsx(3 hunks)apps/mail/store/optimistic-updates.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/ui/app-sidebar.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/lib/hotkeys/navigation-hotkeys.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/lib/hotkeys/thread-display-hotkeys.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/mail/mail-list.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/store/optimistic-updates.ts (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.709Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
Learnt from: dakdevs
PR: Mail-0/Zero#764
File: apps/mail/actions/send.ts:6-8
Timestamp: 2025-04-25T06:09:08.027Z
Learning: The `after` function is a valid export from `next/server` in Next.js versions 14.1.0 and later. It's used to schedule code to run after a Server Action has completed and the response has been streamed to the client, making it ideal for non-critical operations like analytics, logging, or background processing.
apps/mail/hooks/use-mail-navigation.ts (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: Fahad-Dezloper
PR: Mail-0/Zero#1440
File: apps/mail/components/create/ai-chat.tsx:101-113
Timestamp: 2025-06-22T19:23:10.599Z
Learning: On small and medium screens (mobile devices), buttons don't naturally lose focus after being clicked like they do on desktop browsers. This requires manual intervention using setTimeout with additional blur() calls and DOM manipulation to properly clear focus and active states in button click handlers for responsive design.
apps/mail/hooks/use-threads.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
🧬 Code Graph Analysis (6)
apps/mail/lib/hotkeys/navigation-hotkeys.tsx (1)
apps/mail/config/shortcuts.ts (1)
keyboardShortcuts(359-365)
apps/mail/components/ui/nav-main.tsx (1)
apps/mail/hooks/use-stats.ts (1)
useAdjustedStats(22-66)
apps/mail/lib/hotkeys/thread-display-hotkeys.tsx (1)
apps/mail/hooks/use-optimistic-actions.ts (1)
useOptimisticActions(34-473)
apps/mail/components/mail/mail-list.tsx (1)
apps/mail/components/context/thread-context.tsx (1)
ThreadContextMenu(127-493)
apps/mail/hooks/use-stats.ts (1)
apps/mail/store/optimistic-updates.ts (1)
optimisticActionsAtom(11-11)
apps/mail/hooks/use-threads.ts (3)
apps/mail/hooks/use-search-value.ts (1)
useSearchValue(19-21)apps/mail/store/backgroundQueue.ts (2)
backgroundQueueAtom(5-19)isThreadInBackgroundQueueAtom(21-23)apps/mail/store/optimistic-updates.ts (1)
optimisticActionsAtom(11-11)
🔇 Additional comments (16)
apps/mail/components/ui/app-sidebar.tsx (1)
42-42: LGTM: Consistent migration to adjusted stats hook.The migration from
useStatstouseAdjustedStatsproperly supports the optimistic UI improvements by providing mail count statistics that reflect pending thread moves and deletions.Also applies to: 66-66
apps/mail/hooks/use-optimistic-actions.ts (1)
279-279: LGTM: Enhanced optimistic action tracking with source folder.Adding the
sourceproperty enables more precise tracking of thread moves and deletions, which supports:
- Accurate folder count adjustments in
useAdjustedStats- Thread filtering in
useThreadsto hide threads being moved out- Better undo functionality by preserving the source folder context
Also applies to: 339-339
apps/mail/components/mail/mail-list.tsx (1)
606-616: LGTM: Improved separation of concerns by centralizing optimistic state logic.Removing the
optimisticState.shouldHidecheck from the rendering logic simplifies the component and aligns with the centralized thread filtering approach inuseThreads. This improves maintainability by keeping optimistic UI state management in one place.apps/mail/lib/hotkeys/thread-display-hotkeys.tsx (1)
2-2: LGTM: Integration with optimistic deletion system enhances UX.Replacing the previous deletion logic with
optimisticDeleteThreadsprovides immediate UI feedback for hotkey-triggered deletions, creating a smoother user experience that aligns with the PR's goal of implementing a "smooth infinite email deletion experience."Also applies to: 5-5, 24-24, 45-45
apps/mail/lib/hotkeys/navigation-hotkeys.tsx (1)
4-4: LGTM: Good performance optimizations with correct memoization.The
React.useMemoadditions properly prevent unnecessary re-creations:
handlersmemoization with[navigate]dependency is appropriateglobalShortcutsmemoization with empty dependency array is correct for static shortcutsThese optimizations improve performance without changing functionality.
Also applies to: 10-26
apps/mail/components/ui/nav-main.tsx (2)
17-17: LGTM! Clean refactoring to use adjusted stats.The migration from
useStatstouseAdjustedStatsis consistent across bothNavMainandNavItemcomponents, ensuring optimistic UI updates are reflected in the navigation counts.Also applies to: 59-59, 275-275
263-263: Good decoupling of stats prop dependency.Removing the
statsprop fromSidebarLabelsand having the component manage its own stats internally improves encapsulation and reduces prop drilling.apps/mail/hooks/use-stats.ts (1)
4-6: LGTM! Appropriate imports for optimistic UI functionality.The new imports support the optimistic stats adjustment logic effectively.
apps/mail/components/ui/sidebar-labels.tsx (2)
5-5: LGTM! Updated import for optimistic stats.The import change aligns with the overall migration to use adjusted stats throughout the application.
7-10: Excellent encapsulation improvement.Removing the
statsprop and having the component manage its own stats internally:
- Reduces prop drilling
- Makes the component more self-contained
- Ensures optimistic updates are automatically reflected
Also applies to: 12-13
apps/mail/store/optimistic-updates.ts (1)
5-5: LGTM! Essential addition for optimistic UI tracking.Adding the
sourceproperty to MOVE actions enables proper tracking of threads being moved out of folders, which is crucial for the optimistic filtering logic inuseThreads.apps/mail/hooks/use-threads.ts (3)
13-13: LGTM! Proper integration of optimistic actions.The import and usage of
optimisticActionsAtomenables the filtering of threads that are being optimistically moved out of the current folder.Also applies to: 21-21
42-52: Excellent optimistic filtering implementation.The
shouldHideThreadfunction efficiently:
- Uses a Set for O(1) lookup performance
- Correctly identifies threads being moved out of the current folder
- Is properly memoized with appropriate dependencies
60-62: Good integration of optimistic filtering in thread list.The filter integration maintains clean separation of concerns and ensures optimistically moved threads are hidden from the UI immediately.
apps/mail/hooks/use-mail-navigation.ts (2)
27-31: Well-implemented thread deletion trackingThe addition of
prevFocusedThreadIdref provides a robust way to detect when threads are deleted during navigation. The ref is properly initialized and consistently updated across all navigation actions.
135-140: Verify behavior with rapid consecutive thread deletionsThe deletion detection logic looks solid, but please ensure it handles rapid consecutive deletions correctly, especially when multiple threads are deleted while navigation is in progress.
Consider adding a test case that simulates rapid thread deletions to ensure the navigation remains stable.
| // Derived atom that adjusts server stats with optimistic actions | ||
| export const adjustedStatsAtom = atom((get) => { | ||
| // We can't directly access trpc query data here, so this will be used differently | ||
| // This is a placeholder - actual implementation will be in the hook | ||
| return null; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove the unused placeholder atom.
This placeholder atom that returns null serves no purpose and could confuse future developers. Since the actual stats adjustment logic is properly implemented in the useAdjustedStats hook, this atom should be removed.
-// Derived atom that adjusts server stats with optimistic actions
-export const adjustedStatsAtom = atom((get) => {
- // We can't directly access trpc query data here, so this will be used differently
- // This is a placeholder - actual implementation will be in the hook
- return null;
-});🤖 Prompt for AI Agents
In apps/mail/store/optimistic-updates.ts around lines 49 to 54, remove the
entire placeholder atom named adjustedStatsAtom that returns null, as it is
unused and redundant. The actual logic is implemented in the useAdjustedStats
hook, so deleting this atom will clean up the code and prevent confusion.
| const THRESHOLD = 100; | ||
| const PREFETCH_PAGES = 3; | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| threads.length < THRESHOLD && | ||
| threadsQuery.hasNextPage && | ||
| !threadsQuery.isFetchingNextPage && | ||
| !threadsQuery.isLoading | ||
| ) { | ||
| void threadsQuery.fetchNextPage(); | ||
| } | ||
| }, [threads.length, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); | ||
|
|
||
| useEffect(() => { | ||
| const loadedPages = threadsQuery.data?.pages.length ?? 0; | ||
| if ( | ||
| loadedPages < PREFETCH_PAGES && | ||
| threadsQuery.hasNextPage && | ||
| !threadsQuery.isFetchingNextPage && | ||
| !threadsQuery.isLoading | ||
| ) { | ||
| void threadsQuery.fetchNextPage(); | ||
| } | ||
| }, [threadsQuery.data?.pages.length, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); | ||
|
|
||
| useEffect(() => { | ||
| const optimisticlyRemovedCount = Object.values(optimisticActions).reduce((count, action) => { | ||
| if (action.type === 'MOVE' && action.source === folder) { | ||
| return count + action.threadIds.length; | ||
| } | ||
| return count; | ||
| }, 0); | ||
|
|
||
| if ( | ||
| optimisticlyRemovedCount > 20 && | ||
| threadsQuery.hasNextPage && | ||
| !threadsQuery.isFetchingNextPage && | ||
| !threadsQuery.isLoading | ||
| ) { | ||
| void threadsQuery.fetchNextPage(); | ||
| } | ||
| }, [optimisticActions, folder, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Aggressive pagination logic needs refinement.
While the automatic pagination improves UX, the current implementation has several concerns:
- Magic numbers should be constants - Extract
100,3, and20to named constants - Performance impact - Multiple effects triggering
fetchNextPage()could be expensive - Risk of infinite loops - Need safeguards against continuous fetching
+const PAGINATION_THRESHOLD = 100;
+const MAX_PREFETCH_PAGES = 3;
+const OPTIMISTIC_REMOVAL_THRESHOLD = 20;
+
useEffect(() => {
if (
- threads.length < THRESHOLD &&
+ threads.length < PAGINATION_THRESHOLD &&
threadsQuery.hasNextPage &&
!threadsQuery.isFetchingNextPage &&
!threadsQuery.isLoading
) {
void threadsQuery.fetchNextPage();
}
}, [threads.length, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]);
useEffect(() => {
const loadedPages = threadsQuery.data?.pages.length ?? 0;
if (
- loadedPages < PREFETCH_PAGES &&
+ loadedPages < MAX_PREFETCH_PAGES &&
threadsQuery.hasNextPage &&
!threadsQuery.isFetchingNextPage &&
!threadsQuery.isLoading
) {
void threadsQuery.fetchNextPage();
}
}, [threadsQuery.data?.pages.length, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]);
useEffect(() => {
const optimisticlyRemovedCount = Object.values(optimisticActions).reduce((count, action) => {
if (action.type === 'MOVE' && action.source === folder) {
return count + action.threadIds.length;
}
return count;
}, 0);
if (
- optimisticlyRemovedCount > 20 &&
+ optimisticlyRemovedCount > OPTIMISTIC_REMOVAL_THRESHOLD &&
threadsQuery.hasNextPage &&
!threadsQuery.isFetchingNextPage &&
!threadsQuery.isLoading
) {
void threadsQuery.fetchNextPage();
}
}, [optimisticActions, folder, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]);Consider consolidating these effects or adding rate limiting to prevent excessive API calls.
📝 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.
| const THRESHOLD = 100; | |
| const PREFETCH_PAGES = 3; | |
| useEffect(() => { | |
| if ( | |
| threads.length < THRESHOLD && | |
| threadsQuery.hasNextPage && | |
| !threadsQuery.isFetchingNextPage && | |
| !threadsQuery.isLoading | |
| ) { | |
| void threadsQuery.fetchNextPage(); | |
| } | |
| }, [threads.length, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); | |
| useEffect(() => { | |
| const loadedPages = threadsQuery.data?.pages.length ?? 0; | |
| if ( | |
| loadedPages < PREFETCH_PAGES && | |
| threadsQuery.hasNextPage && | |
| !threadsQuery.isFetchingNextPage && | |
| !threadsQuery.isLoading | |
| ) { | |
| void threadsQuery.fetchNextPage(); | |
| } | |
| }, [threadsQuery.data?.pages.length, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); | |
| useEffect(() => { | |
| const optimisticlyRemovedCount = Object.values(optimisticActions).reduce((count, action) => { | |
| if (action.type === 'MOVE' && action.source === folder) { | |
| return count + action.threadIds.length; | |
| } | |
| return count; | |
| }, 0); | |
| if ( | |
| optimisticlyRemovedCount > 20 && | |
| threadsQuery.hasNextPage && | |
| !threadsQuery.isFetchingNextPage && | |
| !threadsQuery.isLoading | |
| ) { | |
| void threadsQuery.fetchNextPage(); | |
| } | |
| }, [optimisticActions, folder, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); | |
| const THRESHOLD = 100; | |
| const PREFETCH_PAGES = 3; | |
| const PAGINATION_THRESHOLD = 100; | |
| const MAX_PREFETCH_PAGES = 3; | |
| const OPTIMISTIC_REMOVAL_THRESHOLD = 20; | |
| useEffect(() => { | |
| if ( | |
| threads.length < PAGINATION_THRESHOLD && | |
| threadsQuery.hasNextPage && | |
| !threadsQuery.isFetchingNextPage && | |
| !threadsQuery.isLoading | |
| ) { | |
| void threadsQuery.fetchNextPage(); | |
| } | |
| }, [threads.length, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); | |
| useEffect(() => { | |
| const loadedPages = threadsQuery.data?.pages.length ?? 0; | |
| if ( | |
| loadedPages < MAX_PREFETCH_PAGES && | |
| threadsQuery.hasNextPage && | |
| !threadsQuery.isFetchingNextPage && | |
| !threadsQuery.isLoading | |
| ) { | |
| void threadsQuery.fetchNextPage(); | |
| } | |
| }, [threadsQuery.data?.pages.length, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); | |
| useEffect(() => { | |
| const optimisticlyRemovedCount = Object.values(optimisticActions).reduce((count, action) => { | |
| if (action.type === 'MOVE' && action.source === folder) { | |
| return count + action.threadIds.length; | |
| } | |
| return count; | |
| }, 0); | |
| if ( | |
| optimisticlyRemovedCount > OPTIMISTIC_REMOVAL_THRESHOLD && | |
| threadsQuery.hasNextPage && | |
| !threadsQuery.isFetchingNextPage && | |
| !threadsQuery.isLoading | |
| ) { | |
| void threadsQuery.fetchNextPage(); | |
| } | |
| }, [optimisticActions, folder, threadsQuery.hasNextPage, threadsQuery.isFetchingNextPage, threadsQuery.isLoading]); |
🤖 Prompt for AI Agents
In apps/mail/hooks/use-threads.ts between lines 64 and 106, the pagination logic
uses hardcoded numbers 100, 3, and 20 which should be extracted as named
constants for clarity. Also, multiple useEffect hooks independently trigger
fetchNextPage(), which can cause performance issues and risk infinite loops.
Refactor by consolidating these effects into a single useEffect that handles all
conditions and add a rate limiting mechanism or a flag to prevent excessive or
continuous fetchNextPage() calls.
| prevFocusedThreadId.current = null; | ||
| return null; | ||
| } else if (currentItem) { | ||
| // Current item existts and wasnt deleted, then navigate to the next one |
There was a problem hiding this comment.
Fix typo: "existts" should be "exists"
- // Current item existts and wasnt deleted, then navigate to the next one
+ // Current item exists and wasn't deleted, then navigate to the next one📝 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.
| // Current item existts and wasnt deleted, then navigate to the next one | |
| // Current item exists and wasn't deleted, then navigate to the next one |
🤖 Prompt for AI Agents
In apps/mail/hooks/use-mail-navigation.ts at line 170, fix the typo in the
comment by changing "existts" to "exists" to correct the spelling.
|
IT'S WIP |
2 participants
before:
https://github.com/user-attachments/assets/47b8d395-d772-484c-9861-79d7f28b43ac
after:
https://github.com/user-attachments/assets/3d6e03d8-7b5c-4465-ac84-b5bc656a3f15
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores