Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Convert DashboardPlugins to WidgetPlugins #1598

Merged
merged 5 commits into from
Oct 30, 2023

Conversation

mattrunyon
Copy link
Collaborator

@mattrunyon mattrunyon commented Oct 25, 2023

Fixes #1573

This fix should be adequate for furthering the Deephaven UI alpha. Rather than trying to refactor 2 1k+ line files (IrisGridPanel and ChartPanel), I opted to add some DOM detection to the Panel component to prevent rendering tab context menu/tooltips if it is nested within another panel.

For Deephaven UI, we will need to ensure the UI panel uses Panel at least and passes through glContainer and glEventHub to the child elements so grids and charts function roughly as normal.

Testing

To test how something would look/function when wrapped, remove the panelComponent line from either of the plugin configs. This will wrap in a generic WidgetPanel which you can see in dev tools. The panel tab will not have any of the custom tooltip for grid or custom context menus.

Test by opening grids/figures, applying some filters/sorts and then reloading the page to ensure the state persisted. Also test adding links and ensuring those persisted on reload. Test that a chart built with chart builder works and survives a reload as well.

Followup

We might be able to convert MarkdownPlugin and FilterPlugin, but I wanted to get the main components in first. Not sure if those would work as WidgetPlugins as currently constructed since they don't listen for PanelEvent.OPEN. FilterPlugin probably makes sense to split into a suite of plugins for each filter type which would need #1587

@mattrunyon mattrunyon requested a review from mofojed October 25, 2023 22:06
@mattrunyon mattrunyon self-assigned this Oct 25, 2023
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (54f7cc5) 46.74% compared to head (8533475) 46.69%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1598      +/-   ##
==========================================
- Coverage   46.74%   46.69%   -0.05%     
==========================================
  Files         583      583              
  Lines       36256    36264       +8     
  Branches     9072     9074       +2     
==========================================
- Hits        16947    16933      -14     
- Misses      19257    19279      +22     
  Partials       52       52              
Flag Coverage Δ
unit 46.69% <38.88%> (-0.05%) ⬇️

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

Files Coverage Δ
...es/dashboard-core-plugins/src/ChartPluginConfig.ts 100.00% <100.00%> (ø)
...ges/dashboard-core-plugins/src/GridPluginConfig.ts 100.00% <100.00%> (ø)
...s/dashboard-core-plugins/src/PandasPluginConfig.ts 100.00% <ø> (ø)
.../dashboard-core-plugins/src/WidgetLoaderPlugin.tsx 100.00% <ø> (ø)
...s/dashboard-core-plugins/src/panels/ChartPanel.tsx 61.09% <100.00%> (+0.12%) ⬆️
.../dashboard-core-plugins/src/panels/PandasPanel.tsx 8.00% <ø> (ø)
...ckages/dashboard-core-plugins/src/panels/Panel.tsx 70.44% <100.00%> (+0.56%) ⬆️
...kages/dashboard-core-plugins/src/useHydrateGrid.ts 0.00% <ø> (-30.00%) ⬇️
...ashboard-core-plugins/src/panels/IrisGridPanel.tsx 42.44% <33.33%> (ø)
packages/dashboard-core-plugins/src/GridPlugin.tsx 25.00% <25.00%> (-75.00%) ⬇️
... and 1 more

... and 15 files with indirect coverage changes

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

@mattrunyon mattrunyon requested a review from mofojed October 26, 2023 19:43
@mattrunyon mattrunyon requested a review from mofojed October 27, 2023 17:23
@mattrunyon mattrunyon merged commit a260842 into deephaven:main Oct 30, 2023
5 checks passed
@mattrunyon mattrunyon deleted the widget-plugin-conversion branch October 30, 2023 18:57
@github-actions github-actions bot locked and limited conversation to collaborators Oct 30, 2023
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.

Convert DashboardPlugins to WidgetPlugins
2 participants