Skip to content

fix(Grid): respect rowStart when copying selected columns - #1271

Open
fallintoplace wants to merge 3 commits into
ClickHouse:mainfrom
fallintoplace:fix/grid-copy-row-start-20261017
Open

fallintoplace wants to merge 3 commits into
ClickHouse:mainfrom
fallintoplace:fix/grid-copy-row-start-20261017

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Description

  • What: Copy selected Grid columns from the current rowStart.
  • Why: Copying on later pages starts from row zero.
  • Implementation: Forward rowStart to the copy helper and keep selected column indexes zero-based.
  • Example: At rowStart={100}, copying three rows uses 100, 101, and 102.

Links and tickets

  • None.

Good to know

  • No visual or breaking changes.

Checklist

  • Breaking changes? (add migration notes in changesets)
  • Visual changes? (specify in changesets)
  • Design review needed?

Contribution

  • Sufficient research before PR
  • Self-reviewed the PR
  • Manually tested the changes (when applies - visual confirmation in Storybook)
  • build and build-storybook work locally
  • Tests and Stories are aligned with the changes

Accessibility

  • Keyboard - every action works with Tab / Enter / Space / Esc; focus is visible and returns to the trigger on close
  • Names — every control has a name that includes its visible label; icon-only controls take aria-label
  • Vision — colors come from tokens, focus ring shows, targets are at least 24×24 px

Screenshots

No visual changes.


Note

Low Risk
Localized clipboard/copy helper behavior with tests; no API or visual changes beyond fixing incorrect copied row/column indices.

Overview
Fixes Grid clipboard copy when the visible window uses a non-zero rowStart (e.g. paged or virtualized data). Column selections were rendered from row 0 instead of the current offset.

The default copy path in Grid now passes rowStart into copyGridElements, which renames pageStart to rowStart and uses rowStart + index when building rows for a columns selection. For rows selection, column indices stay 0-based (0 … columnCount - 1) instead of incorrectly offsetting columns by the row window.

Tests cover integration (Ctrl+C forwards rowStart) and copy helper behavior for column and row selections with rowStart: 100. Patch changeset for @clickhouse/click-ui.

Reviewed by Cursor Bugbot for commit 6f6b15a. Bugbot is set up for automated code reviews on this repo. Configure here.

@changeset-bot

changeset-bot Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 777df0d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@clickhouse/click-ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@XOP XOP added the bug Something isn't working label Oct 21, 2026
@XOP

XOP commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

@fallintoplace please check the formatting,
other than that the fix looks good, thank you!

@workflow-authentication-public

Copy link
Copy Markdown
Contributor

Chromatic Storybook

Check Status Link
Storybook ✅ 664 stories published Open Storybook
UI Tests ✅ 664 tests unchanged Open build

Built from commit: 777df0d90250ef2f031cbd33a98a68e086dcb6a3 · Chromatic run

Statuses as of the end of the run. The build link shows the live review state.

@fallintoplace

Copy link
Copy Markdown
Contributor Author

Thanks, it was already fixed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants