-
Notifications
You must be signed in to change notification settings - Fork 17
fix(ResizeableDataTable): rework loading state #3048
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
0f42fa8 to
e107cdb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the ResizeableDataTable component to support distinct loading states with clearer semantics. The component now distinguishes between initial loading (isLoading) and data fetching (isFetching), following patterns commonly used in RTK Query.
Key changes:
- Renamed the
loadingprop toisLoadingandisFetchingto distinguish between initial load and subsequent data fetches - Added
isLoadingsupport to showTableSkeletonwhen table structure/columns aren't ready - Added
isFetchingsupport to show inline skeleton rows while preserving table headers during data fetches - Wrapped computed values in
useMemohooks for better performance
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/ResizeableDataTable/ResizeableDataTable.tsx | Refactored to support isLoading and isFetching states with memoized columns, data, and settings; added TableSkeleton for initial loading |
| src/containers/Tenant/Diagnostics/TopQueries/QueriesTableWithDrawer.tsx | Updated to use isFetching prop instead of deprecated loading prop |
| src/containers/Tenant/Diagnostics/Partitions/Partitions.tsx | Removed manual loading check and TableSkeleton rendering; updated to use isLoading prop |
| src/containers/Node/Threads/Threads.tsx | Updated to use isLoading prop instead of deprecated loading prop |
Comments suppressed due to low confidence (1)
src/containers/Tenant/Diagnostics/Partitions/Partitions.tsx:134
- The condition
{partitionsData ? renderContent() : null}prevents the table from rendering during initial load whenisLoadingis true. This means theTableSkeletoninResizeableDataTablewill never be shown. The logic should render content when eitherpartitionsDataexists OR whenloadingis true to show the skeleton state properly.
{partitionsData ? renderContent() : null}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, no comments
|
Skipped: This PR does not contain any of your configured keywords: ( |
Part of #2892
Stand: https://nda.ya.ru/t/tNojZU0Y7MWKUx
You could see changes if you turn on 3G in browser network and change sorting in top queries table
Before - no data instead of loader on sort column change in TopQueries:

After - proper loading state on sort column change in TopQueries:

CI Results
Test Status: ❌ FAILED
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: 🔺
Current: 47.22 MB | Main: 47.15 MB
Diff: +0.07 MB (0.14%)
ℹ️ CI Information