Introduced Infinite Scroll using Virtua#1059
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailList
participant DataSource
User->>MailList: Scrolls mail list
MailList->>MailList: Detects scroll near bottom (onScrollEnd)
MailList->>DataSource: loadMore()
DataSource-->>MailList: Returns next page of messages
MailList->>User: Renders additional messages
Possibly related PRs
Suggested reviewers
Poem
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: 0
🧹 Nitpick comments (1)
apps/mail/components/mail/mail-list.tsx (1)
550-558: LGTM! Clean hook destructuring with minor type casting question.The destructuring of the
useThreadshook is well-organized and includes all necessary state variables for infinite scroll functionality.Consider whether the explicit type casting on line 558 is necessary:
-// Ensure items are of type ParsedMessage[] -const parsedItems: ParsedMessage[] = (items as ParsedMessage[]);If the
useThreadshook already returns properly typed items, this casting may be redundant. You could verify the hook's return type to determine if this line can be removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/mail/components/mail/mail-list.tsx(4 hunks)
🔇 Additional comments (4)
apps/mail/components/mail/mail-list.tsx (4)
594-607: Excellent scroll handler implementation with proper safeguards.The scroll detection logic is well-implemented with appropriate checks for loading states and boundary conditions. The 200px threshold provides a good user experience by starting the next page load before reaching the absolute bottom.
610-633: Outstanding event listener setup with proper performance optimization.The scroll event handling demonstrates excellent practices:
requestAnimationFramethrottling prevents excessive function calls- Proper cleanup of event listeners prevents memory leaks
- Initial scroll check on mount handles edge cases where content is already scrollable
- The
tickingflag ensures only one animation frame request is pending
803-803: Essential CSS update for scroll functionality.Adding
overflow-y-autois crucial for enabling the scroll behavior that triggers the infinite loading mechanism.
777-784:Details
❓ Verification inconclusive
Verify complete removal of "Load More" button.
The VList renderer looks clean, but I want to ensure the old "Load More" button was completely removed from the codebase.
🏁 Script executed:
#!/bin/bash # Description: Verify that "Load More" button references have been completely removed # Expected: No remaining references to load more button rendering in mail components echo "Searching for Load More button references..." rg -i "load.?more" --type tsx --type ts -A 3 -B 3 echo -e "\nSearching for button-related pagination..." rg -i "(loadmore|load_more)" --type tsx --type ts -A 3 -B 3 echo -e "\nSearching for handleLoadMore references..." rg "handleLoadMore" --type tsx --type ts -A 3 -B 3Length of output: 549
#!/bin/bash # Re-verify removal of "Load More" references using file globbing echo "Searching for 'Load More' text in .ts and .tsx files..." rg -i "load.?more" -g "*.ts" -g "*.tsx" -A 3 -B 3 || true echo -e "\nSearching for 'loadmore' or 'load_more' in .ts and .tsx files..." rg -i "(loadmore|load_more)" -g "*.ts" -g "*.tsx" -A 3 -B 3 || true echo -e "\nSearching for handleLoadMore references in .ts and .tsx files..." rg "handleLoadMore" -g "*.ts" -g "*.tsx" -A 3 -B 3 || true
Confirm removal of “Load More” references
I ran automated searches across *.ts and *.tsx files and didn’t find any remaining “Load More” or handleLoadMore occurrences. However, please manually verify that no remnants remain elsewhere:
• Tests and fixtures (e.g. tests or *.spec.tsx)
• Storybook stories or examples
• Pagination utilities or helper modules
• Localization/translation filesOnce you’ve confirmed there are no lingering references, we can consider this fully resolved.
hey hey! can you screen record the infinite scroll? id love to check it out! |
|
Hey @nizzyabi Here is the screen record you asked for, Hope this help :) Recording.2025-05-25.012419.mp4 |
|
amazing! thanks. @MrgSub @BlankParticle can you review |
|
@BlankParticle everything oky ? now with loader the UX and typo fixed review |
|
|
||
| useEffect(() => { | ||
| const parent = parentRef.current; | ||
| if (!parent) return; | ||
|
|
||
| let ticking = false; | ||
| const onScroll = () => { | ||
| if (!ticking) { | ||
| window.requestAnimationFrame(() => { | ||
| ticking = false; | ||
| }); | ||
| ticking = true; | ||
| } | ||
| }; | ||
|
|
||
| parent.addEventListener('scroll', onScroll); | ||
|
|
||
|
|
||
| return () => { | ||
| parent.removeEventListener('scroll', onScroll); | ||
| }; | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
i dont think this effect is doing anything?
| itemSize={50} | ||
| onScroll={(offset) => { | ||
| if (parentRef.current) { | ||
| parentRef.current.scrollTop = offset; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
onScroll callback is not needed to set the scrollTop, the list just scrolls automatically, why do we need to set it? I think you can just remove the callback, check if only having scrollEnd works fine.
why added itemSize? it works without it
BlankParticle
force-pushed
the
main
branch
2 times, most recently
from
a8ba39c to
771e17e
Compare
4 participants
Description
Introduces Infinite Scroll using
[Virtua]the feature refer requested from https://0email.featurebase.app/p/automatically-load-more-mail-on-scroll the code removes the load more button and replace with an scroll for more threads fetched, this may effect the overall performance of product need permission to revamp the code breaking changes for component optimal code create a Components to mount and dismount. ik what the code refers and would like get review of possible mergeType 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
Add screenshots or 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