Skip to content

feat: Export plotly-express as a dashboard plugin#329

Merged
vbabich merged 3 commits intomainfrom
plotly-express-dashboard-plugin
Mar 5, 2024
Merged

feat: Export plotly-express as a dashboard plugin#329
vbabich merged 3 commits intomainfrom
plotly-express-dashboard-plugin

Conversation

@vbabich
Copy link
Copy Markdown
Collaborator

@vbabich vbabich commented Feb 28, 2024

Tested with DHE V+ (1.20231218.176) Core 0.32.1
Closes #308

BREAKING CHANGE:

  • widget type in the PanelEvent.OPEN event arguments has changed from VariableDefinition to VariableDescriptor

@vbabich vbabich requested a review from mofojed February 28, 2024 17:26
@vbabich vbabich self-assigned this Feb 28, 2024
@vbabich vbabich added this to the February 2024 milestone Feb 28, 2024
Comment thread plugins/plotly-express/src/js/src/index.ts
@mattrunyon
Copy link
Copy Markdown
Collaborator

I think this will break plotly-express in DHC w/ dh.ui. We currently check if the plugin is the legacy format first when loading, so the legacy definition (named DashboardPlugin export) will override the preferred definition (default export of the plugin object)

Comment thread plugins/plotly-express/src/js/src/index.ts
@vbabich vbabich force-pushed the plotly-express-dashboard-plugin branch from cb4dc26 to e8ed513 Compare February 29, 2024 21:26
@vbabich vbabich marked this pull request as ready for review February 29, 2024 21:27
@vbabich
Copy link
Copy Markdown
Collaborator Author

vbabich commented Feb 29, 2024

I added a named export for the legacy DashboardPlugin next to the default WidgetPlugin.
Updated DHE to prioritize legacy plugins until we implement WidgetLoaderPlugin in Enterprise.

Comment thread plugins/plotly-express/src/js/src/DashboardPlugin.tsx Outdated
@vbabich vbabich requested a review from mofojed March 4, 2024 19:30
): JSX.Element | null {
const { id, layout, registerComponent } = props;

const handlePanelOpen = useCallback(
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.

Side note, we have a useDashboardPanel hook that should do what we're doing here, but it would require other changes as well (since it doesn't use metadata or fetch at all). No need to worry about it here.

Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

I think we'll actually want DHE to register both plugins. Otherwise plotly-express won't work in dh.ui in DHE

@vbabich
Copy link
Copy Markdown
Collaborator Author

vbabich commented Mar 5, 2024

Why? What plugin type is needed for dh.ui in DHE?

@mattrunyon
Copy link
Copy Markdown
Collaborator

dh.ui reads the plugin registry and looks for WidgetPlugins to render widgets within panels. So if you have a dh.ui panel with a table and plotly-express plot, it needs both as WidgetPlugins to render properly. Just the DashboardPlugin won't do anything since that's listening at the dashboard level for a plotly-express widget to be opened as a panel.

@vbabich
Copy link
Copy Markdown
Collaborator Author

vbabich commented Mar 5, 2024

But it should work with just the WidgetPlugin, right? The plan is to phase out DashboardPlugin once we add support for WidgetPlugin in DHE.

@mattrunyon
Copy link
Copy Markdown
Collaborator

Yes once we support WidgetPlugins auto opening like in DHC, we'll only need the WidgetPlugin. I think we'll need both in the interim in DHE so that outside of dh.ui the DashboardPlugin will open the panel, but in dh.ui, the WidgetPlugin is needed so dh.ui knows how to render the widget

@vbabich vbabich merged commit 6212bd5 into main Mar 5, 2024
@vbabich vbabich deleted the plotly-express-dashboard-plugin branch March 5, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DH-16500: Release plotly-express js plugin version that can be loaded as a dashboard plugin

3 participants