facebookarchive / draft-js

A React framework for building text editors.
https://draftjs.org/
MIT License
22.58k stars 2.64k forks source link

Updating dual editorStates not working. #473

Open jussch opened 8 years ago

jussch commented 8 years ago

My scenario: I have two draftjs editors. One is the core input, and the other is a helper input which wraps its input in an entity and adds it to the core input. The core input and the helper input work as intended when the helper only adds to the core input on blur. The problem arrises when I tried to get the helper input to update the core input live, (updating both editorStates on onChange of the helper input) I receive this error:

getUpdatedSelectionState.js:32 Uncaught TypeError: Cannot read property 'getIn' of undefined
  getUpdatedSelectionState @ getUpdatedSelectionState.js:32
  getDraftEditorSelectionWithNodes @ getDraftEditorSelectionWithNodes.js:35
  getDraftEditorSelection @ getDraftEditorSelection.js:33
  editOnSelect @ editOnSelect.js:26
  (anonymous function) @ DraftEditor.react.js:157
  ReactErrorUtils.invokeGuardedCallback @ ReactErrorUtils.js:70
  executeDispatch @ EventPluginUtils.js:87
  executeDispatchesInOrder @ EventPluginUtils.js:110
  executeDispatchesAndRelease @ EventPluginHub.js:42
  executeDispatchesAndReleaseTopLevel @ EventPluginHub.js:53
  forEachAccumulated @ forEachAccumulated.js:24
  processEventQueue @ EventPluginHub.js:215
  runEventQueueInBatch @ ReactEventEmitterMixin.js:18
  handleTopLevel @ ReactEventEmitterMixin.js:29
  handleTopLevelImpl @ ReactEventListener.js:73
  perform @ Transaction.js:136
  batchedUpdates @ ReactDefaultBatchingStrategy.js:63
  batchedUpdates @ ReactUpdates.js:98
  dispatchEvent @ ReactEventListener.js:150

I've boiled down my code as much as possible to find the perpetrator, and I've believe I've found the issue. Updating the editorState of the core Editor through the helper's editorState while the core Editor has readOnly={true} triggers the error. (And without readOnly={true}, the core editor constantly takes focus from the helper input.)

Here is the code to re-produce the error.

export default class DualInput extends Component {
  constructor(props) {
    super(props);
    this.setHelperState = this.setHelperState.bind(this);
    this.state = {
      readOnly: false,
      editorState: EditorState.createEmpty(),
      helperState: EditorState.createEmpty(),
    };
  }

  setHelperState(helperState) {
    this.setState({
      helperState,
      editorState: this.addCoreText(helperState.getCurrentContent().getPlainText('') || ''),
    });
  }

  addCoreText(text) {
    const editorState = this.state.editorState;
    const editorContent = Modifier.insertText(
      editorState.getCurrentContent(),
      editorState.getSelection(),
      text,
      null,
      null
    );

    return EditorState.push(editorState, editorContent, 'insert-characters');
  }

  render() {
    return (
      <div>
        <div onClick={() => this.refs.core.focus()}>
          <Editor
            ref="core"
            editorState={this.state.editorState}
            readOnly={this.state.readOnly}
            onChange={(editorState) => this.setState({ editorState })}
          />
        </div>
        <div onClick={() => this.refs.helper.focus()}>
          <Editor
            ref="helper"
            editorState={this.state.helperState}
            onChange={this.setHelperState}
          />
        </div>
        <button onClick={() => this.setState({ readOnly: !this.state.readOnly })}>
          ReadOnly: {String(this.state.readOnly)}
        </button>
      </div>
    );
  }
}

The above is a simplified version of my scenario, but it still illustrates the problem. Am I perhaps doing something wrong? Or is this a bug?

Thanks.

hellendag commented 8 years ago

Thanks for the details! Admittedly, this is a little tricky for me to follow.

Based on the error, my hunch is that swapping in the new EditorState leads to some kind of mismatch between the DOM and the ContentState, but it's hard to say.

Any chance you could provide a live example for me to try this on and get a better sense?

jussch commented 8 years ago

Thanks for the reply!

Unfortunately I don't know too much about the DraftJS code base, so I won't be of too much help, but I wrote up this example quick this morning: http://schultzjust.in/DraftJs-DualEditor/ . Hopefully this helps illustrate the problem. All the important code is in the HTML file.

Edit: (repo: https://github.com/jussch/DraftJs-DualEditor )

intentionally-left-nil commented 7 years ago

I am also seeing this issue. There is a timing window that exists from when a new editorState is passed into draftjs as a prop until that new state is rendered. If an onSelect event fires in this time, then the code will throw. getUpdatedSelectionState should detect this case and just return editorState.getSelection() in this scenario.

0rvar commented 7 years ago

This is a really annoying bug. Spams console with errors, breaks focus. Any chance we could pull in the fix from textioHQ/draft-js?

jussch commented 7 years ago

I've just updated to DraftJS v0.10.0 and while I haven't been able to extensively test this issue, I think the problem has been fixed. Thanks @AnilRedshift!

cameracker commented 7 years ago

@jussch I'm still seeing this

flarnie commented 7 years ago

Reopening because folks are still encountering this issue.

thundernet8 commented 7 years ago

Also encounter this under ssr

davidnguyen11 commented 6 years ago

Currently It is happening in ssr. So I temporary fixed that by delegating rendering of Editor to client side.

huv1k commented 6 years ago

I have same problem with rendering Editor with server-side. I am using next.js framework.

davidnguyen11 commented 6 years ago

@Huvik I am using next.js too. I solved like this:

import {Editor} from 'draft-js';

class Page1 extends Component {
    constructor(props) {
        super(props);
        this.state = {
            editor: null
        };
   }
   componentDidMount() {
        this.setState({
            editor: withReduxForm(Editor),
        });
   }

  render() {
       return (
           <div>
               {this.state.editor}
           </div>
       )
  }
}
export default Page1;

I bind editor at client side using componentDidMount instead of ssr.

againksy commented 6 years ago

I got this when using draft inside block of other draft. How to fix this? Want to build table with draft in each cell

cameracker commented 6 years ago

I would strongly recommend not attempting to implement a table in draft because it does not effectively deal with complex nested structures of that nature.

kaylee42 commented 6 years ago

I'm also still seeing this

acmoune commented 6 years ago

Just saw it now

flarnie commented 6 years ago

I would strongly recommend not attempting to implement a table in draft because it does not effectively deal with complex nested structures of that nature.

I can confirm what @CameronAckermanSEL said - it's something we have not needed in the past at Facebook, and are just starting to look into. I don't expect we will focus on the editor-within-an-editor use case but I'd be happy to take PRs or examples of how to handle this use case better.

For implementing tables in Draft, there are folks working on this internally but it is still experimental. I expect it will take many months before we have anything ready to share in open source.

jussch commented 6 years ago

For anyone having trouble with this issue, specifically editing a readOnly Editor and a live Editor at the same time, I found this trick solved it for me:

  componentWillReceiveProps(nextProps) {
    const { contentState } = nextProps;

    if (
      contentState &&
      this.props.contentState !== contentState &&
      this.state.editorState.getCurrentContent() !== contentState &&
      this.editorState.getCurrentContent() !== contentState
    ) {
      if (this.props.readOnly) {
        const editorState = EditorState.createWithContent(contentState);
        this.setState({ editorState });
        return;
      }

      this.setState({
        editorState: EditorState.push(
          this.state.editorState,
          contentState,
        ),
      });
    }
  }

Quick context here: I built a wrapper editor for DraftJS that edits the contentState rather than the editorState so that I'm not storing selection state in my Redux. However, to achieve this, the wrapper editor has to convert contentState to editorState to use DraftJS's editor.

So then, to explain my code, if the editor is readOnly, I avoid using EditorState.push(...) because it forces browser selection to happen. This causes a issues if there is more than one editor editing the same contentState. (or maybe its because its changing selection on a readOnly editor, dunno) Instead, I use EditorState.createWithContent(contentState) to generate my new editorState, which bypasses the conflicting selection modification.

Maybe this can help anyone running in to a similar issue.

acmoune commented 6 years ago

I can confirm that I get this error when trying to render Editor server side. I followed @davidnguyen179 way:

class MyEditor extends React.Component {
  constructor(props) {
    super(props)

    this.state = {
      editorState: EditorState.createEmpty(),
      editor: null
    }

    this.onChange = editorState => this.setState({ editorState })
  }

  componentDidMount() {
    this.setState({ editor: Editor })
  }

  render() {
    const ClientEditor = this.state.editor

    return (
      <div>
        {
          this.state.editor ?
            <ClientEditor
              editorState={this.state.editorState}
              onChange={this.onChange}
            /> :
            null
        }
      </div>
    )
  }
}

export default MyEditor

This works fine even with SSR. The thing is to render the Editor only when the component is mounted.

cupcoder commented 6 years ago

@jussch Could you please share the code of your wrapper editor? Really appreciate it! Have been struggling with this issue for a month now.

cupcoder commented 6 years ago

@flarnie Is there a ticket that we can follow to get update on table support in Draftjs? BTW, Slate JS can handle nested editor nicely. You guys may want to take a look at it. http://slatejs.org/

flarnie commented 6 years ago

I don't know of an issue specifically for nested table support, go ahead and open one if you'd like. :)

cameracker commented 6 years ago

https://github.com/facebook/draft-js/issues/600

https://github.com/facebook/draft-js/issues/220

https://github.com/facebook/draft-js/pull/388

There are more of these old issues and PRs that can be dug up from the backlog when searching for "Tables"

FWIW The author of #388 decided to move to Slate after the PR basically died, and went on to write a plugin that provides table support for Slate (out of courtesy I will not provide links to slate ecosystem on another project's backlog but google will help :))

cameracker commented 6 years ago

Also, https://github.com/facebook/draft-js/pull/1084 has been opened for an eternity and resolves this problem. We had been using it for months before we migrated and it resolved this error in the table-via-nested-editor feature. (Once again I'm going to emphasize that nested draft editors to build a table is a bad idea)

cupcoder commented 6 years ago

@CameronAckermanSEL I know nested draft editor is tricky, but it looks like @bkniffler got it working and here is his demo site https://draft-wysiwyg.herokuapp.com/ I was trying to follow his thoughts with the latest draft, but got blocked by this issue due to editOnSelect got triggered for both editors even though one of them (the parent) is marked as readonly.

bkniffler commented 6 years ago

Hey @cupcoder, thanks for bringing up draft-wysiwyg. I've not been following the latest development of draft-js, but when I used it, nesting needed to be implemented in a very hacky way (focus management, events, drag&drop, etc don't work natively). There is no native tree-like data structure, but instead the document is flat.

I've found that using slate-js works much better in this specific case (nested editors with correct focus management, tree document structure). I've been able to use it to produce real nested, editable pages for a content-management system, with containers, accordions, etc.

cameracker commented 6 years ago

@cupcoder I followed a similar approach and ran into the same problems that bkniffler listed above.

Here are some additional problems:

  1. Keyboard navigation between the cells is made much more complicated by the nested editors in the cells.
  2. The serialized document format explodes in size unless very precise serialization is done against the table cell contents (which are not just the cell but the entire document residing in the cell)
  3. Because of the bug here, and other bugs with having multiple active editors mounted, exactly one editor must be readonly=true in the cell because otherwise the editors will try to compete for focus and it's an overall nightmare.
  4. The table performance quickly becomes abysmal as the number of cells increase.

Draft is simply not designed to support a table workflow, which is a shame because it's a very common structure in documentation.

francisrod01 commented 6 years ago

+1 same issue

To solve this I believe we need to another way to visualize this content is other routes as well.

Screenshots:

image

image

Below there is my code:

import React, { Component } from 'react'
import { actions } from 'mirrorx'

import Editor from 'draft-js-plugins-editor'
import createToolbarPlugin from 'draft-js-static-toolbar-plugin'
import 'draft-js-static-toolbar-plugin/lib/plugin.css'

import {
    EditorState,
    convertFromRaw,
    convertToRaw,
} from 'draft-js'

import styles from '../config/editor-styles'

const staticToolbarPlugin = createToolbarPlugin()
const { Toolbar } = staticToolbarPlugin
let plugins = [staticToolbarPlugin]

const handleChangeContent = (content) => {
    const contentState = content.getCurrentContent()
    const rawContent = convertToRaw(contentState)
    const currentContent = JSON.stringify(rawContent)

    actions.editor.setState(currentContent)
}

class MyEditor extends Component {
    constructor(props) {
        super(props)

        const { content, isReadOnly } = props

        if (isReadOnly) {
            plugins = []
        }

        const editorState = this._mountEditorState(content)

        this.state = {
            editorState,
        }

        this.onFocus = () => this.refs.editor.focus()

        this.onChange = (editorState) => {
            this.setState({ editorState })

            handleChangeContent(editorState)
        }
    }
    componentWillReceiveProps(nextProps) {
        const { content, isReadOnly } = nextProps
        if (content && isReadOnly) {
            const editorState = this._mountEditorState(content)

            this.setState({ editorState })
        }
    }
    _mountEditorState(content) {
        let editorState

        try {
            const rawContent = convertFromRaw(JSON.parse(content))
            editorState = EditorState.createWithContent(rawContent)
        } catch (err) {
            editorState = EditorState.createEmpty()
        }

        return editorState
    }
    render() {
        const { isReadOnly } = this.props
        const { editorState } = this.state

        return (
            <div style={styles.root}>

                { !editorState && <div>Loading content...</div> }

                <div style={styles.editor} onClick={this.onFocus}>
                    <Editor
                        readOnly={isReadOnly}
                        editorState={editorState}
                        onChange={this.onChange}
                        ref='editor'
                        spellCheck={true}
                        plugins={plugins}
                    />
                    { !isReadOnly && <Toolbar /> }
                </div>
            </div>
        )
    }
}

export default MyEditor
ahungrynoob commented 6 years ago

aha. I get rid of the error by using createFromBlockArray. Maybe you can try it.

this.state = {
            editMode: false,
            editorState: EditorState.createWithContent(ContentState.createFromBlockArray([this.props.block]))
        }
ahungrynoob commented 6 years ago

And also I found that when I use onMouseDown to handle the sonEditor's focus event, the error never throwed. It seems that the getIn function is fired after the focus Event and is bound with onClick. So we need to let the sonEditor focus before onClick event. I tried onMouseDown ant it worked. Here is my code.

import React, { Component } from 'react'
import { Editor, EditorState, ContentState, Entity, convertFromRaw } from 'draft-js'

const emptyContentState = convertFromRaw({
    entityMap: {},
    blocks: [
        {
            text: '',
            key: 'foo',
            type: 'unstyled',
            entityRanges: [],
        },
    ],
})

export default class Caption extends Component {
    constructor(props) {
        super(props)
        this.state = {
            editMode: false,
            editorState: EditorState.createWithContent(ContentState.createFromText(this._getValue()))
        }
        this.onChange = this._onChange.bind(this)
        this._onClick = this._onClick.bind(this)
        this._onBlur = this._onBlur.bind(this)
    }

    _onChange(editorState) {
        this.setState({
            editorState
        })
    }

    _onClick() {
        this.setState({
            editMode: true,
        }, () => {
            this.props.onStartEdit()
            this.caption.focus()
        })
    }

    _onBlur() {
        if (this.state.editMode) {
            this.setState({
                editMode: false
            }, () => {
                var entityKey = this.props.block.getEntityAt(0)
                this.props.contentState.mergeEntityData(entityKey, { caption: this.state.editorState.getCurrentContent().getPlainText() })
                this.props.onFinishEdit()
            })
        }
    }

    _getValue() {
        return this.props.contentState
            .getEntity(this.props.block.getEntityAt(0))
            .getData()['caption']
    }

    componentDidMount() {
        this._onClick()
    }

    render() {
        return (
            <div className="image-caption" onMouseDown={this._onClick} onBlur={this._onBlur} >
                <Editor
                    textAlignment="center"
                    editorState={this.state.editorState}
                    onChange={this.onChange}
                    readOnly={!this.state.editMode}
                    ref={(ref) => this.caption = ref}
                    editorKey="caption"
                />
                <style jsx global>{`
                    .pape-draft .image-caption:empty{
                        display:inline-block; 
                    }
                `}</style>
            </div>
        )
    }
}
josephluck commented 6 years ago

For those who use next-js, I've gotten around this issue by importing the Draft editor using next's dynamic import with ssr turned off:

const Editor = dynamic(import("draft-js").then(module => module.Editor), {
  ssr: false
});

This will obviously prevent server-side rendering 🙄