-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Added possibility to call CKEDITOR.editor.addCommand
with CKEDITOR.command
instance as parameter
#1786
Conversation
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.
LGTM, just two small things to polish.
core/editor.js
Outdated
@@ -740,12 +740,14 @@ | |||
* } | |||
* } ); | |||
* | |||
* Since 4.10 this method accepts `CKEDITOR.command` instance as param. |
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.
"also accepts" IMO sounds more suitable.
tests/core/command/command.js
Outdated
@@ -302,5 +302,30 @@ bender.test( { | |||
editor._.elementsPath.onClick( 0 ); | |||
|
|||
wait(); | |||
}, | |||
|
|||
'test addCommand from command instance': function() { |
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.
It'd be good to add test using CKEDITOR.command
subclass, just in case to test if inherited commands are treated the same.
Sorry, I've removed the |
5691840
to
d2eb529
Compare
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.
LGTM!
What is the purpose of this pull request?
New Feature
Does your PR contain necessary tests?
All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.
This PR contains
What changes did you make?
I've added check if
CKEDITOR.editor.addComand
has command instance passed it will register this command in editor and return it instead of creating new one.Also added unit test to cover that case.
Closes #1582