Conversation
Reviewer's GuideThis PR adds a standalone Razor class library project for a new Vditor markdown editor component, wiring up all necessary C# and JavaScript interop to initialize, configure, and control the editor in Blazor. Sequence Diagram for Vditor Component InitializationsequenceDiagram
participant U as User/Razor Page
participant CSharp as Vditor.razor.cs
participant JSInterop as Vditor.razor.js
participant VditorLib as vditor.js Library
U->>CSharp: Renders Vditor component
activate CSharp
CSharp->>JSInterop: InvokeAsync("init", id, Interop, {Options, Value})
activate JSInterop
JSInterop->>VditorLib: new Vditor(id, vditorOptionsWithCallbacks)
activate VditorLib
VditorLib-->>JSInterop: Vditor instance created
VditorLib->>JSInterop: Calls options.after() callback
deactivate VditorLib
JSInterop->>CSharp: invokeMethodAsync("TriggerRenderedAsync")
activate CSharp
CSharp->>CSharp: Executes OnRenderedAsync (if any)
deactivate CSharp
deactivate JSInterop
deactivate CSharp
Sequence Diagram for Vditor User Input HandlingsequenceDiagram
actor User
participant VditorLib as vditor.js Library
participant JSInterop as Vditor.razor.js
participant CSharp as Vditor.razor.cs
User->>VditorLib: Types text in editor
activate VditorLib
VditorLib->>JSInterop: Calls options.input(value) callback
activate JSInterop
JSInterop->>CSharp: invokeMethodAsync("TriggerInputAsync", value)
activate CSharp
CSharp->>CSharp: Updates CurrentValue
CSharp->>CSharp: Executes OnInputAsync(value) (if any)
deactivate CSharp
deactivate JSInterop
deactivate VditorLib
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 @ArgoZhang - I've reviewed your changes - here's some feedback:
- The DisableAsync method invokes "disabled" but the Vditor API uses "disable"—please update the invocation to match the correct method name.
- In Vditor.razor.js you’re loading "vditor.js" but the project ships "vditor.min.js"—update the script path to the minified file (or include the unminified version) to avoid 404s and improve performance.
- Rather than bundling all theme CSS variants, consider dynamically loading only the selected theme (e.g. ant-design, light, dark) to trim down the payload size.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The DisableAsync method invokes "disabled" but the Vditor API uses "disable"—please update the invocation to match the correct method name.
- In Vditor.razor.js you’re loading "vditor.js" but the project ships "vditor.min.js"—update the script path to the minified file (or include the unminified version) to avoid 404s and improve performance.
- Rather than bundling all theme CSS variants, consider dynamically loading only the selected theme (e.g. ant-design, light, dark) to trim down the payload size.
## Individual Comments
### Comment 1
<location> `src/components/BootstrapBlazor.Vditor/Vditor.razor.cs:113` </location>
<code_context>
+ /// <param name="value"></param>
+ /// <param name="options"></param>
+ /// <returns></returns>
+ public async Task Reset(string value, VditorOptions options)
+ {
+ if (!string.IsNullOrEmpty(value))
+ {
+ Value = value;
+ }
+ _vditor = await InvokeAsync<IJSObjectReference>("reset", Id, Value, Options);
+ }
+
</code_context>
<issue_to_address>
Reset uses the `Options` property instead of the passed-in parameter
Pass the `options` parameter to JS instead of the component's `Options` property, or update the property before invoking JS to ensure the correct configuration is used.
</issue_to_address>
### Comment 2
<location> `src/components/BootstrapBlazor.Vditor/Vditor.razor.cs:188` </location>
<code_context>
+ /// <summary>
+ /// 禁用编辑器
+ /// </summary>
+ public async ValueTask DisableAsync()
+ {
+ if (_vditor != null)
+ {
+ await _vditor.InvokeVoidAsync("disabled");
+ }
+ }
</code_context>
<issue_to_address>
Calling incorrect Vditor API method for disabling
Use `InvokeVoidAsync("disable")` instead of `InvokeVoidAsync("disabled")` to correctly disable the editor.
</issue_to_address>
### Comment 3
<location> `src/components/BootstrapBlazor.Vditor/wwwroot/js/vditor.js:5878` </location>
<code_context>
+ * @param afterCombine 每个脚注块合并完成后的回调, param: root为合并后的脚注块
+ */
+var combineFootnote = function (elements, afterCombine) {
+ elements.querySelectorAll("[data-type=footnotes-link]").forEach(function (el) {
+ var root = el.parentElement;
+ var footnote = root.nextSibling;
</code_context>
<issue_to_address>
Avoid mutating a live NodeList while iterating
Clone the childNodes into an array before iterating to avoid issues from modifying the live list during the loop.
</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
+113
to
+119
| public async Task Reset(string value, VditorOptions options) | ||
| { | ||
| if (!string.IsNullOrEmpty(value)) | ||
| { | ||
| Value = value; | ||
| } | ||
| _vditor = await InvokeAsync<IJSObjectReference>("reset", Id, Value, Options); |
There was a problem hiding this comment.
issue (bug_risk): Reset uses the Options property instead of the passed-in parameter
Pass the options parameter to JS instead of the component's Options property, or update the property before invoking JS to ensure the correct configuration is used.
Comment on lines
+188
to
+192
| public async ValueTask DisableAsync() | ||
| { | ||
| if (_vditor != null) | ||
| { | ||
| await _vditor.InvokeVoidAsync("disabled"); |
There was a problem hiding this comment.
issue (bug_risk): Calling incorrect Vditor API method for disabling
Use InvokeVoidAsync("disable") instead of InvokeVoidAsync("disabled") to correctly disable the editor.
| * @param afterCombine 每个脚注块合并完成后的回调, param: root为合并后的脚注块 | ||
| */ | ||
| var combineFootnote = function (elements, afterCombine) { | ||
| elements.querySelectorAll("[data-type=footnotes-link]").forEach(function (el) { |
There was a problem hiding this comment.
issue (bug_risk): Avoid mutating a live NodeList while iterating
Clone the childNodes into an array before iterating to avoid issues from modifying the live list during the loop.
Comment on lines
+38
to
+4804
| return range; | ||
| }; | ||
| var setRangeByWbr = function (element, range) { | ||
| var wbrElement = element.querySelector("wbr"); | ||
| if (!wbrElement) { | ||
| return; | ||
| } | ||
| if (!wbrElement.previousElementSibling) { | ||
| if (wbrElement.previousSibling) { | ||
| // text<wbr> | ||
| range.setStart(wbrElement.previousSibling, wbrElement.previousSibling.textContent.length); | ||
| } | ||
| else if (wbrElement.nextSibling) { | ||
| if (wbrElement.nextSibling.nodeType === 3) { | ||
| // <wbr>text | ||
| range.setStart(wbrElement.nextSibling, 0); | ||
| } | ||
| else { | ||
| // <wbr><br> https://github.com/Vanessa219/vditor/issues/400 | ||
| range.setStartBefore(wbrElement.nextSibling); | ||
| } | ||
| } | ||
| else { | ||
| // 内容为空 | ||
| range.setStart(wbrElement.parentElement, 0); | ||
| } | ||
| } | ||
| else { | ||
| if (wbrElement.previousElementSibling.isSameNode(wbrElement.previousSibling)) { | ||
| if (wbrElement.previousElementSibling.lastChild) { | ||
| // <em>text</em><wbr> | ||
| range.setStartBefore(wbrElement); | ||
| range.collapse(true); | ||
| setSelectionFocus(range); | ||
| // fix Chrome set range bug: **c** | ||
| if ((0,_compatibility__WEBPACK_IMPORTED_MODULE_2__/* .isChrome */ .i7)() && (wbrElement.previousElementSibling.tagName === "EM" || | ||
| wbrElement.previousElementSibling.tagName === "STRONG" || | ||
| wbrElement.previousElementSibling.tagName === "S")) { | ||
| range.insertNode(document.createTextNode(_constants__WEBPACK_IMPORTED_MODULE_0__/* .Constants.ZWSP */ .g.ZWSP)); | ||
| range.collapse(false); | ||
| } | ||
| wbrElement.remove(); | ||
| return; | ||
| } | ||
| else { | ||
| // <br><wbr> | ||
| range.setStartAfter(wbrElement.previousElementSibling); | ||
| } | ||
| } | ||
| else { | ||
| // <em>text</em>text<wbr> | ||
| range.setStart(wbrElement.previousSibling, wbrElement.previousSibling.textContent.length); | ||
| } | ||
| } | ||
| range.collapse(true); | ||
| wbrElement.remove(); | ||
| setSelectionFocus(range); | ||
| }; | ||
| var insertHTML = function (html, vditor) { | ||
| // 使用 lute 方法会添加 p 元素，只有一个 p 元素的时候进行删除 | ||
| var tempElement = document.createElement("div"); | ||
| tempElement.innerHTML = html; | ||
| var tempBlockElement = tempElement.querySelectorAll("p"); | ||
| if (tempBlockElement.length === 1 && !tempBlockElement[0].previousSibling && !tempBlockElement[0].nextSibling && | ||
| vditor[vditor.currentMode].element.children.length > 0 && tempElement.firstElementChild.tagName === "P") { | ||
| html = tempBlockElement[0].innerHTML.trim(); | ||
| } | ||
| var pasteElement = document.createElement("div"); | ||
| pasteElement.innerHTML = html; | ||
| var range = getEditorRange(vditor); | ||
| if (range.toString() !== "") { | ||
| vditor[vditor.currentMode].preventInput = true; | ||
| document.execCommand("delete", false, ""); | ||
| } | ||
| if (pasteElement.firstElementChild && | ||
| pasteElement.firstElementChild.getAttribute("data-block") === "0") { | ||
| // 粘贴内容为块元素时，应在下一段落中插入 | ||
| pasteElement.lastElementChild.insertAdjacentHTML("beforeend", "<wbr>"); | ||
| var blockElement = (0,_hasClosest__WEBPACK_IMPORTED_MODULE_1__/* .hasClosestBlock */ .F9)(range.startContainer); | ||
| if (!blockElement) { | ||
| vditor[vditor.currentMode].element.insertAdjacentHTML("beforeend", pasteElement.innerHTML); | ||
| } | ||
| else { | ||
| blockElement.insertAdjacentHTML("afterend", pasteElement.innerHTML); | ||
| } | ||
| setRangeByWbr(vditor[vditor.currentMode].element, range); | ||
| } | ||
| else { | ||
| var pasteTemplate = document.createElement("template"); | ||
| pasteTemplate.innerHTML = html; | ||
| range.insertNode(pasteTemplate.content.cloneNode(true)); | ||
| range.collapse(false); | ||
| setSelectionFocus(range); | ||
| } | ||
| }; | ||
|
|
||
|
|
||
| /***/ }) | ||
|
|
||
| /******/ }); |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
Comment on lines
+71
to
+95
| var diff_match_patch = function () { | ||
| // Defaults. | ||
| // Redefine these in your program to override the defaults. | ||
|
|
||
| // Number of seconds to map a diff before giving up (0 for infinity). | ||
| this.Diff_Timeout = 1.0; | ||
| // Cost of an empty edit operation in terms of edit characters. | ||
| this.Diff_EditCost = 4; | ||
| // At what point is no match declared (0.0 = perfection, 1.0 = very loose). | ||
| this.Match_Threshold = 0.5; | ||
| // How far to search for a match (0 = exact location, 1000+ = broad match). | ||
| // A match this many characters away from the expected location will add | ||
| // 1.0 to the score (0.0 is a perfect match). | ||
| this.Match_Distance = 1000; | ||
| // When deleting a large block of text (over ~64 characters), how close do | ||
| // the contents have to be to match the expected contents. (0.0 = perfection, | ||
| // 1.0 = very loose). Note that Match_Threshold controls how closely the | ||
| // end points of a delete need to match. | ||
| this.Patch_DeleteThreshold = 0.5; | ||
| // Chunk size for context length. | ||
| this.Patch_Margin = 4; | ||
|
|
||
| // The number of bits in an int. | ||
| this.Match_MaxBits = 32; | ||
| }; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| } | ||
|
|
||
| // Check to see if the problem can be split in two. | ||
| var hm = this.diff_halfMatch_(text1, text2); |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| var hm = this.diff_halfMatch_(text1, text2); | ||
| if (hm) { | ||
| // A half-match was found, sort out the return data. | ||
| var text1_a = hm[0]; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| if (hm) { | ||
| // A half-match was found, sort out the return data. | ||
| var text1_a = hm[0]; | ||
| var text1_b = hm[1]; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| // A half-match was found, sort out the return data. | ||
| var text1_a = hm[0]; | ||
| var text1_b = hm[1]; | ||
| var text2_a = hm[2]; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
| var text1_a = hm[0]; | ||
| var text1_b = hm[1]; | ||
| var text2_a = hm[2]; | ||
| var text2_b = hm[3]; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
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
1 participant
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 #465
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add a new Vditor markdown editor component to BootstrapBlazor.Extensions, providing a Blazor wrapper with JS interop, configuration options, event callbacks, and bundled styling and scripts.
New Features: