feat: Widget re-hydration #288
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
mofojed
commented
Feb 15, 2024
mofojed
commented
- Reload widgets on dashboard reload
- Needs my draft PR that stores default dashboard data with the workspace for this to work (fix: Load default dashboard data from workspace data 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 Storing what documents we have open, along with the name of the document #160
- Fixes Re-open document on page refresh (don't worry about previous panels at this step) #161
- Fixes Storing which dashboards/layout/panels are open from this document #162
- Created an `elements` folder and put all elements in that
WIP hydrating data - In the middle of breaking stuff and wiring stuff back up - Had an idea for a scoped ID thing but nowhere to use it yet Re-wire PortalPanel so that it emits an event when it is opened instead - Doesn't throw when the onOpen/onClosed props are not provided anymore - Uses a scoped ID so all panels will be scoped to a particular widget - Does not re-open yet - widget is not saving the IDs of all the panels it's opened, or re-using them WIP wire up ReactPanelManager to dish out IDs - Now the IDs come from ReactPanelManager, so we just need to save those IDs with the widget data and presto WIP Save widget data with the widget - Seems to be trying to save it correctly, but it's never loading? - Maybe because it's the DEFAULT dashboard? Rename ScopedIdWrapper Fix merge conflicts WIP it's rehydrating now, but in a new panel - Need the panel IDs to match, currently it's adding the scope to it again somewhere WIP refactor the PortalPanelManager - Now there is one PortalPanelManager map that all ReactPanels pull the element from - Makes managing the portals easier, panels stay in the same spot upon reload - Still need to clean up code Get rid of debug logging lines Clean up/refactor some code a bit - Move panel stuff into /layout - Move widget stuff into /widget Clean up any opened panels when widget is closed - Console emits the PanelEvent.CLOSE immediately on startup when "Close disconnected panels" is true. In that case, the Widget has not yet loaded so we just need to close any panels that were previously opened by the widget just in case Fix existing tests Add some tests for PortalPanelManager
- Isn't actually used in this PR yet. This is just for deephaven#162
- Fix loading dashboards from plugin data
- In case data formats change
- These were made in another branch
mattrunyon
requested changes
Feb 22, 2024
- Don't pass `WidgetWrapper` directly to `WidgetHandler`, just pass in the individual props - Add a `DashboardWidgetHandler` to handle mapping the `WidgetHandler` callbacks to the dashboard callbacks - It's the only thing that uses the `WidgetId` there, `WidgetHandler` doesn't actually need it - In the future we should be able to get rid of passing through `fetch`, and just have a `useWidget` hook that automatically loads the widget/retries when necessary.
mattrunyon
requested changes
Feb 23, 2024
Comment on lines
+109
to
+113
| export const { | ||
| listen: listenForPortalOpened, | ||
| emit: emitPortalOpened, | ||
| useListener: usePortalOpenedListener, | ||
| } = makeEventFunctions<PortalOpenedPayload>(PORTAL_OPENED); |
Collaborator
There was a problem hiding this comment.
I tried putting a comment inline with this (like you can for interfaces), but doesn't seem to work for a way to document the method. Kind of unfortunate
You can add a comment if you do this though, but defeats some of the point of this util
const portalOpenedEvents = makeEventFunctions<PortalOpenedPayload>(PORTAL_OPENED);
/**
* This is a jsdoc
*/
export emitPortalOpened = portalOpenedEvents.emit;
Member
Author
There was a problem hiding this comment.
That doesn't seem to work for me? I've tried inline comments as well as many other ways but can't seem to do it.
| // On rehydration, yield known IDs first | ||
| // If there are no more known IDs, generate a new one. | ||
| // This can happen if the document hasn't been opened before, or if it's rehydrated and a new panel is added. | ||
| // Note that if the order of panels changes, the worst case scenario is that panels appear in the wrong location in the layout. |
Collaborator
There was a problem hiding this comment.
Could the panel somehow end up with the wrong data? Like let's say you have t1 and t2 with some filters and then their order get flipped. Would t1 now have t2 filters trying to be applied?
Member
Author
There was a problem hiding this comment.
The panel could end up in the wrong place. We should allow a key (or id) to be passed in to ui.panel to allow for being more explicit about the key of your panel. I don't think that should be part of this ticket though.
mattrunyon
approved these changes
Feb 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.