-
Notifications
You must be signed in to change notification settings - Fork 31
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
fix: Load default dashboard data from workspace data #1810
Conversation
mofojed
commented
Feb 15, 2024
- Until we have support for Persist multiple dashboards #1746 , load the default dashboard data from workspace
- Needed for testing the widget hydration PR
- Tested with my changes for widget hydration
- Ensured deephaven.ui widgets re-opened upon refresh
- Ensured links remained between existing tables
- We need to store the Dashboard data in dashboard storage which doesn't exist in Community
- Needed to load filterSets and links from there still
- There's a different layout for each dashboard, so store the map of them that we're listening to
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1810 +/- ##
==========================================
- Coverage 46.05% 45.98% -0.08%
==========================================
Files 628 631 +3
Lines 37771 37862 +91
Branches 9512 9535 +23
==========================================
+ Hits 17394 17409 +15
- Misses 20323 20399 +76
Partials 54 54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
export interface CreateDashboardPayload<T = unknown> { | ||
pluginId: string; | ||
title: string; | ||
data: unknown; | ||
data: T; |
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.
Why the generic? The data comes from the plugin and we only really care it's serializable
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.
So the listen/emit events can add typing for payload. For the emit function which will come from the plugin, can add typing so we know we're not passing a malformed payload.
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.
Looks good. I think we should wait until the plugin side of this change is also ready in case there are any more generics/type changes that are needed
- Reload widgets on dashboard reload - Needs my draft PR that stores default dashboard data with the workspace for this to work (deephaven/web-client-ui#1810) - Opened deephaven.ui widgets from the examples, then refreshed the page. Widgets re-opened in the same positions/panels they were in. Widget internal state (such as input in a text field, or filter set on a table) and ui.dashboards do _not_ re-hydrate correctly yet - ui.dashboard will do after dashboard storage is properly done, but that shouldn't be needed for this to work in Enterprise - Fixes #160 - Fixes #161 - Fixes #162