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

Error when binding ember-mobiledoc-editor to a model #42

Closed
chrisdpeters opened this issue Dec 22, 2015 · 11 comments
Closed

Error when binding ember-mobiledoc-editor to a model #42

chrisdpeters opened this issue Dec 22, 2015 · 11 comments

Comments

@chrisdpeters
Copy link
Contributor

I have an example app hosted on S3 that demonstrates the error that I've been experiencing:
http://mobiledoc-error.s3-website-us-east-1.amazonaws.com/

And here is the accompanying Git repo:
https://github.com/chrisdpeters/mobiledoc-error

I was following @mixonic's advice on how to bind the {{mobiledoc-editor}} mobiledoc attribute to an Ember model attribute. We were discussing it in this gist.

He had suggested that I do something like this:

{{mobiledoc-editor mobiledoc=(readonly article.body) on-change=(action (mut article.body))}}

With the component call setup like this, I get the following error on the 2nd keystroke within the editor:

Uncaught TypeError: Cannot read property 'isCardSection' of null
    handleKeydown   @ editor.js:719
    handleEvent @ editor.js:616
    (anonymous function)    @ editor.js:582

If you run the S3-hosted app above, you'll see the state of the article record in Ember Inspector. It looks like it does get changed in some way but perhaps not how mobiledoc-editor expects:

{ version: 0.2.0, sections: [Array : 2] }

Is there a different type that I should be setting the model attribute to? Right now, it's setup as a string:

body: DS.attr('string')

Bonus issue: When I upgrade Ember and Ember Data to v2.2, I get the following console error on the very first keystroke:

Uncaught TypeError: Failed to execute 'setStart' on 'Range': parameter 1 is not of type 'Node'.
    _moveToNode @ cursor.js:142
    selectRange @ cursor.js:110
    renderRange @ editor.js:295
    PostEditor._renderRange @ post.js:34
    (anonymous function)    @ lifecycle-callbacks.js:21
    runCallbacks    @ lifecycle-callbacks.js:20
    complete    @ post.js:1459
    run @ editor.js:520
    _insertEmptyMarkupSectionAtCursor   @ editor.js:653
    handleKeydown   @ editor.js:667
    handleEvent @ editor.js:616
    (anonymous function)    @ editor.js:582

Then I also get the error that the Ember 1.13 is tripping.

@chrisdpeters
Copy link
Contributor Author

(Sorry, just a second on the S3-hosted app. I regenerated it as an Ember 1.13 app because that's what I've been working with primarily.)

@chrisdpeters
Copy link
Contributor Author

OK, the S3-hosted app should be available. I also took another look at the value of article.body in the Ember Inspector, and it seems to be fine. I was tripped up by the string representation in the attributes panel.

@mixonic
Copy link
Contributor

mixonic commented Dec 31, 2015

@chrisdpeters What version of Mobiledoc-Kit is being used in your demo? I added null and "" blank mobiledocs to the demo against master and can't recreate the issue here.

@mixonic
Copy link
Contributor

mixonic commented Dec 31, 2015

Also, Mobiledocs are an object not a string.

@mixonic
Copy link
Contributor

mixonic commented Dec 31, 2015

Recreated with Mobiledoc-kit 0.7.3 and the repo locally.

@mixonic
Copy link
Contributor

mixonic commented Dec 31, 2015

@chrisdpeters so the issue here is related to cursor position after an editor is destroyed.

The usage you have (being bound to the body, and updating the body) causes the editor to be destroyed and recreated on every edit. Type a letter, and the body gets updated. This causes the mobiledoc-editor to be passed a new mobiledoc, which causes the old editor to be destroyed and a new one to be created for the new mobiledoc. In there the cursor position gets messed up.

To address this immediately, I suggest you create and "initial mobiledoc" property on the calling context and pass that as mobiledoc. That property can then be stable while the change events update model.body. Similar to what we do in the demo.

Basically, the editor component does not understand when you set a mobiledoc it just emitted as a change event. Steps to improve are:

  • Better handling of the focus upon destroy. It should be blurred, I think (this on mobiledoc-kit).
  • Do not re-create the editor when the same mobiledoc we just emitted is passed in.

mixonic added a commit that referenced this issue Dec 31, 2015
Move the editor destroy/setup flow into willRender and didRender.
Compare a new mobiledoc to the local mobiledoc and the upstream one
before destroying and recreating the editor.

Refs #42
mixonic added a commit that referenced this issue Dec 31, 2015
Move the editor destroy/setup flow into willRender and didRender.
Compare a new mobiledoc to the local mobiledoc and the upstream one
before destroying and recreating the editor.

Refs #42
@mixonic
Copy link
Contributor

mixonic commented Dec 31, 2015

@chrisdpeters Your demo is working correctly with a linked version of ember-mobiledoc-editor and mobiledoc-kit master. I'll cut a release in the next few days.

@mixonic mixonic closed this as completed Dec 31, 2015
@chrisdpeters
Copy link
Contributor Author

@mixonic I appreciate this very much. I hope your update makes this addon even more useful for others!

In the model, do you suggest that I change the body to this:

body: DS.attr()

Instead of this?

body: DS.attr('string')

Or does it matter now?

@mixonic
Copy link
Contributor

mixonic commented Dec 31, 2015

@chrisdpeters Mobiledoc is a data structure, not a string. You may need some additional layer in your code to serialize the data into a string for saving, and then to restore it to an object when rendering.

I recommend saving a JSON string and not data, but be aware that the mobiledoc-editor returns the mobiledoc object and not a string.

@chrisdpeters
Copy link
Contributor Author

OK, I have an object transform that I could probably apply:

import DS from 'ember-data';

export default DS.Transform.extend({
  deserialize: function(serialized) {
    if (typeof serialized === 'string') {
      return JSON.parse(serialized);
    }
    else {
      return serialized;
    }
  },

  serialize: function(deserialized) {
    return JSON.stringify(deserialized);
  }
});
body: DS.attr('object')

Do you think this is a worthwhile thing to add to the README, like all of this that we just explored? Or is all of this so fresh that it wouldn't be worthwhile? I'd be happy to do a pull request if you think it is worthwhile.

@mixonic
Copy link
Contributor

mixonic commented Dec 31, 2015

@chrisdpeters I don't think we imply anywhere that Mobiledocs are strings. I'm open to documentation updates of course, though we probably don't need to talk about how to use Ember-Data in this project's README.

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

No branches or pull requests

2 participants