[front] - fix(TablePicker): infinite scroll #11446
Conversation
- Add a new React hook for cursor-based pagination functionality in the app - Provide state management and utility functions for handling pagination logic [front/pages/api] - feature: add search API endpoint for data source views - Create an endpoint to search tables within data source views with cursor pagination - Handle API authentication, validation, and error responses for the search feature
…urce view tables - Add support for pagination in the useDataSourceViewTables hook using cursor and limit parameters - Introduce search functionality for data source view tables by handling search queries - Allow disabling the data tables request using the disabled flag - Include nextPageCursor in the return object to support fetching subsequent pages
…le listing with cursors - Added nextPageCursor to the API response for paginating through table lists - The nextPageCursor indicates the cursor for the next page of table listings
…ion and search - Implement cursor pagination for loading tables in the TablePicker component - Add debounced search functionality to filter tables as user types - Introduce InfiniteScroll component to handle progressive loading of tables - Display a loading indicator when fetching tables or a single table - Refactor useEffect dependencies to correctly update local state with fetched data - Allow excluding tables from the list via `excludeTables` prop - Improve UI responsiveness by including a ScrollBar in the table list display - Optimize table rendering logic based on the search query size and pagination status
- Moved the data source access control check to after GET method validation in search handler - Added a new test case to ensure proper handling of valid requests for search results - Included test cases to handle empty search results effectively and propagate warnings correctly
…s mocking - Tests skipped as proper mocking of the CoreAPI.searchNodes function is problematic - Intentional skip with a placeholder to revisit the mock implementation later
| } | ||
| }, 300); | ||
| return () => clearTimeout(timeout); | ||
| }, [searchFilter, debouncedSearch, resetPagination]); |
There was a problem hiding this comment.
it's for deboucing, right ?
FYI, lodash has an debounce utility function
There was a problem hiding this comment.
Oh nice, didn't know about it. We have this logic spread across a bunch of components, I'll make a pass to harmonize everything 👌🏼
| disabled: !debouncedSearch, | ||
| }); | ||
|
|
||
| const { table, isTableLoading, isTableError } = useDataSourceViewTable({ |
There was a problem hiding this comment.
out of curiosity, why do we need to fetch the table ? Isn't tables containing what we need ?
There was a problem hiding this comment.
Yeah that's an extra fetch but the thing is that if the table you've previously selected is "Dust Table" and you search for "index" then the table isn't contained in tables.
Also, tables is empty until type 2 characters.
| (table) => | ||
| !prevTables.some( | ||
| (prevTable) => prevTable.internalId === table.internalId | ||
| ) |
There was a problem hiding this comment.
nit: this is computation intensive as we iterate on each new table x each previous tables. Especially since we are almost guaranteed to only have new tables. For pages of 100 tables, the second page trigger 10000 iterations.
- option 1: precompute the list of prevTables.internalId once and use it for comparison on each new table.
- option 2: use a different datastructure for prevTables (Map internalId => table ?) and just set so we don't need filtering at all.
| reset: resetPagination, | ||
| handleLoadNext, | ||
| pageIndex, | ||
| } = useCursorPagination(PAGE_SIZE); |
There was a problem hiding this comment.
Should/could this be encapsulated in useDataSourceViewTables ?
There was a problem hiding this comment.
It really depends on how the hook is being used. If used within a DataTable then the way we need to handle the pagination is slightly different.. So I would rather keep it as is tbh
| @@ -0,0 +1,236 @@ | |||
| import { describe, expect, vi } from "vitest"; | |||
| }; | ||
|
|
||
| await handler(req, res); | ||
| true; // Skip for now, I have trouble mocking correctly the CoreAPI.searchNodes |
There was a problem hiding this comment.
soorryyyy again about that :)
- Enhanced user experience by reducing the amount of pagination needed to browse tables - Adjusted PAGE_SIZE constant to accommodate more table entries per page
…r efficiency - Changed the state structure from an array to a Map to optimize table lookups and avoid duplicates - Updated the useEffect to handle pagination logic using the new Map, improving performance with large table lists - Modified rendering logic to convert the Map to an array for display and filter operations
| const newTablesMap = new Map(prevTablesMap); | ||
|
|
||
| tables.forEach((table) => { | ||
| if (!prevTablesMap.has(table.internalId)) { |
There was a problem hiding this comment.
do you really need the test?
- Change the behavior to always update the table entries in the map regardless of their previous existence
Description
This PR implements infinite scroll in the TablePicker component to handle large datasets efficiently. The implementation includes server-side pagination for tables, search functionality with debouncing, and cursor-based pagination to improve performance and user experience.
References:
Risk
Low
Deploy Plan