fix: paginated table in groups scrolled to top on refresh #2291

astandrik · 2025-05-16T08:01:23Z

CI Results

Test Status: ⚠️ FLAKY

Total	Passed	Failed	Flaky	Skipped
318	317	0	1	0

😟 No changes in tests. 😕

Bundle Size: 🔺

Current: 83.62 MB | Main: 83.59 MB
Diff: +0.04 MB (0.04%)

⚠️ Bundle size increased. Please review.

ℹ️ 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.

Copilot

Pull Request Overview

This PR refactors paginated tables to use a scrollContainerRef instead of parentRef and introduces a context-based table state (PaginatedTableProvider) along with automatic scroll adjustment (useTableScroll). It also removes the old renderControls props and centralizes control rendering via context consumers.

Swap parentRef for scrollContainerRef across table components
Introduce PaginatedTableProvider and remove per-component control rendering
Add useTableScroll hook and update TableWithControlsLayout.Table for sticky scroll adjustments

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/containers/Storage/StorageNodes/getNodes.ts	Added mock generation path via URL params
src/components/TableWithControlsLayout/useTableScroll.ts	New hook to sync table and container scrolling
src/components/TableWithControlsLayout/TableWithControlsLayout.tsx	Updated Table component to use `useTableScroll` and refs
src/components/PaginatedTable/ResizeablePaginatedTable.tsx	Added `onStateChange` prop (unused)
src/components/PaginatedTable/PaginatedTable.tsx	Removed `parentRef` and moved to context for scroll handling

Comments suppressed due to low confidence (1)

src/containers/Storage/StorageNodes/nodes.ts:25

[nitpick] The callback parameter Name starts with an uppercase letter and may be confused with a type or class. Rename it to name for consistency with JavaScript naming conventions.

    return poolNames.slice(0, count).map((Name) => ({

src/components/PaginatedTable/ResizeablePaginatedTable.tsx

src/containers/Nodes/NodesTable.tsx

src/components/TableWithControlsLayout/useTableScroll.ts

src/containers/Storage/StorageNodes/getNodes.ts

Copilot

Pull Request Overview

This PR standardizes scrolling behavior by replacing parentRef with scrollContainerRef, introduces automatic table scrolling on updates, and refactors paginated table state to use a React context.

Replace all parentRef props with scrollContainerRef to unify scroll logic.
Add useTableScroll hook and extend TableWithControlsLayout for auto-scrolling and full-height support.
Refactor PaginatedTable to use a context provider for internal state, removing ad-hoc control rendering.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/containers/Tablets/TabletsTable.tsx	Enable full-height layout to support new scroll behavior
src/components/TableWithControlsLayout/useTableScroll.ts	New hook to handle scrolling the table into view
src/components/TableWithControlsLayout/TableWithControlsLayout.tsx	Extend layout with `fullHeight`, `scrollContainerRef`, and dependencies
src/components/PaginatedTable/types.ts	Update types: rename `parentRef` → `scrollContainerRef`, adjust render control signatures
src/components/PaginatedTable/PaginatedTableContext.tsx	Introduce `PaginatedTableProvider` context for table state
src/components/PaginatedTable/useScrollBasedChunks.ts	Rename hook prop to `scrollContainerRef` and update internal logic
src/components/PaginatedTable/PaginatedTable.tsx	Refactor to use context, remove `parentRef`, and simplify rendering
various containers (`Storage`, `Nodes`, `Cluster.tsx`, etc.)	Replace `parentRef` with `scrollContainerRef` in paginated/table components

Comments suppressed due to low confidence (3)

src/containers/Storage/StorageNodes/PaginatedStorageNodes.tsx:37

[nitpick] Multiple *ControlsWithTableState components across containers share identical patterns. Consider extracting a generic higher-order component to avoid duplication and ease future updates.

function StorageNodesControlsWithTableState({

src/components/PaginatedTable/PaginatedTable.tsx:131

The previous implementation scrolled the container to top on filter/reset via parentRef.current.scrollTo(0,0). With parentRef removed, the table no longer resets scroll position on filter changes; consider invoking scrollContainerRef.current.scrollTo(0, 0) after resetting state.

React.useLayoutEffect(() => {

src/components/TableWithControlsLayout/useTableScroll.ts:1

The new useTableScroll hook controls critical scroll behavior but lacks dedicated unit tests. Consider adding tests to verify scroll adjustment logic under different table positions and dependency changes.

import React from 'react';

src/components/TableWithControlsLayout/useTableScroll.ts

src/containers/Tenant/Diagnostics/TopicData/TopicData.tsx

astandrik · 2025-05-19T13:00:19Z

scrolls with small table leaves some extra pixels of cluster overview

astandrik · 2025-05-20T16:58:11Z

@Raubzeug moved pyramid to separate component
fixed scrolling for databases and tablets
fixed extra pixels issue

stand redeployed

Copilot

Pull Request Overview

This PR fixes the issue where paginated tables in groups were not scrolled to the top on refresh by refactoring scrolling logic and updating component props.

Replaces the old "parentRef" prop with the new "scrollContainerRef" across multiple components.
Updates table layout and scroll behavior with the new useTableScroll hook and revised CSS variables.
Introduces wrapper components (e.g., StorageNodesControlsWithTableState, StorageGroupGroup) and switches to PaginatedTableWithLayout for improved state management and scrolling.

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/containers/Storage/TableGroup/TableGroup.tsx	Refactored TableGroup to an arrow function and set displayName.
src/containers/Storage/StorageNodes/PaginatedStorageNodesTable.tsx	Replaced parentRef with scrollContainerRef and removed renderControls.
src/containers/Storage/StorageGroups/PaginatedStorageGroupsTable.tsx	Consistently updated prop names and types; similar changes as StorageNodes table.
src/containers/Storage/PaginatedStorageNodes.tsx	Updated scroll container handling and introduced new wrapper components.
src/containers/Storage/PaginatedStorageGroups.tsx	Similar scroll refactoring and integration with PaginatedTableWithLayout.
Various files in Nodes, PDiskPage, Cluster, and Table component directories	Replaced parentRef with scrollContainerRef and updated scrolling behavior accordingly.
src/components/TableWithControlsLayout	Introduced useTableScroll hook and updated layout styles.
src/components/PaginatedTable/*	Updated types, context, and scroll handling logic to align with new naming and scrolling mechanisms.

Comments suppressed due to low confidence (1)

src/components/PaginatedTable/types.ts:68

Verify that the change to the RenderControls type signature (removing parameters) does not break any existing implementations that might rely on receiving control state parameters.

export type RenderControls = () => React.ReactNode;

src/components/PaginatedTable/PaginatedTable.tsx

astandrik · 2025-05-20T17:55:59Z

splitted PaginatedStorageNodes and PaginatedStorageGroups to directories as they are totally uncomprehensible as single large files

Copilot

Pull Request Overview

This PR refactors table scrolling logic to support a unified scrollContainerRef, replaces legacy parentRef props, and integrates useTableScroll for automatic scroll adjustments.

Swapped parentRef for scrollContainerRef across storage, nodes, cluster, and paginated table components
Introduced useTableScroll in TableWithControlsLayout.Table and propagated scrollDependencies for automatic repositioning
Updated SCSS variables for sticky header offsets and added full-height support

Reviewed Changes

Copilot reviewed 67 out of 67 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/components/TableWithControlsLayout/TableWithControlsLayout.tsx	Added `scrollContainerRef` and `useTableScroll` integration
src/components/TableWithControlsLayout/useTableScroll.ts	New hook to align tables within scroll containers
src/components/PaginatedTable/PaginatedTable.tsx	Removed `parentRef`, adopted context state, reset logic updated
src/components/PaginatedTable/useScrollBasedChunks.ts	Renamed `parentRef` → `scrollContainerRef`
src/components/PaginatedTable/PaginatedTableWithLayout.tsx	New wrapper using `PaginatedTableProvider` and full-height support
src/containers/Storage/...	Replaced `parentRef` with `scrollContainerRef` in all storage-related paginated components
src/containers/Nodes/...	Updated to use `scrollContainerRef` in `Nodes` and `NodesTable`
src/containers/Cluster/Cluster.tsx	Propagated `scrollContainerRef` to `TabletsTable`, `Tenants`, `Nodes`, and `PaginatedStorage`
src/containers/PDiskPage/PDiskPage.tsx	Swapped `parentRef` for `scrollContainerRef` in `<PaginatedStorage>` invocation
src/containers/Storage/PaginatedStorage.tsx	Renamed prop `parentRef` → `scrollContainerRef`
src/containers/Storage/PaginatedStorageGroups/...	Updated all related components to use `scrollContainerRef`

Comments suppressed due to low confidence (3)

src/components/PaginatedTable/PaginatedTable.tsx:128

The manual scroll-to-top on filter or data refresh was removed; without calling scrollContainerRef.current.scrollTo(0, 0), users may remain mid-scroll after updates. Consider adding scrollContainerRef?.current?.scrollTo(0, 0); inside this effect to reset the viewport.

React.useLayoutEffect(() => {

src/containers/Storage/PaginatedStorageGroups/PaginatedStorageGroups.tsx:21

[nitpick] There is a typo in the variable name storageGroupsHandlerHasGroupping (double 'p'). Please rename to storageGroupsHandlerHasGrouping for consistency and clarity.

const storageGroupsHandlerHasGroupping = useStorageGroupsHandlerHasGrouping();

src/components/TableWithControlsLayout/useTableScroll.ts:102

The new scroll adjustment logic in useTableScroll lacks dedicated unit or integration tests. Consider adding tests that simulate container scroll and dependency changes to verify correct offset calculations and behavior.

React.useLayoutEffect(() => {

Raubzeug · 2025-05-21T06:37:42Z

Screen.Recording.2025-05-21.at.09.36.19.mov

astandrik · 2025-05-21T11:57:49Z

@Raubzeug as for now all known issues should be fixed (your screencast too)

Stand redeployed

astandrik · 2025-05-22T12:57:31Z

@Raubzeug conflicts resolved

stand redeployed

Raubzeug · 2025-05-21T06:45:57Z

src/components/PaginatedTable/PaginatedTable.tsx

+    const {sortParams, foundEntities} = tableState;
+
+    // Initialize state with props if available
+    React.useEffect(() => {


Why not pass initialSortParams and initialEntitiesCount as part of initial state?

good point!

Raubzeug · 2025-05-21T06:50:26Z

src/components/TableWithControlsLayout/TableWithControlsLayout.scss


    display: inline-block;

    box-sizing: border-box;
    min-width: 100%;

+    &_fullHeight {


lets not use camel case in css

astandrik force-pushed the astandrik.2200 branch 2 times, most recently from 13bd573 to a5eb2ea Compare May 19, 2025 11:14

astandrik marked this pull request as ready for review May 19, 2025 11:22

astandrik requested review from Copilot, artemmufazalov and Raubzeug May 19, 2025 11:22

Copilot AI reviewed May 19, 2025

View reviewed changes

astandrik requested a review from Copilot May 19, 2025 11:40

Copilot AI reviewed May 19, 2025

View reviewed changes

src/components/TableWithControlsLayout/useTableScroll.ts Show resolved Hide resolved

astandrik commented May 19, 2025

View reviewed changes

src/containers/Tenant/Diagnostics/TopicData/TopicData.tsx Outdated Show resolved Hide resolved

astandrik requested a review from Copilot May 20, 2025 16:58

Copilot AI reviewed May 20, 2025

View reviewed changes

src/components/PaginatedTable/PaginatedTable.tsx Show resolved Hide resolved

astandrik requested a review from Copilot May 20, 2025 18:02

Copilot AI reviewed May 20, 2025

View reviewed changes

astandrik and others added 3 commits May 22, 2025 10:39

chore: refactor paginated tables

b0a2bdd

fix: nanofix

9104e8d

fix: rebase resolve conflicts

c91ec51

astandrik force-pushed the astandrik.2200 branch from d8fafbc to c91ec51 Compare May 22, 2025 12:52

fix: fullHeight -> full-height

55928c3

Raubzeug approved these changes May 22, 2025

View reviewed changes

astandrik mentioned this pull request May 22, 2025

Pass initialSortParams and initialEntitiesCount as part of initial state? #2315

Closed

astandrik added this pull request to the merge queue May 22, 2025

Merged via the queue into main with commit f4971ee May 22, 2025
8 checks passed

astandrik deleted the astandrik.2200 branch May 22, 2025 13:48

ydb-platform-bot mentioned this pull request May 22, 2025

chore(main): release 9.2.1 #2316

Merged

ydb-platform-bot mentioned this pull request Jun 23, 2025

chore(hotfix/v9.6.3-hotfix.2): release 9.6.3-hotfix.2 #2465

Closed

fix: paginated table in groups scrolled to top on refresh #2291

fix: paginated table in groups scrolled to top on refresh #2291

Uh oh!

Conversation

astandrik commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Results

Test Status: ⚠️ FLAKY

Bundle Size: 🔺

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

astandrik commented May 19, 2025

Uh oh!

astandrik commented May 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

astandrik commented May 20, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Raubzeug commented May 21, 2025

Uh oh!

astandrik commented May 21, 2025

Uh oh!

astandrik commented May 22, 2025

Uh oh!

Raubzeug May 21, 2025

Choose a reason for hiding this comment

Uh oh!

astandrik May 22, 2025

Choose a reason for hiding this comment

Uh oh!

astandrik May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Raubzeug May 21, 2025

Choose a reason for hiding this comment

Uh oh!

astandrik May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

astandrik commented May 16, 2025 •

edited

Loading