facebookarchive / draft-js

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

What is the best approach for adding a checklist block to Draft? #132

Closed gscottolson closed 8 years ago

gscottolson commented 8 years ago

I want to extend Draft to accept a checklist/todo block. The block will have a checkbox at the left edge and can be toggled on/off to represent a complete/pending state respectively. The checkbox cannot be moved around the editor (you can’t copy/paste it into the middle of a text block).

I feel like there are a few approaches I can take, so I was wondering if I could get an opinion.

  1. Create a custom block type by extending DraftEditorBlock. I would need to override render() and tuck the checkbox into the component before the children are rendered.
    • One issue with this approach is that Draft does not publicly expose this component. Not a dealbreaker, but I’m assuming there was some thought behind excluding DraftEditorBlock.
  2. Wrap the internal block text in an Entity that injects the checkbox.
    • I tried out this approach using the link example. One side-effect is that you can’t delete all text within the block and have the Entity remain. These concerns were raised in #129, e.g. using a zero-width character.
  3. A better alternative?

Thanks again for continuously improving Draft.

hellendag commented 8 years ago

Can you create a custom block component that includes your checkbox and composes a DraftEditorBlock? (We should never ever extend React components.)

You can pass along props that track the state of the checkbox.

class MyCheckboxEditor extends React.Component {
  constructor(props) {
    this._blockRenderer = (block) => {
      if (block.getType() === 'checkable') {
        return {
          component: CheckableBlock,
          props: {
            checkedState: this.state.checkedState[block.getKey()],
            onCheckedChange: (...) => /* update value in `checkedState` */
          },
        };
      }
    }
  }

  // etc.
}
gscottolson commented 8 years ago

Thanks for the quick reply. Let me take a shot at your suggestion and I will get back to you.

gscottolson commented 8 years ago

@hellendag Can you create a custom block component that includes your checkbox and composes a DraftEditorBlock?

I noticed that DraftEditorBlock requires props to be passed in. As a result, I ended up with:

  this._blockRenderer = (block) => {
    const type = block.getType()
    if (type !== 'taskPending' && type !== 'taskComplete') {
      return // render the default DraftEditorBlock
    }

    return {
      component: CheckableBlock,
      props: {
        checked: type === 'taskComplete',
        ...getDraftEditorBlockProps(this.state.editorState, block),
      },
    }
  }

where getDraftEditorBlockProps() derives the required props.

My component looks like

export default class CheckableBlock extends React.Component {
  static propTypes = {
    blockProps: PropTypes.object.isRequired,
  }

  render() {
    const {checked} = this.props.blockProps
    return (
      <div>
        <input type="checkbox" checked={checked} />
        <DraftEditorBlock {...this.props.blockProps}/>
      </div>
    )
  }
}

This renders as expected and the text contents of the block are editable! Nice. However, using the keyboard to move the insertion cursor over to the input results in an error:

Uncaught Invariant Violation: Unknown node in selection range.

This makes sense. I don’t expect Draft to understand how to handle insertion outside of the DraftEditorBlock. Is there a way to tell Draft “just ignore the <input>” (while still allowing it to be toggled with a mouse/keyboard)?

(I don’t have my heart set on <input type="checkbox">, but I assume I will need a proper React element of some kind to handle events, e.g. toggle the state on/off.)

I don’t believe generated content (:before, :after) or list-image-style will offer me the control to toggle the state.

Any suggestions would be much appreciated.

hellendag commented 8 years ago

where getDraftEditorBlockProps() derives the required props.

The same props should be passed to your block component as to DraftEditorBlock: CustomComponent and DraftEditorBlock.

So I believe you should have access to all the necessary props, and can propagate them through to your DraftEditorBlock using {...this.props}.

Uncaught Invariant Violation: Unknown node in selection range. ... Is there a way to tell Draft “just ignore the ” (while still allowing it to be toggle with a mouse/keyboard)?

Ah, I should have realized that might cause trouble. Does it work if you make the <input> contentEditable={false}?

gscottolson commented 8 years ago

So I believe you should have access to all the necessary props, and can propagate them through to your DraftEditorBlock using {...this.props}.

Sweet!

Does it work if you make the <input> contentEditable={false}?

Unfortunately not. It exhibits the same error when I change the <input> to a <span contentEditable="false">

hellendag commented 8 years ago

Did you should set data-offset-key={this.props.offsetKey} on your root div? This is how our selection logic figures out where it is within the editor. (Not that you would have known to do that, since I probably didn't document it well. :))

gscottolson commented 8 years ago

Thanks for the help. data-offset-key worked!

Alright…making some progress. No more Uncaught Invariant Violations.

Is it possible to special case the selection to “skip” the inline <input> altogether? It would be ideal if the insertion cursor left the current custom block text and landed at the end of the previous block when pressing .

hellendag commented 8 years ago

Awesome!

Is it possible to special case the selection to “skip” the inline altogether?

Not exactly. It's one of the drawbacks of depending on the browser for selection behavior -- it sometimes puts the cursor in awkward places, especially around contentEditable={false} elements.

What does your current SelectionState look like when the cursor is before/after the <input>?

gscottolson commented 8 years ago

It is consistent when moving the cursor before/after the <input>:

anchorKey: "erh0g"
anchorOffset: 0
focusKey: "erh0g"
focusOffset: 0
hasFocus: false
isBackward: false

(I pulled these values from the React Dev Tools.)

gscottolson commented 8 years ago

If I apply certain styles to the <input>, (e.g. position: absolute, float: left) selection behavior changes a little. Going “forward” moves the selection from the end of the current block to the beginning of the next block. Going “backward” from the first character of the current block moves the cursor around the input before placing it at the end previous block.

I haven’t dug through the code enough, but it seems Draft knows how to solve the “forward” case (because it can put the cursor at offset: 0 of the next block). It’s the “backward” case where Draft “loses track” of the selection.

hellendag commented 8 years ago

Would a handler do the job? For instance, you could use a key binding function that maps the key to a command, then in handleKeyCommand, logic for that command that:

We could also expose leftArrow and rightArrow as first-class keydown handler citizens, too.

You could probably still end up in a weird cursor state at times, but that could help.

gscottolson commented 8 years ago

We could also expose leftArrow and rightArrow as first-class keydown handler citizens, too.

I think custom key bindings and handleKeyCommand will get me to a workable solution. But exposing onLeftArrow could be a nice API enhancement for the future. :space_invader:

Would a handler do the job?

Yes, I think so. However, I’m confused about the interplay between the method assigned to the keyBindingFn prop and handleKeyCommand.

If I pass a command to handleKeyCommand and the left keypress is not inside a checkable block, then I will have lost the chance to call getDefaultKeyBinding appropriately (handleKeyCommand doesn’t have access to event, right?)

Currently I have:

  // some pseudocode for brevity
  myKeyBinder = (event) => {
    // if key isnt ArrowLeft return getDefaultKeyBinding(event)
    // if selection isnt collapsed return getDefaultKeyBinding(event)
    // if focus offset isnt 0 return getDefaultKeyBinding(event)
    // if block type isnt checkable return getDefaultKeyBinding(event)
    // if prev block doesn’t exist return getDefaultKeyBinding(event)

    const prevBlock = contentState.getBlockBefore(startKey)
    const prevLength = prevBlock.getLength()
    const nextSelection = new SelectionState({
      anchorKey: prevKey,
      anchorOffset: prevLength,
      focusKey: prevKey,
      focusOffset: prevLength,
      isBackward: false,
      hasFocus: false,
    })
    // move to end of previous block
    const selectionState = EditorState.forceSelection(editorState, nextSelection)
    const nextState = EditorState.push(selectionState, contentState, 'move-selection-to-end-of-block')
    this.handleChange(nextState)

    return event.key
  }

FWIW, this solves my needs, but I imaging I may be missing something.

hellendag commented 8 years ago

http://facebook.github.io/draft-js/docs/advanced-topics-key-bindings.html#customization

Your key binding function should fall through to return getDefaultKeyBinding(event);. :)

gscottolson commented 8 years ago

Your key binding function should fall through to return getDefaultKeyBinding(event);. :)

It does. :wink: That is all the pseudocode at the top. I return getDefaultKeyBinding(event) for each non-relevant condition before I generate a new EditorState. My code seems similar to the example you referenced:

function myKeyBindingFn(e: SyntheticKeyboardEvent): string {
  if (e.keyCode === 83 /* `S` key */ && hasCommandModifier(e)) {
    return 'myeditor-save';
  }
  return getDefaultKeyBinding(e);
}

However, if I don’t respect the default key binding for I would have to manually move the selection one character left each time inside handleKeyCommand, right?

hellendag commented 8 years ago

Oh ha, I just totally ignored that since it was commented out! My fault. :)

I think I see what the issue is here. There are actually two things you need to implement.

  1. Your key binding function, which should return a string like 'move-selection-up' in your left-arrow case.
  2. A handleKeyCommand function, which receives the command string and consumes it accordingly.

Both should be provided as props to Editor.

class MyEditor extends React.Component {
  constructor(props) {
    ...
    this.handleKeyCommand = (command) => {
      if (command === 'move-selection-left') {
        // do stuff to this.state.editorState, call this.setState(), return true
      }
      return false;
    }
  }

  render() {
    return (
      <Editor
        handleKeyCommand={this.handleKeyCommand}
        keyBindingFn={myKeyBindingFn}
        ...
      />
    );
  }
}

function myKeyBinding(event) {
  if (/* left key and other stuff */) {
    return 'move-selection-left';
  }
  return getDefaultKeyBinding(event);
}

So you're defining a command, then separately consuming and handling that command. Does that make sense?

gscottolson commented 8 years ago

Yes. I think my hangup is that I have to duplicate much of the EditorState/ContentState/SelectionState introspection in both methods. However, my issue has been solved and that is the important part.

I just wanted to throw out a huge THANK YOU for taking the time to respond thoroughly (and thoughtfully) to all of my questions and comments. Your help was immensely valuable. :gem:

hellendag commented 8 years ago

I think my hangup is that I have to duplicate much of the EditorState/ContentState/SelectionState introspection in both methods.

Ah, true. That's kind of annoying. Are there any top-level API methods that you think might help streamline this? I'm going to go ahead and close this issue for queue maintenance, but we can continue discussing on the thread.

I just wanted to throw out a huge THANK YOU for taking the time to respond thoroughly (and thoughtfully) to all of my questions and comments. Your help was immensely valuable. :gem:

You're welcome! :)

gscottolson commented 8 years ago

Are there any top-level API methods that you think might help streamline this?

Hmm. Good question. Off the top of my head:

That would save me a small amount of code here and there, but it might not be worth the API bloat.

I’ll keep my eyes open for common patterns as I continue to build with Draft.

hellendag commented 8 years ago

These would generally be wrappers like this, right?:

static function getBlockBeforeSelectionStart(editorState) {
  const selection = editorState.getSelection();
  const content = editorState.getCurrentContent();
  return content.getBlockMap().getBlockForKey(selection.getStartKey());
}

Plucking information off an EditorState can be a little boilerplate-y and tedious, I know. :)

It might be useful to have a bag of shortcut utilities like these, perhaps not on EditorState itself, but either available within the Draft repo or in a utility repo.

gscottolson commented 8 years ago

Yes, these would probably be better suited as a set of utility functions, similar to RichUtils. Thanks for following up.

glenn-allen commented 8 years ago

Hi guys, sorry if I shouldn't be asking this here, but I have been trying to do something very similar for the last few hours and I'm stumped by the following error:

warning.js:44 Warning: React.createElement: type should not be null, undefined, boolean, or number. It should be a string (for DOM elements) or a ReactClass (for composite components). Check the render method of ChecklistEditorBlock.

I'm on the latest version of draft (0.4.0) which I can see is considerably different to the code referenced here.

This error is only occurring when my custom block component tries to use the DraftEditorBlock, otherwise it renders as expected (except my cursor handling is messed up).

This is the code for my ChecklistEditorBlock with everything except the DraftEditorBlock removed:

class ChecklistEditorBlock extends React.Component {
    render() {
        return <DraftEditorBlock {...this.props} />;
    }
}

And this is the code for my block rendered function:

blockRendererFn(contentBlock) {
    const type = contentBlock.getType();
    switch (type) {
        case "checklist":
            return {
                component: ChecklistEditorBlock,
            };
        default:
            return;
    }
}

I've tried to step into the code to make sense of the warning, but I'm only 2 months into react so I'm struggling. Would either of you have any ideas, or is this potentially a regression? Thanks

cgestes commented 8 years ago

Is your DraftEditorBlock defined?

(Try to print it)

On Tue, 12 Apr 2016, 09:20 Glenn Allen, notifications@github.com wrote:

Hi guys, sorry if I shouldn't be asking this here, but I have been trying to do something very similar for the last few hours and I'm stumped by the following error:

warning.js:44 Warning: React.createElement: type should not be null, undefined, boolean, or number. It should be a string (for DOM elements) or a ReactClass (for composite components). Check the render method of ChecklistEditorBlock.

I'm on the latest version of draft (0.4.0) which I can see is considerably different to the code referenced here.

This error is only occurring when my custom block component tries to use the DraftEditorBlock, otherwise it renders as expected (except my cursor handling is messed up).

This is the code for my ChecklistEditorBlock with everything except the DraftEditorBlock removed:

class ChecklistEditorBlock extends React.Component { render() { return <DraftEditorBlock {...this.props} />; } }

And this is the code for my block rendered function:

blockRendererFn(contentBlock) { const type = contentBlock.getType(); switch (type) { case STYLE_COMMANDS.CHECKLIST: return { component: ChecklistEditorBlock, }; default: return; } }

I've tried to step into the code to make sense of the warning, but I'm only 2 months into react so I'm struggling. Would either of you have any ideas, or is this potentially a regression? Thanks

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/facebook/draft-js/issues/132#issuecomment-208751351

gscottolson commented 8 years ago

@glenn-allen Can you provide a Gist with more code or a (non-working) example using jsFiddle or equivalent?

glenn-allen commented 8 years ago

Thank you both for your quick reply! So @cgestes, you were correct - DraftEditorBlock was undefined, pretty embarrassed I didn't think to do that myself. Upon further inspection of the draftjs source I realised theDraftEditorBlock is exposed instead as just EditorBlock. Correcting that solved my problems, thanks so much again!

sugarshin commented 8 years ago

Hi. I have tried the implementation of the checkable list to these messages to the reference. Thank you! However, we are worried about where to convert from html to contentState. What would be better to make modifications to convertFromHTML? Thank you

epalese commented 8 years ago

Hi, is it possible to change the content of the DraftEditorBlock according to some external events (e.g. the status of the checkbox)?

export default class CheckableBlock extends React.Component {
  constructor(props) {
    super(props);
    ...
    this._onChange() {
      // update the content of DraftEditorBlock
    }
  }

  render() {
    const {checked} = this.props.blockProps
    return (
      <div>
        <input type="checkbox"
            checked={checked}
            onChange={this.onChange} />
        <DraftEditorBlock {...this.props} />
      </div>
    )
  }
}

Thanks

Iuriy-Budnikov commented 8 years ago

Hey guys.

How to send type «checklist» from action to blockRendererFn?

blockRendererFn(contentBlock) {
    const type = contentBlock.getType();
    switch (type) {
        case "checklist":
            return {
                component: ChecklistEditorBlock,
            };
        default:
            return;
    }
}
Iuriy-Budnikov commented 8 years ago

@glenn-allen could you please share you action for adding checkbox

Iuriy-Budnikov commented 8 years ago

@hellendag could you please help ? : )

jan4984 commented 8 years ago

Hi @gscottolson, Could you please provide a run able source of your check able custom block component?

glenn-allen commented 8 years ago

Hi Lury, sorry I'm on vacation at the moment so I'm unable to look into this right now. I'll be back in just over a week, so if it's still unanswered I'll have a look when I'm back. On 7 Jun 2016 20:26, "Iuriy Budnikov" notifications@github.com wrote:

@glenn-allen https://github.com/glenn-allen could you please share you action for adding checkbox

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebook/draft-js/issues/132#issuecomment-224279241, or mute the thread https://github.com/notifications/unsubscribe/ANzfSq7n4R4WFlprzV1gLzUvm2QwOCQjks5qJXFqgaJpZM4Hmvi1 .

myyellowshoe commented 8 years ago

@glenn-allen @gscottolson Getting ready to head down this path myself, Anyone willing to open source it as a plugin for us to build on? We could even use the architecture to release it https://github.com/draft-js-plugins/draft-js-plugins

Would just be cool to have this up in the community.

Thanks!

glenn-allen commented 8 years ago

Hi @myyellowshoe & @luriy-Budnikov (sorry for forgetting!), My current implementation is fairly intertwined with the app I'm building and it's using a workaround for block metadata which as far as I'm aware is now possible via the draft API in Draft 0.8. I'll be trying to remove that soon, but not sure when I'll get the chance.

In the meantime, to get you started this is my general approach:

  1. Add a "blockRenderMap" and "blockRenderFn" to your editor:
<Editor  ...
                        blockRenderMap={this.blockRenderMap}
                        blockRendererFn={this.blockRendererFn}
                        ... />

The block render map tells draft how to automatically wrap the block (Draft divs and classes):

const checklistRenderProp = {
    element: 'li',
    wrapper: {
        type: 'ul',
        props: {
            className: 'checklist-ul',
        },
    },
};
this.blockRenderMap = DefaultDraftBlockRenderMap.set('checklist', checklistRenderProp);

The block render fn tells Draft how to actually render the block and what properties to pass it:

blockRendererFn(contentBlock) {
        const type = contentBlock.getType();
        switch (type) {
            case 'checklist':
                return {
                    component: ChecklistEditorBlock,
                    props: {
                        updateMetadataFn // NOTE: This is my workaround to updating metadata for a block (I don't think this is necessary in Draft 0.8),
                        checked: true // NOTE: This should be calculated from the block metadata,
                        // ... Any other props you want to add
                 };
            default:
                return;
        }
    }

As you can see we specify the component that we want to be rendered when a block with this type is encountered and provide it with props to render itself.

A simple version of that component is as follows:

import React from 'react';
import { EditorBlock } from 'draft-js';
import classNames from 'classnames';   // Makes adding classes simpler

export default class ChecklistEditorBlock extends React.Component {
    constructor(props) {
        super(props);
        this.toggleChecked = this.toggleChecked.bind(this);
    }

    toggleChecked(event) {
        const { blockProps, block } = this.props;
        const { updateMetadataFn, returnFocusToEditor, checked } = blockProps;

        const newChecked = !checked;
        updateMetadataFn(block.getKey(), newChecked);

        // I also stop propagation, return focus to the editor and set some state here, but that's probably specific to my app 
    }

    render() {
        const { offsetKey, blockProps } = this.props;
        const { checked } = blockProps;

        const blockClassNames = classNames('ChecklistEditorBlock', { checked });

        return (
            <div className={blockClassNames} data-offset-key={offsetKey}>
                <Checkbox checked={checked} onClick={this.toggleChecked} />
                <div className="text"><EditorBlock {...this.props} /></div>
            </div>
        );
    }
}

NOTE: "Checkbox" is simply a React component that uses CSS and an SVG to visually toggle between a checked and unchecked state (i.e. when the class "checked" is added or removed.

Also - "EditorBlock" is the Draft JS component which has all the smarts to handle the text entry.

Further to the above you'll need a way of triggering a block to become a checklist. This can simply be in an event handler or similar:

const newEditorState = RichUtils.toggleBlockType(editorState, 'checklist')

And to help with the arrow movement near the checkbox you'd also want to add a key binding function to your Draft :

checklistKeyBindingFn(editorState, event) {
         // left key
         if (event.keyCode === 37) {
            const selection = editorState.getSelection();
            const startKey = selection.getStartKey();
            const blockType = editorState.getCurrentContent().getBlockForKey(startKey).getType();
            if (blockType === 'checklist' && selection.getStartOffset() === 0
                && (selection.isCollapsed() || selection.getIsBackward())) {
                return 'move-selection-to-end-of-prev-block';
            }
        }
        return null;
}

keyBindingFn(event) {
        const { editorState } = this.state;

        return this.checklistKeyBindingFn(editorState, event)
            || getDefaultKeyBinding(event);
    }

And you'll need to handle that new command "move-selection-to-end-of-prev-block":

const handleMoveSelectionToEndOfPreviousBlockCommand = (editorState) => {
    const selection = editorState.getSelection();
    const startKey = selection.getStartKey();
    const contentState = editorState.getCurrentContent();
    const prevBlock = contentState.getBlockBefore(startKey);

    // If there's no previous block, then do nothing
    if (!prevBlock) return null;

    const prevKey = prevBlock.getKey();
    const prevLength = prevBlock.getLength();

    // Move the focus offset to the end of the previous line
    let selectionChanges = {
        focusKey: prevKey,
        focusOffset: prevLength,
    };
    // If the selection is collapsed, keep it collapsed by also moving the anchor
    if (selection.isCollapsed()) {
        selectionChanges = {
            ...selectionChanges,
            anchorKey: prevKey,
            anchorOffset: prevLength,
        };
    }

    const nextSelection = selection.merge(selectionChanges);

    // Update the selection state.
    const updatedEditorState = EditorState.forceSelection(editorState, nextSelection);
    return EditorState.push(updatedEditorState, contentState, 'move-selection-to-end-of-prev-block');
};

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

        let newState;
        if (command === 'move-selection-to-end-of-prev-block') {
            newState = this.handleMoveSelectionToEndOfPreviousBlockCommand(editorState);
        }

        if (newState) {
            this.onChange(newState);
            return true;
        }
        return false;
    }

So your final Editor will look something like:

<Editor ...
                        blockRenderMap={this.blockRenderMap}
                        blockRendererFn={this.blockRendererFn}
                        editorState={editorState}
                        onChange={this.onChange}
                        handleKeyCommand={this.handleKeyCommand}
                        keyBindingFn={this.keyBindingFn} />

I've heavily modified my code so there might be a couple of gaps sorry and this might not be the recommended way, but it's worked for me. Hope it helps, sorry it's so long!

myyellowshoe commented 8 years ago

Super cool @glenn-allen. Thanks for posting that! Would like to plugintize it so this should be more than enough to get started down that path. Thanks a bunch!

glenn-allen commented 8 years ago

FWIW @myyellowshoe, I've just updated to use the Draft 0.8 block data and this is my update metadata function:

updateBlockMetadata(blockKey, metadata) {
        let contentState = this.state.editorState.getCurrentContent();
        let updatedBlock = contentState
            .getBlockForKey(blockKey)
            .mergeIn(['data'], metadata);

        let blockMap = contentState.getBlockMap();
        blockMap = blockMap.merge({ [blockKey]: updatedBlock });
        contentState = contentState.merge({ blockMap });

        const newEditorState = EditorState.push(this.state.editorState, contentState, 'metadata-update');
        this.onChange(newEditorState);
    }

And my block render function now returns:

{
    component: ChecklistEditorBlock,
    props: {
        ...props,
        updateMetadataFn,
        checked: !!contentBlock.getData().get('checked'),
    },
};
myyellowshoe commented 8 years ago

Perfect. Thanks!

mkrn commented 8 years ago

Thanks a lot for this thread and throughout answers!

I'm implementing something similar and would like to ask how would one receive text updates in components props?

I'd like to invoke some API calls when text changes from within the component that's rendered.. But it seems that props are not changing when one types text into the EditorBlock

<EditorBlock {...this.props} /> Inspecting source code there are also no update callback props for EditorBlock

Most unclear is when blockRendererFn is actually invoked (tried passing text as this: text: contentBlock.getText() to blockProps.

Any idea would be appreciated! Thanks

lvdang commented 6 years ago

@glenn-allen - Can you show me how you are importing "DraftEditorBlock"

So far this doesn't work. I think I am declaring this wrong. import DraftEditorBlock from 'draft-js';

Actually I figured it out. I had to declare like this because EditorBlock is alias to 'DraftEditorBlock' import {EditorBlock} from 'draft-js';