fix(DockView): correctness fixes for group delete/re-insert, float and dock-back#1047
Merged
Conversation
…cross close and re-insert
… the resized width
…e group is floated
|
Thanks for your PR, @zhaijunlei955. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's GuideAdjusts DockView group lifecycle behavior (delete → re-insert → float → dock) to correctly reuse placeholder groups, preserve floating position/size and grid width, keep action buttons in sync, and ensure floating panels close and re-open without desynchronizing state. Sequence diagram for panel re-open into floating group and placeholder reusesequenceDiagram
actor User
participant DockviewComponent
participant addPanelWidthGroupId
participant GridGroup as Grid_group_placeholder
participant FloatingGroup as Floating_group
User->>DockviewComponent: reopenPanel(panel)
DockviewComponent->>addPanelWidthGroupId: addPanelWidthGroupId(dockview, panel, index)
addPanelWidthGroupId->>DockviewComponent: dockview.api.getGroup(panel.groupId)
addPanelWidthGroupId->>GridGroup: [group.panels.length === 0 && location.type === grid]
addPanelWidthGroupId->>DockviewComponent: dockview.api.getGroup(getFloatingId(group.id))
addPanelWidthGroupId->>FloatingGroup: [floated.api.location.type === floating]
addPanelWidthGroupId->>DockviewComponent: dockview.addPanel({ id, title, component, position, initialWidth })
addPanelWidthGroupId->>FloatingGroup: group.panels.find(...).api.setActive()
addPanelWidthGroupId->>GridGroup: [reusedEmptyGroup && location.type === grid]
addPanelWidthGroupId->>DockviewComponent: reRenderActionStates(group)
addPanelWidthGroupId->>GridGroup: group.api._pendingSize = undefined
Sequence diagram for closing a floating group via panel closessequenceDiagram
actor User
participant DockviewComponent
participant removeGroupFn as removeGroup
participant Group as Floating_group
participant Panel
User->>DockviewComponent: removeGroup(group)
DockviewComponent->>Group: group.api.location.type
alt type == floating
DockviewComponent->>Group: removeDrawerBtn(group)
DockviewComponent->>Group: [...group.panels]
loop each panel
DockviewComponent->>Panel: panel.api.close()
end
Note over Group: last close re-enters removeGroup as empty
DockviewComponent->>removeGroupFn: removeGroup(group)
else type != floating
DockviewComponent->>removeGroupFn: removeGroup(group)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
addPanelWidthGroupId, clearinggroup.api._pendingSizereaches into a private/underscored field; if possible, prefer using or adding a public API hook for resetting pending size so this logic doesn’t silently break on upstream changes. reRenderActionStatesassumesgroup.headeris always present; adding a null guard aroundgroup.headerbefore querying.dv-right-actions-containerwould make this more robust against groups without headers or lifecycle edge cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `addPanelWidthGroupId`, clearing `group.api._pendingSize` reaches into a private/underscored field; if possible, prefer using or adding a public API hook for resetting pending size so this logic doesn’t silently break on upstream changes.
- `reRenderActionStates` assumes `group.header` is always present; adding a null guard around `group.header` before querying `.dv-right-actions-container` would make this more robust against groups without headers or lifecycle edge cases.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… lives (grid or floating)
ArgoZhang
previously approved these changes
Jun 25, 2026
ArgoZhang
approved these changes
Jun 25, 2026
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
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
close #1048
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); 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.
instead of resurrecting the hidden grid placeholder. This avoids forking one group
into two (a grid copy + the float) — which collided on the float id and left the
second floating group unable to 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 behavior when deleting, re-inserting, floating, and docking groups to avoid placeholder glitches and stale layout state.
Bug Fixes:
Enhancements: