bustle / ember-mobiledoc-editor

MIT License
86 stars 48 forks source link

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

Closed chrisdpeters closed 8 years ago

chrisdpeters commented 8 years ago

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 commented 8 years ago

(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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

Also, Mobiledocs are an object not a string.

mixonic commented 8 years ago

Recreated with Mobiledoc-kit 0.7.3 and the repo locally.

mixonic commented 8 years ago

@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:

mixonic commented 8 years ago

@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.

chrisdpeters commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

@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.