feat(PaginatedStorage): add grouping #1364
Conversation
| const chunkOffset = id * limit; | ||
| const remainingLenght = totalLength - chunkOffset; | ||
| const calculatedChunkLength = remainingLenght < limit ? remainingLenght : limit; |
There was a problem hiding this comment.
This allows not to show 50 (limit) loading rows for the last chunk, when we know how many entities are left. Also it allows to set some predefined initial length to table
| <StorageGroupsControls | ||
| withTypeSelector | ||
| entitiesCountCurrent={storageGroups.length} | ||
| entitiesCountTotal={groupsTotalCount} | ||
| entitiesLoading={isLoading} | ||
| columnsToSelect={storageGroupsColumnsToSelect} | ||
| handleSelectedColumnsUpdate={setStorageGroupsSelectedColumns} | ||
| /> | ||
| ) : null} | ||
| {isNodes ? ( | ||
| <StorageNodesControls | ||
| withTypeSelector | ||
| entitiesCountCurrent={storageNodes.length} | ||
| entitiesCountTotal={nodesTotalCount} | ||
| entitiesLoading={isLoading} | ||
| columnsToSelect={storageNodesColumnsToSelect} | ||
| handleSelectedColumnsUpdate={setStorageNodesSelectedColumns} | ||
| /> | ||
| ) : null} |
There was a problem hiding this comment.
Added different controls to the same component. As the next step Storage could be split into two components like PaginatedStorage
| const getConcurrentId = (limit?: number, offset?: number) => { | ||
| return `getStorageGroups|offset${offset}|limit${limit}`; | ||
| }; |
There was a problem hiding this comment.
Requests are managed by PaginatedTable and rtk-query, there is no need to manage it additionally by axios, moreover - in case with grouping it creates a bug
artemmufazalov
force-pushed
the
1302-paginated-storage-grouping
branch
2 times, most recently
from
a34479b
to
a16b0be
Compare
artemmufazalov
force-pushed
the
1302-paginated-storage-grouping
branch
from
a16b0be
to
451e279
Compare
|
Let's add padding on the right of the groups container. |
|
If filter is set, I would probably expand groups, that contain filtered entities. |
|
I think we should preserve selected GroupBy setting when switch between Groups and Nodes storage. |
|
|
||
| const renderTitle = () => { | ||
| return ( | ||
| <div onClick={toggleCollapsed} className={b('title-wrapper')}> |
There was a problem hiding this comment.
Lets try to use button here, otherwise the element isn't reachable from keyboard
There was a problem hiding this comment.
Button applies it's own styles, that doesn't fit here, so I added tabIndex to div
There was a problem hiding this comment.
For now I may focus, but it doesn't expand on Enter.
Lets use button css api, like in this example.
artemmufazalov
force-pushed
the
1302-paginated-storage-grouping
branch
from
085e585
to
03dd19b
Compare
Groups are filtered as well, so all groups on the screen will have only filtered entities |
There is an issue with groups being closed every time search changes, I will fix it |
It's preserved if it's the property, that exist for both nodes and groups. You may try "Disk space usage". However, if it's a property, that doesn't valid for one type of storage - it isn't used, so if groups will be grouped by In the code single search query // StorageGroups
export const storageGroupsGroupByParamSchema = z
.custom<
GroupsGroupByField | undefined
>((value) => STORAGE_GROUPS_GROUP_BY_PARAMS.includes(value))
.catch(undefined);
// StorageNodes
export const storageNodesGroupByParamSchema = z
.custom<NodesGroupByField | undefined>((value) => STORAGE_NODES_GROUP_BY_PARAMS.includes(value))
.catch(undefined);
// useStorageQueryParams
const storageGroupsGroupByParam = storageGroupsGroupByParamSchema.parse(queryParams.groupBy);
const storageNodesGroupByParam = storageNodesGroupByParamSchema.parse(queryParams.groupBy); |
I'm not sure this is a proper way to handle it... as for me, we have to different storage types. And we should treat them separately. All controls should belong to selected storage type. |
It seems you did a fix, for now expand state doesn't reset on search change. But what do you think if we automatically expand a group, if 1) search is not empty 2) only one group is in result. |
I don't like the idea to expand something automatically. This groups can be used not only to group entities, but also to view overall cluster statistics. By enabling some group by option, I can see, how cluster feels in term of that option. Automatic expand does't really suit this case. There is no indication on how much table groups are rendered. We have only one group -> it's automatically expanded -> it's not obvious for me, that we have only one group until I scroll to bottom. Also it's optimization - content is not loaded and rendered until group is expanded. To sum up, I don't think we should auto expand, but we can discuss it and implement in next iteration if needed |
Agree with your args! lets stick to this implementation. Think about it if we have users request. |
Closes: #1365
Stand: https://nda.ya.ru/t/_9dz2Iw678WGs4
To test grouping, enable
Use paginated tablessetting in experimentsCI Results
Test Status:⚠️ FLAKY
📊 Full Report
Bundle Size: 🔺
Current: 79.13 MB | Main: 79.08 MB
Diff: +0.04 MB (0.06%)
ℹ️ CI Information