bmcmahen / react-wysiwyg

retain some control over contenteditable input
169 stars 37 forks source link

Confused about noLineBreaks and onEnterKey. #15

Open dtuite opened 9 years ago

dtuite commented 9 years ago

It seems to me like noLineBreaks sounds like an option which would allow me to control whether or not the user can insert line breaks when they're typing. However, even when the noLineBreaks option is not set, the user still can't insert newlines.

Imagine using your example with the noLineBreaks deleted. I type a word and then press enter. I would expect this to take me down to the next line where I could continue typing. Instead it unfocuses the content editable element.

Am I misunderstanding the intention here?

bmcmahen commented 9 years ago

I think you understand correctly, but it looks like this might be a bug. I only ever use this module with the noLineBreaks option enabled, but we should have a test to make sure this functionality works as intended. Ideally you should be able to disable line-breaks while still receiving the callback for onEnterKey. I'll try to look at this soon.

dtuite commented 9 years ago

Cool. That's exactly how I would have expected it to work. I'll try to get a PR for this together tomorrow.

bmcmahen commented 9 years ago

I understand why this doesn't work, now. It's because the first argument in the onChange callback is only a text representation of the content entered into input field. So if you merely pass this back as the html prop, it won't actually include the newly created line-break. I suppose a solution right now, with the current api, would be to listen for the 'onEnterKey', and then manually insert a line-break into the text. That's pretty messy, though, and basically means that either way, the 'noLineBreaks' prop isn't necessary.

dtuite commented 9 years ago

Right. Although I suspect that noLineBreaks would still be necessary because there are situations where you want to control whether or not the user can move to a new line.

I see it as something like this.

onEnterKey(args) {
  if (!this.props.noLineBreaks) {
    // Add a newline into the text content...
  } 
  return this.props.onEnterKey(args);
}
avk commented 8 years ago

I'm experiencing this issue as well: noLineBreaks seems to be incompatible with onEnterKey:

this code https://github.com/bmcmahen/react-wysiwyg/blob/master/index.js#L229

    // prevent linebreaks
    if (this.props.noLinebreaks && (key === 13)) {
      return prev()
    }

comes before this code: https://github.com/bmcmahen/react-wysiwyg/blob/master/index.js#L251

        case 13:
          // 'Enter' key
          prev();
          if (this.props.onEnterKey) {
            this.props.onEnterKey();
          }
          break;

And they seem incompatible though I haven't spent very long looking at this.

Any updates @bmcmahen?