Skip to content

fix: Grid making unnecessary onSelectionChanged calls#2471

Merged
mattrunyon merged 2 commits intodeephaven:mainfrom
mattrunyon:dh-19292
Jun 24, 2025
Merged

fix: Grid making unnecessary onSelectionChanged calls#2471
mattrunyon merged 2 commits intodeephaven:mainfrom
mattrunyon:dh-19292

Conversation

@mattrunyon
Copy link
Copy Markdown
Collaborator

Needed as part of DH-19292 to ensure we don't emit duplicate selection events. Even with debouncing, we might emit the event, and then emit the same selection (in a different array reference) on mouse up. This fixes that so we don't emit the unnecessary events if the mouse up did not remove selections

There are still some quirks around the selection events like when using ctrl+click+drag to deselect part of a range, onSelectionChanged is emitted with the subtracted portion as an extra range (e.g., [[0, 5], [1, 2]]) until mouse up which commits the selection and removes the overlapping ranges ([[0, 0], [3, 5]]). On the dh.ui side I'm just consolidating the ranges, but would probably be good to fix that at some point.

@mattrunyon mattrunyon requested a review from a team June 23, 2025 23:49
@mattrunyon mattrunyon self-assigned this Jun 23, 2025
@mattrunyon mattrunyon requested review from bmingles and removed request for a team June 23, 2025 23:49
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.58%. Comparing base (1c9a8ed) to head (842692d).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2471      +/-   ##
==========================================
- Coverage   47.27%   44.58%   -2.69%     
==========================================
  Files         727      757      +30     
  Lines       39767    42397    +2630     
  Branches     9949    10847     +898     
==========================================
+ Hits        18800    18903     +103     
- Misses      20956    23483    +2527     
  Partials       11       11              
Flag Coverage Δ
unit 44.58% <100.00%> (-2.69%) ⬇️

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.

Comment thread packages/grid/src/Grid.tsx Outdated
@mattrunyon mattrunyon requested a review from bmingles June 24, 2025 15:44
Copy link
Copy Markdown
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

LGTM

@mattrunyon mattrunyon merged commit 495b4b4 into deephaven:main Jun 24, 2025
10 of 11 checks passed
@mattrunyon mattrunyon deleted the dh-19292 branch June 24, 2025 15:59
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants