Proposed simpler usage for the editor
Closed this issue · 9 comments
First of all, I'd like to say I love what you are doing with this project. I've been long awaiting something like this.
However, I feel like there is an issue with the current implementation: Instead of having to call a function when you need the editor's data, why not create a listener to the changes in the editor, so the content is automatically saved?
You don't need to worry about making sure that the content is saved in every possible scenario of the user leaving the editor screen, and the implementation becomes very similar to the react native TextInput.
Current implementation - You first set a reference to the editor, which is used to call this.webViewQuillEditor.getDelta(), which then calls the function provided in getDeltaCallback, which then will set the new text in the state. That text will be received again by the editor in contentToDisplay.
Suggested implementation - Having a onChangeText listener, which when the editor is updated, receives a function to update the state with the new text. This text is passed to the value prop of the editor, which sets the initial and current value of the editor.
@reggie3, If you believe this is something worth working on, I'd love to try to help!
Thanks. I hope you find it useful.
A quick glance at the Quilljs documentation makes it look like what you're speaking of could be implemented using (text-change)[https://quilljs.com/docs/api/#text-change] which could fire a callback that sends a message to the WebViewQuillEditor which would in turn fire a callback to the parent compoment.
However, this still means there is a callback involved, so it wouldn't make the component any simpler since the user would still have to create a function to receive the contents.. And in this case, the callback would be getting called every time the editor's contents changed. Let me know if you're seeing a different design for what you are talking about.
Yes, that is exactly what I'm talking about! Even though there is still a callback, that would mean the data would be constantly persisted, and there wouldn't be any need to be calling the reference to the editor every time you needed the data/needed to persist it.
I'm kind of busy at the moment with another project. Would you be willing to submit a pull request to implement that feature? I'd definitely be willing to help explain the code changes that need to take place to make it work.
Unfortunately, for the project I'm working on, I've found that it would make more sense to implement my own custom webView, so unfortunately I won't be able to work on that. However, I would like to thank you for your work in this project and in your Medium posts. You definitely helped steer me to the right direction. Thank you very much!
Hey guys, I'm using this package in my project too and thought it might be helpful to have the text-change callback so I went ahead and implemented it. Checkout #8
Also, I agree e with @filiptdz that it requires a bit more polishing up to make the usage easier. I'm happy to contribute the changes I'm making on it for my project.
@foysalit I saw your pull request and it looks like it will work. I'll have to hand merge your changes in since I've been updating the library myself to try to get it to work with Expo Asset Bundling. You can see some of my posts about it here: https://forums.expo.io/t/what-is-path-to-location-of-files-bundled-with-assetbundlepatterns-in-sdk-v25/7311/7. Bundling will be a big enough change that it would be easier to just copy and paste your changes into the new code base.
That sounds good. I think your link to the post is broken, can you please post the link again? Also, let me know if you need a hand with anything to offload some work and ship it faster. I could use the updated version asap. thanks again.
Version 0.2.5 should add what you guys are looking for. I've update the readme to include an "onDeltaChangeCallback" callback for the editor. @foysalit, thanks for the code inputs.
Please give it a test, and let me know how it works for you guys. I published a new version of the Expo demonstration application, and it is working there.
Haven't received any feedback on whether this feature works in the wild now that it's been incorporated. I'm assuming it does, so I'm closing it.