feat(mail): Email selection checkboxes and Empty Folder functionality#1642
feat(mail): Email selection checkboxes and Empty Folder functionality#1642hackice20 wants to merge 18 commits intoMail-0:stagingfrom hackice20:feat/inbox-selection-updated
Conversation
- Add individual checkboxes to each email row for bulk selection - Integrate with existing bulk selection system and keyboard shortcuts - Add Empty Folder button for Spam and Bin folders - Spam folder: moves all emails to Bin - Bin folder: permanently deletes all emails - Include input validation, rate limiting, and safety mechanisms - Built on latest staging with all improvements and translations
|
""" WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailToolbar
participant EmptyFolderButton
participant MailAPI
participant Toast
User->>MailToolbar: Opens Spam or Bin folder
MailToolbar->>EmptyFolderButton: Renders button (if folder is spam/bin)
User->>EmptyFolderButton: Clicks "Empty Folder" button
EmptyFolderButton->>User: Prompts for confirmation
User->>EmptyFolderButton: Confirms action
loop For each batch (up to 1000)
EmptyFolderButton->>MailAPI: Fetch threads (batch)
EmptyFolderButton->>MailAPI: Move or delete threads
end
EmptyFolderButton->>Toast: Show success/error notification
sequenceDiagram
participant User
participant MailList
participant MailState
User->>MailList: Clicks checkbox on mail thread
MailList->>MailState: Toggle thread ID in bulkSelected array
MailList->>MailList: Prevents event propagation
Estimated code review effort3 (~45 minutes) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ 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
Enhanced email management with bulk selection and folder cleanup functionality in the mail interface.
- Added
apps/mail/components/mail/empty-folder-button.tsxcomponent with safety confirmations and pagination for emptying Spam/Bin folders - Integrated checkbox selection in
apps/mail/components/mail/mail-list.tsxwith existing bulk selection system and keyboard modifiers (Ctrl/Cmd, Shift) - Modified
apps/mail/components/mail/mail.tsxto support the new empty folder button and bulk selection features - Implemented validation and rate limiting for folder operations with optimistic updates
3 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile
| {/* Selection checkbox */} | ||
| <Checkbox | ||
| className="mr-2 h-4 w-4" | ||
| checked={isMailBulkSelected} | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| }} | ||
| onCheckedChange={() => { | ||
| if (!idToUse) return; | ||
| setMail((prev) => { | ||
| const already = prev.bulkSelected.includes(idToUse); | ||
| return { | ||
| ...prev, | ||
| bulkSelected: already | ||
| ? prev.bulkSelected.filter((id) => id !== idToUse) | ||
| : [...prev.bulkSelected, idToUse], | ||
| }; | ||
| }); | ||
| }} | ||
| /> |
There was a problem hiding this comment.
logic: Missing keyboard shortcut handler. The checkbox should be keyboard accessible via space/enter when focused
apps/mail/components/mail/mail.tsx
Outdated
| {/* Bulk action buttons when emails are selected */} | ||
| {mail.bulkSelected.length > 0 && ( | ||
| <div className="dark:bg-iconDark/20 relative ml-2 rounded-full" aria-hidden> | ||
| <BulkSelectActions /> | ||
| </div> | ||
| )} | ||
| {/* Empty folder actions for Spam & Bin */} | ||
| {['spam', 'bin'].includes(folder) && ( | ||
| <EmptyFolderButton folder={folder} /> | ||
| )} |
There was a problem hiding this comment.
logic: New Empty Folder button lacks disabled state while action is in progress - could lead to multiple simultaneous folder clearing attempts
| {/* Bulk action buttons when emails are selected */} | |
| {mail.bulkSelected.length > 0 && ( | |
| <div className="dark:bg-iconDark/20 relative ml-2 rounded-full" aria-hidden> | |
| <BulkSelectActions /> | |
| </div> | |
| )} | |
| {/* Empty folder actions for Spam & Bin */} | |
| {['spam', 'bin'].includes(folder) && ( | |
| <EmptyFolderButton folder={folder} /> | |
| )} | |
| {/* Bulk action buttons when emails are selected */} | |
| {mail.bulkSelected.length > 0 && ( | |
| <div className="dark:bg-iconDark/20 relative ml-2 rounded-full" aria-hidden> | |
| <BulkSelectActions /> | |
| </div> | |
| )} | |
| {/* Empty folder actions for Spam & Bin */} | |
| {['spam', 'bin'].includes(folder) && ( | |
| <EmptyFolderButton folder={folder} loading={isPending} /> | |
| )} |
|
@MrgSub updated |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| if (!ids.length) { | ||
| toast.success('Folder already empty'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The loading state should be reset before returning when the folder is already empty. Consider updating to:
if (!ids.length) {
toast.success('Folder already empty');
setIsLoading(false);
return;
}This ensures the button doesn't remain in a loading state when there's nothing to process.
| if (!ids.length) { | |
| toast.success('Folder already empty'); | |
| return; | |
| } | |
| if (!ids.length) { | |
| toast.success('Folder already empty'); | |
| setIsLoading(false); | |
| return; | |
| } |
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: 0
🧹 Nitpick comments (1)
apps/mail/components/mail/empty-folder-button.tsx (1)
24-87: Consider improving pagination efficiency and user experience.The current implementation has several areas for potential improvement:
Pagination efficiency: Loading all threads upfront could be memory-intensive for large folders. Consider implementing batched processing where you process and delete threads in smaller chunks.
User experience:
window.confirm()provides a basic confirmation dialog. Consider using a proper Modal/Dialog component for better UX and consistency with the design system.Safety limits: The
MAX_PAGESlimit of 1000 pages (100k emails) might be insufficient for very large corporate mailboxes.Consider this alternative approach for better efficiency:
// Process in smaller batches instead of loading everything upfront const BATCH_SIZE = 50; let processedCount = 0; while (true) { const page = await trpcClient.mail.listThreads.query({ folder, q: '', max: BATCH_SIZE, cursor, }); if (page?.threads?.length) { const batchIds = page.threads.map((t: { id: string }) => t.id); // Process this batch immediately if (folder === 'spam') { await optimisticMoveThreadsTo(batchIds, folder, 'bin'); } else { await optimisticDeleteThreads(batchIds, folder); } processedCount += batchIds.length; // Show progress to user toast.loading(`Processed ${processedCount} emails...`); } if (!page?.nextPageToken) break; cursor = page.nextPageToken; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
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)
🧰 Additional context used
🧠 Learnings (4)
📓 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.
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.
apps/mail/components/mail/mail-list.tsx (3)
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: 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#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.731Z
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.
apps/mail/components/mail/empty-folder-button.tsx (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: 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.
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.
🧬 Code Graph Analysis (2)
apps/mail/components/mail/empty-folder-button.tsx (3)
apps/mail/hooks/use-optimistic-actions.ts (1)
useOptimisticActions(34-471)apps/mail/providers/query-provider.tsx (1)
trpcClient(86-103)apps/mail/components/ui/button.tsx (1)
Button(71-71)
apps/mail/components/mail/mail.tsx (1)
apps/mail/components/mail/empty-folder-button.tsx (1)
EmptyFolderButton(20-100)
🔇 Additional comments (4)
apps/mail/components/mail/mail-list.tsx (1)
415-434: LGTM: Clean checkbox implementation for bulk selection.The checkbox implementation correctly integrates with the existing
bulkSelectedstate and follows good React patterns. The event propagation is properly stopped to prevent triggering the thread click handler, and the selection toggle logic is sound.apps/mail/components/mail/empty-folder-button.tsx (2)
27-30: Strong input validation for security.Good security practice to validate the folder parameter and restrict functionality to only spam and bin folders. This prevents misuse of the component.
43-52: Good safety mechanism for pagination loop.The page count limit prevents infinite loops and provides a safety net. The limit of 1000 pages (potentially 100k emails) should handle most real-world scenarios.
apps/mail/components/mail/mail.tsx (1)
519-528: Clean integration of new UI components.The conditional rendering logic is well-implemented:
BulkSelectActionsonly appears when emails are selected (mail.bulkSelected.length > 0)EmptyFolderButtononly appears for spam and bin folders- Components are properly positioned within the existing toolbar structure
The integration follows existing patterns in the codebase and maintains good separation of concerns.
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
| try { | ||
| const page = await trpcClient.mail.listThreads.query({ | ||
| folder, | ||
| q: '', |
There was a problem hiding this comment.
this should use the current query no?
|
|
||
| setIsLoading(true); | ||
| try { | ||
| const ids: string[] = []; |
| try { | ||
| const page = await trpcClient.mail.listThreads.query({ |
There was a problem hiding this comment.
please move this logic to the server
- Use Set instead of string[] for better performance - Add setIsLoading(false) when folder is already empty - Use current search query instead of empty string - Fix error type from any to proper error shape - Remove unused Trash import
- Keep Set approach for better performance - Keep current search query instead of empty string - Add try-catch for pagination error handling - Maintain all code review improvements
|
UI looks like it needs work, will review after |
|
@MrgSub pls look at the video one more time. |
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (3)
apps/mail/messages/lv.json (3)
166-179: Add numeric placeholder to keep pluralised messages consistent
noteCount(below) and most other counters include the#placeholder so the rendered string shows the actual number (“1 piezīme”,“3 piezīmes”, …).
attachmentCountomits it, so users will only see the word “pielikums/-i” with no number, which is inconsistent with the rest of the UI."countPlural=0": "pielikumi", - "countPlural=one": "pielikums", - "countPlural=other": "pielikumi" + "countPlural=one": "# pielikums", + "countPlural=other": "# pielikumi"
184-195: Same issue forfileCount– the number is missingReplicate the fix suggested for
attachmentCountso the string reads e.g. “2 faili” instead of only “faili”."countPlural=0": "faili", - "countPlural=one": "fails", - "countPlural=other": "faili" + "countPlural=one": "# fails", + "countPlural=other": "# faili"
330-341: Add missingopenInNewTabtranslation in Latvian localeThe component at
apps/mail/components/context/thread-context.tsx:276still callsm['common.mail.openInNewTab']()but
apps/mail/messages/lv.jsonno longer defines that key—so at runtime you’ll see the raw lookup instead of a translated string.Please:
- In
apps/mail/messages/lv.json, under thecommon.mailsection, re-add:"openInNewTab": "Atvērt jaunā cilnē"- Run
rg -n "openInNewTab"to confirm no unresolved references remain.- If removal was intentional, instead update
thread-context.tsxto stop using this key.
♻️ Duplicate comments (3)
apps/mail/messages/es.json (3)
184-190: Duplicate of the comment above – same declaration/selector pattern, same verdict.
262-268: Duplicate of the comment above – same declaration/selector pattern, same verdict.
330-336: Duplicate of the comment above – same declaration/selector pattern, same verdict.
🧹 Nitpick comments (8)
apps/mail/messages/zh_TW.json (1)
182-191: Consider adding the numeric placeholder to stay consistent with other plural forms
fileCountomits the#placeholder whilenoteCount/replieskeep it ("# 檔案"vs"# 筆記"). Unless the design deliberately hides the number here, consider:- "countPlural=one": "檔案", - "countPlural=other": "檔案" + "countPlural=one": "# 個檔案", + "countPlural=other": "# 個檔案"This keeps UX consistent across counters.
apps/mail/messages/ja.json (2)
168-174: Redundant plural branches – simplify to a single “添付ファイル” stringAll three plural branches (
0,one,other) resolve to the exact same text, so the plural-rules machinery is never providing differentiated output. Consider replacing the entire plural block with a single plain string or drop the pluralisation layer:- "attachmentCount": [ - { - "declarations": [ … ], - "selectors": [ … ], - "match": { - "countPlural=0": "添付ファイル", - "countPlural=one": "添付ファイル", - "countPlural=other": "添付ファイル" - } - } - ], + "attachmentCount": "添付ファイル",Same comment applies to
fileCountbelow.
184-190: Duplicate plural mapping – same output for every branch
fileCountrepeats the exact same string across all plural cases. Unless you plan to differentiate later, remove the plural wrapper as suggested forattachmentCount.apps/mail/messages/hi.json (2)
168-174: Improve plural-form translations for “attachment”“अटैचमेंटस” is not a natural Hindi plural; “अटैचमेंट्स” is the widely accepted transliteration.
Proposed patch:- "countPlural=0": "अटैचमेंटस" - "countPlural=one": "अटैचमेंट" - "countPlural=other": "अटैचमेंटस" + "countPlural=0": "अटैचमेंट्स" + "countPlural=one": "अटैचमेंट" + "countPlural=other": "अटैचमेंट्स"
184-190: Fix singular spelling for “file”The current singular form
"फाइल्"ends with a halant and is clipped. Use"फ़ाइल"(with nukta) for correct spelling.- "countPlural=one": "फाइल्" + "countPlural=one": "फ़ाइल"apps/mail/components/ui/confirmation-dialog.tsx (2)
12-17: Consider enhancing the Props interface.The current Props interface is functional but could be improved for better type safety and flexibility:
interface Props { title: string; description: string; onConfirm: () => void; trigger: React.ReactNode; + confirmText?: string; + cancelText?: string; + confirmVariant?: "default" | "destructive" | "outline" | "secondary" | "ghost" | "link"; + isLoading?: boolean; }This would allow customization of button text and variants for different use cases.
29-31: Ensure dialog closes after Cancel button click.The Cancel button uses
DialogTriggerwithasChildwhich should close the dialog, but consider adding explicit close handling for better control:- <DialogTrigger asChild> - <Button variant="outline">Cancel</Button> - </DialogTrigger> + <DialogTrigger asChild> + <Button variant="outline" type="button">Cancel</Button> + </DialogTrigger>Adding
type="button"prevents form submission if this dialog is used within forms.apps/mail/components/mail/bulk-select-actions.tsx (1)
9-12: Consider removing unused LABELS constant.The
LABELSconstant is defined but never used in the component. Either utilize it or remove it to keep the code clean:-const LABELS = { - SPAM: 'spam', - TRASH: 'bin', -}; -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
apps/mail/components/mail/bulk-select-actions.tsx(1 hunks)apps/mail/components/mail/empty-folder-button.tsx(1 hunks)apps/mail/components/mail/mail.tsx(2 hunks)apps/mail/components/ui/confirmation-dialog.tsx(1 hunks)apps/mail/config/shortcuts.ts(1 hunks)apps/mail/messages/ar.json(5 hunks)apps/mail/messages/ca.json(5 hunks)apps/mail/messages/cs.json(5 hunks)apps/mail/messages/de.json(5 hunks)apps/mail/messages/en.json(5 hunks)apps/mail/messages/es.json(5 hunks)apps/mail/messages/fa.json(5 hunks)apps/mail/messages/fr.json(5 hunks)apps/mail/messages/hi.json(5 hunks)apps/mail/messages/hu.json(5 hunks)apps/mail/messages/ja.json(5 hunks)apps/mail/messages/ko.json(5 hunks)apps/mail/messages/lv.json(5 hunks)apps/mail/messages/nl.json(5 hunks)apps/mail/messages/pl.json(5 hunks)apps/mail/messages/pt.json(5 hunks)apps/mail/messages/ru.json(5 hunks)apps/mail/messages/tr.json(5 hunks)apps/mail/messages/vi.json(5 hunks)apps/mail/messages/zh_CN.json(5 hunks)apps/mail/messages/zh_TW.json(4 hunks)docker-compose.db.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- apps/mail/messages/ko.json
- apps/mail/messages/fa.json
- apps/mail/messages/pl.json
- apps/mail/messages/cs.json
- apps/mail/messages/fr.json
- apps/mail/messages/nl.json
- apps/mail/messages/zh_CN.json
- apps/mail/messages/ca.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/mail/components/mail/empty-folder-button.tsx
- apps/mail/components/mail/mail.tsx
🧰 Additional context used
🧠 Learnings (17)
📓 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.
apps/mail/messages/zh_TW.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/es.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/config/shortcuts.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.
apps/mail/messages/hi.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/en.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/ar.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/de.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/ja.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/vi.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/hu.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/tr.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/components/mail/bulk-select-actions.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/messages/ru.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/pt.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
apps/mail/messages/lv.json (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1734
File: apps/server/src/lib/driver/google.ts:211-221
Timestamp: 2025-07-15T06:46:33.349Z
Learning: In apps/server/src/lib/driver/google.ts, the normalization of "draft" to "drafts" in the count() method is necessary because the navigation item in apps/mail/config/navigation.ts has id: 'drafts' (plural) while the Google API returns "draft" (singular). The nav-main.tsx component matches stats by comparing stat.label with item.id, so the backend must return "drafts" for the draft counter badge to appear in the sidebar.
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.
🧬 Code Graph Analysis (1)
apps/mail/components/ui/confirmation-dialog.tsx (2)
apps/mail/components/ui/dialog.tsx (7)
Dialog(107-107)DialogTrigger(111-111)DialogContent(112-112)DialogHeader(113-113)DialogTitle(115-115)DialogDescription(116-116)DialogFooter(114-114)apps/mail/components/ui/button.tsx (1)
Button(71-71)
🪛 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 (20)
apps/mail/messages/zh_TW.json (4)
166-175: 👍 Readability boost looks goodThe expanded
declarations/selectorsblocks make the ICU message clearer without touching runtime behaviour.
No further action needed.
260-269: Consistent pattern – looks fine
noteCountmirrors the same plural-rule structure adopted elsewhere in the file. No issues spotted.
328-337: Plural rule formatting OK, but verify placeholderAs with
noteCount, make sure the consuming code replaces#with the numeric value; otherwise users will literally see “# 則回覆”.
If that substitution is already handled globally, you’re good.
392-395:openInNewTabkey still present – conflicts with PR summaryThe AI summary says this key was removed across all locales, yet it remains here. Confirm whether it should stay or be deleted to avoid dead strings.
Likely an incorrect or invalid review comment.
apps/mail/messages/hu.json (1)
168-179: Add the numeric placeholder to the pluralised attachment / file stringsThe 0/one/other variants for both
attachmentCountandfileCountomit the#placeholder that is present in the equivalent English entries and that you still use fornoteCount/repliesbelow.
Without the placeholder the UI will render just “csatolmány” / “fájl”, i.e. the number is silently dropped.- "countPlural=0": "csatolmány", - "countPlural=one": "csatolmány", - "countPlural=other": "csatolmány" + "countPlural=0": "# csatolmány", + "countPlural=one": "# csatolmány", + "countPlural=other": "# csatolmány"(and the analogous three-line change for
fileCount)If the count is intentionally rendered elsewhere (e.g. a badge next to the label) please double-check; otherwise the placeholder should be reinstated for consistency with the rest of the locale files.
Also applies to: 184-197
apps/mail/messages/hi.json (1)
393-393: Translation looks goodThe updated key
failedToArchiveremains consistent with the rest of the error messages. No action needed.apps/mail/messages/vi.json (1)
262-270: 👍noteCountkeeps the#placeholder
noteCountcorrectly exposes the count value and even provides a distinct zero-form (“Thêm ghi chú”). Looks good.apps/mail/messages/lv.json (2)
262-268: LGTM – declarations/selectors block correctly addedThe plural-rules boilerplate matches the updated inlang message-format syntax.
No further action required here.
393-394: Minor move looks correctKey has simply been re-ordered; string unchanged.
No issues spotted.apps/mail/config/shortcuts.ts (1)
36-36: LGTM! Excellent defensive programming.The navigator availability checks prevent runtime errors in server-side rendering environments and other contexts where the
navigatorobject might be undefined. This is a solid improvement for cross-environment compatibility.Also applies to: 43-43
apps/mail/messages/en.json (1)
168-174: LGTM! Improved readability for pluralization config.The expansion of declarations and selectors arrays from single-line to multi-line format significantly improves readability without changing the functional behavior. This is good maintenance for large localization files.
Also applies to: 184-190, 262-268, 330-336
apps/mail/messages/pt.json (1)
168-174: LGTM! Improved readability for pluralization config.The expansion of declarations and selectors arrays to multi-line format improves readability without changing functionality.
Also applies to: 184-190, 262-268
apps/mail/messages/de.json (1)
168-174: LGTM! Improved readability and correct translations.The multi-line formatting of pluralization arrays improves readability, and the German translations are correct. The "replies" section properly uses "Antwort/Antworten" for singular/plural forms.
Also applies to: 184-190, 262-268, 330-336
apps/mail/messages/ar.json (1)
168-174: LGTM: Improved readability of pluralization arrays.The reformatting of pluralization declarations and selectors from single-line to multi-line arrays significantly improves readability and maintainability of the localization file.
Also applies to: 184-190, 262-268, 330-336
apps/mail/messages/ru.json (1)
168-174: LGTM: Consistent formatting improvements.The multi-line array formatting for pluralization entries matches the pattern established in other localization files and improves code readability.
Also applies to: 184-190, 262-268, 330-336
apps/mail/components/mail/bulk-select-actions.tsx (1)
54-68: LGTM: Excellent accessibility implementation.The component properly implements accessibility with:
- Screen reader labels using
sr-onlyclass- Descriptive
titleattributes for tooltips- Semantic button elements with appropriate ARIA
apps/mail/messages/tr.json (2)
168-174: LGTM: Consistent pluralization formatting.The multi-line array formatting maintains consistency across all localization files and improves maintainability.
Also applies to: 184-190, 262-268, 330-336
272-272: Good implementation of Turkish pluralization.The Turkish pluralization correctly uses distinct forms: "# not" (singular) and "# notlar" (plural) for notes, and "# cevap" (singular) and "# cevaplar" (plural) for replies. This appears more linguistically appropriate than the other language files.
Also applies to: 339-340
apps/mail/messages/es.json (2)
168-174: Plural-rules declarations & selectors look correctExplicitly declaring
countandcountPluraland wiring the selector improves readability and prevents future tooling from guessing implicit variables. No issues spotted.
393-393: No missing Spanish translations for “Empty Folder”We’ve verified that
apps/mail/messages/es.jsonmatches all keys present in the English locale undercommon.mail. The only “empty…” entries inen.jsonare:
common.mail.emptycommon.mail.emptyDescriptionNo
emptyFolder,emptyBin, confirmation-dialog titles, or similar keys exist in the English locale, so there are no corresponding Spanish keys to add.Please ensure the PR first introduces the new English
common.mailkeys for the “Empty Folder” feature before localizing them.
| volumes: | ||
| valkey-data: | ||
| postgres-data: | ||
| postgres-data: No newline at end of file |
There was a problem hiding this comment.
Re-introduce trailing newline to satisfy linters
The change removes the terminating newline, tripping YAML lint (new-line-at-end-of-file) and potentially breaking CI hooks that enforce POSIX newline conventions. Add the newline back.
postgres-data:
+📝 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.
| postgres-data: | |
| postgres-data: | |
🧰 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 trailing newline at
the end, which causes YAML lint errors and may break CI hooks. Add a newline
character after the last line to restore the proper file ending and satisfy the
linter requirements.
| "declarations": [ | ||
| "input count", | ||
| "local countPlural = count: plural" | ||
| ], | ||
| "selectors": [ | ||
| "countPlural" | ||
| ], | ||
| "match": { | ||
| "countPlural=0": "Add notes", | ||
| "countPlural=one": "# 件のメモ", | ||
| "countPlural=other": "# 件のメモ" | ||
| "countPlural=one": "# note", | ||
| "countPlural=other": "# notes" | ||
| } |
There was a problem hiding this comment.
English strings slipped into Japanese locale – restore JA translations
noteCount now shows English text ("# note", "# notes"). This breaks language consistency and looks like a copy-paste artefact.
- "countPlural=0": "Add notes",
- "countPlural=one": "# note",
- "countPlural=other": "# notes"
+ "countPlural=0": "メモを追加",
+ "countPlural=one": "# 件のメモ",
+ "countPlural=other": "# 件のメモ"(Feel free to adjust wording; {count} or the implicit # placeholder can be positioned before the unit as Japanese usually does: # 件のメモ).
📝 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.
| "declarations": [ | |
| "input count", | |
| "local countPlural = count: plural" | |
| ], | |
| "selectors": [ | |
| "countPlural" | |
| ], | |
| "match": { | |
| "countPlural=0": "Add notes", | |
| "countPlural=one": "# 件のメモ", | |
| "countPlural=other": "# 件のメモ" | |
| "countPlural=one": "# note", | |
| "countPlural=other": "# notes" | |
| } | |
| "declarations": [ | |
| "input count", | |
| "local countPlural = count: plural" | |
| ], | |
| "selectors": [ | |
| "countPlural" | |
| ], | |
| "match": { | |
| - "countPlural=0": "Add notes", | |
| - "countPlural=one": "# note", | |
| - "countPlural=other": "# notes" | |
| + "countPlural=0": "メモを追加", | |
| + "countPlural=one": "# 件のメモ", | |
| + "countPlural=other": "# 件のメモ" | |
| } |
🤖 Prompt for AI Agents
In apps/mail/messages/ja.json around lines 262 to 273, the strings for noteCount
are in English ("# note", "# notes") instead of Japanese. Replace these English
strings with appropriate Japanese translations, such as using the placeholder
"#" followed by "件のメモ" to maintain natural Japanese phrasing and language
consistency.
| "declarations": [ | ||
| "input count", | ||
| "local countPlural = count: plural" | ||
| ], | ||
| "selectors": [ | ||
| "countPlural" | ||
| ], | ||
| "match": { | ||
| "countPlural=0": "返信", | ||
| "countPlural=one": "# 件の返信", | ||
| "countPlural=other": "# 件の返信" | ||
| "countPlural=one": "返信 #個", | ||
| "countPlural=other": "返信 #個" |
There was a problem hiding this comment.
Mixed language & unnatural counter – fix “返信 #個”
replies uses English order and an odd counter suffix “個”. Suggested correction:
- "countPlural=0": "返信",
- "countPlural=one": "返信 #個",
- "countPlural=other": "返信 #個"
+ "countPlural=0": "返信なし",
+ "countPlural=one": "# 件の返信",
+ "countPlural=other": "# 件の返信"This restores full Japanese phrasing and keeps plural handling minimal (Japanese rarely distinguishes plural, but the count placeholder is still helpful).
📝 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.
| "declarations": [ | |
| "input count", | |
| "local countPlural = count: plural" | |
| ], | |
| "selectors": [ | |
| "countPlural" | |
| ], | |
| "match": { | |
| "countPlural=0": "返信", | |
| "countPlural=one": "# 件の返信", | |
| "countPlural=other": "# 件の返信" | |
| "countPlural=one": "返信 #個", | |
| "countPlural=other": "返信 #個" | |
| "declarations": [ | |
| "input count", | |
| "local countPlural = count: plural" | |
| ], | |
| "selectors": [ | |
| "countPlural" | |
| ], | |
| "match": { | |
| "countPlural=0": "返信なし", | |
| "countPlural=one": "# 件の返信", | |
| "countPlural=other": "# 件の返信" |
🤖 Prompt for AI Agents
In apps/mail/messages/ja.json around lines 330 to 340, the translation for
replies uses an unnatural mix of English order and the counter suffix "個" which
is not appropriate for Japanese. Replace "返信 #個" with a natural Japanese
phrasing that omits the counter suffix and keeps the count placeholder, ensuring
the text reads naturally in Japanese while maintaining plural handling.
| "declarations": [ | ||
| "input count", | ||
| "local countPlural = count: plural" | ||
| ], | ||
| "selectors": [ | ||
| "countPlural" | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expose reply count value instead of literal #
Same issue as above for replies. Substitute the placeholder with {count} so users see the real number.
- "countPlural=one": "# जवाब"
- "countPlural=other": "# जवाब"
+ "countPlural=one": "{count} जवाब"
+ "countPlural=other": "{count} जवाब"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/mail/messages/hi.json around lines 330 to 336, the placeholder for reply
count is currently a literal `#` instead of the actual count value. Replace the
literal `#` with the variable `{count}` in the message string so that the real
number of replies is displayed to users.
| "declarations": [ | ||
| "input count", | ||
| "local countPlural = count: plural" | ||
| ], | ||
| "selectors": [ | ||
| "countPlural" | ||
| ], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use numeric placeholder for note count
The strings in noteCount still contain the # placeholder. Replace it with {count} to surface the actual number.
- "countPlural=one": "# नोट"
- "countPlural=other": "# नोट्स"
+ "countPlural=one": "{count} नोट"
+ "countPlural=other": "{count} नोट्स"📝 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.
| "declarations": [ | |
| "input count", | |
| "local countPlural = count: plural" | |
| ], | |
| "selectors": [ | |
| "countPlural" | |
| ], | |
| "declarations": [ | |
| "input count", | |
| "local countPlural = count: plural" | |
| ], | |
| "selectors": [ | |
| "countPlural" | |
| ], | |
| "countPlural=one": "{count} नोट", | |
| "countPlural=other": "{count} नोट्स" |
🤖 Prompt for AI Agents
In apps/mail/messages/hi.json around lines 262 to 268, the noteCount strings use
the '#' placeholder for the count, which should be replaced with the numeric
placeholder '{count}'. Update all occurrences of '#' in the noteCount strings to
'{count}' to correctly display the actual number.
| "countPlural=0": "إضافة ملاحظات", | ||
| "countPlural=one": "ملاحظة واحدة", | ||
| "countPlural=other": "# ملاحظات" | ||
| "countPlural=other": "# ملاحظة" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify pluralization form changes for Arabic.
The pluralization strings for "noteCount" and "replies" were changed from plural forms ("# ملاحظات", "# ردود") to singular forms ("# ملاحظة", "# رد") for the "other" category. Please ensure these changes align with Arabic pluralization rules and are consistent with the intended user experience.
Also applies to: 340-340
🏁 Script executed:
#!/bin/bash
# Check if similar singular/plural changes were made consistently across other Arabic localization keys
rg -A 3 -B 3 '"countPlural=other":' apps/mail/messages/ar.jsonLength of output: 774
Update Arabic pluralization for notes and replies
The “other” category for notes and replies currently uses singular nouns, which is inconsistent with both the other entries (attachments, files) and Arabic grammar. Please update to the plural form:
• apps/mail/messages/ar.json
• Change the noteCount entry:
diff - "countPlural=other": "# ملاحظة" + "countPlural=other": "# ملاحظات"
• Change the replies entry:
diff - "countPlural=other": "# رد" + "countPlural=other": "# ردود"
📝 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.
| "countPlural=other": "# ملاحظة" | |
| "countPlural=other": "# ملاحظات" |
| "countPlural=other": "# ملاحظة" | |
| "countPlural=other": "# ردود" |
🤖 Prompt for AI Agents
In apps/mail/messages/ar.json at line 272, the "countPlural=other" entry for
notes and replies uses a singular noun, which is inconsistent with other entries
and Arabic grammar. Update the value to use the correct plural form for both the
noteCount and replies entries to ensure proper pluralization in Arabic.
| "countPlural=0": "Добавьте заметки", | ||
| "countPlural=one": "# заметка", | ||
| "countPlural=other": "# заметок" | ||
| "countPlural=other": "# заметки" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Russian case changes for grammatical correctness.
The pluralization strings were changed from genitive case forms ("# заметок", "# ответов") to nominative case forms ("# заметки", "# ответы"). Please verify that these changes maintain grammatical correctness in Russian when displayed in the UI context.
Also applies to: 340-340
🌐 Web query:
What is the correct Russian case to use for count expressions like "5 заметок" vs "5 заметки" in user interfaces?
💡 Result:
The correct Russian case to use for count expressions like "5 заметок" is the genitive plural. In Russian, when you use numerals greater than 4 (such as 5, 6, 10, and so on), the noun that follows is always put into the genitive plural form[4].
- For example, the correct phrase is "5 заметок" ("заметок" is genitive plural of "заметка").
- The form "5 заметки" is incorrect, as "заметки" is nominative plural and does not match the Russian grammar rules for numerals above 4.
Here is a brief breakdown:
- 1 заметка (nominative singular)
- 2, 3, 4 заметки (genitive singular, a unique rule in Russian)
- 5+ заметок (genitive plural)
This rule is essential for user interfaces and for ensuring natural-sounding Russian text[4][2][3].
Citations:
- 1: https://mangolanguages.com/resources/learn/grammar/russian/how-to-use-the-genitive-case-in-russian-
- 2: https://storylearning.com/learn/russian/russian-tips/russian-genitive-case
- 3: https://www.hackyourrussian.com/post/russian-genitive-case
- 4: https://en.wikipedia.org/wiki/Russian_declension
- 5: https://www.thoughtco.com/russian-cases-4768614
- 6: https://www.russianforfree.com/grammar-of-russian-nouns-genitive.php
- 7: https://www.russianlessons.net/grammar/nouns_genitive.php
Revert plural forms to genitive plural for counts ≥5
Russian numerals above 4 require the noun in genitive plural. The strings for countPlural=other must use “# заметок” and “# ответов,” not “# заметки” or “# ответы.”
Files and locations needing fix:
- apps/mail/messages/ru.json: line 272 (notes), line 340 (answers)
Suggested diff:
--- a/apps/mail/messages/ru.json
+++ b/apps/mail/messages/ru.json
@@ 272,272c272
- "countPlural=other": "# заметки"
+ "countPlural=other": "# заметок"
@@ 340,340c340
- "countPlural=other": "# ответы"
+ "countPlural=other": "# ответов"📝 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.
| "countPlural=other": "# заметки" | |
| --- a/apps/mail/messages/ru.json | |
| +++ b/apps/mail/messages/ru.json | |
| @@ 272,272c272 | |
| - "countPlural=other": "# заметки" | |
| + "countPlural=other": "# заметок" | |
| @@ 340,340c340 | |
| - "countPlural=other": "# ответы" | |
| + "countPlural=other": "# ответов" |
🤖 Prompt for AI Agents
In apps/mail/messages/ru.json at line 272, the plural form for counts greater or
equal to 5 is incorrect; it uses "заметки" instead of the required genitive
plural "заметок." Change the string for "countPlural=other" from "# заметки" to
"# заметок." Also, check line 340 for a similar issue with "ответы" and correct
it to "ответов."
| <DialogTrigger asChild> | ||
| <Button | ||
| variant="destructive" | ||
| onClick={() => { | ||
| onConfirm(); | ||
| }} | ||
| > | ||
| Confirm | ||
| </Button> | ||
| </DialogTrigger> |
There was a problem hiding this comment.
The DialogTrigger wrapping the Confirm button will close the dialog immediately when clicked, potentially before the async onConfirm action completes. This could lead to a confusing user experience where the dialog disappears but the action is still processing.
Consider either:
- Using controlled dialog state with
openandonOpenChangeprops to manually close the dialog after the action completes - Moving the
onConfirm()call outside the button to theonConfirmprop of theDialogContentcomponent - Adding a loading state to prevent multiple clicks while processing
This would ensure users see appropriate feedback during the entire operation.
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/mail/components/ui/confirmation-dialog.tsx (1)
24-32: Consider adding error handling feedback to the user.While the error handling ensures proper cleanup, users won't see any feedback if the
onConfirmoperation fails. Consider adding toast notifications or error state management for better UX.const handleConfirm = async () => { setIsLoading(true); try { await onConfirm(); + // Consider adding success toast here if needed } catch (error) { + // Consider adding error toast or error state handling + console.error('Confirmation action failed:', error); } finally { setIsLoading(false); setIsOpen(false); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/mail/components/mail/empty-folder-button.tsx(1 hunks)apps/mail/components/ui/confirmation-dialog.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/mail/components/mail/empty-folder-button.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 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.
apps/mail/components/ui/confirmation-dialog.tsx (3)
Learnt from: danteissaias
PR: Mail-0/Zero#902
File: apps/mail/components/connection/add.tsx:77-77
Timestamp: 2025-05-07T16:55:46.513Z
Learning: For the "Upgrade" link in AddConnectionDialog, using a proper <button> element instead of a <span> with onClick is recognized as an accessibility improvement but was deferred as out of scope in PR #902 (CSS variables PR).
Learnt from: danteissaias
PR: Mail-0/Zero#618
File: apps/mail/components/mail/mail-iframe.tsx:102-102
Timestamp: 2025-04-07T20:48:48.213Z
Learning: In the Zero mail application, when implementing the "Trust Sender" feature, the banner should disappear immediately when a user clicks the "Trust Sender" button without showing a loading state. This is achieved by setting `setImagesEnabled(true)` at the beginning of the `onTrustSender` function, providing immediate visual feedback while the backend operation continues asynchronously.
Learnt from: AnjanyKumarJaiswal
PR: Mail-0/Zero#1732
File: apps/mail/components/create/email-composer.tsx:634-657
Timestamp: 2025-07-15T03:31:14.991Z
Learning: In draft deletion operations, using setTimeout with a delay (like 500ms) before showing success toast notifications improves UX by allowing UI state changes (like closing composers and clearing IDs) to complete before displaying the toast, preventing jarring immediate toast appearances that could disappear quickly during interface transitions.
🧬 Code Graph Analysis (1)
apps/mail/components/ui/confirmation-dialog.tsx (2)
apps/mail/components/ui/dialog.tsx (7)
Dialog(107-107)DialogTrigger(111-111)DialogContent(112-112)DialogHeader(113-113)DialogTitle(115-115)DialogDescription(116-116)DialogFooter(114-114)apps/mail/components/ui/button.tsx (1)
Button(71-71)
🔇 Additional comments (5)
apps/mail/components/ui/confirmation-dialog.tsx (5)
1-18: LGTM: Clean imports and well-defined interface.The imports are appropriately structured and the Props interface clearly defines the expected component contract. The
onConfirmfunction signature as() => Promise<void>correctly handles async operations.
20-22: Good state management approach.Using controlled state for both dialog visibility (
isOpen) and loading status (isLoading) provides proper user feedback during async operations. This addresses the previous feedback about dialog close handling.
24-32: Excellent async error handling implementation.The
handleConfirmfunction properly manages loading states and ensures the dialog closes regardless of success or failure through thefinallyblock. This addresses the previous concern about dialog behavior during async operations.
35-35: Proper controlled dialog implementation.Using controlled state with
open={isOpen}andonOpenChange={setIsOpen}provides the correct dialog behavior that was suggested in previous reviews.
43-48: Well-implemented button states and accessibility.Both buttons are properly disabled during loading operations, preventing multiple submissions. The dynamic button text ("Confirming...") provides clear user feedback. The use of appropriate button variants (
outlinefor cancel,destructivefor confirm) follows UI conventions.
- Add visible select all checkbox with sidebar toggle - Improve empty folder button styling with cleaner design - Fix layout positioning and spacing in mail toolbar
- Keep improved select all + empty folder layout structure - Integrate upstream/staging changes with mt-1 class - Maintain clean two-row toolbar design
| <Button | ||
| key={action.id} | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={action.action} | ||
| className="h-8 w-8 rounded-full" | ||
| title={action.label} | ||
| > | ||
| {action.icon} | ||
| <span className="sr-only">{action.label}</span> | ||
| </Button> | ||
| ))} |
There was a problem hiding this comment.
The icon-only buttons need proper accessibility attributes. While they include title and <span className="sr-only">, these aren't sufficient for all screen readers. Consider adding aria-label={action.label} to each Button component to ensure proper accessibility:
<Button
key={action.id}
variant="ghost"
size="icon"
onClick={action.action}
className="h-8 w-8 rounded-full"
title={action.label}
aria-label={action.label}
>
{action.icon}
<span className="sr-only">{action.label}</span>
</Button>This ensures the buttons are accessible across all assistive technologies, not just those that recognize the title attribute.
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
| <button | ||
| onClick={handleToggle} | ||
| disabled={isFetchingIds} | ||
| className="flex items-center gap-2 text-sm font-medium text-gray-700 dark:text-gray-300 hover:text-gray-900 dark:hover:text-gray-100 transition-colors" | ||
| > | ||
| <Checkbox | ||
| ref={checkboxRef} | ||
| disabled={isFetchingIds} | ||
| checked={isIndeterminate ? 'indeterminate' : isAllLoadedSelected} | ||
| onCheckedChange={handleToggle} | ||
| className={cn('hidden', className)} | ||
| id="select-all" | ||
| className="h-4 w-4 pointer-events-none" | ||
| /> | ||
| <label | ||
| htmlFor="select-all" | ||
| className={cn( | ||
| 'text-muted-foreground flex items-center gap-1 text-xs font-medium transition-colors', | ||
| isIndeterminate && 'text-primary', | ||
| )} | ||
| > | ||
| <span | ||
| className={cn( | ||
| 'border-muted-foreground flex items-center justify-center rounded border p-0.5 transition-colors', | ||
| { | ||
| 'border-primary bg-primary': isAllLoadedSelected, | ||
| }, | ||
| )} | ||
| > | ||
| <Check | ||
| className={cn('text-muted-foreground/30 h-2 w-2 transition-colors', { | ||
| 'text-black': isAllLoadedSelected, | ||
| })} | ||
| /> | ||
| </span> | ||
| Select all | ||
| </label> | ||
| </div> | ||
| Select all | ||
| </button> |
There was a problem hiding this comment.
The button element needs proper accessibility attributes. While it contains a checkbox and "Select all" text, the pointer-events-none on the checkbox could interfere with screen reader interpretation. Add an explicit aria-label attribute to ensure screen readers correctly announce the button's purpose:
<button
onClick={handleToggle}
disabled={isFetchingIds}
className="flex items-center gap-2 text-sm font-medium text-gray-700 dark:text-gray-300 hover:text-gray-900 dark:hover:text-gray-100 transition-colors"
aria-label="Select all emails"
>This ensures the button remains accessible even if the visual presentation changes.
| <button | |
| onClick={handleToggle} | |
| disabled={isFetchingIds} | |
| className="flex items-center gap-2 text-sm font-medium text-gray-700 dark:text-gray-300 hover:text-gray-900 dark:hover:text-gray-100 transition-colors" | |
| > | |
| <Checkbox | |
| ref={checkboxRef} | |
| disabled={isFetchingIds} | |
| checked={isIndeterminate ? 'indeterminate' : isAllLoadedSelected} | |
| onCheckedChange={handleToggle} | |
| className={cn('hidden', className)} | |
| id="select-all" | |
| className="h-4 w-4 pointer-events-none" | |
| /> | |
| <label | |
| htmlFor="select-all" | |
| className={cn( | |
| 'text-muted-foreground flex items-center gap-1 text-xs font-medium transition-colors', | |
| isIndeterminate && 'text-primary', | |
| )} | |
| > | |
| <span | |
| className={cn( | |
| 'border-muted-foreground flex items-center justify-center rounded border p-0.5 transition-colors', | |
| { | |
| 'border-primary bg-primary': isAllLoadedSelected, | |
| }, | |
| )} | |
| > | |
| <Check | |
| className={cn('text-muted-foreground/30 h-2 w-2 transition-colors', { | |
| 'text-black': isAllLoadedSelected, | |
| })} | |
| /> | |
| </span> | |
| Select all | |
| </label> | |
| </div> | |
| Select all | |
| </button> | |
| <button | |
| onClick={handleToggle} | |
| disabled={isFetchingIds} | |
| className="flex items-center gap-2 text-sm font-medium text-gray-700 dark:text-gray-300 hover:text-gray-900 dark:hover:text-gray-100 transition-colors" | |
| aria-label="Select all emails" | |
| > | |
| <Checkbox | |
| checked={isIndeterminate ? 'indeterminate' : isAllLoadedSelected} | |
| className="h-4 w-4 pointer-events-none" | |
| /> | |
| Select all | |
| </button> |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
| <Button | ||
| key={action.id} | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={action.action} | ||
| className="h-8 w-8 rounded-full" | ||
| title={action.label} | ||
| > | ||
| {action.icon} | ||
| <span className="sr-only">{action.label}</span> | ||
| </Button> | ||
| ))} |
There was a problem hiding this comment.
The icon-only buttons need proper accessibility attributes. While they include title and screen reader text via sr-only spans, this implementation doesn't fully meet accessibility standards. For better screen reader support, add aria-label={action.label} to each Button component:
<Button
key={action.id}
variant="ghost"
size="icon"
onClick={action.action}
className="h-8 w-8 rounded-full"
title={action.label}
aria-label={action.label}
>
{action.icon}
<span className="sr-only">{action.label}</span>
</Button>This ensures the buttons are properly announced by assistive technologies regardless of how the screen reader handles title attributes.
| <Button | |
| key={action.id} | |
| variant="ghost" | |
| size="icon" | |
| onClick={action.action} | |
| className="h-8 w-8 rounded-full" | |
| title={action.label} | |
| > | |
| {action.icon} | |
| <span className="sr-only">{action.label}</span> | |
| </Button> | |
| ))} | |
| <Button | |
| key={action.id} | |
| variant="ghost" | |
| size="icon" | |
| onClick={action.action} | |
| className="h-8 w-8 rounded-full" | |
| title={action.label} | |
| aria-label={action.label} | |
| > | |
| {action.icon} | |
| <span className="sr-only">{action.label}</span> | |
| </Button> | |
| ))} |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
|
Dear @MrgSub , I have resolved all conflicts. the UI is clean AF. and everything looks good on my machine. Please review my code. regards |
- Pull latest commits from hackice20 staging branch - Keep all our select all + empty folder improvements on top priority - Maintain accessibility fixes intact
Bug Report
Comments? Email us. Your free trial ends in 7 days. |
| <Button | ||
| key={action.id} | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={action.action} | ||
| className="h-8 w-8 rounded-full" | ||
| title={action.label} | ||
| aria-label={action.label} | ||
| > | ||
| {action.icon} | ||
| <span className="sr-only">{action.label}</span> | ||
| </Button> |
There was a problem hiding this comment.
The icon-only buttons have both aria-label and a visually hidden <span className="sr-only"> containing the same text. This creates duplicate announcements for screen readers. For better accessibility, use either:
- Keep the
aria-labelattribute and remove the<span className="sr-only">element, or - Remove the
aria-labelattribute and keep the<span className="sr-only">element
Both approaches are valid, but using both simultaneously can cause redundant announcements. The title attribute is fine to keep as it provides tooltips for sighted mouse users.
// Example of preferred approach:
<Button
key={action.id}
variant="ghost"
size="icon"
onClick={action.action}
className="h-8 w-8 rounded-full"
title={action.label}
aria-label={action.label}
>
{action.icon}
</Button>| <Button | |
| key={action.id} | |
| variant="ghost" | |
| size="icon" | |
| onClick={action.action} | |
| className="h-8 w-8 rounded-full" | |
| title={action.label} | |
| aria-label={action.label} | |
| > | |
| {action.icon} | |
| <span className="sr-only">{action.label}</span> | |
| </Button> | |
| <Button | |
| key={action.id} | |
| variant="ghost" | |
| size="icon" | |
| onClick={action.action} | |
| className="h-8 w-8 rounded-full" | |
| title={action.label} | |
| aria-label={action.label} | |
| > | |
| {action.icon} | |
| </Button> |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
🐛 Bug Fixes: 1. EmptyFolderButton: Add user notification on pagination failures 2. BulkSelectActions: Replace generic errors with specific action feedback 3. SelectAllCheckbox: Fix stale cache by clearing on all search changes - Improve error visibility and user experience - Prevent selection of wrong emails due to stale cache - Provide actionable error messages with retry guidance
Bug Report
Comments? Email us. Your free trial ends in 7 days. |
| <Button | ||
| key={action.id} | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={action.action} | ||
| className="h-8 w-8 rounded-full" | ||
| title={action.label} | ||
| > | ||
| {action.icon} | ||
| <span className="sr-only">{action.label}</span> | ||
| </Button> |
There was a problem hiding this comment.
The icon-only buttons need proper accessibility labels. While they include title attributes and sr-only spans, best practices for assistive technology support recommend explicit aria-label attributes for icon-only buttons. Consider adding aria-label={action.label} to each button to ensure they're fully accessible to screen reader users.
<Button
key={action.id}
variant="ghost"
size="icon"
onClick={action.action}
className="h-8 w-8 rounded-full"
title={action.label}
aria-label={action.label}
>
{action.icon}
<span className="sr-only">{action.label}</span>
</Button>| <Button | |
| key={action.id} | |
| variant="ghost" | |
| size="icon" | |
| onClick={action.action} | |
| className="h-8 w-8 rounded-full" | |
| title={action.label} | |
| > | |
| {action.icon} | |
| <span className="sr-only">{action.label}</span> | |
| </Button> | |
| <Button | |
| key={action.id} | |
| variant="ghost" | |
| size="icon" | |
| onClick={action.action} | |
| className="h-8 w-8 rounded-full" | |
| title={action.label} | |
| aria-label={action.label} | |
| > | |
| {action.icon} | |
| <span className="sr-only">{action.label}</span> | |
| </Button> |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
| <button | ||
| onClick={handleToggle} | ||
| disabled={isFetchingIds} | ||
| className="flex items-center gap-2 text-sm font-medium text-gray-700 dark:text-gray-300 hover:text-gray-900 dark:hover:text-gray-100 transition-colors" | ||
| aria-label="Select all emails" | ||
| aria-pressed={isAllLoadedSelected} | ||
| type="button" | ||
| > | ||
| <Checkbox | ||
| ref={checkboxRef} | ||
| disabled={isFetchingIds} | ||
| checked={isIndeterminate ? 'indeterminate' : isAllLoadedSelected} | ||
| onCheckedChange={handleToggle} | ||
| className={cn('hidden', className)} | ||
| id="select-all" | ||
| className="h-4 w-4 pointer-events-none" | ||
| aria-hidden="true" | ||
| /> | ||
| <label | ||
| htmlFor="select-all" | ||
| className={cn( | ||
| 'text-muted-foreground flex items-center gap-1 text-xs font-medium transition-colors', | ||
| isIndeterminate && 'text-primary', | ||
| )} | ||
| > | ||
| <span | ||
| className={cn( | ||
| 'border-muted-foreground flex items-center justify-center rounded border p-0.5 transition-colors', | ||
| { | ||
| 'border-primary bg-primary': isAllLoadedSelected, | ||
| }, | ||
| )} | ||
| > | ||
| <Check | ||
| className={cn('text-muted-foreground/30 h-2 w-2 transition-colors', { | ||
| 'text-black': isAllLoadedSelected, | ||
| })} | ||
| /> | ||
| </span> | ||
| Select all | ||
| </label> | ||
| </div> | ||
| Select all | ||
| </button> |
There was a problem hiding this comment.
The button-based implementation of the "Select all" control changes the semantic meaning from a checkbox to a button, which may confuse users and assistive technologies. While the code does include appropriate ARIA attributes (aria-pressed), this approach deviates from the expected checkbox pattern established elsewhere in the application.
Consider maintaining consistency by either:
- Keeping the original checkbox implementation with its label, or
- If a button-like appearance is desired, use a properly styled checkbox with a label that maintains the correct semantics
This would ensure that:
- The interaction pattern remains consistent throughout the application
- Screen readers announce the control correctly as a checkbox
- Keyboard navigation works as expected for form controls
The current implementation mixes button and checkbox semantics, which could create an inconsistent user experience.
| <button | |
| onClick={handleToggle} | |
| disabled={isFetchingIds} | |
| className="flex items-center gap-2 text-sm font-medium text-gray-700 dark:text-gray-300 hover:text-gray-900 dark:hover:text-gray-100 transition-colors" | |
| aria-label="Select all emails" | |
| aria-pressed={isAllLoadedSelected} | |
| type="button" | |
| > | |
| <Checkbox | |
| ref={checkboxRef} | |
| disabled={isFetchingIds} | |
| checked={isIndeterminate ? 'indeterminate' : isAllLoadedSelected} | |
| onCheckedChange={handleToggle} | |
| className={cn('hidden', className)} | |
| id="select-all" | |
| className="h-4 w-4 pointer-events-none" | |
| aria-hidden="true" | |
| /> | |
| <label | |
| htmlFor="select-all" | |
| className={cn( | |
| 'text-muted-foreground flex items-center gap-1 text-xs font-medium transition-colors', | |
| isIndeterminate && 'text-primary', | |
| )} | |
| > | |
| <span | |
| className={cn( | |
| 'border-muted-foreground flex items-center justify-center rounded border p-0.5 transition-colors', | |
| { | |
| 'border-primary bg-primary': isAllLoadedSelected, | |
| }, | |
| )} | |
| > | |
| <Check | |
| className={cn('text-muted-foreground/30 h-2 w-2 transition-colors', { | |
| 'text-black': isAllLoadedSelected, | |
| })} | |
| /> | |
| </span> | |
| Select all | |
| </label> | |
| </div> | |
| Select all | |
| </button> | |
| <div className="flex items-center gap-2"> | |
| <Checkbox | |
| id="select-all-checkbox" | |
| checked={isIndeterminate ? 'indeterminate' : isAllLoadedSelected} | |
| onCheckedChange={handleToggle} | |
| disabled={isFetchingIds} | |
| className="h-4 w-4" | |
| aria-label="Select all emails" | |
| /> | |
| <label | |
| htmlFor="select-all-checkbox" | |
| className="text-sm font-medium text-gray-700 dark:text-gray-300 hover:text-gray-900 dark:hover:text-gray-100 transition-colors cursor-pointer" | |
| > | |
| Select all | |
| </label> | |
| </div> |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
🚀 Major Improvements: 1. SelectAllCheckbox: Restore proper checkbox semantics (fix semantic consistency) - Replace button-wrapped-checkbox with proper checkbox+label pattern - Add race condition protection with cache key validation - Prevent stale results when search changes during fetch 2. BulkSelectActions: Re-add aria-label for full accessibility compliance - Both aria-label and sr-only span for maximum compatibility - Fix toast interpolation with explicit error messages 3. EmptyFolderButton: Enhanced error handling for all failure scenarios - Handle immediate server connection failures - Distinguish between complete vs partial fetch failures - Provide actionable user feedback ✅ Addresses all jazzberry-ai critical bugs ✅ Satisfies graphite-app accessibility requirements ✅ Maintains consistent UX patterns across components
Bug ReportBug: Race Condition in Comments? Email us. Your free trial ends in 7 days. |
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.
Description
Fixes #1560
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
Screencast.From.2025-07-20.11-13-29.mp4
Updates Made
✅ Merged latest staging changes
✅ Resolved all merge conflicts
✅ Applied all code review feedback:
Ready for re-review! @MrgSub
By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.
Summary by CodeRabbit
Summary by CodeRabbit