chore!: only pagination tables in storage #1745
Conversation
There was a problem hiding this comment.
Copilot reviewed 5 out of 19 changed files in this pull request and generated 1 suggestion.
Files not reviewed (14)
- src/containers/UserSettings/i18n/en.json: Language not supported
- src/containers/Storage/Storage.tsx: Evaluated as low risk
- src/containers/Storage/StorageWrapper.tsx: Evaluated as low risk
- src/containers/Storage/StorageNodes/StorageNodesTable.tsx: Evaluated as low risk
- src/containers/VDiskPage/VDiskPage.tsx: Evaluated as low risk
- src/containers/Storage/PaginatedStorage.tsx: Evaluated as low risk
- src/containers/Cluster/Cluster.tsx: Evaluated as low risk
- src/containers/UserSettings/settings.tsx: Evaluated as low risk
- src/containers/StorageGroupPage/StorageGroupPage.tsx: Evaluated as low risk
- src/containers/Storage/PaginatedStorageGroups.tsx: Evaluated as low risk
- src/containers/Tenant/Diagnostics/Diagnostics.tsx: Evaluated as low risk
- src/services/settings.ts: Evaluated as low risk
- src/utils/constants.ts: Evaluated as low risk
- src/containers/PDiskPage/PDiskPage.tsx: Evaluated as low risk
Comments skipped due to low confidence (1)
tests/suites/paginatedTable/paginatedTable.ts:193
- The value 'instant' for the 'behavior' property is invalid. It should be 'auto' or 'smooth'.
top: Math.floor(container.scrollHeight / 2), behavior: 'instant'
astandrik
changed the title
tests/suites/paginatedTable/mocks.ts
Outdated
| await page.route(`${backend}/api/settings`, async (route) => { | ||
| await new Promise((resolve) => setTimeout(resolve, MOCK_DELAY)); | ||
|
|
||
| await route.fulfill({ | ||
| status: 200, | ||
| contentType: 'application/json', | ||
| body: JSON.stringify({ | ||
| theme: 'light', | ||
| language: 'en', | ||
| autoRefreshInterval: 10000, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Could you please clarify, why do we need such mock? Our settings is stored in LS only, currently we don't have any api to store settings
There was a problem hiding this comment.
dont think we really need it
| '.g-tree-select.g-table-column-setup button', | ||
| ); | ||
| this.columnSetupPopup = page.locator('.g-popup .g-select-popup.g-tree-select__popup'); | ||
| this.scrollContainer = '.ydb-cluster'; |
There was a problem hiding this comment.
Note for future: current PaginatedTable class is not really reusable, it's actually cluster table with set of controls, that is typical for nodes and storage tables. So, for example, you cannot use it to test table on disks pages. Also I had some difficulties with using it inside table groups - class is set up for one table and one set of controls. I suggest splitting it in the future, so it will be PaginatedTable, TableControls and some PagePaginatedTable for every page or particular component.
Example for scrollContainer property:
class PaginatedTable {
constructor(page: Page, scrollContainer: string) {}
}
class ClusterNodesTable extends PaginatedTable {
constructor(page: Page){
super(page, '.ydb-cluster')
// some code
}
}
astandrik
deleted the
astandrik.move-storage-tables-to-paginated-1693-2
branch
This pull request includes several changes to the
src/containersdirectory to replace the usage ofStorageWrapperwithPaginatedStorageand makeviewContextoptional. Additionally, it removes theStoragecomponent and updates related utility functions and types accordingly.StorageWrapperwithPaginatedStorageStoragecomponentviewContextoptionalStorageWrappercomponentCloses #1693
CI Results
Test Status: ✅ PASSED
📊 Full Report
Test Changes Summary ✨5
✨ New Tests (5)
Bundle Size: 🔽
Current: 65.85 MB | Main: 65.88 MB
Diff: 0.02 MB (-0.03%)
✅ Bundle size decreased.
ℹ️ CI Information