ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Dynamic toolbar configuration options

Open jodator opened this issue 5 years ago • 6 comments

📝 Provide a description of the improvement

After quick search i can see that most of the features reads configuration options at the plugin init stage. Only few read configuration option afterwards but that feels like a side-effect of the code rather then dynamic configuration.

Should we have dynamic configuration for some options?

Pros:

  • ability to change some values on the fly

Cons:

  • making it dynamic require to re-render UI, update some state and the previously created content may not have sense

Edit:

Other things to consider:

  • if we allow some options to be dynamic - how?
    • By annotating that in the docs
    • By providing dynamic configuration key or new construct (something like editor.properties) that can be change during runtime? 

References issues: #7278.


If you'd like to see this improvement implemented, add a 👍 reaction to this post.

jodator avatar Jun 05 '20 07:06 jodator

I was always against dynamic config as it creates many problems:

  • Which properties should be dynamic? There are some which cannot be due to immense technical issues that that would cause. There are some that will never be changed by anyone. So, the ROI here would need to be checked for every case.
  • The above means that the developer will have to change which property is dynamic anyway.
  • Some properties (e.g. lists of some values) might need a smarter way to be changed than just being overridden.
  • The complexity will rise with every such property.

So, I'm rather looking for API-driven ways to change the editor behavior on the fly. Every case is different and handling it via API, when needed,  makes to me the most sense.

Reinmar avatar Jul 20 '20 08:07 Reinmar

That could be one reason for having some of the configuration dynamic: ckeditor/ckeditor5-vue#125 (this, and the linked issue).

Reinmar avatar Jul 28 '20 11:07 Reinmar

Another reason for having configuration dynamic: #7871. And there's a solution that is kinda OK.

The plugin has an observable property which one can set after the plugin reads a configuration. The plugins listens on this property change and updates internal state.

jodator avatar Aug 27 '20 07:08 jodator

I agree that the notion of making all configuration values dynamic is problematic -- it's sometimes not going to be possible for a plugin to handle config values changing, and even when it is it's likely to cause friction in writing plugins because of the effort needed to handle dynamic changes, even in cases where it's not very useful.

In my opinion, it should be opt-in by the plugin authors, which I think means one of three things:

  1. Provide some patterns or implement some mixin-style utilities to make it easy for plugins to provide an API for updating configuration values outside the Config class
  2. Modify the Config class to allow plugins to define properties as dynamic, and provide an API on Config for updating them (that maybe throws an error when trying to update a non-dynamic config value)
  3. Make all config values technically dynamic it that they provide notifications when the values are changed, but plugins aren't expected to respond to any/all changes, but rather to document which ones can and can't be dynamic.

(3) seems kinda like the worst of both worlds. (2) kinda makes sense, but potentially risks the nested object thing (e.g. can I modify config.foo.bar without replacing all of config.foo and still produce a change notification to the plugin?). I'd probably think about doing something like (1), then seeing how it shakes out and if dynamic config values end up widespread enough, maybe then implement (2) and deprecate then remove the various (1) implementations.

My two cents. For now we're running off of a fork of the TextTransformation plugin (see #7871), which isn't ideal, but gets us what we need.

bendemboski avatar Aug 27 '20 16:08 bendemboski

Merged #7278 into this issue. There are some more use cases listed.

Reinmar avatar Nov 02 '20 08:11 Reinmar

I see that this issue went from dynamic toolbar configuration to a generic dynamic configuration discussion.

Last week as a part of CKE5 team workshops we played with making the toolbar configurable and it was fairly easy to achieve:

https://user-images.githubusercontent.com/5353898/191475304-9ef161c3-e11c-4cb2-aae1-27528e1b547b.mp4

The code can be found on the workshops/dynamic-config branch.

The demo can be found in manual tests: http://localhost:8125/ckeditor5-utils/tests/manual/config/config.html

The implementation simply adds the emitter interface for now - but it's just 2h of hacking.

mlewand avatar Sep 21 '22 10:09 mlewand