Skip to content

fix: Simple Pivot prerequisites#2437

Merged
vbabich merged 9 commits intodeephaven:mainfrom
vbabich:simple-pivot-prerequisites
May 14, 2025
Merged

fix: Simple Pivot prerequisites#2437
vbabich merged 9 commits intodeephaven:mainfrom
vbabich:simple-pivot-prerequisites

Conversation

@vbabich
Copy link
Copy Markdown
Collaborator

@vbabich vbabich commented May 7, 2025

  • Reset movedColumns in IrisGrid and re-calculate metrics on TABLE_CHANGED event
  • Fix the issue with grid metrics calculations based on stale state in Grid.componentDidUpdate
  • Disable advanced filters for non-filterable columns
  • Expose DisplayColumn type export
  • Trigger COLUMNS_CHANGED instead of TABLE_CHANGED in updateFrozenColumns

@vbabich vbabich self-assigned this May 7, 2025
@vbabich vbabich requested review from Copilot and mofojed May 7, 2025 23:44
Copy link
Copy Markdown
Contributor

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 fixes issues related to simple pivot prerequisites by resetting movedColumns on TABLE_CHANGED events, recalculating grid metrics based on updated state, and disabling advanced filters for non-filterable columns while also exposing the DisplayColumn type export.

  • Updated context menu actions to disable advanced filtering on non-filterable columns.
  • Added TABLE_CHANGED event listener in IrisGrid to reset movedColumns on table updates.
  • Refactored state updates in Grid to batch changes and update metrics accordingly.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/iris-grid/src/mousehandlers/IrisGridContextMenuHandler.tsx Added a disabled property based on model.isFilterable to the context menu action.
packages/iris-grid/src/index.ts Added a wildcard export from IrisGridModel to expose additional members.
packages/iris-grid/src/IrisGrid.tsx Bound and registered a new handleTableChanged event handler for TABLE_CHANGED events.
packages/grid/src/Grid.tsx Refactored state update calls to batch state changes before updating metrics.
Comments suppressed due to low confidence (2)

packages/iris-grid/src/index.ts:15

  • The wildcard export may inadvertently expose internal members. Consider verifying that this export is intentional and does not lead to naming collisions or leaking implementation details.
export * from './IrisGridModel';

packages/grid/src/Grid.tsx:611

  • [nitpick] Batching state updates and passing the updated state to updateMetrics is a good optimization; please double-check that the state used in updateMetrics accurately reflects all intended changes before the canvas is re-rendered.
this.setState(updatedState);

Comment thread packages/iris-grid/src/IrisGrid.tsx
@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2025

Codecov Report

Attention: Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.

Project coverage is 47.23%. Comparing base (8dd24c0) to head (1858adb).

Files with missing lines Patch % Lines
packages/grid/src/Grid.tsx 50.00% 4 Missing ⚠️
packages/iris-grid/src/IrisGrid.tsx 33.33% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #2437    +/-   ##
========================================
  Coverage   47.23%   47.23%            
========================================
  Files         718      718            
  Lines       39594    39603     +9     
  Branches    10098     9909   -189     
========================================
+ Hits        18701    18706     +5     
- Misses      20882    20886     +4     
  Partials       11       11            
Flag Coverage Δ
unit 47.23% <42.85%> (+<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.

mofojed
mofojed previously approved these changes May 8, 2025
@mofojed
Copy link
Copy Markdown
Member

mofojed commented May 8, 2025

@vbabich Might need to update snapshots... for some reason I can't download the playwright report right now.

Comment thread packages/iris-grid/src/IrisGridTableModel.ts
@vbabich vbabich merged commit 75fe9c1 into deephaven:main May 14, 2025
11 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators May 14, 2025
@vbabich vbabich deleted the simple-pivot-prerequisites branch May 14, 2025 18:56
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.

3 participants