feat(DockView): bump version 10.0.10#989
Merged
Merged
Conversation
Reviewer's GuideRefactors DockView configuration and initialization to remove the legacy fixObject helper, make layout config reconstruction more robust, add a fallback path when stored/layout configs are invalid, and adjust rendering/maximize behavior to avoid unnecessary or incorrect panel loading; also exports new helpers and minorly updates group/panel logic. Sequence diagram for Dockview initialization with config fallbackssequenceDiagram
participant Init as initDockview
participant Dockview as DockviewComponent
participant Config as dockview_config
participant Options as DockviewOptions
Init->>Dockview: set init()
activate Dockview
Dockview->>Config: getConfig(Options)
activate Config
alt getConfig throws
Config-->>Dockview: error
Dockview->>Config: getConfigFromContent(Options)
Config-->>Dockview: configFromContent
else getConfig succeeds
Config-->>Dockview: config
end
deactivate Config
Dockview->>Dockview: fromJSON(config)
alt fromJSON throws
Dockview-->>Dockview: error logged
Dockview->>Config: getConfigFromContent(Options)
activate Config
Config-->>Dockview: configFromContent
deactivate Config
Dockview->>Dockview: fromJSON(configFromContent)
else fromJSON succeeds
Dockview-->>Dockview: layout initialized
end
Dockview->>Dockview: params.floatingGroups = config.floatingGroups || []
deactivate Dockview
Sequence diagram for panel visibility during group maximizationsequenceDiagram
actor User
participant Group as DockviewGroup
participant Toggle as toggleFull
participant Panel as DockviewPanel
User->>Group: click maximize button
Group->>Toggle: toggleFull(group, actionContainer, maximize)
activate Toggle
Toggle->>Group: group.api.accessor.maximizing = true
alt grid group
Toggle->>Group: group.api.maximize() or exitMaximized()
else floating group
Toggle->>Group: floatingMaximize(group) or floatingExitMaximized(group)
end
Toggle->>Group: group.api.accessor.maximizing = false
deactivate Toggle
Note over Panel,Group: Later, panel visibility changes
Panel->>Panel: onDidVisibilityChange(event)
Panel->>Panel: if dockview._isDisposed or dockview.maximizing return
Panel->>Panel: else handle renderer onlyWhenVisible
Updated class diagram for Dockview modules and config helpersclassDiagram
class DockviewComponent {
+any params
+any panels
+any floatingGroups
+boolean _isDisposed
+boolean _inited
+void fromJSON(config)
+void updateTheme()
+void init()
+void update(options)
}
class DockviewParams {
+DockviewOptions options
+any observer
+any floatingGroups
+any theme
}
class DockviewOptions {
+string localStorageKey
+string layoutConfig
+string renderer
+boolean lock
+string theme
+any content
+number width
+number height
}
class DockviewGroup {
+any api
+any model
+any header
+any panels
+HTMLElement element
}
class DockviewGroupApiAccessor {
+boolean maximizing
}
class DockviewPanel {
+any api
+any accessor
+any params
+void onDidVisibilityChange(handler)
}
class DockviewConfigModule {
+any getConfig(options)
+any getConfigFromStorage(options)
+any getConfigFromOptions(options)
+any getConfigFromLayoutString(options)
+any getConfigFromContent(options)
+any renewConfigFromOptions(config, options)
+any reloadFromConfig(dockview, options)
+void saveConfig(dockview)
+void loadPanelsFromLocalstorage(dockview)
}
class ConfigRoot {
+string activeGroup
+ConfigGrid grid
+ConfigPanels panels
}
class ConfigGrid {
+number width
+number height
+string orientation
+ConfigBranch root
}
class ConfigBranch {
+string type
+any data
+any position
}
class ConfigPanel {
+string id
+any params
}
DockviewComponent o-- DockviewParams
DockviewParams o-- DockviewOptions
DockviewComponent "*" o-- DockviewGroup
DockviewGroup o-- DockviewGroupApiAccessor : accessor
DockviewComponent "*" o-- DockviewPanel
DockviewPanel --> DockviewComponent : accessor
DockviewConfigModule --> ConfigRoot
ConfigRoot o-- ConfigGrid
ConfigRoot "*" o-- ConfigPanels
ConfigGrid o-- ConfigBranch
ConfigBranch "*" o-- ConfigBranch : data
ConfigPanels "*" o-- ConfigPanel
class ConfigPanels {
}
class ObservePanelActiveChange {
+void observePanelActiveChange(panel)
}
ObservePanelActiveChange ..> DockviewPanel : uses
ObservePanelActiveChange ..> DockviewOptions : reads renderer
class ToggleFullHelper {
+void toggleFull(group, actionContainer, maximize)
}
ToggleFullHelper ..> DockviewGroup
ToggleFullHelper ..> DockviewGroupApiAccessor : sets maximizing
class DockviewUtilsModule {
+void initDockview(dockview, options, template)
+void update(dockview, options)
}
DockviewUtilsModule ..> DockviewComponent
DockviewUtilsModule ..> DockviewConfigModule : getConfig, getConfigFromContent
DockviewUtilsModule ..> ToggleFullHelper
DockviewUtilsModule ..> ObservePanelActiveChange
Flow diagram for Dockview config reconstruction without fixObjectflowchart TD
A[Start getConfig] --> B[Has options.layoutConfig?]
B -->|Yes| C[Parse layoutConfig JSON]
C --> D[renewConfigFromOptions config and options]
D --> E[Final config]
B -->|No| F[loadPanelsFromLocalstorage]
F --> G[Has stored layout?]
G -->|Yes| H[Parse stored JSON]
H --> I[renewConfigFromOptions config and options]
I --> E
G -->|No| J[getConfigFromContent options]
J --> E
E --> K[Return config]
subgraph renewConfigFromOptions
R1[Iterate panels in config]
R1 --> R2[Find optionPanel by id]
R2 -->|Found| R3[Merge params and visible]
R3 --> R4[Set optionPanel.id = panel.id]
R4 --> R5[Replace config panels by panel id]
R2 -->|Not found| R6[Remove panel from config]
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 found 2 issues, and left some high level feedback:
- In
dockview-config.js,getConfigFromLayoutStringnow callsrenewConfigFromOptions(config)without passingoptions, butrenewConfigFromOptionsstill expects and usesoptions, so this will result inoptionsbeingundefinedat runtime and likely break layout renewal. - The new
maximizingflag intoggleFullis set before and cleared after the maximize operation, but not in atry/finally; if an exception occurs during maximize/exitMaximized,maximizingcould remaintrueand suppress visibility handling unexpectedly. - In
dockview.update, the theme change check now comparesoldOptions.theme.classNametooptions.themebut no longer updatesdockview.options.theme.className(or another target) to the new value, soupdateTheme()may run with stale theme data unless the theme is being set elsewhere.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `dockview-config.js`, `getConfigFromLayoutString` now calls `renewConfigFromOptions(config)` without passing `options`, but `renewConfigFromOptions` still expects and uses `options`, so this will result in `options` being `undefined` at runtime and likely break layout renewal.
- The new `maximizing` flag in `toggleFull` is set before and cleared after the maximize operation, but not in a `try/finally`; if an exception occurs during maximize/exitMaximized, `maximizing` could remain `true` and suppress visibility handling unexpectedly.
- In `dockview.update`, the theme change check now compares `oldOptions.theme.className` to `options.theme` but no longer updates `dockview.options.theme.className` (or another target) to the new value, so `updateTheme()` may run with stale theme data unless the theme is being set elsewhere.
## Individual Comments
### Comment 1
<location path="src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-group.js" line_range="451-454" />
<code_context>
}
const toggleFull = (group, actionContainer, maximize) => {
+ group.api.accessor.maximizing = true;
const type = group.model.location.type;
if (type === 'grid') {
</code_context>
<issue_to_address>
**issue (bug_risk):** maximizing flag should be reset even if maximizing/exitMaximized throws, to avoid leaving the dockview in a bad state.
If any of the maximize/exitMaximized variants (`maximize`, `exitMaximized`, `floatingMaximize`, `floatingExitMaximized`) throws, `group.api.accessor.maximizing` will remain `true` and `observePanelActiveChange`’s visibility handling will never run again. Consider wrapping the relevant calls in `try { ... } finally { group.api.accessor.maximizing = false; }` to guarantee the flag is reset.
</issue_to_address>
### Comment 2
<location path="src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-utils.js" line_range="61-64" />
<code_context>
+ const oldOptions = dockview.params.options;
+ dockview.params.options = {...options, renderer: options.renderer || 'onlyWhenVisible' };
+
+ if (oldOptions.lock !== options.lock) {
toggleGroupLock(dockview, options);
}
- if (dockview.options.theme.className !== options.theme) {
- dockview.options.theme.className = options.theme;
+ if (oldOptions.theme.className !== options.theme) {
dockview.updateTheme();
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using oldOptions for theme comparison may be fragile if the theme shape changes or is not an object.
The previous code compared `dockview.options.theme.className` to `options.theme` and then updated `dockview.options.theme.className`. Now it compares `oldOptions.theme.className` to `options.theme` without updating the dockview theme object. If `options.theme` is a string or `oldOptions.theme` is absent/non-object, this can throw or fail to detect theme changes. Consider normalizing `options.theme` to a consistent shape (e.g., an object with `className`) and updating the dockview theme’s source of truth before calling `updateTheme()`.
Suggested implementation:
```javascript
dockview.update = options => {
const oldOptions = dockview.params.options || {};
// normalize theme to a consistent shape
const nextTheme =
typeof options.theme === 'string'
? { className: options.theme }
: (options.theme || {});
dockview.params.options = {
...options,
theme: nextTheme,
renderer: options.renderer || 'onlyWhenVisible',
};
// toggle lock state if it changed
if (oldOptions.lock !== options.lock) {
toggleGroupLock(dockview, options);
}
// safely compare previous theme to new theme and update dockview theme source of truth
const previousTheme = oldOptions.theme;
const previousClassName =
previousTheme && typeof previousTheme === 'object'
? previousTheme.className
: previousTheme;
if (previousClassName !== nextTheme.className) {
if (!dockview.options) {
dockview.options = {};
}
dockview.options.theme = nextTheme;
dockview.updateTheme();
}
try {
```
If other parts of the codebase assume `dockview.params.options.theme` is a string, you should update those usages to expect an object with a `className` property, or adjust the normalization logic to keep both forms (e.g., store the string separately while still providing an object to `dockview.options.theme`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+451
to
454
| group.api.accessor.maximizing = true; | ||
| const type = group.model.location.type; | ||
| if (type === 'grid') { | ||
| maximize ? group.api.exitMaximized() : group.api.maximize(); |
There was a problem hiding this comment.
issue (bug_risk): maximizing flag should be reset even if maximizing/exitMaximized throws, to avoid leaving the dockview in a bad state.
If any of the maximize/exitMaximized variants (maximize, exitMaximized, floatingMaximize, floatingExitMaximized) throws, group.api.accessor.maximizing will remain true and observePanelActiveChange’s visibility handling will never run again. Consider wrapping the relevant calls in try { ... } finally { group.api.accessor.maximizing = false; } to guarantee the flag is reset.
Comment on lines
+61
to
+64
| if (oldOptions.lock !== options.lock) { | ||
| toggleGroupLock(dockview, options); | ||
| } | ||
| if (dockview.options.theme.className !== options.theme) { | ||
| dockview.options.theme.className = options.theme; | ||
| if (oldOptions.theme.className !== options.theme) { |
There was a problem hiding this comment.
suggestion (bug_risk): Using oldOptions for theme comparison may be fragile if the theme shape changes or is not an object.
The previous code compared dockview.options.theme.className to options.theme and then updated dockview.options.theme.className. Now it compares oldOptions.theme.className to options.theme without updating the dockview theme object. If options.theme is a string or oldOptions.theme is absent/non-object, this can throw or fail to detect theme changes. Consider normalizing options.theme to a consistent shape (e.g., an object with className) and updating the dockview theme’s source of truth before calling updateTheme().
Suggested implementation:
dockview.update = options => {
const oldOptions = dockview.params.options || {};
// normalize theme to a consistent shape
const nextTheme =
typeof options.theme === 'string'
? { className: options.theme }
: (options.theme || {});
dockview.params.options = {
...options,
theme: nextTheme,
renderer: options.renderer || 'onlyWhenVisible',
};
// toggle lock state if it changed
if (oldOptions.lock !== options.lock) {
toggleGroupLock(dockview, options);
}
// safely compare previous theme to new theme and update dockview theme source of truth
const previousTheme = oldOptions.theme;
const previousClassName =
previousTheme && typeof previousTheme === 'object'
? previousTheme.className
: previousTheme;
if (previousClassName !== nextTheme.className) {
if (!dockview.options) {
dockview.options = {};
}
dockview.options.theme = nextTheme;
dockview.updateTheme();
}
try {If other parts of the codebase assume dockview.params.options.theme is a string, you should update those usages to expect an object with a className property, or adjust the normalization logic to keep both forms (e.g., store the string separately while still providing an object to dockview.options.theme).
There was a problem hiding this comment.
Pull request overview
Updates the BootstrapBlazor.DockView package to v10.0.10 while also adjusting DockView client-side (JS) layout/config loading and some runtime behaviors (maximize/visibility handling, theme/update flow).
Changes:
- Bumped
BootstrapBlazor.DockViewpackage version from10.0.9to10.0.10. - Updated DockView JS config loading (fallback to content-derived config) and removed the unused
dockview-fix.js. - Adjusted panel visibility handling during maximize and tweaked update logic for renderer/theme.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-utils.js | Adds config-loading fallbacks and changes update/theme handling + tab loading behavior. |
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-panel.js | Skips visibility-change handling while maximizing. |
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-group.js | Sets a maximizing flag during maximize/restore operations. |
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-fix.js | Removes the file entirely. |
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-extensions.js | Removes an unused import from config module. |
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-config.js | Refactors config renewal and exports (now exports getConfigFromContent). |
| src/components/BootstrapBlazor.DockView/BootstrapBlazor.DockView.csproj | Version bump to 10.0.10. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| }); | ||
| return fixObject(config); | ||
| return renewConfigFromOptions(config); |
| dockview.fromJSON(config); | ||
| } catch (error) { | ||
| console.error(error); | ||
| dockview.fromJSON(getConfigFromContent(options)); |
Comment on lines
46
to
47
| dockview.params.floatingGroups = config.floatingGroups || [] | ||
| dockview.fromJSON(config); | ||
| } |
| } | ||
| if (dockview.options.theme.className !== options.theme) { | ||
| dockview.options.theme.className = options.theme; | ||
| if (oldOptions.theme.className !== options.theme) { |
Comment on lines
3
to
5
| <PropertyGroup> | ||
| <Version>10.0.9</Version> | ||
| <Version>10.0.10</Version> | ||
| </PropertyGroup> |
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 #988
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Improve DockView layout initialization and rendering behavior while cleaning up obsolete configuration utilities.
New Features:
Bug Fixes:
Enhancements:
Build: