feat: add shimmer loading effect and starred mail section#1589
feat: add shimmer loading effect and starred mail section#1589Pankajkumar2608 wants to merge 3 commits intoMail-0:stagingfrom
Conversation
|
""" WalkthroughThe changes introduce a new "Starred" mail folder throughout the mail application, updating constants, navigation, localization, and folder-label mappings to support it. Additionally, a skeleton (shimmer) loading effect is implemented for the mail list, replacing the previous spinner-based loading UI with placeholder components for a smoother loading experience. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant MailApp
participant Server
User->>Sidebar: Clicks "Starred"
Sidebar->>MailApp: Navigates to /mail/starred
MailApp->>Server: Requests mails with label "STARRED"
MailApp->>MailApp: Shows MailListSkeleton (shimmer loading)
Server-->>MailApp: Returns starred mails
MailApp->>MailApp: Renders MailList with starred mails
sequenceDiagram
participant MailApp
participant MailList
participant MailListSkeleton
MailApp->>MailList: Renders MailList (loading=true)
MailList->>MailListSkeleton: Display shimmer skeleton
MailList-->>MailApp: Once data loaded, render mail threads
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Implements a new Starred mail section and shimmer loading effects for improved UX during data fetching. The changes span both client and server components with careful attention to data management.
- Added new
apps/mail/components/mail/thread-skeleton.tsximplementing loading placeholders with shimmer animation effects - Updated
apps/server/src/trpc/routes/mail.tsto handle starred mail functionality with proper label management - Modified
apps/mail/config/navigation.tsto include Starred section under Management group with appropriate shortcuts - Enhanced
apps/mail/components/mail/mail-list.tsxwith optimistic UI updates for star toggling - Added localization support in
apps/mail/messages/en.jsonfor the new Starred section
7 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
| id: 'starred', | ||
| title: m['navigation.sidebar.starred'](), | ||
| url: '/mail/starred', | ||
| icon: Stars, |
There was a problem hiding this comment.
logic: potential keyboard shortcut conflict between 'g + s' for starred and settings route defined on line 157
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (7)
apps/mail/app/(routes)/mail/[folder]/page.tsx(1 hunks)apps/mail/components/mail/mail-list.tsx(3 hunks)apps/mail/components/mail/thread-skeleton.tsx(1 hunks)apps/mail/config/navigation.ts(1 hunks)apps/mail/messages/en.json(1 hunks)apps/server/src/lib/utils.ts(3 hunks)apps/server/src/trpc/routes/mail.ts(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: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.
apps/server/src/trpc/routes/mail.ts (2)
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:386-391
Timestamp: 2025-06-27T04:59:29.709Z
Learning: In apps/server/src/trpc/routes/mail.ts, the attachment processing logic conditionally handles mixed attachment types - it preserves existing File-like objects with arrayBuffer methods while only converting serialized attachments that need processing through toAttachmentFiles.
Learnt from: retrogtx
PR: Mail-0/Zero#1468
File: apps/server/src/trpc/routes/mail.ts:331-331
Timestamp: 2025-06-28T03:56:09.351Z
Learning: In apps/server/src/trpc/routes/mail.ts, the user indicated they are not using ISO format for the scheduleAt parameter, despite the frontend code showing toISOString() usage in the ScheduleSendPicker component.
apps/mail/app/(routes)/mail/[folder]/page.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/mail/mail-list.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/config/navigation.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
🧬 Code Graph Analysis (2)
apps/mail/config/navigation.ts (1)
apps/mail/components/icons/icons.tsx (1)
Stars(1342-1373)
apps/server/src/lib/utils.ts (1)
apps/mail/lib/utils.ts (2)
FOLDERS(10-17)LABELS(19-26)
🪛 Biome (1.9.4)
apps/mail/components/mail/mail-list.tsx
[error] 1017-1017: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
🔇 Additional comments (12)
apps/mail/components/mail/thread-skeleton.tsx (1)
1-19: Well-implemented skeleton loading component.The
ThreadSkeletoncomponent follows excellent skeleton UI patterns with proper visual hierarchy, responsive design, and consistent spacing. The layout effectively mimics a mail thread item structure.apps/server/src/trpc/routes/mail.ts (1)
22-22: Correct implementation of starred folder mapping.The addition follows the established pattern and correctly maps the "starred" folder to the "STARRED" label, maintaining consistency with existing folder mappings.
apps/mail/app/(routes)/mail/[folder]/page.tsx (1)
9-9: Necessary addition for starred folder routing.Adding "starred" to
ALLOWED_FOLDERScorrectly enables route validation for the new starred folder, preventing "folder not found" errors when users navigate to/mail/starred.apps/mail/messages/en.json (1)
404-404: Proper localization addition for starred folder.The addition of the "starred" label under
navigation.sidebaris correctly placed and follows the established pattern for navigation localization.apps/server/src/lib/utils.ts (1)
50-50: LGTM! Consistent implementation of STARRED folder and label constants.The additions follow the established patterns in the codebase:
- Folder name uses lowercase ('starred')
- Label name uses uppercase ('STARRED')
- Proper mapping in FOLDER_TAGS
These changes align well with the PR's objective to add starred mail functionality.
Also applies to: 60-60, 80-80
apps/mail/components/mail/mail-list.tsx (7)
61-61: Good addition of ThreadSkeleton import.Adding the skeleton component import supports the enhanced loading experience being implemented.
729-735: Excellent skeleton loading implementation.The
MailListSkeletoncomponent provides a much better user experience during loading states compared to a simple spinner. The parameterized count allows for flexibility.
982-982: Improved container layout structure.The change to
flex-collayout properly supports the new skeleton loading and empty state implementations.
986-1008: Enhanced loading and empty states.The replacement of the simple loading spinner with
MailListSkeletonsignificantly improves the user experience. The empty state now usesflex-1for better space utilization.
1010-1034: Well-structured virtual list implementation.The VList container structure is properly organized with flex layout. The scroll handling logic for pagination remains intact and functional.
1037-1037: Improved fetching state condition.The condition
isFetching && !isLoadingproperly separates the pagination loading state from the initial skeleton loading, providing clearer visual feedback to users.
1017-1017: Ignore static-analysis warning: VList children is a render-functionThe
VListcomponent fromvirtuatypes itschildrenprop as a render function(index: number) => ReactNode. PassingvListRendererhere matches the intended API, so the linter’s “non-canonical children” warning can be safely ignored.
|
Please address comments and provide a video |
Untitled.video.-.Made.with.Clipchamp.2.mp4 |
|
Hi @MrgSub, I noticed that an unintended merge commit (be57303) got added to this branch. 3775d92: feat: add shimmer loading effect and starred mail section 317fb68: fixed issue with between shortcut If possible, could you please cherry-pick or squash only these two commits when merging to keep the history clean? Thanks a lot! |
|
Thank you, can we remove the loading effect and just add the starred mail section? perhaps open another PR for the skeleton as it's not needed atm |
I added the loading effect to improve the user experience during longer load times — it's better to show a shimmer effect than a blank screen. But sure, I can remove it for now and focus on just adding the starred mail section. We can introduce the skeleton in a separate PR later if needed. |
|
This PR has merge conflicts and has been open for more than 3 days. It will be automatically closed. Please resolve the conflicts and reopen the PR if you'd like to continue working on it. |
2 participants
Uploading Untitled video - Made with Clipchamp (1).mp4…
Fixes #1588
Description
This PR adds two enhancements:
Type of Change
Areas Affected
Testing Done
Security Considerations
Checklist
Additional Notes
thread-skeleton.tsxfor shimmer effect.navigation.ts, route config, anden.jsonto support the new "Starred" section.Screenshots/Recordings
Add screenshots or screen recordings here if applicable.
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
Enhancements
Bug Fixes