Skip to content

feat: Table plugin support for GridWidgetPlugin#2478

Merged
mattrunyon merged 3 commits intodeephaven:mainfrom
mattrunyon:dh-18601
Jul 15, 2025
Merged

feat: Table plugin support for GridWidgetPlugin#2478
mattrunyon merged 3 commits intodeephaven:mainfrom
mattrunyon:dh-18601

Conversation

@mattrunyon
Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon commented Jun 28, 2025

Also fixes selection and panelState reactivity (for GridWidget, not IrisGridPanel). Could fix for IrisGridPanel here as well if desired.

Passes the panel name as tableName since tables do not actually have names. Mimics the panel prop passed to plugins to give irisGrid and getTableName which might have been previously used. Should keep most things backwards compatible unless there were other items from the panel being used.

One concern I have is the filters from plugins are completely invisible to users once they are applied. They don't go to quick filters or anywhere else, just applied with no visual. This probably makes sense as a separate ticket though.

Testing

You can checkout my plugins branch https://github.com/mattrunyon/deephaven-plugins/tree/table-plugin-example and use the local JS plugin server.

Then use the following code to test (stocks will not update the counter state since it uses the IrisGridPanel version which doesn't update state prop to the component)

import deephaven.ui as ui
import deephaven.plot.express as dx

stocks = dx.data.stocks().with_attributes({'PluginName' : 'table-example'})
t = ui.panel(stocks)

@mattrunyon mattrunyon requested a review from mofojed June 28, 2025 02:08
@mattrunyon mattrunyon self-assigned this Jun 28, 2025
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Good overall, I haven't actually tested it with a plugin. If you update the Test Plan on the ticket attach a built plugin to make it easy for testers.

Comment thread packages/dashboard-core-plugins/src/TablePluginWrapper.tsx Outdated
Comment thread packages/dashboard-core-plugins/src/useTablePlugin.tsx
Comment thread packages/dashboard-core-plugins/src/useIrisGridModel.ts Outdated
* @param item Golden layout content item to search for the content item
* @param searchId the ID
*/
static getContentItemById(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm surprised this didn't already exist.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I was too. It looks like we always just used the panel or glContainer to find the item since we only accessed the GL content item from the panel component.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It looks like we have PanelManager.getContainerByPanelId which is close. I'm going to leave this util I think because the closest we can do is the logic from that which gets the stack for the item with config containing the id, then gets the item from the stack

This also lets you get a row or column which wouldn't work with the other logic (since they're not in a stack), but I can't think of a useful reason for that.

* The model for the table this plugin is associated with.
*/
model: IrisGridModel;
model: IrisGridTableModelTemplate;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mmm at some point wish we could clean up our GridModel types. Should all be defined as types/interfaces instead of classes.

@mattrunyon mattrunyon requested a review from mofojed July 8, 2025 02:56
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 8, 2025

Codecov Report

Attention: Patch coverage is 71.42857% with 16 lines in your changes missing coverage. Please review.

Project coverage is 44.65%. Comparing base (5b572c3) to head (cb52739).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
.../dashboard-core-plugins/src/TablePluginWrapper.tsx 12.50% 14 Missing ⚠️
...ages/dashboard-core-plugins/src/useTablePlugin.tsx 94.44% 1 Missing ⚠️
packages/dashboard/src/layout/LayoutUtils.ts 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2478      +/-   ##
==========================================
+ Coverage   44.54%   44.65%   +0.11%     
==========================================
  Files         759      761       +2     
  Lines       42526    42597      +71     
  Branches    10877    10902      +25     
==========================================
+ Hits        18942    19021      +79     
+ Misses      23528    23520       -8     
  Partials       56       56              
Flag Coverage Δ
unit 44.65% <71.42%> (+0.11%) ⬆️

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.

@mattrunyon mattrunyon merged commit d698759 into deephaven:main Jul 15, 2025
12 checks passed
@mattrunyon mattrunyon deleted the dh-18601 branch July 15, 2025 14:24
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 15, 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