Skip to content

fix: Context menu using stale selection#2414

Merged
ericlln merged 2 commits intodeephaven:release/v0.85from
ericlln:cherry-pick-v0.85-context-menu-using-stale-selections
Apr 10, 2025
Merged

fix: Context menu using stale selection#2414
ericlln merged 2 commits intodeephaven:release/v0.85from
ericlln:cherry-pick-v0.85-context-menu-using-stale-selections

Conversation

@ericlln
Copy link
Copy Markdown
Contributor

@ericlln ericlln commented Apr 10, 2025

Cherry-pick #2407 and #2412 to v0.85

ericlln added 2 commits April 9, 2025 15:58
- Although we handle the case where the grid is right-clicked outside of
the current selection in `GridSelectionHandler.ts`, where the new
selection will be updated to the row the cursor is on, state changes are
batched in event handlers so `IrisGridContextMenuHandler.tsx` will be
using a stale value for `selectedRanges` when it runs.
- To fix this we can implement the exact same logic in both components,
such that the selected ranges used in `onContextMenu` in
`IrisGridContextMenuHandler.tsx` will be the same as what
`onContextMenu` in `GridSelectionHandler.ts` will set it to after all
handlers have ran.

Test Snippet (any table will work though)
```
from deephaven import empty_table, input_table
source = empty_table(10).update(["X = i", "Y = i"])
result = input_table(init_table=source)
```

Before this change, it was seen that right clicking any cell when the
table first opens will not produce the full context menu as expected,
and only subsequent clicks will open the full context menu (probably for
the wrong cell).

(cherry picked from commit c43f306)
…aven#2412)

After merging in deephaven#2407, I
thought a bit more about my implementation and decided to refactor it
(before picking this back to `v0.85`)

It makes more sense for `getLatestSelection` to live in
`GridSelectionMouseHandler` since its logic also depends on the
implementation of `onContextMenu` in this class, and this will make it
more straightforward for other mouse handlers that might want to use
this method to get the Grid's most recent selected ranges.

(cherry picked from commit 23cefe3)
@ericlln ericlln self-assigned this Apr 10, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 10, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 46.73%. Comparing base (632f2f2) to head (6989889).
Report is 1 commits behind head on release/v0.85.

Files with missing lines Patch % Lines
...d/src/mousehandlers/IrisGridContextMenuHandler.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           release/v0.85    #2414   +/-   ##
==============================================
  Coverage          46.72%   46.73%           
==============================================
  Files                695      695           
  Lines              38877    38883    +6     
  Branches            9883     9886    +3     
==============================================
+ Hits               18167    18172    +5     
- Misses             20658    20659    +1     
  Partials              52       52           
Flag Coverage Δ
unit 46.73% <85.71%> (+<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.

@ericlln ericlln requested a review from a team April 10, 2025 16:09
@ericlln ericlln marked this pull request as ready for review April 10, 2025 16:09
@ericlln ericlln requested review from dgodinez-dh and removed request for a team April 10, 2025 16:09
@ericlln ericlln merged commit 5644dad into deephaven:release/v0.85 Apr 10, 2025
12 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 10, 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