-
Notifications
You must be signed in to change notification settings - Fork 32
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: Multiple dashboards #1714
Conversation
}: AppDashboardsProps): JSX.Element { | ||
const connection = useConnection(); | ||
|
||
const hydratePanel = useCallback( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will this work from Enterprise? We need to be able to pass in the hydration function and pull the query from the metadata for Enterprise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this hydration specifically will affect Enterprise. This is just moving the hydration function from AppMainContainer
to AppDashboards
since AppMainContainer
doesn't need to know anything about this once the dashboards component is split out
Enterprise has its own hydration which would be part of the Dashboard
already
As far as Enterprise changes go, it will need to add dashboardPluginData
and also listen to the event for creating a dashboard like DHC does. There might be something w/ the plugin and plugin data that we need to modify to work w/ enterprise PQs since right now the plugin is responsible for fetching its widget
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought about extending WidgetPlugin
in some manner so that it doesn't have to mount a panel. Right now it's kind of hacky IMO because the dashboard widget from dh.ui is loaded as part of a dashboard plugin. Instead it could be a widget plugin that mounts, but doesn't render a panel.
Or we need some sort of context/hook that lets the plugin get its own fetch function or create one with just the widget name and type. In DHE the hook could wrap any other info like PQ data into the fetch function it provides
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A plugin that mounts but doesn't have a panel sounds kinda neat. Need to give it more thought.
I've fixed the console focus issue. The problem was if the console was focused when a dashboard was opened, it didn't resize monaco when it became visible again. In enterprise we're doing some extra stuff that calls |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1714 +/- ##
==========================================
- Coverage 46.05% 45.99% -0.06%
==========================================
Files 623 626 +3
Lines 37514 37576 +62
Branches 9433 9450 +17
==========================================
+ Hits 17276 17284 +8
- Misses 20184 20237 +53
- Partials 54 55 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
}: AppDashboardsProps): JSX.Element { | ||
const connection = useConnection(); | ||
|
||
const hydratePanel = useCallback( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A plugin that mounts but doesn't have a panel sounds kinda neat. Need to give it more thought.
Fixes #1683
Test with the dh.ui plugin from deephaven/deephaven-plugins#176
Example dashboard code