React-admin: RichTextInput: Spaces are trimmed, cannot add spaces

Created on 28 Mar 2019  路  4Comments  路  Source: marmelab/react-admin

Note: This bug is a regression of https://github.com/marmelab/react-admin/pull/2930

What you were expecting:

Spaces inside the RichTextInput not to be trimmed.

What happened instead:

Spaces inside the RichTextInput are trimmed, so we cannot write two words or more.

Steps to reproduce:

Just use a RichTextInput with defaults and try to write 2 words (i.e. use space at the end).

Related code:

The problem is that quill's clipboard.convert code eventually goes here, which trims the spaces:

function matchText(node, delta) {
  var text = node.data;
  // Word represents empty line with <o:p>&nbsp;</o:p>
  if (node.parentNode.tagName === 'O:P') {
    return delta.insert(text.trim());
  }
  if (text.trim().length === 0 && node.parentNode.classList.contains('ql-clipboard')) {
    return delta;
  }
  if (!computeStyle(node.parentNode).whiteSpace.startsWith('pre')) {
    // eslint-disable-next-line func-style
    var replacer = function replacer(collapse, match) {
      match = match.replace(/[^\u00a0]/g, ''); // \u00a0 is nbsp;
      return match.length < 1 && collapse ? ' ' : match;
    };
    text = text.replace(/\r\n/g, ' ').replace(/\n/g, ' ');
    text = text.replace(/\s\s+/g, replacer.bind(replacer, true)); // collapse whitespace
    if (node.previousSibling == null && isLine(node.parentNode) || node.previousSibling != null && isLine(node.previousSibling)) {
      text = text.replace(/^\s+/, replacer.bind(replacer, false));
    }
    if (node.nextSibling == null && isLine(node.parentNode) || node.nextSibling != null && isLine(node.nextSibling)) {
      text = text.replace(/\s+$/, replacer.bind(replacer, false));
    }
  }
  return delta.insert(text);
}

The above code is triggered by the traverse line, at the middle of the following quill's code:

          key: 'convert',
          value: function convert(html) {
            if (typeof html === 'string') {
              this.container.innerHTML = html.replace(/\>\r?\n +\</g, '><'); // Remove spaces between tags

              return this.convert();
            }

            var formats = this.quill.getFormat(this.quill.selection.savedRange.index);

            if (formats[_code2.default.blotName]) {
              var text = this.container.innerText;
              this.container.innerHTML = '';
              return new _quillDelta2.default().insert(text, _defineProperty({}, _code2.default.blotName, formats[_code2.default.blotName]));
            }

            var _prepareMatching = this.prepareMatching(),
                _prepareMatching2 = _slicedToArray(_prepareMatching, 2),
                elementMatchers = _prepareMatching2[0],
                textMatchers = _prepareMatching2[1];

            var delta = traverse(this.container, elementMatchers, textMatchers); // Remove trailing newline

            if (deltaEndsWith(delta, '\n') && delta.ops[delta.ops.length - 1].attributes == null) {
              delta = delta.compose(new _quillDelta2.default().retain(delta.length() - 1).delete(1));
            }

            debug.log('convert', this.container.innerHTML, delta);
            this.container.innerHTML = '';
            return delta;

Other information:

Environment

  • React-admin version:
  • Last version that did not exhibit the issue (if applicable):
  • React version:
  • Browser:
  • Stack trace (in case of a JS error):
bug

Most helpful comment

Hi!
@fzaninotto @djhi do you think you might have some time to look at this issue and my suggestion? Don鈥檛 want to come off as pushy but I鈥檓 really willing to work on this 馃檪
Thanks!

All 4 comments

Hi,
I also noticed that this regression affects new lines too: when inserting a new line (hitting Enter), it is briefly displayed in the editor before being deleted.
You can reproduce this behavior in the Demo app: https://marmelab.com/react-admin-demo/#/products/8/description

I'd gladly push a PR fixing this, however I'm not quite sure about which strategy to adopt. I do have an idea, although I would appreciate some early eyeballing before trying to implement it.

The problem seems to be caused by triggering the componentDidUpdate method even though the change came from inside the RichTextInput:

    componentDidUpdate(prevProps) {
        if (prevProps.input.value !== this.props.input.value) {
            const selection = this.quill.getSelection();
            this.quill.setContents(
                this.quill.clipboard.convert(this.props.input.value) // This call deletes all trailing whitespace / newlines
            );
            if (selection && this.quill.hasFocus()) {
                this.quill.setSelection(selection);
            }
        }
    }

If we add a state property, like isUpdateInternal, we could detect whether the input value prop change was indeed external to the component, or instead resulted from the user typing into it. A quick proof of concept:

export class RichTextInput extends Component {
+    constructor(props) {
+       super(props);
+      this.state = { isUpdateInternal: false };
    }
    // ...
    componentDidUpdate(prevProps) {
+        if (!this.state.isUpdateInternal &&
+            prevProps.input.value !== this.props.input.value) {
            const selection = this.quill.getSelection();
            this.quill.setContents(
                this.quill.clipboard.convert(this.props.input.value)
            );
            if (selection && this.quill.hasFocus()) {
                this.quill.setSelection(selection);
            }
        }
+        this.setState({isUpdateInternal: false});
    }
   // ...
    onTextChange = () => {
+        this.setState({isUpdateInternal: true})
        const value =
            this.editor.innerHTML == '<p><br></p>' ? '' : this.editor.innerHTML;
        this.props.input.onChange(value);
    };
}

What do you think? Would be happy to go with another solution as well.

Hi!
@fzaninotto @djhi do you think you might have some time to look at this issue and my suggestion? Don鈥檛 want to come off as pushy but I鈥檓 really willing to work on this 馃檪
Thanks!

This looks fixed! Cannot reproduce the issue in the Demo app anymore : https://marmelab.com/react-admin-demo/#/products/8/description

@Kmaschta I think you can close the issue, thanks! :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

9747749366 picture 9747749366  路  3Comments

waynebloss picture waynebloss  路  3Comments

yangjiamu picture yangjiamu  路  3Comments

samanmohamadi picture samanmohamadi  路  3Comments

ericwb picture ericwb  路  3Comments