Skip to content
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

refactor(TenantOverviewTableLayout): pass table as child #2001

Merged
merged 1 commit into from
Mar 7, 2025

Conversation

artemmufazalov
Copy link
Member

@artemmufazalov artemmufazalov commented Mar 6, 2025

Make TenantOverviewTableLayout accept table as child instead of rendering it itself. The goal - make possible to reuse some tables as components like:

<TenantOverviewTableLayout>
    <MyCustomTable/>
</TenantOverviewTableLayout>

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total Passed Failed Flaky Skipped
264 262 0 1 1
Test Changes Summary ⏭️1

⏭️ Skipped Tests (1)

  1. Streaming query shows some results and banner when stop button is clicked (tenant/queryEditor/queryEditor.test.ts)

Bundle Size: ✅

Current: 80.81 MB | Main: 80.81 MB
Diff: +2.08 KB (0.00%)

✅ Bundle size unchanged.

ℹ️ CI Information
  • Test recordings for failed tests are available in the full report.
  • Bundle size is measured for the entire 'dist' directory.
  • 📊 indicates links to detailed reports.
  • 🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

...DEFAULT_TABLE_SETTINGS,
stickyHead: 'fixed',
dynamicRender: false,
sortable: false,
Copy link
Member Author

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)

@artemmufazalov artemmufazalov marked this pull request as ready for review March 6, 2025 12:40
@artemmufazalov artemmufazalov requested a review from sareyu March 6, 2025 12:40
@astandrik astandrik requested a review from Copilot March 7, 2025 05:22

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,
@artemmufazalov artemmufazalov added this pull request to the merge queue Mar 7, 2025
Merged via the queue into main with commit 259695a Mar 7, 2025
8 checks passed
@artemmufazalov artemmufazalov deleted the refactor-tenant-tables branch March 7, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants