feat: add select all checkbox to mail layout#1467
Conversation
|
Warning Rate limit exceeded@retrogtx has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
""" WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SelectAllCheckbox
participant MailState
participant API
participant Toast
User->>SelectAllCheckbox: Toggle checkbox
alt Some threads selected
SelectAllCheckbox->>MailState: Clear selection
else None selected
SelectAllCheckbox->>MailState: Select all loaded threads
SelectAllCheckbox->>API: Fetch all matching thread IDs (paginated)
API-->>SelectAllCheckbox: Return all thread IDs
SelectAllCheckbox->>MailState: Select all fetched threads
SelectAllCheckbox->>Toast: Show selection count and "select all" action
end
Note right of SelectAllCheckbox: Handles loading, indeterminate state, and errors
Suggested labels
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/mail/components/mail/select-all-checkbox.tsx (1)
31-56: Consider adding request cancellation for better UX.The pagination logic is well-implemented with proper error handling. However, for large datasets, consider adding an AbortController to allow cancelling the fetch operation if the user changes their mind.
const fetchAllMatchingThreadIds = useCallback(async (): Promise<string[]> => { + const abortController = new AbortController(); const ids: string[] = []; let cursor = ''; const MAX_PER_PAGE = 100; try { while (true) { + if (abortController.signal.aborted) break; const page = await trpcClient.mail.listThreads.query({ folder, q: query, max: MAX_PER_PAGE, cursor, }); // ... rest of logic } } catch (err: any) { + if (err.name === 'AbortError') return ids; console.error('Failed to fetch all thread IDs', err); toast.error(err?.message ?? 'Failed to select all emails'); } return ids; }, [folder, query]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/components/mail/mail.tsx(2 hunks)apps/mail/components/mail/select-all-checkbox.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/mail/components/mail/select-all-checkbox.tsx
[error] 13-13: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (6)
apps/mail/components/mail/select-all-checkbox.tsx (4)
1-9: LGTM! Well-organized imports.All necessary dependencies are properly imported and the imports are logically grouped.
20-29: LGTM! Well-implemented state calculations.The computed values properly handle the three checkbox states with appropriate memoization dependencies.
58-81: LGTM! Excellent UX design for bulk selection.The two-step selection process provides immediate feedback while fetching all threads asynchronously. The toast notification with action button is a great UX pattern.
83-91: LGTM! Clean component rendering with proper state handling.The checkbox correctly reflects selection state and provides appropriate user feedback during loading.
apps/mail/components/mail/mail.tsx (2)
68-68: LGTM! Proper import statement.The import follows the project's conventions and uses the correct relative path.
496-496: LGTM! Well-positioned component integration.The SelectAllCheckbox is appropriately placed in the sidebar header with proper spacing and integrates seamlessly with the existing UI.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/mail/mail.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/mail/components/mail/mail.tsx (1)
apps/mail/components/mail/select-all-checkbox.tsx (1)
SelectAllCheckbox(11-92)
🪛 Biome (1.9.4)
apps/mail/components/mail/mail.tsx
[error] 69-69: Shouldn't redeclare 'IConnection'. Consider to delete it or rename it.
'IConnection' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (2)
apps/mail/components/mail/mail.tsx (2)
70-70: LGTM! Clean import of the new component.The import of
SelectAllCheckboxis properly structured and follows the project's import conventions.
496-496: Excellent integration of the SelectAllCheckbox component.The component is placed appropriately in the sidebar header next to the
SidebarTogglebutton with proper spacing (ml-2). This aligns perfectly with the PR objective of adding select all functionality to the mail layout.
now you can select all mails in the current view, and all of them that are not visible in the current view too.
checkbox.mp4
Summary by CodeRabbit
New Features
User Interface