bootstrapworld / codemirror-blocks

A library for building language-specific, CodeMirror-friendly editors that are a11y-friendly.
27 stars 11 forks source link

Multiple-delete doesn't #181

Closed justinpombrio closed 5 years ago

justinpombrio commented 5 years ago

If you select multiple block and then delete them or cut them, the first block goes away but the rest are transformed into plain text.

The issue seems to be around codeMirror.js. Here's what it looks like is happening:

justinpombrio commented 5 years ago

Upon committing the operation of deleting two lines, react-codemirror-2/Controlled fires the onBeforeChange event twice, as expected, and with the correct values. It also fires the onChange event once, with the wrong value (only one of the two lines is deleted). And after the whole ordeal, our global cm instance has only deleted one of the two lines.

justinpombrio commented 5 years ago

Here's an google sheet:

showing the order in which various things happen. It's all fine, including up through when DragAndDropEditor passes the correct value={this.props.value} to the <CodeMirror.../> element, but after that somehow CodeMirror's value is incorrect and only contains one of the two deletions.

schanzer commented 5 years ago

OK, did some poking around - I think my CodeMirror knowledge will come in handy here: the code assumes that the changes object is immutable, and that it can be passed to one editor and then to another and have the same effect. That assumption is invalid.

If we print out data inside the handleChange method in ToggleEditor, we can see what the change object looks like (we should really rename this variable!). Notice that the canceled property is set to true, which will result in the operation not being processed at all! I couldn't find any documentation about this, but it's a somewhat educated guess. This is alluded to in the documentation for beforeChange, which says that the event exposes a cancel() event. I wonder if our controlled component is calling cancel somehow, or if tmpCM (native library) does it once it processes the change?

Update: yep, it's happening inside react-codemirror2. See https://github.com/scniro/react-codemirror2/blob/master/src/index.tsx#L539

Manually setting data.canceled = false appears to resolve the issue.

schanzer commented 5 years ago

This now appears to be fixed.

schanzer commented 5 years ago

Confirmed fixed. Closing.