fix(DockView): correctness fixes for group delete/re-insert, float and dock-back#1046
Closed
zhaijunlei955 wants to merge 4 commits into
Closed
fix(DockView): correctness fixes for group delete/re-insert, float and dock-back#1046zhaijunlei955 wants to merge 4 commits into
zhaijunlei955 wants to merge 4 commits into
Conversation
…cross close and re-insert
… the resized width
Reviewer's GuideAdjusts DockView group lifecycle behavior for delete → re-insert → float → dock flows, ensuring placeholder groups defer action states, floating groups preserve size/position, origin groups restore width and action buttons correctly, and floating-group closes propagate per-panel visibility updates. Sequence diagram for DockView group delete → re-insert → float → dock lifecyclesequenceDiagram
actor User
participant DockviewComponent
participant Group
participant Panel
participant dockview_group_js as addPanelWidthGroupId
participant dockview_extensions_js as DockviewComponent_removeGroup
participant dockview_panel_js as onRemovePanel
participant DockFunction as dock
participant ActionStates as reRenderActionStates
User->>DockviewComponent: removeGroup(group)
DockviewComponent->>DockviewComponent_removeGroup: DockviewComponent.prototype.removeGroup
DockviewComponent_removeGroup->>Group: type == floating
DockviewComponent_removeGroup->>Group: removeDrawerBtn
alt floating group has panels
DockviewComponent_removeGroup->>Group: [...group.panels]
loop each panel
DockviewComponent_removeGroup->>Panel: panel.api.close()
Panel->>dockview_panel_js: onRemovePanel
dockview_panel_js->>Panel: set panel.params.currentPosition
end
else no panels
DockviewComponent_removeGroup->>DockviewComponent: removeGroup.apply(this, args)
end
User->>dockview_group_js: addPanelWidthGroupId(dockview, panel, index)
dockview_group_js->>Group: dockview.api.getGroup(panel.groupId)
alt reused empty group
dockview_group_js->>dockview_group_js: reusedEmptyGroup = true
dockview_group_js->>dockview_group_js: restoreWidth = panel.params.currentPosition.width
else new floating group
dockview_group_js->>dockview_group_js: isNewFloatingGroup = true
dockview_group_js->>dockview_group_js: floatingGroupRect from currentPosition or rect
end
dockview_group_js->>DockviewComponent: dockview.addPanel({ initialWidth })
alt new floating group
dockview_group_js->>Panel: panel.api.setActive()
end
alt reused empty grid group
dockview_group_js->>ActionStates: reRenderActionStates(group)
dockview_group_js->>Group: group.api._pendingSize = undefined
end
User->>DockFunction: dock(group, floatType)
DockFunction->>DockviewComponent: moveGroupOrPanel
DockFunction->>Group: originGroup
DockFunction->>ActionStates: reRenderActionStates(originGroup)
DockFunction->>DockviewComponent: saveConfig
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for your PR, @zhaijunlei955. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Directly assigning to
group.api._pendingSizeinaddPanelWidthGroupIdcouples this fix to an internal/private field on the Dockview API; consider tracking pending sizes in your own metadata or via a public Dockview API to avoid breakage if upstream internals change. - In
DockviewComponent.prototype.removeGroupfor floating groups, the method now always returnsundefinedafter closing panels (sinceremoveGroup.applyis only called when there are no panels); if callers rely on the original return value, consider capturing and returning the result of the lastpanel.api.close()or explicitly documenting/standardizing the new behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Directly assigning to `group.api._pendingSize` in `addPanelWidthGroupId` couples this fix to an internal/private field on the Dockview API; consider tracking pending sizes in your own metadata or via a public Dockview API to avoid breakage if upstream internals change.
- In `DockviewComponent.prototype.removeGroup` for floating groups, the method now always returns `undefined` after closing panels (since `removeGroup.apply` is only called when there are no panels); if callers rely on the original return value, consider capturing and returning the result of the last `panel.api.close()` or explicitly documenting/standardizing the new behavior.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
A set of related fixes for the group lifecycle (delete → re-insert → float → dock).
Fixes
group is empty (avoids
[].every()showing wrong buttons that stick), and restorethe pre-delete width instead of collapsing to the ~100px minimum.
which were deferred while it sat empty as a placeholder.
individually, so external panel buttons stay highlight-synced; a re-added floating
group gets its active panel set (no blank), and a hidden→re-inserted floating group
keeps its last size & position (offset-based capture, immune to
style:auto)._pendingSizeleft by thewidth restore, so a group resized after re-opening no longer snaps back to its old
width on dock-back.
Scope
DockView only, 3 JS files (
dockview-group.js,dockview-extensions.js,dockview-panel.js). Only the delete/re-insert/float/dock paths are touched; adding atab to an existing group and all non-floating paths are unchanged.
Summary by Sourcery
Improve DockView group lifecycle handling across delete, re-insert, float, and dock-back flows to preserve layout, state, and actions.
Bug Fixes: