Problem/Motivation
Removing CKEditor5PluginContextualInterface
Quoting #3215506-11: Plugins should be enableable based on toolbar configuration
We can't just conditionally enable
ckeditor5.imagethough, because if there's no PHP code, one cannot implementCKEditor5PluginContextualInterfaceeither. So I see at least 4 solutions:
- The simplest possible approach here AFAICT is to move the
elementsannotation fromckeditor5.imagetockeditor5.imageUpload.- Merge
ckeditor5.imageandckeditor5.imageUploadinto a single plugin definition again. This is also still very simple. It would just mean one fewer CKEditor 5 plugin definition defined in YAML.- Add
classtockeditor5.imageand add a tiny amount of code to implementCKEditor5PluginContextualInterface.- Add infrastructure to conditionally enable a particular CKEditor plugin from YAML only. Something like
enableIfToolbarItemIsPresent: foobar. This is an API addition though. It feels a bit logic-y. Which is bad for an annotation.
I advocated against
- Add infrastructure to conditionally enable a particular CKEditor plugin from YAML only. Something like
enableIfToolbarItemIsPresent: foobar. This is an API addition though. It feels a bit logic-y. Which is bad for an annotation.
… but I actually do think this could be a good idea. Because it will actually allow writing less PHP code:
ckeditor5.media- would get
conditions: filter: media_embed ckeditor5.mediaLibrary- would get
conditions: filter: media_embed toolbarItem: drupalMedia ckeditor5.image- would get
conditions: toolbarItem: uploadImage imageUploadStatus: true ckeditor5.imageUpload- would get
conditions: toolbarItem: uploadImage imageUploadStatus: true - Consequence
- All
CKEditor5PluginContextualInterfaceimplementations would disappear!
Removing CKEditor5PluginContextualValidationInterface
Better still, this would allow us to remove CKEditor5PluginContextualValidationInterface as well! The only use for that so far is \Drupal\ckeditor5\Plugin\CKEditor5Plugin\MediaLibrary::validateFilterForm(), which explicitly checks whether the drupalMedia filter is enabled. That requirement/validation logic can easily be inferred from that metadata.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Issue fork ckeditor5-3216244
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
wim leersBetter still 🤓
Looks like this could end up helping #3201641: Improve the HTML filter configuration UX too!
Comment #3
wim leersComment #4
wim leersComment #6
wim leersComment #7
wim leersI'm saying that at least 90% of the validation we'll need is going to be covered by the things in this proposal.
Rather than each reimplementing the same thing many times but slightly differently and slightly broken, this is far more robust.
And the best part: if the need for custom validation reoccurs, we can reintroduce this functionality 🤓😊
Comment #8
wim leersThis conflicted heavily with:
Rebased.
And beautifully, because it is now metadata instead of inexaminable logic, the code in
\Drupal\ckeditor5\Plugin\CKEditor5PluginManager::shouldHaveVisiblePluginSettingsForm()added by #3215597: CKEditor 5 plugin settings form available always, even when the plugin is not enabled now has no need for guesswork anymore!See https://git.drupalcode.org/project/ckeditor5/-/merge_requests/43/diffs?c... for the simplifications I was able to make after I rebased. 😊
End result:
Comment #10
lauriiiIt seems fine to move forward with this. If any plugins ever needed this functionality, it could be brought back. Some generic use cases could be also added as conditions to core, without adding the more advanced functionality to allow handling more complex rules for enabling plugins or validating the editor form.
Comment #12
wim leers🥳