feat: separate pagination for runs and nested runs #415
Conversation
RohitR311
added
Type: Bug
WalkthroughThe pull request introduces a comprehensive pagination management system for the Changes
Sequence DiagramsequenceDiagram
participant User
participant RunsTable
participant PaginationState
User->>RunsTable: Interact with Accordion
RunsTable->>PaginationState: Update Page/Rows
PaginationState-->>RunsTable: Return Pagination State
RunsTable->>RunsTable: Render Rows
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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
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 (3)
src/components/run/RunsTable.tsx (3)
160-174: Consider simplifying default pagination state initialization.
Using asetTimeoutto lazily initialize missing states works but might introduce timing complexities. A direct setState check or a simple memoization could simplify the logic:- if (!paginationStates[robotMetaId]) { - setTimeout(() => { - setPaginationStates(prev => ({ - ...prev, - [robotMetaId]: defaultState - })); - }, 0); - return defaultState; - } + // Directly check and set missing state if needed + if (!paginationStates[robotMetaId]) { + setPaginationStates(prev => ({ + ...prev, + [robotMetaId]: defaultState + })); + return defaultState; + }
186-193: Avoid spread syntax on reduce accumulators for better performance.
Repeatedly spreading accumulators can degrade performance dramatically. Instead, create a new object once per iteration:- const reset = Object.keys(prev).reduce((acc, robotId) => ({ - ...acc, - [robotId]: { ...prev[robotId], page: 0 } - }), {}); + const reset: PaginationState = {}; + Object.keys(prev).forEach(robotId => { + reset[robotId] = { ...prev[robotId], page: 0 }; + }); return reset;🧰 Tools
🪛 Biome (1.9.4)
[error] 189-189: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
360-443: Validate the accordion label choice.
Using the last item’sname(indexdata[data.length - 1]) might be intentional, but confirm this is desired. In some cases, the first item or a shared property might be more representative of the entire group.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/run/RunsTable.tsx(7 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/run/RunsTable.tsx
[error] 189-189: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (8)
src/components/run/RunsTable.tsx (8)
73-79: Interface definition looks solid.
No immediate issues with this dictionary-based interface for storing pagination states byrobotMetaId. It’s clear and straightforward.
89-90: Naming clarity is good; no issues found.
The separate state for outer pagination (accordion-level) is clearly named and doesn’t conflict with the nested pagination logic.
123-124: Pagination states management is well-initialized.
Introducing a dedicated React state for each accordion’s pagination is a substantial improvement over a single global pagination.
131-139: Outer (accordion-level) pagination logic is clearly implemented.
The handlershandleAccordionPageChangeandhandleAccordionsPerPageChangeneatly handle top-level pagination. No further changes needed.
140-147: Nested pagination update logic is straightforward.
Updating the page index perrobotMetaIdlooks correct. No further suggestions here.
150-157: Rows-per-page handler is clear and resets page index correctly.
This ensures users automatically see page 0 of new page-size increments. Nicely done.
273-273: Correct retrieval of pagination state for row rendering.
Accessing the correct page and rowsPerPage per accordion ensures each group is paginated independently. Implementation is sound.
448-452: Outer pagination controls appear well-integrated.
Governing how many accordions to display on the page further refines the user’s navigational experience.
Closes: #414
Maxun._.Open.Source.No.Code.Web.Data.Extraction.Platform.-.Brave.2025-01-29.22-57-17.mp4
Summary by CodeRabbit
New Features
Refactor