feat: Enhance infinite scrolling to use action button for load more data#39792
feat: Enhance infinite scrolling to use action button for load more data#39792jacquesikot merged 13 commits intoreleasefrom
Conversation
…utton to trigger scroll
WalkthroughThis pull request refactors the virtual list rendering in Table Widget V2. In the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code Definitions (1)app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.test.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (19)
🪧 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
|
jacquesikot
changed the title
jacquesikot
changed the title
![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 (3)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (2)
39-51: Consider extracting inline styles for better maintainabilityThe inline styles for the "Load More" button could be extracted to a constant or styled component for better readability and maintenance.
+const loadMoreButtonStyle = { + display: "flex", + justifyContent: "flex-start", + alignItems: "center", + paddingLeft: "10px", + cursor: "pointer", + zIndex: 1000, +}; // Then in the component: - style={{ - ...style, - display: "flex", - justifyContent: "flex-start", - alignItems: "center", - paddingLeft: "10px", - cursor: "pointer", - zIndex: 1000, - }} + style={{ + ...style, + ...loadMoreButtonStyle, + }}
83-83: Fix typo in constant nameThere's a typo in the constant name - "BUTON" should be "BUTTON".
-const LOAD_MORE_BUTON_ROW = 1; +const LOAD_MORE_BUTTON_ROW = 1;Don't forget to update the reference on line 120 as well.
app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteVirtualization.tsx (1)
70-77: Remove unused dependency in useMemoThe
totalRecordsCountdependency is included in the useMemo dependency array, but it's no longer used in the calculation.- }, [totalRecordsCount, cachedRows.length]); + }, [cachedRows.length]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx(2 hunks)app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/index.tsx(2 hunks)app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteVirtualization.tsx(4 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/index.tsx (2)
app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteVirtualization.tsx (1) (1)
useInfiniteVirtualization(20-84)app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (1) (1)
FixedInfiniteVirtualList(136-140)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
🔇 Additional comments (7)
app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/index.tsx (2)
24-29: Improved hook integration for infinite scrollingThe refactoring to use
hasMoreDatasimplifies the component by removing the need for separateisItemLoadedandloadMoreItemsfunctions. This approach aligns well with the new action button loading strategy.
33-43: Clean prop structure for FixedInfiniteVirtualListThe component now receives the necessary props directly (
hasMoreDataandloadMore), eliminating the need for theInfiniteLoadercomponent. This reduces complexity and improves code readability.app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (3)
13-67: Well-structured MemoizedRow component with clear conditional renderingThe new component handles three distinct scenarios appropriately:
- Rendering data rows when index is within bounds
- Rendering the "Load More" button when at the boundary with more data available
- Rendering empty rows otherwise
The custom equality comparison function helps prevent unnecessary re-renders.
97-109: Good use of memoization for performance optimizationThe memoization of both
rowsWithLoadMoreandrowRendereris excellent for preventing unnecessary re-renders, especially in a virtualized list where performance is critical.
120-120: Correct adjustment for itemCount with Load More buttonThe itemCount calculation correctly accounts for the additional "Load More" button row when more data is available.
app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteVirtualization.tsx (2)
10-14: Simplified hook return interfaceThe change to include
hasMoreDatainstead ofisItemLoadedandloadMoreItemsstreamlines the interface and aligns with the new loading approach.
47-50: Efficient initial loading strategyThe initial load logic is simplified while maintaining good UX by pre-loading enough data (2 pages worth) to ensure smooth scrolling before requiring user interaction.
…MoreData optional
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (3)
83-83: Fix typo in constant nameThere's a typo in the constant name: "BUTON" should be "BUTTON".
-const LOAD_MORE_BUTON_ROW = 1; +const LOAD_MORE_BUTTON_ROW = 1;Remember to update all references to this constant as well.
97-99: Avoid mutating the rows arrayUsing
Object.assign(rows, { loadMore, hasMoreData })mutates the originalrowsarray. While this works, it's better to avoid mutations in React.- const rowsWithLoadMore = React.useMemo(() => { - return Object.assign(rows, { loadMore, hasMoreData }); - }, [rows, loadMore, hasMoreData]); + const rowsWithLoadMore = React.useMemo(() => { + return { ...rows, loadMore, hasMoreData }; + }, [rows, loadMore, hasMoreData]);
120-120: Update the constant reference after fixing the typoOnce you fix the typo in the constant name, update this reference too.
- itemCount={hasMoreData ? itemCount + LOAD_MORE_BUTON_ROW : itemCount} + itemCount={hasMoreData ? itemCount + LOAD_MORE_BUTTON_ROW : itemCount}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (2)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/Row.tsx (1) (1)
Row(16-101)app/client/src/widgets/TableWidgetV2/component/cellComponents/EmptyCell.tsx (1) (1)
EmptyRows(28-193)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
🔇 Additional comments (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (1)
13-67: Implementation of Load More button looks good 👍The
MemoizedRowcomponent intelligently handles three different rendering scenarios:
- Regular row display when the index is within data range
- "Load More" button when we're at the end and more data exists
- Empty row when neither condition is met
The custom memo comparison function correctly optimizes re-renders by checking for changes in the
loadMoreandhasMoreDataprops first.
…and item tracking
…bility features and Tailwind styling
…e and item tracking functions
…b.com/appsmithorg/appsmith into feat/infinite-scroll-load-more-button
jacquesikot
added
ok-to-test
…o feat/infinite-scroll-load-more-button
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.test.tsx (2)
137-139: Consider extracting string literals as constants.The "Load more records" aria-label is repeated across multiple test cases. Consider extracting it to a constant for better maintainability.
+const LOAD_MORE_BUTTON_LABEL = "Load more records"; const loadMoreButton = screen.getByRole("button", { - name: "Load more records", + name: LOAD_MORE_BUTTON_LABEL, });
58-88: Consider simplifying the mock row creation with a default object.The
createMockRowsfunction contains a lot of mocked properties with Jest functions. You could simplify it by creating a default mock object and spreading it.const createMockRows = (count: number): ReactTableRowType<Record<string, unknown>>[] => { + const defaultRow = { + getRowProps: jest.fn(), + toggleRowExpanded: jest.fn(), + toggleRowSelected: jest.fn(), + getToggleRowExpandedProps: jest.fn(), + getToggleRowSelectedProps: jest.fn(), + setState: jest.fn(), + isExpanded: false, + canExpand: false, + depth: 0, + state: {}, + isSelected: false, + isSomeSelected: false, + isGrouped: false, + groupByID: "", + groupByVal: "", + subRows: [], + leafRows: [], + allCells: [], + cells: [], + values: {}, + }; + return Array.from({ length: count }, (_, i) => ({ + ...defaultRow, id: `${i + 1}`, original: { id: i + 1, name: `Test ${i + 1}` }, index: i, - cells: [], - values: {}, - getRowProps: jest.fn(), - allCells: [], - subRows: [], - isExpanded: false, - canExpand: false, - depth: 0, - toggleRowExpanded: jest.fn(), - state: {}, - toggleRowSelected: jest.fn(), - getToggleRowExpandedProps: jest.fn(), - isSelected: false, - isSomeSelected: false, - isGrouped: false, - groupByID: "", - groupByVal: "", - leafRows: [], - getToggleRowSelectedProps: jest.fn(), - setState: jest.fn(), })); };app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (4)
41-58: Consider using a Button component instead of a div for better accessibility.While the current implementation has proper role and tabIndex attributes, using a proper Button component would be more semantic and provide better accessibility defaults.
-<div - aria-label="Load more records" - className="flex items-center justify-start cursor-pointer z-[1000]" - onClick={loadMore} - role="button" - style={{ ...style }} - tabIndex={0} -> +<button + aria-label="Load more records" + className="flex items-center justify-start cursor-pointer z-[1000] border-0 bg-transparent" + onClick={loadMore} + style={{ ...style }} +> <Text className="underline pl-[10px]" style={{ fontWeight: "var(--ads-v2-font-weight-normal)", fontSize: TEXT_SIZES.PARAGRAPH, color: Colors.GRAY, }} > Load More </Text> -</div> +</button>
90-90: Fix typo in constant name.The constant name has a typo: "BUTON" should be "BUTTON".
-const LOAD_MORE_BUTON_ROW = 1; +const LOAD_MORE_BUTTON_ROW = 1;
127-127: Update reference to the fixed constant name.Update to use the corrected constant name.
-itemCount={hasMoreData ? itemCount + LOAD_MORE_BUTON_ROW : itemCount} +itemCount={hasMoreData ? itemCount + LOAD_MORE_BUTTON_ROW : itemCount}
15-25: Consider creating a named type for the component props.The component props type is complex and could be reused elsewhere. Consider extracting it to a named type.
+type MemoizedRowProps = ListChildComponentProps & { + loadMore?: () => void; + hasMoreData?: boolean; +}; export const MemoizedRow = React.memo( function RowComponent({ data, hasMoreData, index, loadMore, style, - }: ListChildComponentProps & { - loadMore?: () => void; - hasMoreData?: boolean; - }) { + }: MemoizedRowProps) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.test.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx(3 hunks)app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteVirtualization.test.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/widgets/TableWidgetV2/component/VirtualTable/InifiniteScrollBody/useInfiniteVirtualization.test.tsx
🧰 Additional context used
🧬 Code Definitions (2)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.test.tsx (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (1) (1)
FixedInfiniteVirtualList(143-147)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (2)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/Row.tsx (1) (1)
Row(16-101)app/client/src/widgets/TableWidgetV2/component/cellComponents/EmptyCell.tsx (1) (1)
EmptyRows(28-193)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (2)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.test.tsx (1)
1-193: Test suite looks good with comprehensive test cases.The test suite covers all the main scenarios for the
FixedInfiniteVirtualListcomponent, including row rendering, "Load More" button display logic, and interaction handling.app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (1)
14-74: New MemoizedRow component looks good with clear conditional rendering logic.The component properly handles three scenarios: rendering a row, a "Load More" button, or an empty row, with appropriate conditional checks.
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (1)
86-88:⚠️ Potential issueAvoid mutating the rows array with Object.assign.
Using
Object.assign(rows, {...})mutates the original rows array. Create a new object instead to avoid potential side effects.const rowsWithLoadMore = React.useMemo(() => { - return Object.assign(rows, { loadMore, hasMoreData }); + return { ...rows, loadMore, hasMoreData }; }, [rows, loadMore, hasMoreData]);
🧹 Nitpick comments (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (1)
72-72: Fix typo in constant name.The constant name has a spelling error.
-const LOAD_MORE_BUTON_ROW = 1; +const LOAD_MORE_BUTTON_ROW = 1;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/widgets/TableWidgetV2/component/LoadMoreButton.tsx(1 hunks)app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx(3 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
app/client/src/widgets/TableWidgetV2/component/LoadMoreButton.tsx (1)
app/client/src/ce/constants/messages.ts (1) (1)
TABLE_LOAD_MORE(2634-2634)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (2)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/Row.tsx (1) (1)
Row(16-101)app/client/src/widgets/TableWidgetV2/component/cellComponents/EmptyCell.tsx (1) (1)
EmptyRows(28-193)
🔇 Additional comments (5)
app/client/src/ce/constants/messages.ts (1)
2634-2634: Clean addition of internationalized text constant.Adding this constant centralizes UI text and enables internationalization, following the established pattern in the file.
app/client/src/widgets/TableWidgetV2/component/LoadMoreButton.tsx (1)
1-42: Well-structured and accessible LoadMoreButton component.The component follows best practices with proper accessibility attributes (aria-label, role, tabIndex) and uses the internationalized message constant. The styling also includes appropriate cursor and z-index properties.
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (3)
17-56: Good implementation of MemoizedRow with appropriate conditional rendering.The component cleanly handles three different rendering scenarios based on the index and hasMoreData flag. The custom equality function properly optimizes re-renders.
109-109: Good adjustment of itemCount for the Load More button.The component correctly adjusts the itemCount to include space for the "Load More" button when hasMoreData is true, using the constant for clarity.
90-98: Effective memoization of the row renderer.The rowRenderer is properly memoized with the correct dependency array, preventing unnecessary re-renders when only unrelated props change.
…o feat/infinite-scroll-load-more-button
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (1)
86-88:⚠️ Potential issueAvoid mutating the rows array with Object.assign
Using
Object.assign(rows, {...})mutates the original rows array. Create a new object instead to avoid potential side effects.const rowsWithLoadMore = React.useMemo(() => { - return Object.assign(rows, { loadMore, hasMoreData }); + return { ...rows, loadMore, hasMoreData }; }, [rows, loadMore, hasMoreData]);
🧹 Nitpick comments (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (1)
72-72: Typo in constant nameThere's a typo in the constant name - "BUTON" should be "BUTTON".
-const LOAD_MORE_BUTON_ROW = 1; +const LOAD_MORE_BUTTON_ROW = 1;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (3)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/Row.tsx (1) (1)
Row(21-131)app/client/src/widgets/TableWidgetV2/component/cellComponents/EmptyCell.tsx (1) (1)
EmptyRows(28-193)app/client/src/widgets/TableWidgetV2/component/VirtualTable/BaseVirtualList.tsx (1) (1)
BaseVirtualListProps(61-71)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
🔇 Additional comments (4)
app/client/src/widgets/TableWidgetV2/component/TableBodyCoreComponents/VirtualList.tsx (4)
16-56: Well-structured MemoizedRow component with optimized renderingThe new MemoizedRow component handles three distinct rendering scenarios: regular rows, a load more button, and empty rows. The custom equality function in the memo wrapper correctly prevents unnecessary re-renders when only data changes but not the loading state props.
68-69: Interface extension maintains backward compatibilityGood approach adding optional props to the existing interface instead of creating a new one. This maintains compatibility with existing code while supporting the new functionality.
90-98: Good use of memoization for rowRendererProper use of useMemo for the rowRenderer function helps prevent unnecessary re-renders. The dependencies array correctly includes only the props that would affect rendering.
108-110: Smart conditional item count handlingThe itemCount calculation correctly accounts for the additional "Load More" button row when there's more data available. This ensures the virtual list correctly sizes itself.
…alList in tests and components This update transitions the VirtualList component to utilize VariableInfiniteVirtualList, enhancing its functionality for handling variable row heights. The changes include updates to test cases and component imports to reflect this new implementation, ensuring improved performance and user experience in infinite scrolling scenarios.
jacquesikot
added
ok-to-test
…o feat/infinite-scroll-load-more-button
| isItemLoaded: (index: number) => boolean; | ||
| itemCount: number; | ||
| loadMoreItems: (startIndex: number, stopIndex: number) => Promise<void>; | ||
| hasMoreData: boolean; |
There was a problem hiding this comment.
If we are going via this route, does it even need this variable? I mean, can this not be calculated by page number?
There was a problem hiding this comment.
Yes, we can calculate with pageSize and rows, but putting it in the hook encapsulates that logic in a related scope. ALternatively we can remove it and have it in the InfiniteScrollBody, but we'd still have to pass it as props to BaseVirtualList. I prefer to encapsulate the logic for this inside the hook.
github-actions
bot
added
Query Widgets & IDE Pod
2 participants
Summary
This PR replaces the automatic infinite scrolling behavior in TableWidgetV2 with an explicit "Load More" button. This change improves user experience by:
Changes
Removed Dependencies
react-window-infinite-loaderdependency as it's no longer needed with the new approachKey Implementation Details
Technical Implementation
VirtualList.tsxto include a "Load More" button row when more data is availableuseInfiniteVirtualizationhook to track data availability without automatic loadingInfiniteScrollBodycomponent to pass the necessary props for the load more functionalityFixes #39084
Automation
/ok-to-test tags="@tag.Widget, @tag.Sanity, @tag.Binding"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13971123087
Commit: 42733b8
Cypress dashboard.
Tags:
@tag.Widget, @tag.Sanity, @tag.BindingSpec:
Thu, 20 Mar 2025 15:41:02 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
LoadMoreButtoncomponent for enhanced user interaction when loading more records.Refactor
Tests