Conversation
|
Thanks for your PR, @j4587698. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's GuideThis PR adds file upload callback support to the SummerNote Editor component by introducing a new OnFileUpload parameter in Editor.razor.cs, enhancing the JavaScript interop to handle image uploads, and providing a file metadata model for processing and saving uploads. Sequence diagram for Editor file upload callback processsequenceDiagram
participant JS as Editor.razor.js
participant Editor as Editor.razor.cs
participant User as actor User
participant Callback as OnFileUpload (callback)
User->>JS: Uploads image in Editor
JS->>Editor: invokeMethodAsync('ImageUpload', file metadata)
Editor->>JS: fetch file stream
Editor->>Callback: OnFileUpload(uploadFile)
Callback-->>Editor: Returns image URL
Editor-->>JS: Returns image URL
JS->>JS: Insert image into editor
Entity relationship diagram for Editor and EditorUploadFileerDiagram
EDITOR ||--o| EDITORUPLOADFILE : handles
EDITORUPLOADFILE {
string FileName
long FileSize
string LastModified
string ContentType
Stream UploadStream
int Index
int Code
string Error
}
Class diagram for new EditorUploadFile model and Editor changesclassDiagram
class EditorUploadFile {
+string? FileName
+long FileSize
+string? LastModified
+string? ContentType
+Stream? UploadStream
+int Index
+int Code
+string? Error
+Task<bool> SaveToFile(string fileName, CancellationToken token)
}
class Editor {
+Func<EditorUploadFile, Task<string>>? OnFileUpload
+Task<string> ImageUpload(EditorUploadFile uploadFile)
}
Editor --> "1" EditorUploadFile: uses
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/components/BootstrapBlazor.SummerNote/Components/Editor/EditorUploadFile.cs:94` </location>
<code_context>
+ // 打开文件流
+ var stream = UploadStream;
+
+ var buffer = new byte[4 * 1096];
+ int bytesRead = 0;
+
</code_context>
<issue_to_address>
Buffer size calculation uses 1096 instead of 1024.
Please confirm whether 1096 is intentional or if 1024 was intended, as powers of two are standard for buffer sizes.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
var buffer = new byte[4 * 1096];
=======
// Use a standard power-of-two buffer size for optimal performance
var buffer = new byte[4 * 1024];
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/components/BootstrapBlazor.SummerNote/Components/Editor/EditorUploadFile.cs:80` </location>
<code_context>
+ }
+
+ var folder = Path.GetDirectoryName(fileName);
+ if (!string.IsNullOrEmpty(folder) && !Directory.Exists(folder))
+ {
+ Directory.CreateDirectory(folder);
</code_context>
<issue_to_address>
Directory creation does not handle exceptions.
Wrap Directory.CreateDirectory(folder) in a try-catch block to handle potential exceptions and update Code and Error as needed.
</issue_to_address>
### Comment 3
<location> `src/components/BootstrapBlazor.SummerNote/Components/Editor/EditorUploadFile.cs:87` </location>
<code_context>
+
+ if (Code == 0)
+ {
+ using var uploadFile = File.OpenWrite(fileName);
+
+ try
</code_context>
<issue_to_address>
File.OpenWrite is not wrapped in a try-catch block.
Move File.OpenWrite inside the try-catch block to handle exceptions and ensure Code/Error are set appropriately.
</issue_to_address>
### Comment 4
<location> `src/components/BootstrapBlazor.SummerNote/Components/Editor/Editor.razor.js:58` </location>
<code_context>
+ fileName: file.name,
+ fileSize: file.size,
+ contentType: file.type,
+ lastModified: new Date(file.lastModified).toISOString(),
+ index: i
+ }).then(data => {
</code_context>
<issue_to_address>
lastModified conversion may not be accurate for all browsers.
Add a check to verify that file.lastModified is a valid number before converting it to an ISO string, as its type or presence may vary across browsers.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
lastModified: new Date(file.lastModified).toISOString(),
=======
lastModified: (typeof file.lastModified === 'number' && !isNaN(file.lastModified))
? new Date(file.lastModified).toISOString()
: null,
>>>>>>> REPLACE
</suggested_fix>
### Comment 5
<location> `src/components/BootstrapBlazor.SummerNote/Components/Editor/EditorUploadFile.cs:60` </location>
<code_context>
+ /// <param name="fileName"></param>
+ /// <param name="token"></param>
+ /// <returns></returns>
+ public async Task<bool> SaveToFile(string fileName, CancellationToken token = default)
+ {
+ var ret = false;
</code_context>
<issue_to_address>
Consider simplifying the SaveToFile method by using built-in file and stream operations to handle folder creation, file overwriting, and stream copying.
Here’s a drastically simplified version of `SaveToFile` that preserves your error‐reporting (via `Code`/`Error`), but collapses all of the manual folder‐creation, delete‐if‐exists and read/write loops into a few lines:
```csharp
public async Task<bool> SaveToFile(string filePath, CancellationToken token = default)
{
try
{
// ensure folder exists (no need to check Exists first)
var folder = Path.GetDirectoryName(filePath);
if (!string.IsNullOrEmpty(folder))
Directory.CreateDirectory(folder);
// File.Create will truncate/overwrite existing file
using var output = File.Create(filePath);
if (UploadStream != null)
{
// CopyToAsync handles the buffer loops for you
await UploadStream.CopyToAsync(output, 81920, token);
}
return true;
}
catch (Exception ex)
{
// you can distinguish codes if you really need to
Code = 1003;
Error = ex.Message;
return false;
}
}
```
Steps to apply:
1. Remove your manual `File.Exists`/`File.Delete` block and the `new byte[...]` read/write loop.
2. Call `Directory.CreateDirectory(...)` unconditionally – it’s a no‐op if the folder already exists.
3. Use `File.Create(...)` (which overwrites) instead of `File.OpenWrite`.
4. Let `Stream.CopyToAsync(...)` do the heavy lifting.
5. Wrap the whole thing in a single `try/catch` for error reporting.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // 打开文件流 | ||
| var stream = UploadStream; | ||
|
|
||
| var buffer = new byte[4 * 1096]; |
There was a problem hiding this comment.
suggestion: Buffer size calculation uses 1096 instead of 1024.
Please confirm whether 1096 is intentional or if 1024 was intended, as powers of two are standard for buffer sizes.
Suggested change
| var buffer = new byte[4 * 1096]; | |
| // Use a standard power-of-two buffer size for optimal performance | |
| var buffer = new byte[4 * 1024]; |
| } | ||
|
|
||
| var folder = Path.GetDirectoryName(fileName); | ||
| if (!string.IsNullOrEmpty(folder) && !Directory.Exists(folder)) |
There was a problem hiding this comment.
issue: Directory creation does not handle exceptions.
Wrap Directory.CreateDirectory(folder) in a try-catch block to handle potential exceptions and update Code and Error as needed.
|
|
||
| if (Code == 0) | ||
| { | ||
| using var uploadFile = File.OpenWrite(fileName); |
There was a problem hiding this comment.
issue (bug_risk): File.OpenWrite is not wrapped in a try-catch block.
Move File.OpenWrite inside the try-catch block to handle exceptions and ensure Code/Error are set appropriately.
| fileName: file.name, | ||
| fileSize: file.size, | ||
| contentType: file.type, | ||
| lastModified: new Date(file.lastModified).toISOString(), |
There was a problem hiding this comment.
suggestion: lastModified conversion may not be accurate for all browsers.
Add a check to verify that file.lastModified is a valid number before converting it to an ISO string, as its type or presence may vary across browsers.
Suggested change
| lastModified: new Date(file.lastModified).toISOString(), | |
| lastModified: (typeof file.lastModified === 'number' && !isNaN(file.lastModified)) | |
| ? new Date(file.lastModified).toISOString() | |
| : null, |
| /// <param name="fileName"></param> | ||
| /// <param name="token"></param> | ||
| /// <returns></returns> | ||
| public async Task<bool> SaveToFile(string fileName, CancellationToken token = default) |
There was a problem hiding this comment.
issue (complexity): Consider simplifying the SaveToFile method by using built-in file and stream operations to handle folder creation, file overwriting, and stream copying.
Here’s a drastically simplified version of SaveToFile that preserves your error‐reporting (via Code/Error), but collapses all of the manual folder‐creation, delete‐if‐exists and read/write loops into a few lines:
public async Task<bool> SaveToFile(string filePath, CancellationToken token = default)
{
try
{
// ensure folder exists (no need to check Exists first)
var folder = Path.GetDirectoryName(filePath);
if (!string.IsNullOrEmpty(folder))
Directory.CreateDirectory(folder);
// File.Create will truncate/overwrite existing file
using var output = File.Create(filePath);
if (UploadStream != null)
{
// CopyToAsync handles the buffer loops for you
await UploadStream.CopyToAsync(output, 81920, token);
}
return true;
}
catch (Exception ex)
{
// you can distinguish codes if you really need to
Code = 1003;
Error = ex.Message;
return false;
}
}Steps to apply:
- Remove your manual
File.Exists/File.Deleteblock and thenew byte[...]read/write loop. - Call
Directory.CreateDirectory(...)unconditionally – it’s a no‐op if the folder already exists. - Use
File.Create(...)(which overwrites) instead ofFile.OpenWrite. - Let
Stream.CopyToAsync(...)do the heavy lifting. - Wrap the whole thing in a single
try/catchfor error reporting.
# Conflicts: # src/components/BootstrapBlazor.CodeEditor/BootstrapBlazor.CodeEditor.csproj
ArgoZhang
approved these changes
Aug 18, 2025
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
2 participants
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 #531
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add custom file upload support to the SummerNote Editor component by introducing a new OnFileUpload callback, wiring up JS interop for image uploads, and encapsulating upload details in an EditorUploadFile class.
New Features: