Atomic block's entity should be deleted on backspace command too

[aight8]

I have a simple use case which bothers me since a week. When I add an atomic block:

1.

"‸"

2.

"Some text...‸"

3. add atomic block with entity space (programatically)

   "Sometext..."
   " "
   "‸"

4. Hit backspace (we jump on line 1, atomic block is deleted how we except BUT the space from the atomic block with the entity in it is now on line 1 right next to the cursor.)

   "Sometext...‸ "

Normally you not recognize this space with the entity - but when you render the content state side-by-side to the editor you recognize that an entity in still present there on line 1. You can delete this space, than it's solved theoretically.

I except that the entity space should be removed too when I delete an atomic block by a backspace operation.

Otherwise the content state is now just invalid - there is an atomic entity somewhere free outside of an atomic block.

[aight8]

Additional research

Same behaviour on delete/backspace

I figured out that I have the exact same behaviour when using the delete command. In the first post the backspace command was executed. Both are handled by RichUtils. How I see in the code, onBackspace and onDelete keeps responsibility to delete atomic blocks. However the empty space with the entity range is just there.

Details to the atomic block insertion

Just for completeness, I used the following function to insert the atomic block.

AtomicBlockUtils.insertAtomicBlock(this.state.editorState, entityKey, ' ')

The character parameter is a space. (I read this somewhere) I tried the whole procedure with another readable string as character - for example "MYIMAGE". In this case there is the exact same behaviour.

The atomic block will be deleted but the entity range with the characters appears but the MYIMAGE string which contains the entity range appears on the upper line.

[aight8]

Okey I solved it. I don't returned "handled" in Editor::handleKeyCommand when it was handled and updated by RichUtils.handleKeyCommand - this caused this problem.

This issue can be closed but I moved an entry in issue #931 for documentation purposes (more infos there)

[ndelage]

I'm reviewing the same bug in our codebase, but we don't have a custom handleKeyCommand function. However, deleting an atomic block element with backspace results in the leftover space (from the insertAtomicBlock call), just like you described.

Any idea what might fix this? Do we need to write our own handleKeyCommand?

[aight8]

RichUtils.handleKeyCommand is not bound by default to handle commands. That's the truth. In the RichUtils.handleKeyCommand method you see all the commands this method handles. They are several, but in most cases it's okey.

Simple example:

  handleKeyCommand(command) {
    const { editorState } = this.state;

    const newContent = RichUtils.handleKeyCommand(editorState, command);
    if (newContent) {
      this.handleEditorChange(newContent); // .. update editor state ...
      return 'handled';
    } else {
      return 'not-handled';
    }
  }

Detailed explanation When you don't want that RichUtils.handleKeyCommand handle some command (for example some inline styles) just catch those commands and don't call RichUtils.handleKeyCommand for it.

The most important point however is that you return the handled string when your custom code or RichUtils.handleKeyCommand handled the command - this will prevent the default key action. When you don't return handled even when RichUtils.handleKeyCommand was handled the command and updated the editor state, still 90% of the commands are working as excepted. But deleting atomic block will not work correctly since draft.js thinks the command was not handled and executes the default action which break the atomic block removal.

It's a tricky point.

I suggested that draft.js should warn the user when handleKeyCommand function not returns explicitly "handled" or "not-handled". Or clarify this case in the docs even more. (--> https://facebook.github.io/draft-js/docs/advanced-topics-key-bindings.html)

[stevensacks]

Here's my workaround.

auditEditorState.js

export const auditEditorState = editorState => {
    // Need to make sure when an atomic block is deleted that its corresponding entity is, as well

    const atomicTypes = {image: true, twitter: true, youtube: true};

    const selectionState = editorState.getSelection();
    const anchorKey = selectionState.getAnchorKey();
    const currentContent = editorState.getCurrentContent();
    const currentContentBlock = currentContent.getBlockForKey(anchorKey);

    const type = currentContentBlock.getType();
    if (type === 'unstyled') {
        let orphan = false;
        currentContentBlock.getCharacterList().forEach(cm => {
            const entityKey = cm.getEntity();
            if (entityKey) {
                const entityType = currentContent.getEntity(entityKey).getType();
                if (entityKey && atomicTypes[entityType]) {
                    console.log(`${entityType} not allowed in unstyled block ${currentContentBlock.getKey()}`);
                    orphan = true;
                }
            }
        });

        if (orphan) {
            const newEditorState = editorState;
            newEditorState._immutable = newEditorState._immutable.set('allowUndo', false); // hack to allow pushing a new state without creating an undo
            const updatedSelection = selectionState.merge({anchorOffset: 0});
            const newContent = Modifier.removeRange(currentContent, updatedSelection, 'forward');
            return EditorState.push(
                newEditorState,
                newContent,
                'remove-orphaned-entities'
            );
        }
    }
    return editorState;
};

[stevensacks]

Sorry that's a slightly older version. This one's been cleaned up.

export const auditEditorState = editorState => {
    // Need to make sure when an atomic block is deleted that its corresponding entity is, as well
    const atomicTypes = {image: true, twitter: true, youtube: true};

    const selectionState = editorState.getSelection();
    const anchorKey = selectionState.getAnchorKey();
    const currentContent = editorState.getCurrentContent();
    const currentContentBlock = currentContent.getBlockForKey(anchorKey);

    const type = currentContentBlock.getType();
    if (type === 'unstyled') {
        let orphan = false;
        currentContentBlock.getCharacterList().forEach(cm => {
            if (!orphan) {
                const entityKey = cm.getEntity();
                if (entityKey) {
                    const entityType = currentContent.getEntity(entityKey).getType();
                    if (entityKey && atomicTypes[entityType]) {
                        //console.log(`${entityType} not allowed in unstyled block ${currentContentBlock.getKey()}`);
                        orphan = true;
                    }
                }
            }
        });

        if (orphan) {
            const newEditorState = editorState;
            newEditorState._immutable = newEditorState._immutable.set('allowUndo', false); // hack to allow pushing a new state without creating an undo
            const updatedSelection = selectionState.merge({anchorOffset: 0});
            const newContent = Modifier.removeRange(currentContent, updatedSelection, 'forward');
            return EditorState.push(
                newEditorState,
                newContent,
                'remove-orphaned-entities'
            );
        }
    }
    return editorState;
};```

[betancourtl]

You might be able to simplify the logic to find the orphans by doing something like this.

  const orphan = currentContentBlock.getCharacterList().some(cm => {
    const entityKey = cm.getEntity();

    return entityKey && atomicTypes[currentContent.getEntity(entityKey).getType()];
  });

[stevensacks]

Why stop there? :)

// Need to make sure when an atomic block is deleted that its corresponding entity is, as well
export const auditEditorState = editorState => {
    const selectionState = editorState.getSelection();
    const anchorKey = selectionState.getAnchorKey();
    const currentContent = editorState.getCurrentContent();
    const currentContentBlock = currentContent.getBlockForKey(anchorKey);
    const atomicTypes = {image: true, twitter: true, youtube: true};
    if (currentContentBlock.getType() === 'unstyled' && currentContentBlock.getCharacterList().some(cm => cm.getEntity() && atomicTypes[currentContent.getEntity(cm.getEntity()).getType()]) {
        editorState._immutable = editorState._immutable.set('allowUndo', false);
        return EditorState.push(
            editorState,
            Modifier.removeRange(currentContent, selectionState.merge({anchorOffset: 0}), 'forward'),
            'remove-orphaned-entities'
        );
    }
    return editorState;
};

[stevensacks]

This issue is actually worse. Eventually, you get runtime errors when you try to add or delete an atomic block.

Uncaught Error: Unknown DraftEntity key.
    at invariant (eval at <anonymous> (dekki.js:861), <anonymous>:44:15)
    at Object.__get (eval at <anonymous> (dekki.js:2058), <anonymous>:166:285)
    at ContentState.getEntity (eval at <anonymous> (dekki.js:2518), <anonymous>:155:24)
    at AtomicBlock (eval at <anonymous> (dekki.js:4840), <anonymous>:30:37)
    at StatelessComponent.render (eval at <anonymous> (dekki.js:8194), <anonymous>:44:17)
    at eval (eval at <anonymous> (dekki.js:8194), <anonymous>:795:21)
    at measureLifeCyclePerf (eval at <anonymous> (dekki.js:8194), <anonymous>:75:12)
    at ReactCompositeComponentWrapper._renderValidatedComponentWithoutOwnerOrContext (eval at <anonymous> (dekki.js:8194), <anonymous>:794:25)
    at ReactCompositeComponentWrapper._renderValidatedComponent (eval at <anonymous> (dekki.js:8194), <anonymous>:821:32)
    at ReactCompositeComponentWrapper._updateRenderedComponent (eval at <anonymous> (dekki.js:8194), <anonymous>:745:36)

[stevensacks]

I found the issue: https://github.com/facebook/draft-js/blob/master/examples/draft-0-10-0/media/media.html#L223-L225

        const entity = props.contentState.getEntity(
          props.block.getEntityAt(0)
        );

If you go "too fast" then props.block.getEntityAt(0) returns null sometimes which causes a runtime error. You have to check to see if that entity key exists before you try to get it.

[stevensacks]

    const entityKey = props.block.getEntityAt(0);
    if (entityKey) {
        const entity = props.contentState.getEntity(entityKey);

[stevensacks]

Sorry for that diversion. It's an unrelated bug to this ticket.

[againksy]

@aight8 can you please provide^ how to fix this exactly peace of code?

[againksy]

@aight8 @stevensacks how to use this auditEditorState.js ?

[betancourtl]

This is what I used to do.

// handleBeforeInput Editor prop functions
export const BACKSPACE_ON_ATOMIC_BLOCK = 'BACKSPACE_ON_ATOMIC_BLOCK';
export const ENTER_ON_ATOMIC_BLOCK = 'ENTER_ON_ATOMIC_BLOCK';

export const atomicKeyBindings = (editorState, e) => {
  const backSpace = 8;
  const enter = 13;
  if (e.keyCode === backSpace) {
    if (atomicSelection(editorState)) return BACKSPACE_ON_ATOMIC_BLOCK;
  }
  if (e.keyCode === enter) {
    if (atomicSelection(editorState)) return ENTER_ON_ATOMIC_BLOCK;
  }

  return null;
};

export const atomicKeyCommands = (command, editorState, updateEditorState) => {
  if (command === BACKSPACE_ON_ATOMIC_BLOCK) {
    const blockKey = atomicSelection(editorState); // check the block type
    const newEditorState = removeBlockWithKey(editorState, blockKey); // handle the backspae myself
    updateEditorState(newEditorState);
    return 'handled';
  }

  if (command === ENTER_ON_ATOMIC_BLOCK) {
    const newEditorState = insertNewBlock(editorState);
    updateEditorState(newEditorState);
    return 'handled';
  }

  return 'not-handled';
};

// method used to remove the atomic block
export const removeBlockWithKey = (editorState, blockKey) => {
  const contentState = editorState.getCurrentContent();
  const blockMap = contentState.getBlockMap();
  const newBlockMap = blockMap.filter(block => block.getKey() !== blockKey);

  const newContentState = contentState.merge({
    blockMap: newBlockMap,
  });

  const selection = editorState.getSelection();
  const startKey = selection.getStartKey();
  const blockBeforeKey = contentState.getBlockBefore(startKey).getKey();
  const newSelection = SelectionState.createEmpty().merge({
    ...selection.toJS(),
    anchorKey: blockBeforeKey,
    focusKey: blockBeforeKey,
  });

  return EditorState.forceSelection(EditorState.push(editorState, newContentState), newSelection);
};

export const insertNewBlock = editorState => {
  const contentState = editorState.getCurrentContent();
  const blockList = contentState.getBlockMap().toList();
  const selection = editorState.getSelection();
  const startKey = selection.getStartKey();
  const blockIndex = blockList.findIndex(block => block.getKey() === startKey);
  const blockMap = contentState.getBlockMap();
  const firstSlice = blockMap.slice(0, blockIndex + 1);
  const lastSlice = blockMap.slice(blockIndex + 1);
  const tempBlockMap = ContentState.createFromText('').getBlockMap();
  const fistBlock = tempBlockMap.first();

  // Generate the blockMap
  const newBlockMap = [fistBlock].reduce((prevBlockMap, newBlock) => {
    return prevBlockMap.set(newBlock.getKey(), newBlock);
  }, firstSlice).concat(lastSlice);

  // Modify selection to place cursor at new block
  const selectionKey = fistBlock.getKey();
  const newSelection = SelectionState.createEmpty().merge({
    anchorKey: selectionKey,
    focusKey: selectionKey,
    anchorOffset: 0,
    focusOffset: 0,
  });

  const newContentState = contentState.merge({
    blockMap: newBlockMap,
  });

  // Force the editor to render the cursor correctly
  return EditorState.forceSelection(
    EditorState.push(editorState, newContentState),
    newSelection);
};

[againksy]

@webdeveloperpr is there anyway, in which i can fix existing broken draft state?