feat(DockView): add onPanelVisibleChanged event#980
Open
feat(DockView): add onPanelVisibleChanged event#980
Conversation
Reviewer's GuideImplements a new panel visibility change handling flow for DockView, improves panel lifecycle and toggle logic, and hardens initialization and cleanup against disposed instances and missing DOM, while adding minor behavioral tweaks like inactive panel creation and safer panel deletion. Sequence diagram for the updated panel visibility change handlingsequenceDiagram
participant Panel
participant PanelApi
participant Group
participant Dockview
participant LoadTabsEvent
Panel->>PanelApi: onDidVisibilityChange callback(isVisible)
activate PanelApi
PanelApi->>Dockview: get accessor
Note over Dockview: dockview._isDisposed?
alt Dockview is disposed
PanelApi-->>Panel: return
else Dockview is not disposed
PanelApi->>Dockview: read params.options.renderer
Note over Dockview: renderer === onlyWhenVisible && dockview._inited?
alt onlyWhenVisible and inited
alt isVisible
PanelApi->>Dockview: saveConfig(dockview)
PanelApi->>Dockview: collect visible panels from groups
Dockview-->>LoadTabsEvent: _loadTabs.fire(visiblePanelKeys)
else not isVisible
PanelApi->>Panel: appendTemplatePanelEle(panel)
end
end
Note over Group: group.getParams().floatType == drawer?
alt isVisible and floatType drawer
PanelApi->>Group: setDrawerTitle(group)
end
PanelApi->>PanelApi: setTimeout(0)
Note over PanelApi: after timeout
PanelApi->>Dockview: check dockview._isDisposed
alt Dockview is disposed
PanelApi-->>Panel: return
else Dockview is not disposed
PanelApi->>Panel: moveAlwaysRenderPanel(panel)
end
end
deactivate PanelApi
Updated class diagram for Dockview panel lifecycle and extensionsclassDiagram
class Dockview {
+any params
+array~DockviewGroup~ groups
+array~DockviewPanel~ panels
+HTMLElement element
+boolean _inited
+boolean _isDisposed
+any _loadTabs
+any gridview
+addPanel(id, title, inactive, renderer, component, position, params)
}
class DockviewGroup {
+array~DockviewPanel~ panels
+getParams()
}
class DockviewPanel {
+any api
+any accessor
+any group
+any params
+string renderer
+string component
+string id
+string title
}
class DockviewGroupPanel {
+array~DockviewPanel~ panels
+any accessor
+getParams()
+setParams(data)
}
class DockviewGroupPanelModel {
+any accessor
+closePanel(panel, triggerVisibleChange, moveToTemplate)
}
class DockviewUtils {
+initDockview(dockview, options, template)
+toggleComponent(dockview, options)
+cleanUndefined(obj)
}
Dockview "1" o-- "many" DockviewGroup : groups
Dockview "1" o-- "many" DockviewPanel : panels
DockviewGroup "1" o-- "many" DockviewPanel : panels
DockviewGroupPanel "1" o-- "many" DockviewPanel : panels
DockviewGroupPanelModel "1" o-- "1" DockviewGroupPanel : controls
class PanelVisibilityObserver {
+observePanelActiveChange(panel)
}
PanelVisibilityObserver ..> DockviewPanel : observes
PanelVisibilityObserver ..> Dockview : uses accessor
class InitFlow {
+savePanel(dockview, panel)
+deletePanel(dockview, panel)
+setWidth(target, dockview)
+addPanelWidthGroupId(dockview, panel, index)
+addPanelWidthCreatGroup(dockview, panel, panels)
}
InitFlow ..> Dockview : manages panels
%% Key behavioral updates
%% - DockviewPanel.api.onDidVisibilityChange now drives visibility logic
%% - Dockview.panels deletion uses key-based findIndex
%% - initDockview guards against disposed dockview and missing DOM
%% - toggleComponent merges panels with cleanUndefined
%% - addPanel* helpers mark new panels as inactive at creation
Flow diagram for the updated toggleComponent panel merge logicflowchart TD
A[Start toggleComponent] --> B[getPanelsFromOptions options]
B --> C[Filter panels where params.visible is true]
C --> D[Get panels from dockview.params.panels as localPanels]
D --> E[For each visible options panel p]
E --> F{Exists in localPanels?}
F -- Yes --> G[Use local panel instance pan]
G --> H[Update group and index based on panels and parentId]
F -- No --> I[Find existingPanel in dockview.params.panels matching key]
I --> J{existingPanel found?}
J -- Yes --> K[Create merged panel object]
K --> L[Merge top-level fields using cleanUndefined]
L --> M[Merge params using cleanUndefined]
M --> H
J -- No --> N[Use new panel definition p]
N --> H
H --> O[Insert or move panel into group at computed index]
O --> P[Continue for next visible panel]
P --> Q[End toggleComponent]
subgraph "cleanUndefined behavior"
R[Input object] --> S[Remove entries with null or undefined values]
S --> T[Return cleaned object]
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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:
- The helper
cleanUndefinedcurrently filters out bothundefinedandnullvalues (v != null); if the intent is truly only to ignoreundefinedwhile preserving explicitnulls, adjust the predicate or rename the function to better reflect the semantics. observePanelActiveChangeis now wired toonDidVisibilityChangeand no longer uses the panel active state; consider renaming this function (and any related identifiers) to reflect visibility-based behavior to keep the lifecycle logic easier to follow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The helper `cleanUndefined` currently filters out both `undefined` and `null` values (`v != null`); if the intent is truly only to ignore `undefined` while preserving explicit `null`s, adjust the predicate or rename the function to better reflect the semantics.
- `observePanelActiveChange` is now wired to `onDidVisibilityChange` and no longer uses the panel active state; consider renaming this function (and any related identifiers) to reflect visibility-based behavior to keep the lifecycle logic easier to follow.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Adds support for a panelVisibleChanged event in DockView (fixes #979), and adjusts panel/group initialization and visibility handling to better align UI state with saved/localStorage state.
Changes:
- Fire
panelVisibleChangedevents during layout restore and when panels are closed. - Refactor panel visibility handling to drive tab loading and template DOM placement from
onDidVisibilityChange. - Adjust panel creation/merge behavior (avoid overwriting stored values with
undefined/null, addinactiveflag when adding panels).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-utils.js |
Adds disposed-guarding during post-layout init; introduces cleanUndefined and changes panel merge behavior; minor event wiring robustness tweaks. |
src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-panel.js |
Consolidates behavior into onDidVisibilityChange to save config/load tabs and move DOM nodes based on visibility; improves panel deletion matching by key. |
src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-group.js |
Adds inactive: true when adding panels to avoid auto-activation. |
src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-extensions.js |
Removes debug logging; simplifies closePanel template move logic; ensures close triggers panelVisibleChanged(false). |
Comments suppressed due to low confidence (2)
src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-group.js:84
- Passing
inactive: truetodockview.addPanelprevents Dockview from setting the new panel (and possibly the group) active. When this code creates a brand-new group (e.g. whengroupdidn’t exist yet), this can leave the group with no active panel/content rendered until something else activates it. Consider only settinginactive: truewhen adding into an already-active group (to avoid stealing focus), or explicitly activating the first panel after creation when the group has no active panel.
dockview.addPanel({
id: panel.id,
title: panel.title,
inactive: true,
renderer: panel.renderer,
component: panel.component,
position: { referenceGroup: group, index: index || 0 },
params: { ...panel.params, rect, packup, visible: true }
})
src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-group.js:121
inactive: truehere changes the behavior of adding a panel so it won’t become the active/visible panel. If this path is used to show a newly-visible panel (e.g. viatoggleComponent), the UI may not switch to it and may even render an empty group if no other panel is active. If the intent is “don’t steal focus”, consider making this conditional (only when the target group already has an active panel) or activating the panel based onpanel.params.isActive/index after insertion.
let option = {
id: panel.id,
title: panel.title,
inactive: true,
renderer: panel.renderer,
component: panel.component,
position: { referenceGroup: group },
params: { ...panel.params, isPackup, packupHeight, isMaximized, position }
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Link issues
fixes #979
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Handle DockView panel visibility changes uniformly and improve panel lifecycle robustness.
Bug Fixes:
Enhancements: