fix(DockView): restore a re-shown group's size only when collapsed to its minimum#1051
Merged
Conversation
|
Thanks for your PR, @zhaijunlei955. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts how DockView restores panel group sizes when re-showing a previously deleted group, restoring saved width/height only when the placeholder group is actually collapsed to its minimum size and clearing pending sizes after use. Sequence diagram for restoring DockView group size on re-showsequenceDiagram
participant DockView as dockview
participant Group as group
participant Panel as panel
participant Parent as group_parent_element
DockView->>Group: addPanelWidthGroupId(dockview, panel, index)
Note over Group,Panel: reusedEmptyGroup && group.api.location.type === grid
Group->>Parent: read offsetWidth, offsetHeight
Group->>Panel: read params.currentPosition (cp.width, cp.height)
alt [parent collapsed to minimum]
Group->>DockView: set initialWidth = cp.width (if width > offsetWidth && offsetWidth <= minimumWidth + 2)
Group->>DockView: set initialHeight = cp.height (if height > offsetHeight && offsetHeight <= minimumHeight + 2)
else [parent has real size]
Group->>DockView: leave initialWidth, initialHeight undefined
end
DockView->>Group: addPanel({ initialWidth, initialHeight, ... })
Group->>Group: reRenderActionStates(group)
Group->>Group: group.api._pendingSize = undefined (if initialWidth > 0 || initialHeight > 0)
Flow diagram for conditional restoration of DockView group width/heightflowchart TD
A[reusedEmptyGroup && location.type === grid && parentElement exists] --> B[get cp.width, cp.height]
B --> C[get parent offsetWidth, offsetHeight]
C --> D{offsetWidth <= minimumWidth + 2\nand cp.width > offsetWidth}
D -->|yes| E[initialWidth = cp.width]
D -->|no| F[initialWidth undefined]
C --> G{offsetHeight <= minimumHeight + 2\nand cp.height > offsetHeight}
G -->|yes| H[initialHeight = cp.height]
G -->|no| I[initialHeight undefined]
E --> J[call dockview.addPanel with initialWidth/initialHeight]
F --> J
H --> J
I --> J
J --> K{initialWidth > 0 or initialHeight > 0}
K -->|yes| L[group.api._pendingSize = undefined]
K -->|no| M[leave _pendingSize unchanged]
File-Level Changes
Possibly linked issues
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:
- The
+ 2tolerance when comparingoffsetWidth/offsetHeighttominimumWidth/Heightis a bit opaque; consider extracting this into a named constant or helper to document the rationale and keep the magic number in one place. - The
reusedEmptyGroup && group.api.location.type === 'grid'guard is now used in two places; consider centralizing the grid-specific reuse logic into a small helper to avoid duplicating conditions and keep behavior changes localized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `+ 2` tolerance when comparing `offsetWidth/offsetHeight` to `minimumWidth/Height` is a bit opaque; consider extracting this into a named constant or helper to document the rationale and keep the magic number in one place.
- The `reusedEmptyGroup && group.api.location.type === 'grid'` guard is now used in two places; consider centralizing the grid-specific reuse logic into a small helper to avoid duplicating conditions and keep behavior changes localized.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ArgoZhang
approved these changes
Jun 26, 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.
#Issues
close #1052
Follow-up to #1047, which restored a re-opened group's width unconditionally and never restored height. In a left/right + nested (top/bottom) layout, re-showing a previously-closed group could then misbehave:
Problem
Fix
Restore a saved dimension (width and height) only when the group is actually collapsed to its minimum (
offset <= group.minimum + 2); if the layout already gave it a real size, keep it..dv-viewis an unstyled absolute wrapper, so its offset size equals the splitview size exactly — at collapse that is the group's minimum, making the check reliable.Known limitation
Deleting both groups of one side in sequence then re-adding them can still squeeze one (the later-deleted group captured the full size while alone). Inherent to storing absolute sizes at delete time; a proportional / redistribute-on-re-add fix is deferred.
Summary by Sourcery
Adjust DockView group size restoration when re-showing previously closed groups to avoid layout issues in nested grid splits.
Bug Fixes:
Enhancements: