instructure-react / react-tinymce

React TinyMCE component
181 stars 115 forks source link

Fix issues with updating props #23

Closed JasonTolliver closed 7 years ago

JasonTolliver commented 8 years ago
JasonTolliver commented 8 years ago

Would fix https://github.com/instructure-react/react-tinymce/issues/21, thanks @jonathaningram for the inspiration.

jonathaningram commented 8 years ago

Edit: but your change is slightly different to mine, so in the end you may not need the cursor change I added...

@JasonTolliver you may want to update it to correct the cursor:

  componentWillReceiveProps(nextProps) {
    if (!isEqual(this.props.config, nextProps.config)) {
      this._init(nextProps.config, nextProps.content)
    }
    if (!isEqual(this.props.id, nextProps.id)) {
      this.id = nextProps.id
    }
    if (!isEqual(this.props.content, nextProps.content)) {
      let editor = tinymce.EditorManager.get(this.id)
      editor.setContent(nextProps.content)
      // added:
      editor.selection.select(editor.getBody(), true)
      editor.selection.collapse(false)
    }
  },

Without those added lines, when you focus into the editor and start typing, after the first character, the cursor will jump back to the start causing the result to look like this (after typing "Foobar"):

image

JasonTolliver commented 8 years ago

Great call @jonathaningram! https://github.com/JasonTolliver/react-tinymce/commit/524431336f90879066ce5d28e7f5e0f172e24628

JasonTolliver commented 8 years ago

One more fix added to this in cc24267, because TinyMCE is completely handling its own inner state, you cannot compare nextProps.content with this.props.content. So instead we compare it with editor.getContent() and we don't get false positives for updating.

andrewmclagan commented 7 years ago

any chance of a merge, outstanding since Jan?

sassanh commented 7 years ago

I'm looking forward to see this merged too.

sassanh commented 7 years ago

It'd be great if @JasonTolliver could update his fork with latest version so that we could use his fork meanwhile.

kevin1024 commented 7 years ago

I pushed a package to npm including this commit if anyone wants to use it. https://www.npmjs.com/package/@realgeeks/react-tinymce

sassanh commented 7 years ago

@kevin1024 thank you :-) @JasonTolliver using editor.getContent() makes it vulnerable considering this issue in tinymce: https://github.com/tinymce/tinymce/issues/2722 I changed it back to this.props.content. I guess we should keep using this.props.content till that issue is fixed or we handle that issue. It was unusable for me till I rolled it back to this.props.content. I guess you can reproduce the problem but changing content prop fast when it's initializing.