Move Controller outside of configurations data class #2037
Replies: 3 comments 6 replies
-
The configurations seem to be used as a convenient way to set and access the QuillController which is required for all operations. I did a quick search for 'configurations.controller' and found 17 usages. These seem to be confined to editor and toolbar, so if these were supplied with the controller directly, you would not need access via the configurations. Developers will need to move setting the controller from configurations to whatever since the same controller instance must be available to the toolbar and editor. Is this a breaking change? Would it be possible to link toolbar and editor such that the controller only needs to be set in one place? If this is possible then most developers would not even need to set a controller - a default plain controller would suffice. Other than that, I totally agree with you - it makes no sense to have the controller in the configurations. Configurations should be const. |
Beta Was this translation helpful? Give feedback.
-
@EchoEllet Are you working on this? I have a need to access some of the configurations from the controller and would like to move the controller out of the configurations as you are recommending. It would be good to eventually have all of the configurations as const's and sourced from a single common point. If you are not already working on this, I can start the re-organization. I would propose either a 1-step breaking change or a 2-step with intermediate non-breaking change. Since you are more experienced in this project, I defer to your better judgement or I can outline my thoughts. |
Beta Was this translation helpful? Give feedback.
-
I've submitted a PR to start the process. At some point, I will remove the deprecated controller parameter in the configurations and that will enable configurations to be const. This will be a breaking change. @EchoEllet When do you recommend making the breaking change? Wait 2 or 4 weeks in case anyone reports a problem? |
Beta Was this translation helpful? Give feedback.
-
For now both
QuillEditorConfigurations
andQuillSimpleToolbarConfigurations
require theQuillController
which is mutable and requires to removal of theconst
keyword when creating an instance of the configuration classScreenshot
The
QuillController
is required regardless of the configurations.Moving the
controller
property outside ofQuillEditorConfigurations
andQuillSimpleToolbarConfigurations
will make it possible to use the
const
keyword or not have to create an instance of the configuration class.If there is a value passed to a property in
QuillEditorConfigurations
that needs to be computed in runtime then theconst
will need to be removed as well though for most use cases, the developer doesn't need to.We could do this without breaking change by making both not required and temporarily using
assert
to check if one of them is null, deprecate the old one and once we remove it we no longer need to check it usingassert
.Beta Was this translation helpful? Give feedback.
All reactions