-
Notifications
You must be signed in to change notification settings - Fork 17
refactor(TenantOverviewTableLayout): pass table as child #2001
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
| ...DEFAULT_TABLE_SETTINGS, | ||
| stickyHead: 'fixed', | ||
| dynamicRender: false, | ||
| sortable: false, |
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.
Tenant tables display tops, there is no need in sorting there (and this helps to get rid of unneeded code that modifies columns)
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.
PR Overview
This PR refactors the TenantOverviewTableLayout to allow passing a custom table component as a child rather than having the layout render the table itself. Key changes include moving the table rendering logic into child components using the ResizeableDataTable, updating the interface of TenantOverviewTableLayout to remove generic data props, and streamlining column definitions by removing redundant sorting modifications.
Reviewed Changes
| File | Description |
|---|---|
| src/containers/Tenant/Diagnostics/TenantOverview/TenantStorage/TopGroups.tsx | Updated to pass the table as a child with added settings imports. |
| src/containers/Tenant/Diagnostics/TenantOverview/TenantOverviewTableLayout.tsx | Refactored to remove outputting a built‐in table and now accepts children and a withData flag. |
| src/containers/Tenant/Diagnostics/TenantOverview/TenantCpu/TopNodesByLoad.tsx | Modified to pass table as a child and removed per-column unsortable mapping. |
| src/containers/Tenant/Diagnostics/TenantOverview/TenantMemory/TopNodesByMemory.tsx | Updated similarly to TopNodesByLoad with table passed as a child. |
| src/containers/Tenant/Diagnostics/TenantOverview/TenantCpu/TopNodesByCpu.tsx | Converted to use the new layout and child table rendering. |
| src/containers/Tenant/Diagnostics/TenantOverview/TenantStorage/TopTables.tsx | Updated to wrap the ResizeableDataTable in TenantOverviewTableLayout. |
| src/containers/Tenant/Diagnostics/TenantOverview/TenantCpu/TopShards.tsx | Refactored to use the new refactored layout pattern. |
| src/containers/Tenant/Diagnostics/TenantOverview/TenantCpu/TopQueries.tsx | Changes made to pass table as child, including moving onRowClick into the child table. |
| src/utils/constants.ts | TENANT_OVERVIEW_TABLES_SETTINGS updated with an explicit 'sortable: false' property. |
| src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx & src/containers/Storage/StorageGroups/columns/columns.tsx | Simplified column definitions by removing the extra unsortable mapping. |
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
src/containers/Tenant/Diagnostics/TenantOverview/TenantOverviewTableLayout.tsx:29
- [nitpick] Consider renaming the 'withData' prop to 'hasData' to better reflect that it represents the presence of data and improve clarity in the error handling logic.
if (error && !withData) {
src/utils/constants.ts:96
- [nitpick] Ensure the new 'sortable' setting in TENANT_OVERVIEW_TABLES_SETTINGS is compatible with all table implementations, as column definitions no longer override this property.
sortable: false,
Make
TenantOverviewTableLayoutaccept table as child instead of rendering it itself. The goal - make possible to reuse some tables as components like:CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️1
⏭️ Skipped Tests (1)
Bundle Size: ✅
Current: 80.81 MB | Main: 80.81 MB
Diff: +2.08 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information