Android-RTEditor icon indicating copy to clipboard operation
Android-RTEditor copied to clipboard

Architecture questions

Open 3dm1 opened this issue 7 years ago • 1 comments

First of all, amazing library and thank you for writing it.

I've been going through the library for the last couple of day and I've noticed a couple of things that I would like to share and, if there's interest, write a PR to address them.

  1. The ColorPicker dependency is added to the RTEditor module, but only used by the HorizontalRTToolbar on the RTEditor-Toolbar module.
  2. The RTManager relies on the pre-defined number of effects to update the toolbar so it is hard to append any custom effect without a new PR to the library or extending and overriding the onSelectionChanged method. It might be interesting to allow registering a delegate into the RTManager that will receive the toolbar and update the appropriate button as necessary.
  3. Currently the RTManager relies on an EventBus to receive images/links and it feels this is doing too much as it ties me to a library I'd rather not use, specially if I decide to write my custom implementation of the toolbar. I believe that insertImage should be made public and a insertLink should be created for the same purpose. That way it is up to the user to decide how to provide the editor with the appropriate elements.
  4. Likewise, having CropImageActivity/MediaChooserActivity seems to go beyond the responsibility. Maybe this could be bundled with the toolbar module, but definitely not with the core module. As I mentioned before, the editor should not care how the image is provided.

If you think these points make sense, I'd be glad to fork the library and create a PR related to some of them.

3dm1 avatar Jan 24 '18 09:01 3dm1

The ColorPicker dependency is added to the RTEditor module, but only used by the HorizontalRTToolbar on the RTEditor-Toolbar module.

I agree and I just changed that

The RTManager relies on the pre-defined number of effects to update the toolbar so it is hard to append any custom effect without a new PR to the library or extending and overriding the onSelectionChanged method. It might be interesting to allow registering a delegate into the RTManager that will receive the toolbar and update the appropriate button as necessary.

I agree again. There's much I would do differently nowadays but unfortunately I don't have time for larger changes like this one.

Currently the RTManager relies on an EventBus to receive images/links and it feels this is doing too much as it ties me to a library I'd rather not use, specially if I decide to write my custom implementation of the toolbar.

I would use an reactive approach instead of publish-subscribe pattern meaning I would pick RxJava (or RxKotlin) instead of EventBus. Same for your last point. This shouldn't be too hard and I might do that in the near future to eliminate EventBus.

1gravity avatar Jan 28 '18 02:01 1gravity