-
Notifications
You must be signed in to change notification settings - Fork 7
Fix: Crashed when CKEditor 4 objects were in toolbar config #10
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's Guide by SourceryThis pull request fixes a crash that occurred when CKEditor 4 objects were present in the toolbar configuration. The fix involves updating the toolbar building logic to correctly handle different types of toolbar items, including arrays and items with a nested 'items' property. Additionally, a type check was added to ensure that block items are strings before being added to the block toolbar in inline mode. Sequence diagram for building toolbarssequenceDiagram
participant CmsCKEditor5Plugin
participant toolbarConfig
CmsCKEditor5Plugin->>toolbarConfig: buildToolbars(toolbarConfig)
loop For each item in toolbarConfig
alt item is array or item.items is array
CmsCKEditor5Plugin->>CmsCKEditor5Plugin: buildToolbars(item or item.items)
else item is a block item and inline mode
CmsCKEditor5Plugin->>CmsCKEditor5Plugin: Add item to blockToolbar if item is string
else
CmsCKEditor5Plugin->>CmsCKEditor5Plugin: Add item to topToolbar or blockToolbar
end
end
Updated class diagram for CmsCKEditor5PluginclassDiagram
class CmsCKEditor5Plugin {
-_blockItems
+init()
+buildToolbars(toolbarConfig)
}
note for CmsCKEditor5Plugin "Fixes a crash when CKEditor 4 objects were in toolbar config"
File-Level Changes
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 @fsbraun - I've reviewed your changes - here's some feedback:
Overall Comments:
- The condition
Array.isArray[item.items]
looks wrong, did you meanArray.isArray(item.items)
? - The ternary operator
Array.isArray[item] ? item : item.items
also looks wrong, did you meanArray.isArray(item) ? item : item.items
?
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Setting: