feat: implement infinite scroll hook#40050
Merged
jacquesikot merged 3 commits intoreleasefrom Apr 3, 2025
Merged
Conversation
Contributor
WalkthroughThe changes refactor the infinite scrolling logic in the table widget. The code replaces the previous computation of Changes
Possibly related issues
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteScroll.tsx (4)
15-20: Consider potential edge case in loading condition.The current implementation triggers loadMore when rows.length equals pageSize, which may cause unnecessary loading operations in some cases.
- if (rows.length > 0 && rows.length <= pageSize) { + if (rows.length > 0 && rows.length < pageSize) { loadMore(); }
16-16: Update comment to match code variable.The comment refers to "cachedRows" but the variable is named "rows" in the code.
- // If cachedRows is just a single page, call loadMore to fetch the next page + // If rows is just a single page, call loadMore to fetch the next page
22-23: No need for explicit return statement.The function doesn't return any value, so the explicit
return;statement is unnecessary.- return;
15-20: Add error handling for loadMore function.Consider adding error handling to manage cases where the loadMore function might fail.
useEffect(() => { // If cachedRows is just a single page, call loadMore to fetch the next page if (rows.length > 0 && rows.length <= pageSize) { - loadMore(); + try { + loadMore(); + } catch (error) { + console.error("Failed to load more rows:", error); + } } }, [rows.length, pageSize, loadMore]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/index.tsx(2 hunks)app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteScroll.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (4)
app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/index.tsx (3)
1-1: Import statements look good.The new import for useInfiniteScroll is correctly referenced from the relative path.
Also applies to: 7-7
25-29: Good extraction of infinite scroll logic.Extracting the infinite scrolling logic into a separate hook improves code organization and separation of concerns.
37-37: Simplified itemCount usage.Directly using
rows.lengthfor itemCount is simpler than the previous approach using totalRecordsCount. This change makes the code more straightforward.app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteScroll.tsx (1)
4-8: Interface is well-defined.The UseInfiniteScrollProps interface properly defines the required properties with appropriate types.
jacquesikot
added
ok-to-test
rahulbarwal
reviewed
Apr 3, 2025
...t/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteScroll.tsx
Show resolved
Hide resolved
…d row height calculations
AmanAgarwal041
approved these changes
Apr 3, 2025
2 tasks
github-actions bot
pushed a commit
to Zeral-Zhang/appsmith
that referenced
this pull request
Apr 12, 2025
# ✨ Optimized Infinite Scrolling for TableWidgetV2 ## 📌 Problem The **infinite scrolling** feature in **TableWidgetV2** did not load the correct number of pages (2) on init, and the table rows length was more than the data fetched ## ✅ Solution This PR optimizes **infinite scrolling** by: - 📥 **Automatically loading the next page of data** when the initial rows are **≤ 1 page** - 📊 **Simplifying row count management** by using the actual **rows length** instead of relying on `totalRecordsCount` which does not tell actual rows loaded into the table --- ### 🚀 How to Test 1. Enable **Infinite Scroll** in **TableWidgetV2**. 2. Ensure that additional pages (page 2) loads **automatically** when the initial rows are ≤ 1 page. 3. Verify that **pagination behavior remains consistent** with expected results. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Widget, @tag.Table, @tag.Binding, @tag.Sanity" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/14241762644> > Commit: 8bf69c9 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14241762644&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Widget, @tag.Table, @tag.Binding, @tag.Sanity` > Spec: > <hr>Thu, 03 Apr 2025 12:53:32 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced a new `useInfiniteScroll` hook to enhance infinite scrolling functionality in the table widget. - **Refactor** - Simplified pagination logic by directly utilizing the length of the rows array, improving performance and clarity. - **Bug Fixes** - Adjusted test data and assertions in the Cypress test suite to better reflect the expected behavior of the table widget under varying content conditions. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Rahul Barwal <rahul.barwal@appsmith.com>
2 tasks
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ Optimized Infinite Scrolling for TableWidgetV2
📌 Problem
The infinite scrolling feature in TableWidgetV2 did not load the correct number of pages (2) on init, and the table rows length was more than the data fetched
✅ Solution
This PR optimizes infinite scrolling by:
totalRecordsCountwhich does not tell actual rows loaded into the table🚀 How to Test
Fixes #
Issue Numberor
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Widget, @tag.Table, @tag.Binding, @tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14241762644
Commit: 8bf69c9
Cypress dashboard.
Tags:
@tag.Widget, @tag.Table, @tag.Binding, @tag.SanitySpec:
Thu, 03 Apr 2025 12:53:32 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
useInfiniteScrollhook to enhance infinite scrolling functionality in the table widget.Refactor
Bug Fixes