Skip to content

fix: DH-22758: Fix column resize bug on columns with the same display text#2691

Open
vbabich wants to merge 4 commits into
deephaven:mainfrom
vbabich:vlad-DH-22758
Open

fix: DH-22758: Fix column resize bug on columns with the same display text#2691
vbabich wants to merge 4 commits into
deephaven:mainfrom
vbabich:vlad-DH-22758

Conversation

@vbabich

@vbabich vbabich commented May 28, 2026

Copy link
Copy Markdown
Collaborator
  • Fix the issue with unclickable column separators between column groups with the same display text
  • Fix hover on the separators after the last column, updated ci snapshots

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 50.48%. Comparing base (d3474f6) to head (50ac447).

Files with missing lines Patch % Lines
packages/grid/src/GridRenderer.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2691      +/-   ##
==========================================
+ Coverage   50.47%   50.48%   +0.01%     
==========================================
  Files         787      787              
  Lines       44836    44841       +5     
  Branches    11609    11610       +1     
==========================================
+ Hits        22629    22639      +10     
- Misses      22161    22183      +22     
+ Partials       46       19      -27     
Flag Coverage Δ
unit 50.48% <88.88%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@vbabich vbabich requested a review from Copilot May 28, 2026 20:51
@vbabich vbabich requested review from a team, dgodinez-dh and mattrunyon and removed request for a team and mattrunyon May 28, 2026 20:56
@vbabich vbabich enabled auto-merge (squash) May 28, 2026 20:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment on lines 485 to 487
// Always show separator for the last column
if (nextColumnIndex == null) {
return true;
Comment on lines +1262 to +1267
shouldDrawSeparator = GridUtils.hasColumnSeparatorAtDepth(
model,
depth,
columnIndex,
nextColumnIndex
);
@vbabich vbabich disabled auto-merge May 28, 2026 21:43

@dgodinez-dh dgodinez-dh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code changes look good to me. Copilot brought up two issues. One of them looks to be a comment on pre-existing code. The other, I'm not sure if it is relevant to the intendend change.

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.

3 participants