fix: Re-opening widgets after re-hydrated #379
Merged
Conversation
This file contains 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
- There was a race condition when replacing the widget, where the old panel content was getting removed before the new panel was being put up - In that case, we want to wait for the portal opened event before emitting an open event - Also check that we're not getting a duplicate open event in the panel manager - The duplicate panel ID was getting logged with the widget that was saved, which would sometimes cause widgets to randomly re-open - Still screwing up in some cases. Panel manager is getting the open event twice and therefore not keeping track of the IDs correctly
- Need to update unit tests - Tested with two widgets, `foo` and `bar` - Tested that they saved with the layout when opened - Tested that they re-activated when re-creating the item in the console (re-opening it) - Tested that they were removed from the plugin data when all panels closed - TODO: Handling if there's errors loading the widget on startup, showing a loading spinner in panels while loading the widget
- Basically reverted to what they were before, which is good
mattrunyon
requested changes
Mar 25, 2024
Comment on lines
51
to
52
| const [widgetData] = useState<WidgetData>(() => structuredClone(data)); | ||
| const [panelIds] = useState<string[]>([]); |
There was a problem hiding this comment.
Why useState with no setter instead of useRef? Also, why is panelIds separate from widgetData.panelIds here?
There was a problem hiding this comment.
Added a comment about why I did it specifically for widgetData.
For panelIds, I could use useRef. Just means I'd be adding .current everywhere else.
- Extract getPreservedData method - Add tests - Add some more comments
mattrunyon
requested changes
Mar 27, 2024
Co-authored-by: Matthew Runyon <[email protected]>
mattrunyon
requested changes
Apr 3, 2024
mattrunyon
approved these changes
Apr 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
LayoutUtils.openComponentto open the panel, which would replace the existing panel; which triggered a panel close event, then a panel open event.panelIdswidget data, which would cause them to re-open even if you "closed" all the panelsLayoutUtils.openComponentif the panel does not yet existpanelIdsmultiple times, and we're removing it correctly on close