singerdmx / flutter-quill

Rich text editor for Flutter
https://pub.dev/packages/flutter_quill
MIT License
2.6k stars 839 forks source link

Remove the QuillToolbarProvider, QuillEditorProvider and related classes and members #2301

Closed EchoEllet closed 1 week ago

EchoEllet commented 1 month ago

A feature allows children of QuillEditor and QuillToolbar and related widgets to access their configuration and properties so that we have one place to change some things and access them across different places inside those widgets.

This feature was added initially so that we can access the configuration and properties in the widget tree using the BuildContext without passing the property across multiple constructors, unfortunately, this change wasn't introduced with enough planning and I don't see any valid use case, or how it's useful, it's problematic and caused bugs that we had to manually fix in a non-clean way that's not type-safe, sometimes we had to write some minor tests for bug fixes, making it inefficient, instead we should have one data class that hold all related parameters and passing them all around and requiring them if necessary.

I also expect minor memory leak issues from this feature since not being disposed of properly. It doesn't impact the performance much if that's our main concern.

It's not documented properly, poorly written and tested and this was a mistake I made last year, we don't have a strong reason to keep using it, as such, in the long term we will remove it in either backward compatible change or release it as a major release. Luckily it doesn't expose APIs that are difficult to remove or change to the public.

This is one of the things to fix in 11.0.0, though we should have more planning about how it will be done and why this is an issue in the first place.

Why might want to know why would want this feature? Is customization options to different things all from one place should be a feature in the first place?

We should also discuss whether we want to fix more issues in 11.0.0 or simply introduce it with fewer breaking changes or fixes and then introduce 12.0.0 after a while or have them all in one version. IMO That will depend on the user feedback.

After planning, it will be a while until I start to fix this issue though it could also done soon.