ckeditor
Add config variable to control default state of Lock Ratio button
What is the purpose of this pull request?
New feature
Does your PR contain necessary tests?
All patches that 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
- [x] Unit tests
- [x] Manual tests
Did you follow the CKEditor 4 code style guide?
Your code should follow the guidelines from the CKEditor 4 code style guide which helps keep the entire codebase consistent.
- [x] PR is consistent with the code style guide
What is the proposed changelog entry for this pull request?
* [#5219](https://github.com/ckeditor/ckeditor4/issues/5219): Added the [`config.image2_defaultLockRatio`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-image2_defaultLockRatio) config variable to allow setting the default value of the "Lock ratio" option in the [Enhanced Image](https://ckeditor.com/cke4/addon/image2) dialog.
What changes did you make?
I've added the config.image2_defaultLocRatio boolean variable to allow user to control the default state of the "Lock ratio" button. The variable is set to true by default.
However, the change is not that straightforward. The "Lock ratio" had the second function in the image dialog – it informed the user if the loaded image has preserved its natural aspect ratio. In that case, the "Lock ratio" button was automatically set to "locked" after loading the image. Yet, it made this feature completely unusable – as even after setting the default value of the "Lock ratio" to unlocked, it was automatically switching to locked when any image was loaded. Due to that I've removed the whole mechanism of toggling lock ratio after loading the image. I'm not sure if it's the right thing to do – it is kind of a breaking change. Maybe the implementation should be updated, e.g. to preserve the after-load behavior if the config variable is not set at all?
Which issues does your PR resolve?
Closes #5219.
@Comandeer what do you think about providing 3 state option instead? Like:
CKEDITOR.config.image2_defaultLockRatio = OFF | ON | AUTO
We even have these CKEDITOR.TRISTATE_ON/OFF/DISABLED enums but I'm not sure if DISABLED would be understandable to integrators for AUTO mode.
Yup, we would probably need to keep another set of tristate-like constants. In fact it's very similar to true | false | undefined yet I find the latter more intuitive to use – omit the option if you want the old/default behaviour, set it if you want to set a unchanging value to the "Lock ratio".
I think it's a fine compromise to avoid adding too many enums to the global scope. Could you implement the required changes? Right now we indeed introduce regression (and basically remove a valid feature from the image plugin).
I've updated the config variable to take three possible values: true, false and undefined. The undefined is the default one, connected with the current behavior of "Lock ratio" (so indicating if the loaded image has the correct aspect ratio).
I've also rebased the branch onto latest master.
I've updated the manual tests. Now there are three separate tests for each value of the config variable (as testing steps are different enough for that). I've also enabled aspectratio test.
I've also reenabled the legacy lock behavior which will be used if the config variable is not set.