feat(mail): inbox selection checkbox and Empty Bin/Spam actions#1561
feat(mail): inbox selection checkbox and Empty Bin/Spam actions#1561hackice20 wants to merge 1 commit intoMail-0:stagingfrom hackice20:feat/inbox-selection
Conversation
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EmptyFolderButton
participant Backend
User->>EmptyFolderButton: Click "Empty Folder" button
EmptyFolderButton->>User: Show confirmation prompt
User->>EmptyFolderButton: Confirm action
loop For each page of threads
EmptyFolderButton->>Backend: Fetch thread IDs (batch of 100)
Backend-->>EmptyFolderButton: Return thread IDs
end
alt Folder is "spam"
EmptyFolderButton->>Backend: Move all threads to "bin" (optimistic)
else Folder is "bin"
EmptyFolderButton->>Backend: Permanently delete all threads (optimistic)
end
Backend-->>EmptyFolderButton: Success or error
EmptyFolderButton->>User: Show success or error toast
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
apps/mail/components/mail/empty-folder-button.tsx (1)
25-25: Replace window.confirm with proper dialog component.Using
window.confirmprovides a poor user experience compared to the existing dialog components in the codebase.Consider using the existing Dialog components from the UI library:
+import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle, DialogTrigger } from '@/components/ui/dialog'; +const [showConfirm, setShowConfirm] = useState(false); - if (!window.confirm(confirmText)) return; + setShowConfirm(true);Then render a proper confirmation dialog in the component JSX.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/mail/components/mail/empty-folder-button.tsx(1 hunks)apps/mail/components/mail/mail-list.tsx(2 hunks)apps/mail/components/mail/mail.tsx(2 hunks)docker-compose.db.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
docker-compose.db.yaml (1)
Learnt from: adiologydev
PR: Mail-0/Zero#871
File: docker-compose.yaml:48-53
Timestamp: 2025-05-04T19:53:46.788Z
Learning: In docker-compose.yaml, the PGDATA environment variable for PostgreSQL containers should be properly indented under the environment block. The current indentation in the project is correct.
apps/mail/components/mail/mail.tsx (2)
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
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/empty-folder-button.tsx (2)
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:12-12
Timestamp: 2025-04-07T20:46:11.697Z
Learning: In the Mail-0/Zero application, sender emails are guaranteed to be non-empty when passed to components that handle them, making additional empty string validation unnecessary.
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.
🪛 YAMLlint (1.37.1)
docker-compose.db.yaml
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (2)
apps/mail/components/mail/mail.tsx (1)
70-70: Clean integration of new bulk action features.The import and conditional rendering logic are well-implemented. The separation between bulk selection actions and folder-specific actions creates a clear and intuitive user interface.
Also applies to: 523-532
apps/mail/components/mail/mail-list.tsx (1)
61-61: Well-implemented bulk selection checkbox.The checkbox integration is clean with proper event handling, state management, and user interaction. The event propagation prevention and state synchronization are correctly implemented.
Also applies to: 424-443
| volumes: | ||
| valkey-data: | ||
| postgres-data: | ||
| postgres-data: No newline at end of file |
There was a problem hiding this comment.
Add missing newline at end of file.
The YAMLlint tool correctly identified that the file should end with a newline character for POSIX compliance and better Git handling.
Apply this diff to fix the formatting issue:
- postgres-data:
+ postgres-data:
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 39-39: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In docker-compose.db.yaml at line 39, the file is missing a newline character at
the end. Add a single newline character after the last line to ensure POSIX
compliance and proper handling by Git and YAMLlint.
| const handleEmptyFolder = useCallback(async () => { | ||
| if (isLoading) return; | ||
|
|
||
| const confirmText = | ||
| folder === 'spam' | ||
| ? 'This will delete all emails in the Spam folder and move them to Bin. Continue?' | ||
| : 'This will permanently delete all emails in the Bin folder. Continue?'; | ||
|
|
||
| if (!window.confirm(confirmText)) return; | ||
|
|
||
| setIsLoading(true); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and rate limiting.
The function lacks folder validation and protection against rapid successive clicks.
Add proper validation and debouncing:
const handleEmptyFolder = useCallback(async () => {
- if (isLoading) return;
+ if (isLoading || !['spam', 'bin'].includes(folder)) return;
const confirmText =
folder === 'spam'
? 'This will delete all emails in the Spam folder and move them to Bin. Continue?'
: 'This will permanently delete all emails in the Bin folder. Continue?';📝 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 handleEmptyFolder = useCallback(async () => { | |
| if (isLoading) return; | |
| const confirmText = | |
| folder === 'spam' | |
| ? 'This will delete all emails in the Spam folder and move them to Bin. Continue?' | |
| : 'This will permanently delete all emails in the Bin folder. Continue?'; | |
| if (!window.confirm(confirmText)) return; | |
| setIsLoading(true); | |
| const handleEmptyFolder = useCallback(async () => { | |
| if (isLoading || !['spam', 'bin'].includes(folder)) return; | |
| const confirmText = | |
| folder === 'spam' | |
| ? 'This will delete all emails in the Spam folder and move them to Bin. Continue?' | |
| : 'This will permanently delete all emails in the Bin folder. Continue?'; | |
| if (!window.confirm(confirmText)) return; | |
| setIsLoading(true); |
🤖 Prompt for AI Agents
In apps/mail/components/mail/empty-folder-button.tsx around lines 17 to 27, the
handleEmptyFolder function lacks validation for the folder value and does not
prevent rapid successive clicks. Add validation to ensure the folder variable
only accepts expected values like 'spam' or 'bin'. Implement debouncing or a
rate-limiting mechanism to ignore or delay repeated clicks while the function is
already processing, preventing multiple simultaneous executions.
| q: '', | ||
| max: MAX_PER_PAGE, | ||
| cursor, | ||
| } as any); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unsafe type assertion.
The as any cast bypasses TypeScript's type safety and could lead to runtime errors if the API contract changes.
Define a proper type for the query parameters:
+interface ListThreadsParams {
+ folder: string;
+ q: string;
+ max: number;
+ cursor: string;
+}
- const page = await trpcClient.mail.listThreads.query({
- folder,
- q: '',
- max: MAX_PER_PAGE,
- cursor,
- } as any);
+ const page = await trpcClient.mail.listThreads.query({
+ folder,
+ q: '',
+ max: MAX_PER_PAGE,
+ cursor,
+ } satisfies ListThreadsParams);📝 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.
| } as any); | |
| // Define a proper parameter type | |
| interface ListThreadsParams { | |
| folder: string; | |
| q: string; | |
| max: number; | |
| cursor: string; | |
| } | |
| // …inside your async function… | |
| - const page = await trpcClient.mail.listThreads.query({ | |
| - folder, | |
| - q: '', | |
| - max: MAX_PER_PAGE, | |
| - cursor, | |
| - } as any); | |
| + const page = await trpcClient.mail.listThreads.query({ | |
| + folder, | |
| + q: '', | |
| + max: MAX_PER_PAGE, | |
| + cursor, | |
| + } satisfies ListThreadsParams); |
🤖 Prompt for AI Agents
In apps/mail/components/mail/empty-folder-button.tsx at line 39, remove the
unsafe 'as any' type assertion used for query parameters. Instead, define and
use a proper TypeScript type or interface that accurately represents the
expected shape of the query parameters to ensure type safety and prevent
potential runtime errors.
| while (true) { | ||
| // Fetch mails in pages of 100 until there is no nextPageToken | ||
| const page = await trpcClient.mail.listThreads.query({ | ||
| folder, | ||
| q: '', | ||
| max: MAX_PER_PAGE, | ||
| cursor, | ||
| } as any); | ||
| if (page?.threads?.length) { | ||
| ids.push(...page.threads.map((t: { id: string }) => t.id)); | ||
| } | ||
| if (!page?.nextPageToken) break; | ||
| cursor = page.nextPageToken; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add safety mechanism for pagination loop.
The while loop could theoretically run indefinitely if the backend returns inconsistent pagination data, leading to an infinite loop.
Add a safety counter to prevent infinite loops:
const ids: string[] = [];
let cursor = '';
const MAX_PER_PAGE = 100;
+ const MAX_PAGES = 1000; // Safety limit
+ let pageCount = 0;
while (true) {
+ if (++pageCount > MAX_PAGES) {
+ console.warn('Maximum page limit reached, stopping pagination');
+ break;
+ }
// Fetch mails in pages of 100 until there is no nextPageToken📝 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.
| while (true) { | |
| // Fetch mails in pages of 100 until there is no nextPageToken | |
| const page = await trpcClient.mail.listThreads.query({ | |
| folder, | |
| q: '', | |
| max: MAX_PER_PAGE, | |
| cursor, | |
| } as any); | |
| if (page?.threads?.length) { | |
| ids.push(...page.threads.map((t: { id: string }) => t.id)); | |
| } | |
| if (!page?.nextPageToken) break; | |
| cursor = page.nextPageToken; | |
| } | |
| const ids: string[] = []; | |
| let cursor = ''; | |
| const MAX_PER_PAGE = 100; | |
| + const MAX_PAGES = 1000; // Safety limit | |
| + let pageCount = 0; | |
| while (true) { | |
| + if (++pageCount > MAX_PAGES) { | |
| + console.warn('Maximum page limit reached, stopping pagination'); | |
| + break; | |
| + } | |
| // Fetch mails in pages of 100 until there is no nextPageToken | |
| const page = await trpcClient.mail.listThreads.query({ | |
| folder, | |
| q: '', | |
| max: MAX_PER_PAGE, | |
| cursor, | |
| } as any); | |
| if (page?.threads?.length) { | |
| ids.push(...page.threads.map((t: { id: string }) => t.id)); | |
| } | |
| if (!page?.nextPageToken) break; | |
| cursor = page.nextPageToken; | |
| } |
🤖 Prompt for AI Agents
In apps/mail/components/mail/empty-folder-button.tsx around lines 32 to 45, the
while loop fetching paginated mail threads lacks a safety mechanism and could
run indefinitely if pagination data is inconsistent. Add a counter variable
initialized before the loop and increment it each iteration; break the loop if
the counter exceeds a reasonable maximum (e.g., 100) to prevent infinite
looping.
|
outdated, please address comments and reopen |
ill get it donw |
2 participants
READ CAREFULLY THEN REMOVE
Remove bullet points that are not relevant.
PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.
Fixes #1560
Description
Adds two productivity features to the mailbox:
Checkbox Selection – Every thread row now includes a checkbox so users can multi-select messages with a click. The selection synchronises with the existing mail.bulkSelected state and respects keyboard modifiers (Ctrl/Cmd, Shift, Alt+Shift).
“Empty Bin / Empty Spam” Button – When the user is in the Bin or Spam folder, a contextual red button appears in the header.
Spam → moves all conversations to Bin
Bin → permanently deletes all conversations
A confirmation dialog and toast feedback are provided.
No new backend routes were required; implementation re-uses existing TRPC endpoints and optimistic-action helpers.
Type of Change
Please delete options that are not relevant.
Areas Affected
Please check all that apply:
Testing Done
Describe the tests you've done:
Security Considerations
For changes involving data or authentication:
Checklist
Additional Notes
Add any other context about the pull request here.
Screenshots/Recordings
working.mp4
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
New Features
Style