| 1 | | No the command state (if it is disable or not) is based on the "setState" method which is called by many plugins. It can be changed when selection change, content in selection change (#12618), mode change, editor is in readOnly mode and maybe some more cases. Also many plugins set state of the commands manually. There should be one mechanism to synchronize all this cases. Every time there is chance that a command state may change a special event is fired (e.g. `commandContextChanged)` and every listener check if command should be disabled. If it should the listener return `false`. This way we would avoid the situation when one part of code disable command the another enable it back. |
| | 1 | The problem: |
| | 2 | |
| | 3 | Now the command state (if it is disable or not) is based on the "setState" method which is called by many plugins. It can be changed when selection change, content in selection change (#12618), mode change, editor is in readOnly mode and maybe some more cases. Also many plugins set state of the commands manually. We get a state spaghetti and it is very often that one part of code try to disable command and another enable it back. |
| | 4 | |
| | 5 | The solution: |
| | 6 | |
| | 7 | There should be one entry point for all parts of code which want to change the state. Good candidate seems to be `refresh` method and event. The example scenario can be readonly feature. It will: |
| | 8 | - add `refresh` event listener and disable every command if editor is in the read only mode, |
| | 9 | - call `refresh()` function when editor switch to the `readonly` mode. |
| | 10 | |
| | 11 | This way every time the state of the command is suppose to be changed all listeners are called and we can be sure that we do not enable feature which should be disabled. |
| | 12 | |
| | 13 | In fact direct usage of `setState()` is always wrong because we can not be sure if any other plugin want to change the state too. |
| | 14 | |
| | 15 | The idea of changes is: |
| | 16 | - mark `setState()` as deprecated, |
| | 17 | - `refresh` event get `evt.data.state` parameter which can be changed by any listener. |
| | 18 | - `commandDefinition` methods (`refresh` and `exec`) are only (other then `command.refresh()`) methods which can change state directly (by adding data.state parameter or returning new state), |
| | 19 | - `refresh()` method contents may be moved to the `refresh` listeners, so it will be possible to add listener before them, |
| | 20 | - everything what `refresh()` will do is firing `refresh` event and setting the state based on what listeners will set. |
| | 21 | |
| | 22 | This way if someone want to disable command and be sure no one else will enable it back, can add `refresh` listener with `0` priority, set `evt.data.state = CKEDITOR.TRISTATE_DISABLED` and cancel event. |
| | 23 | |
| | 24 | It seems to be possible to apply these changes without breaking backward compatibility. `setState` method will still work (as bad as it works now), but it will be deprecated. Also it is pretty huge change, because `setState` is used in many plugins. |
