feat(DockView): add DockViewRenderMode.Partial parameter#957
Merged
feat(DockView): add DockViewRenderMode.Partial parameter#957
Conversation
Reviewer's GuideAdds a new DockViewRenderMode.Partial rendering mode and corresponding lazy/partial tab loading behavior by wiring Blazor components to new JS events, refactoring DockViewComponent into a code-only component that consults DockViewV2.ShowTab, and updating DockView/JS config and utilities to coordinate which tabs are rendered on the server based on client visibility state. Sequence diagram for DockViewRenderMode.Partial tab loadingsequenceDiagram
actor User
participant Browser
participant DockviewJS as Dockview_JS
participant DotNet as DockViewV2_dotnet
participant Component as DockViewComponent
User->>Browser: Interact with dockview layout
Browser->>DockviewJS: Dockview layout/visibility changes
Note over DockviewJS: onDidLayoutFromJSON / onDidVisibilityChange
DockviewJS->>DockviewJS: Compute visiblePanels keys
DockviewJS->>DotNet: invokeMethodAsync(LoadTabs, tabs)
DotNet->>DotNet: LoadTabs(tabs)
DotNet->>DotNet: _loadTabs.Clear() then add each tab key
DotNet->>DotNet: StateHasChanged()
loop Render cycle
DotNet->>Component: BuildRenderTree(builder)
Component->>DotNet: DockView.ShowTab(Key)
alt Renderer == Always
DotNet-->>Component: true
else Renderer == Partial or OnlyWhenVisible
DotNet-->>Component: _loadTabs.Contains(Key)
end
alt ShowTab(Key) == true
Component->>Component: Render ChildContent
else ShowTab(Key) == false
Component-->>Component: Skip ChildContent
end
end
Class diagram for DockView partial rendering and tab loadingclassDiagram
class DockViewComponentBase
class DockViewComponent {
+bool ShowTitleBar
+string? TitleBarIcon
+string? TitleBarIconUrl
+string? Id
+string? Key
+string? Title
+RenderFragment? TitleTemplate
+RenderFragment? ChildContent
+Func~Task~? OnClickTitleBarCallback
-DockViewV2 DockView
+void OnInitialized()
+void BuildRenderTree(RenderTreeBuilder builder)
-Task OnClickBar()
}
class DockViewV2 {
-HashSet~string~ _loadTabs
+DockViewRenderMode Renderer
+Task LoadTabs(List~string~ tabs)
+bool ShowTab(string? key)
+Task PanelVisibleChangedCallbackAsync(string title, bool status)
}
class DockViewRenderMode {
<<enumeration>>
OnlyWhenVisible
Always
Partial
}
class DockViewConfig {
+string? PanelVisibleChangedCallback
+string? LockChangedCallback
+string? SplitterCallback
+string? LoadTabs
+IEnumerable~DockViewComponent~ Contents
}
DockViewComponentBase <|-- DockViewComponent
DockViewComponent ..> DockViewV2 : cascadingParameter
DockViewV2 --> DockViewRenderMode : uses
DockViewV2 --> DockViewConfig : configuredBy
DockViewConfig o--> DockViewComponent : contents
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:
- In
dockview-utils.jstheif (options.renderer === 'always') {}branch is empty and severalrendererassignments are commented out, which makes the behavior of the different render modes harder to follow; consider either implementing thealwayscase explicitly or removing/commenting consistently so the effective behavior is clear. - The
LoadTabsJS-invokable method clears_loadTabsand the JS code for thepartialmode calls_loadTabs?.firetwice (once with visible panels and once with all panels), effectively discarding the first set; if this is not intentional, you may want to consolidate the calls or adjustLoadTabsso you don't overwrite the initially visible set. - In
DockViewComponent.BuildRenderTree,DockView.ShowTab(Key)is used on a[CascadingParameter][NotNull]property; to make the component more robust against missing cascades or initialization-order issues, consider adding a null guard or fallback behavior rather than assumingDockViewis always non-null at render time.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `dockview-utils.js` the `if (options.renderer === 'always') {}` branch is empty and several `renderer` assignments are commented out, which makes the behavior of the different render modes harder to follow; consider either implementing the `always` case explicitly or removing/commenting consistently so the effective behavior is clear.
- The `LoadTabs` JS-invokable method clears `_loadTabs` and the JS code for the `partial` mode calls `_loadTabs?.fire` twice (once with visible panels and once with all panels), effectively discarding the first set; if this is not intentional, you may want to consolidate the calls or adjust `LoadTabs` so you don't overwrite the initially visible set.
- In `DockViewComponent.BuildRenderTree`, `DockView.ShowTab(Key)` is used on a `[CascadingParameter][NotNull]` property; to make the component more robust against missing cascades or initialization-order issues, consider adding a null guard or fallback behavior rather than assuming `DockView` is always non-null at render time.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 a new DockView rendering mode (Partial) and introduces a JS→.NET “loadTabs” signal so the Blazor side can conditionally render panel content based on which tabs should be materialized.
Changes:
- Add
DockViewRenderMode.Partialand wire it through the DockView config/options. - Introduce a
loadTabsevent from JS to .NET ([JSInvokable] LoadTabs) and gate panel rendering viaShowTab. - Adjust JS initialization/visibility handling to request tab rendering via
_loadTabs?.fire(...).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-utils.js | Sets default options.renderer; fires loadTabs during layout initialization |
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-panel.js | Fires loadTabs on panel visibility changes; changes how panel renderer is populated |
| src/components/BootstrapBlazor.DockView/wwwroot/js/dockview-config.js | Changes how renderer is populated when rebuilding panels from config |
| src/components/BootstrapBlazor.DockView/Components/DockViewV2.razor.js | Subscribes to loadTabs and forwards to .NET callback |
| src/components/BootstrapBlazor.DockView/Components/DockViewV2.razor.cs | Adds LoadTabs JS-invokable + ShowTab gate |
| src/components/BootstrapBlazor.DockView/Components/DockViewComponent.cs | Moves panel rendering to BuildRenderTree and gates by ShowTab |
| src/components/BootstrapBlazor.DockView/Components/DockViewComponent.razor | Removed (logic moved into BuildRenderTree) |
| src/components/BootstrapBlazor.DockView/Components/DockViewRenderMode.cs | Adds Partial enum value + doc updates |
| src/components/BootstrapBlazor.DockView/Components/DockViewConfig.cs | Adds LoadTabs callback name to config |
| src/components/BootstrapBlazor.DockView/Components/DockViewV2.razor | BOM/format-only change |
| src/components/BootstrapBlazor.DockView/BootstrapBlazor.DockView.csproj | Package version updated from beta to stable |
Comments suppressed due to low confidence (1)
src/components/BootstrapBlazor.DockView/Components/DockViewComponent.cs:182
BuildRenderTreeno longer respects the component’sVisibleparameter (previously@if (Visible) { @ChildContent }). As written, content can render even whenVisible == falseas soon asShowTab(Key)is true, which is a behavior change and can lead to hidden/removed panels still rendering (and running) their child components. Consider gating content on both flags (e.g.,Visible && DockView.ShowTab(Key)).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
278
to
283
| title: item.title, | ||
| tabComponent: item.componentName, | ||
| contentComponent: item.componentName, | ||
| renderer: item.renderer || options.renderer, | ||
| // renderer: item.renderer || options.renderer, | ||
| params: { ...item, parentId: parent.id } | ||
| } |
There was a problem hiding this comment.
Same as in dockview-panel.js: commenting out renderer: item.renderer || options.renderer means panels created from layout/config won’t carry a renderer value, but the rest of the JS uses panel.renderer to decide how to manage DOM/content. This likely causes inconsistent behavior when reloading layouts. Please restore setting renderer (or set a default elsewhere).
Comment on lines
303
to
309
| panels[contentItem.id] = { | ||
| id: contentItem.id, | ||
| title: contentItem.title, | ||
| renderer: contentItem.renderer || options.renderer, | ||
| // renderer: contentItem.renderer || options.renderer, | ||
| tabComponent: contentItem.componentName, | ||
| contentComponent: contentItem.componentName, | ||
| params: { ...contentItem, parentId: parent.id } |
There was a problem hiding this comment.
Same issue as above: renderer is commented out for leaf panels, which can leave renderer undefined and break renderer-mode logic. Please ensure renderer is always set (e.g., contentItem.renderer || options.renderer).
Comment on lines
+265
to
+267
| /// | ||
| /// </summary> | ||
| /// <param name="tabs"></param> |
There was a problem hiding this comment.
The XML doc for LoadTabs is empty and tabs parameter is undocumented. Given this is a JS-invokable public method that affects rendering, please add a clear summary (ideally bilingual if that’s the convention in this component) describing when JS calls it and what tabs contains.
Suggested change
| /// | |
| /// </summary> | |
| /// <param name="tabs"></param> | |
| /// 标签页加载回调方法，由 JavaScript 在激活标签变化或需重新渲染时调用，用于更新当前需要渲染的标签集合。 | |
| /// Tab loading callback invoked from JavaScript when the set of active/visible tabs changes, updating which tabs should be rendered. | |
| /// </summary> | |
| /// <param name="tabs"> | |
| /// 需要在服务器端渲染的标签标识列表，由客户端脚本传入。每一项为一个标签的唯一标识。 | |
| /// List of tab identifiers that should currently be rendered on the server, provided by client-side script. Each item represents a single tab id. | |
| /// </param> |
|
|
||
| /// <summary> | ||
| /// <para lang="zh">部分渲染 可见版面渲染 不可见版面异步渲染</para> | ||
| /// <para lang="en"></para> |
There was a problem hiding this comment.
DockViewRenderMode.Partial has an empty English <para lang="en"> doc. Please fill it in to match the bilingual XML doc convention used for the rest of the enum values.
Suggested change
| /// <para lang="en"></para> | |
| /// <para lang="en">Partial rendering: render visible panels and asynchronously render invisible panels.</para> |
| const delPanelsStr = localStorage.getItem(dockview.params.options.localStorageKey + '-panels'); | ||
| const delPanels = delPanelsStr && JSON.parse(delPanelsStr) || []; | ||
| if (options.renderer === 'always') { | ||
|
|
There was a problem hiding this comment.
options.renderer === 'always' branch is empty. This conditional can be removed (and let the later branches handle the other modes) or implemented explicitly; leaving an empty branch makes the initialization logic harder to follow and suggests incomplete behavior.
Suggested change
| if (dockview.panels.length > 0) { | |
| dockview._loadTabs?.fire(dockview.panels.map(p => p.params.key)); | |
| } |
| groupId: contentItem.groupId, | ||
| title: contentItem.title, | ||
| renderer: contentItem.renderer || options.renderer, | ||
| // renderer: contentItem.renderer || options.renderer, |
There was a problem hiding this comment.
renderer is no longer populated when building the panels list from options.content, but other code relies on panel.renderer (e.g., filtering onlyWhenVisible panels and passing renderer into dockview.addPanel). With this commented out, new panels created from options will have renderer === undefined, which can break render-mode behavior. Consider restoring renderer: contentItem.renderer || options.renderer (or otherwise ensuring a default is always set).
Suggested change
| // renderer: contentItem.renderer || options.renderer, | |
| renderer: contentItem.renderer || options.renderer, |
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 #956
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Introduce a new partial render mode for DockView and wire up server-side and JavaScript logic to only render active or requested tabs.
New Features:
Enhancements: