Skip to content

Conversation

@JasonTolliver
Copy link
Contributor

  • A simple content change was causing a complete re-render
  • Altering id prop was not initiating re-_init causing a selector mis-match
  • Initiating re-_init was not cloning the new props, _init was then adding new properties to the non-bound props and triggering false re-renders

@JasonTolliver
Copy link
Contributor Author

Would fix #21, thanks @jonathaningram for the inspiration.

- A simple content change was causing a complete re-render
- Altering id prop was not initiating re-_init causing a selector mis-match
- Initiating re-_init was not cloning the new props, _init was then adding new properties and triggering false re-renders
@JasonTolliver JasonTolliver force-pushed the feature/content_update_causes_unneeded_rerender branch from 62ff95f to 4b5fca8 Compare January 21, 2016 01:29
@jonathaningram
Copy link

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
Copy link
Contributor Author

Great call @jonathaningram! JasonTolliver@5244313

- Because TinyMCE is completely handling its own inner state, you cannot compare nextProps.content with this.props.content.  Instead you must compare with editor.getContent()
@JasonTolliver
Copy link
Contributor Author

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
Copy link

any chance of a merge, outstanding since Jan?

@sassanh
Copy link

sassanh commented Aug 18, 2016

I'm looking forward to see this merged too.

@sassanh
Copy link

sassanh commented Aug 18, 2016

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

kevin1024 added a commit to RealGeeks/react-tinymce that referenced this pull request Aug 18, 2016
@kevin1024
Copy link

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

@sassanh
Copy link

sassanh commented Aug 18, 2016

@kevin1024 thank you :-)
@JasonTolliver using editor.getContent() makes it vulnerable considering this issue in tinymce: tinymce/tinymce#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.

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.

6 participants