Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split render from editor instantiation #89

Merged
merged 1 commit into from
Aug 25, 2015

Conversation

mixonic
Copy link
Contributor

@mixonic mixonic commented Aug 25, 2015

Drop the element argument from editor instantiation.

Should this perhaps be editor.append?

Fixes #88

@@ -71,7 +71,8 @@ const defaults = {
unknownCardHandler: () => {
throw new Error('Unknown card encountered');
},
mobiledoc: null
mobiledoc: null,
pasteInput: null
Copy link
Collaborator

Choose a reason for hiding this comment

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

html may be a more obvious name for this. pasteInput implies that is the result of a copy-paste operation (which needn't be true) and is vague about what the input is (text, html, dom node, etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dom felt pretty lame. Bleh, I guess it can be html and I can make it recognize when DOM is passed instead.

@bantic
Copy link
Collaborator

bantic commented Aug 25, 2015

Was adding an option like renderImmediately: false considered? Using an extra call to render seems like it may come as a mild surprise to an implementer; the 80% use case seems like it would be to instantiate an editor and immediately render it.
Otherwise 👍

@mixonic
Copy link
Contributor Author

mixonic commented Aug 25, 2015

I'd prefer one flow over making immediate render the default case. IMO Having a default render behavior encourages us to be lazy about what things look like before rendering has occurred. Several APIs should fail if you have not yet rendered.

Having the editor pre-render allows the user to call configuration methods like disableInput without us also needing to translate those into options.

@mixonic
Copy link
Contributor Author

mixonic commented Aug 25, 2015

@bantic let us continue to discuss. I'm going to merge and cut a release to move forward though.

mixonic added a commit that referenced this pull request Aug 25, 2015
Split render from editor instantiation
@mixonic mixonic merged commit 5449871 into bustle:master Aug 25, 2015
@mixonic mixonic deleted the split-out-render branch August 25, 2015 19:09
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.

2 participants