Skip to content

feat: drawer table scroll #2364

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: drawer table scroll #2364

wants to merge 11 commits into from

Conversation

astandrik
Copy link
Collaborator

@astandrik astandrik commented Jun 4, 2025

Closes #2173
Stand

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total Passed Failed Flaky Skipped
322 319 0 2 1
Test Changes Summary ✨2 ⏭️1

✨ New Tests (2)

  1. Top Query rows components have consistent height across different query lengths (tenant/diagnostics/tabs/queries.test.ts)
  2. Scroll to row, get shareable link, navigate to URL and verify row is scrolled into view (tenant/diagnostics/tabs/queries.test.ts)

⏭️ Skipped Tests (1)

  1. Scroll to row, get shareable link, navigate to URL and verify row is scrolled into view (tenant/diagnostics/tabs/queries.test.ts)

Bundle Size: 🔺

Current: 83.76 MB | Main: 83.76 MB
Diff: +7.77 KB (0.01%)

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

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds automatic scrolling to the selected row in the TopQueries drawer and standardizes row heights by replacing the truncated query component.

  • Introduces useScrollToSelected hook and wires it into TopQueriesData with dynamicInnerRef
  • Adds dynamicRenderType: 'uniform' to table settings for fixed row heights
  • Creates FixedHeightQuery component (replacing TruncatedQuery) and corresponding styles

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/containers/Tenant/Diagnostics/TopQueries/utils.ts Added dynamicRenderType: 'uniform' to table settings
src/containers/Tenant/Diagnostics/TopQueries/hooks/useScrollToSelected.ts New hook to scroll the react-list to the selected row
src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx Swapped out TruncatedQuery for FixedHeightQuery
src/containers/Tenant/Diagnostics/TopQueries/TopQueriesData.tsx Integrated scroll hook, updated settings to use dynamicInnerRef
src/components/FixedHeightQuery/FixedHeightQuery.tsx New component for rendering queries with fixed height and optional clipboard
src/components/FixedHeightQuery/FixedHeightQuery.scss Styles for the fixed-height query display
Comments suppressed due to low confidence (3)

src/containers/Tenant/Diagnostics/TopQueries/hooks/useScrollToSelected.ts:25

  • Consider adding unit tests for this custom hook to verify scrolling logic (visible range check, fallback behavior) under different scenarios.
export function useScrollToSelected({selectedRow, rows, reactListRef}: UseScrollToSelectedParams) {

src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx:5

  • The YDBSyntaxHighlighter import is no longer used after replacing TruncatedQuery, consider removing this unused import.
import {YDBSyntaxHighlighter} from '../../../../../components/SyntaxHighlighter/YDBSyntaxHighlighter';

src/components/FixedHeightQuery/FixedHeightQuery.tsx:20

  • [nitpick] Add a JSDoc comment above the FixedHeightQuery component to describe its purpose and the meaning of its props (lines, hasClipboardButton, etc.).
export const FixedHeightQuery = ({

() => ({
...TOP_QUERIES_TABLE_SETTINGS,
dynamicInnerRef: reactListRef,
// Using 'uniform' type - react-list automatically calculates size from first item
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment

@@ -0,0 +1,42 @@
.kv-fixed-height-query {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use .ydb prefix instead of .kv please. kv is Kikimr Viewer - old name of our UI, it is not used anymore, although it's present in some old components

@astandrik astandrik requested a review from artemmufazalov June 6, 2025 15:48
@astandrik
Copy link
Collaborator Author

@artemmufazalov fixed issues, added couple of tests

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.

feat: drawer table scroll
2 participants