Skip to content

Conversation

@c-w
Copy link
Owner

@c-w c-w commented Jun 25, 2019

This change adds a new top-level function window.mathquill4quill which acts as a factory method for the enableMathQuillFormulaAuthoring call. Decoupling the enableMathQuillFormulaAuthoring call from the Quill instance enables integrating the mathquill4quill library with other projects that are built on-top of Quill such as react-quill mentioned in #9.

@c-w
Copy link
Owner Author

c-w commented Jun 25, 2019

@boomanaiden154 given that you've contributed a lot to this project recently, would you mind taking a look at this pull request and let me know your thoughts? Thanks in advance!

@c-w c-w force-pushed the react-quill-integration branch from 92f376a to 6f4fddd Compare June 25, 2019 19:58
@c-w c-w force-pushed the react-quill-integration branch from 6f4fddd to 05d669c Compare June 25, 2019 20:09
Copy link
Collaborator

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. The preservation of the functionality outside react-quill would be what I am most concerned about, and this is preserved with a minor breaking change upon update to the latest version(just need to slightly refactor the code enabling mathquill4quill). I don't have much experience with react, but it looks good.

@c-w
Copy link
Owner Author

c-w commented Jun 25, 2019

Thanks for taking a look @boomanaiden154. Note that the change is fully backwards compatible and I've verified that the previous method of initialization via quill.enableMathQuillFormulaAuthoring(options) continues to work. The only change is that now we expose a top-level function on window but this shouldn't impact existing applications (unless mathquill4quill is already defined as a global which seems unlikely).

@c-w c-w merged commit f4adf00 into master Jun 25, 2019
@c-w c-w deleted the react-quill-integration branch June 25, 2019 22:57
@boomanaiden154
Copy link
Collaborator

@c-w guess I didn't see the part at the end that exposed the function with quill.prototype. Everything looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants