feat: migrate ThreadList to PaginatedVirtualList#40845
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
WalkthroughThreadList component is migrated from react-virtuoso to PaginatedVirtualList. The change removes useResizeObserver-based container sizing and updates test mocks accordingly. Pagination behavior is refined to disable onEndReached while pending. ChangesThreadList Migration to PaginatedVirtualList
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
1 issue found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/components/VirtualList/VirtualList.tsx">
<violation number="1" location="apps/meteor/client/components/VirtualList/VirtualList.tsx:67">
P1: Pagination lock can get permanently stuck after a successful `onEndReached` call that does not change tracked item metadata, blocking all future pagination.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (isEndReachedLockedRef.current) { | ||
| return; | ||
| } | ||
| isEndReachedLockedRef.current = true; |
There was a problem hiding this comment.
P1: Pagination lock can get permanently stuck after a successful onEndReached call that does not change tracked item metadata, blocking all future pagination.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/components/VirtualList/VirtualList.tsx, line 67:
<comment>Pagination lock can get permanently stuck after a successful `onEndReached` call that does not change tracked item metadata, blocking all future pagination.</comment>
<file context>
@@ -0,0 +1,109 @@
+ if (isEndReachedLockedRef.current) {
+ return;
+ }
+ isEndReachedLockedRef.current = true;
+
+ try {
</file context>
6908462 to
019d3de
Compare
019d3de to
e21d902
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #40845 +/- ##
===========================================
+ Coverage 70.02% 70.07% +0.04%
===========================================
Files 3355 3355
Lines 129162 129153 -9
Branches 22337 22392 +55
===========================================
+ Hits 90443 90500 +57
+ Misses 35421 35356 -65
+ Partials 3298 3297 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
f0a93f0 to
f0c1e62
Compare
|
Rebased onto the latest develop after the Virtua foundation PR merged. No functional changes beyond the ThreadList migration. |
Proposed changes
This PR migrates
ThreadListfromreact-virtuosoto the sharedPaginatedVirtualListcomponent introduced by the Virtua foundation work.Changes include:
react-virtuosoimplementation withPaginatedVirtualListThreadListwith the same consumer pattern used inDiscussionsListThe goal is to keep
ThreadListconsistent with the new virtualization approach while keeping the scope of the PR limited to ThreadList-specific changes.Issue(s)
Part of the Virtua migration work for the contextual bar lists.
Further comments
This PR should be merged after the Virtua foundation work (
PaginatedVirtualListandDiscussionsListmigration), #39394 as it reuses the shared virtualization infrastructure introduced there and only contains ThreadList-specific changes.Summary by CodeRabbit
Refactor
Tests