-
-
Notifications
You must be signed in to change notification settings - Fork 5
refactor(Editor): remove BootstrapBlazor.SummerNote.callbacks #863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the SummerNote editor integration by removing the global BootstrapBlazor.SummerNote.callbacks wiring in JavaScript and making a small documentation improvement to the Editor IsEditor parameter. Sequence diagram for SummerNote editor initialization and image upload callback after callbacks refactorsequenceDiagram
actor Developer
participant EditorComponent
participant JSRuntime
participant EditorJs
participant DataStore
participant SummernoteInstance
Developer->>EditorComponent: Render Editor
EditorComponent->>JSRuntime: InvokeVoidAsync init(id, invoker, methods, height, value, lang,...)
JSRuntime->>EditorJs: init(id, invoker, methodGetPluginAttrs, methodClickPluginItem, height, value, lang, langUrl, hasUpload)
EditorJs->>EditorJs: const el = document.getElementById(id)
EditorJs->>EditorJs: Build toolbar and option
EditorJs->>EditorJs: option.toolbar = toolbar
alt hasUpload is true
EditorJs->>EditorJs: option.callbacks.onImageUpload = async files => {...}
end
EditorJs->>SummernoteInstance: Initialize summernote with option
EditorJs->>DataStore: Data.set(id, editor)
rect rgb(230,230,250)
SummernoteInstance-->>EditorJs: onImageUpload(files)
EditorJs->>EditorJs: editor.files = files
EditorJs->>EditorJs: await addUploadScriptIfNeeded()
EditorJs->>JSRuntime: invoker.invokeMethodAsync(methodClickPluginItem, ...)
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Removing the
window.BootstrapBlazor.SummerNote.callbackswiring is a breaking change for anyone using those global callbacks; consider either providing a migration path (e.g., a new event API on the component) or adding a runtime warning when callbacks are still present to make the breakage more discoverable. - Now that the global
BootstrapBlazorobject creation is removed from this file, double-check that no other SummerNote-related scripts still rely onwindow.BootstrapBlazorbeing defined implicitly here, or consider centralizing its initialization in a single shared script if it is still needed elsewhere.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Removing the `window.BootstrapBlazor.SummerNote.callbacks` wiring is a breaking change for anyone using those global callbacks; consider either providing a migration path (e.g., a new event API on the component) or adding a runtime warning when callbacks are still present to make the breakage more discoverable.
- Now that the global `BootstrapBlazor` object creation is removed from this file, double-check that no other SummerNote-related scripts still rely on `window.BootstrapBlazor` being defined implicitly here, or consider centralizing its initialization in a single shared script if it is still needed elsewhere.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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the Editor component by removing the legacy BootstrapBlazor.SummerNote.callbacks functionality that allowed external JavaScript code to register custom event callbacks. The change simplifies the codebase by removing unused infrastructure while maintaining all existing component functionality.
Key Changes:
- Removed the
reloadCallbacksfunction and associated callback registration mechanism from the JavaScript initialization - Removed the
BootstrapBlazornamespace initialization check that was only needed for the callbacks feature - Updated documentation for the
IsEditorproperty to include default value - Bumped package version to 10.0.2
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Editor.razor.js | Removed callback registration infrastructure including the reloadCallbacks function and BootstrapBlazor namespace initialization |
| Editor.razor.cs | Enhanced documentation by adding default value information to the IsEditor property comment |
| BootstrapBlazor.SummerNote.csproj | Version bump from 10.0.1 to 10.0.2 reflecting the refactoring changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Link issues
fixes #862
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Simplify the SummerNote editor integration by removing support for global JavaScript callback registration and clarifying the IsEditor parameter documentation.
Enhancements: