Refactor to prepare virtual scrolling on big tables #360
Conversation
…and delete PortalContainerProvider
There was a problem hiding this comment.
Pull request overview
This PR refactors the HighTable component architecture to prepare for virtual scrolling on big tables by decomposing a monolithic component into smaller, focused modules.
- Extracts scroll management logic into a new
Scrollercomponent - Separates table rendering logic into a new
Slicecomponent - Creates a new
Wrappercomponent to manage provider composition - Introduces
ErrorContextto centralize error handling across components - Removes
ViewportProviderandPortalContainerProviderin favor of simpler patterns
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/providers/ViewportProvider.tsx | Deleted - viewport tracking moved to Scroller component |
| src/providers/PortalContainerProvider.tsx | Deleted - replaced with inline context provider in Wrapper |
| src/contexts/ViewportContext.ts | Deleted - viewport width now passed as prop instead of context |
| src/contexts/ErrorContext.ts | New context for centralized error handling across components |
| src/components/HighTable/constants.ts | New file extracting shared constants (row height, padding, overscan, aria offset) |
| src/components/HighTable/Wrapper.tsx | New wrapper component managing provider composition and viewport width state |
| src/components/HighTable/Slice.tsx | New component handling data fetching and table rendering for visible rows |
| src/components/HighTable/Scroller.tsx | New component managing scroll state, viewport tracking, and cell navigation scrolling |
| src/components/HighTable/HighTable.tsx | Simplified to only provide ErrorContext and DataProvider, delegating to Wrapper |
| src/providers/SelectionProvider.tsx | Updated to use ErrorContext instead of onError prop parameter |
| src/providers/ColumnWidthsProvider.tsx | Updated to receive viewportWidth as prop instead of from ViewportContext |
| src/providers/CellNavigationProvider.tsx | Minor TODO comment clarification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
OK, merging. I think it's a clarity and decorrelation improvement |
Pure refactoring.
To prepare #347, where we will have two modes (normal vs virtual scroll, depending on the number of rows)