I/5862: Disable text transformations inside code blocks

[panr]

Suggested merge commit message (convention)

Fix: Disabled text transformation inside code blocks. Closes ckeditor/ckeditor5#5862.

[jodator]

• Tension: What's the best way for implementing this._disableStack

Looking at what we have already and what is easiest to use

1. EmiterMixin and ObservableMixin - It's OK to add if( !this._disabledStack) check directly in mixin methods. Or some initializer method that will be called.
2. The other case I've found is ElementApiMixin which throws if a required property is not set. It does that because it must be mixed with proper editor class. So this case is not for ForceDisablableMixin

Should ObservableMixin implement this

I don't think so - they don't know about each other and shouldn't know.

The name ForceDisabledMixin - it's pretty clear for me what it does (adds forceDisabled() and clearForceDisabled() methods ).

🤔 Another name that I could think of is DisableableMixn (as in ObservableMixin) - which is kinda funny, so no :D We also do not follow any pattern here (like using -able suffix) so again I don't have a better name.

TL;DR:

1. this._forceDisabled set by methods (no throw) similar to EmitterMixin.
2. The name is OK for me.

[Reinmar]

What do you think about not exporting this to a mixin but rather keeping duplicated in the base Command and Plugin classes? Cons: duplication. Pros: no problem how this mixin should work. I'd consider this option as a possible middle ground between full flavoured completely reusable mixin and lots of code duplication (when every plugin and command would implement this manually).

[Reinmar]

Cause: "Duplication is better than the wrong abstraction".

[Reinmar]

Also, @panr mentioned that the implementation of this functionality differs slightly between commands and plugins (no refresh() in plugins) which is exactly the reason why it may be easier to duplicate the code.

[jodator]

Actually, it makes more sense as you mentioned. Do we want an interface for this? Probably we do as I remember the usage of this. Both Plugin and Command would implement the ForceDisableable interfaces (doh, the name...) in this case.

[jodator]

I think that we could go a bit further and make TextWatcher disablable. At this stage I don't think we need to implement the TextWatcher.forceDisabled() et all - just simple listener to isEnabled property.

[panr]

There's a little commit madness. Should I squash some of them?

[jodator]

Only small changes needed:

1. The EmitterMixin is not needed.
2. The isEnabled property needs documentation.

ps.: I've also tested how the removing listeners could work and as it didn't break the test I've added this to your PR.

[jodator]

LGTM 👍