mweiss / elm-rte-toolkit

A toolkit for creating rich text editors in Elm
https://mweiss.github.io/elm-rte-toolkit/
BSD 3-Clause "New" or "Revised" License
149 stars 14 forks source link

Backspace doesn't work on Chrome when inline text node contains initial newline #32

Open matsjoyce opened 3 years ago

matsjoyce commented 3 years ago

If you have a document like (span is just a mark):

[doc]
    [paragraph]
        [hard_break]
        [span] "\nh"

Backspace doesn't work on Chrome. This is because the backspace deletes the last h, and then turns the \n into a <br> which causes https://github.com/mweiss/elm-rte-toolkit/blob/master/js/elmEditor.js#L327 to reject the event and rerender, since the mutationsList includes a childList mutation:

Is there anything wrong with changing characterDataMutations to:

    characterDataMutations(mutationsList) {
        if (!mutationsList) {
            return null;
        }

        let mutations = [];
        for (let mutation of mutationsList) {
            if (mutation.type !== "characterData") {
                continue;
            }
            mutations.push({
                path: getSelectionPath(mutation.target, this, 0),
                text: mutation.target.nodeValue
            });
        }
        return mutations.length === 0 ? null : mutations;
    }

Or does something more specific need to be done? I haven't noticed any issues, but my testing has not been particularly thorough.

mweiss commented 3 years ago

Thanks for reporting this. I think this is a good start, but it would also require logic changes on the Elm side as well. Currently, if characterDataMutations are passed to Elm, the elm logic will skip validation. This could cause the vdom to crash in some cases if the browser removed a node but the validation didn't catch it. Perhaps we could experiment with first applying the character data mutations, then doing the validation if there are other mutations; maybe with 2 different events or with an extra parameter on the current editorchange event.

matsjoyce commented 3 years ago

For the time being, I have fixed this be performing a String.replace "\n" "" on the initial HTML, since newlines aren't currently significant. However, that workaround won't work if code blocks are in use, since newlines inside pre are significant.