mweiss / elm-rte-toolkit Goto Github PK

Platform: iOS12 Safari

If you select the editor by tapping text, the caret sometimes doesn't appear on iOS. However, the keyboard is still available and text can be updated. I thought this might be an issue with contenteditable in general on iOS, but other contenteditable editors, like DraftJS and ProseMirror, do not have this issue.

The expected behavior is that the cursor is visible.

In the Definition of link, the htmlToNode function seems to confuse href with src in here.
This has the consequence that pasting (or parsing html with a link) won't work because the link has an href not an src.

Am I missing something?

It would be nice to be able to have a draggable property on ElementDefinition which allowed you to drag inline and block leaves around in the document.

It would also be nice to support drag+drop API in general, and be able to drop arbitrary HTML into the editor and have it parsed similar to how we handle pasted html.

in the demo on https://mweiss.github.io/elm-rte-toolkit/, I noticed that if you:

1. select some text
2. click outside the editor (inc. the toolbar)
3. click a formatting button in the toolbar

then the range you selected will be restored and the format will be applied. I expected it to stay deselected.

edit: in case it matters, I'm using Safari 13.0.5

When you have multiple rte's at the same time the second rte doesn't update the selection-state as it should. Only the first one on the page is updated correctly.

Cutting and copying work fine, but pasting into the editor doesn't seem to have any effect in Firefox for OSX.

Currently, the toolkit does not scale up for larger documents. On my MacBook, I started to notice lag for some commands on a documents with around 2500 nodes. It would be nice to identify the bottlenecks, add benchmarking, and have a story for how to scale up an editor.

IME is broken in Chrome and Safari, although it seems to work mostly in Firefox. This needs to work in all browsers.

I am working with a codebase that is using Elm 0.19.0. I tried compiling this for 0.19.0 and it works just fine, so I don't think there is any reason to restrict the package range to >= 0.19.1?

Right now, there's no real effective way for a developer to react efficiently to state changes in the editor. It would be nice to have a way of adding extra effects after a state change.

In other input fields, when I cmd+backspace (on mac) it will delete the entire line for me. The RTE toolkit does not.

1. Select google IME ひらがな.

2. Type something like こんにちは and click the result. The entire word will be selected

3. Press enter, the selection persists over the entire selection. The expected behavior is that the cursor will be at the end of the word. This appears to be due to the editor detecting the selection state change during composition and saving it instead of ignoring it.

Note: this happens mostly in chrome

See this Ellie. This issue is also observable in code blocks in the demo. I've injected a bunch of logging statements into the JS to try to understand what's going on, but haven't really made any headway. Any help would be much appreciated!

Environment: Win 10 (MS Edge) VM

On my windows 10 legacy VM, I had trouble sometimes selecting a range within the editor. I think this may be due to calling the selection API too many times. I'm not sure what the cause or fix is though.

I get above runtime exception from elmEditor.js.
The complaint seems to be on line 364 on .call(this):

var _this2 = _possibleConstructorReturn(this, (ElmEditor.__proto__ || Object.getPrototypeOf(ElmEditor)).call(this));

I am just trying out the package and I am not doing anything more than trying to render a paragraph with the below Editor.Config. But the render seems to work just fine even given the above error.

        { decorations = Decorations.emptyDecorations
        , spec = Definitions.markdown
        , commandMap = Command.emptyCommandMap
        , toMsg = EditorMsg
        }

Steps to reproduce:

• Go to https://mweiss.github.io/elm-rte-toolkit/#/examples/basic using Chrome
• Turn the text that is present into a link
• Place the caret after the link
• Typing at that position is impossible

Cause: Chrome adds the text after the link, which is ignored by the editor. See https://bugs.chromium.org/p/chromium/issues/detail?id=1115085#c3 for details

Possible fix:

• Rendering links without the href does fix the issue (but will also stop them being rendered as links). Unfortunately, removing the href cannot be done though decorations, so it will have to be done though the linkToHtmlNode which will then affect the Html.toHtml function.
• Add a new command for typing after a link. However, it is hard to determine which nodes render to an a, since the only place the a element is used is in the linkToHtmlNode function.
• Modify the JS that handles the mutation list to move the changes into the link. This can be complicated if the link contains nested markup. Going down that route results is a monstrosity like this:

   characterDataMutations(mutationsList) {
      if (!mutationsList) {
          return null;
      }
  
      let mutations = [], allCharacterData = true, self = this;
      mutationsList.forEach(function (mutation, i) {
          if (mutation.type === "childList"
              && mutation.addedNodes.length === 1 // Added a single text node
              && mutation.addedNodes[0].nodeType === Node.TEXT_NODE
              && mutation.previousSibling // Previous node is a link
              && mutation.previousSibling.nodeType === Node.ELEMENT_NODE
              && mutation.previousSibling.nodeName === "A"
          ) {
              var n = mutation.previousSibling;
              while (n.nodeType !== Node.TEXT_NODE) {
                  n = n.childNodes[n.childNodes.length - 1];
              }
              n.nodeValue += mutation.addedNodes[0].nodeValue;
              mutationsList[i + 1] = {target: n, type: "characterData"};
              mutation.addedNodes[0].remove();
              return;
          }
          if (mutation.type !== "characterData") {
              allCharacterData = false;
              return;
          }
          mutations.push({
              path: getSelectionPath(mutation.target, self, 0),
              text: mutation.target.nodeValue
          });
      });
      return allCharacterData ? mutations : null;
  }

This problem does not occur on Firefox.

a new space at the end of a line in firefox returns the old model and doesn't update

If there's an empty paragraph with no spaces and no zero width spaces, this will fail to be parsed by htmlToElementArray. It will instead hit the "Invalid node type for empty fragment result array" error branch.

emptyParagraph =
    "<p></p>"


test "Tests that an empty paragraph works as expected" <|
    \_ ->  case htmlToElementArray markdown emptyParagraph of
          Ok _ -> Expect.pass
          Err _ -> Expect.fail "Failed to parse :("

The content type here when parsing is a TextBlockNodeType with an empty children array, instead of a BlockLeafNodeType as expect for that case

Here's my current working branch but I'm not confident the fix I landed on is actually what should be changed here (UPDATE: yeah already found issues with it). I would expect the empty paragraph to have a node tree that looks like:

block
    (element paragraph [])
    (inlineChildren (Array.fromList [])

but instead this yields:

block
    (element paragraph [])
    Leaf

and I'm wondering if I really need to just get it to output that former structure and have everything else be ok with it- or if I'm overthinking it and the "paragraph without inline children" is an acceptable case.

The following cases DO work though:
elements with inline children where the only child is a zero width space
"<p>\u{200B}</p>"
or a single space
"<p> </p>"

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.

Environment: Mac OSX, Firefox 74

To reproduce:

1. Select Japanese IME, Hiragana
2. Type something in the editor so that composition line shows underneath the text, for example こんにちは.
3. Without pressing enter, backspace the text to the end. the first character (こ) is not deleted. I think this has to do with event ordering, namely the compositionend seems to be firing before the mutation observer has time to update the buffered editor state.

Note: I can't get this to reproduce in Chrome for Mac.

These 2 functions are documented as such:

_joinBackward_
If the selection is collapsed and at the start of a text block, **tries to join the current block the previous one**.

_joinForward_
If the selection is collapsed and at the end of a text block, **tries to join the current block the next one**.

(emphases are mine)

But in reality they both try to join either backward or forward with the first text block it finds, not the previous block (whatever it might be). That is to say that if the previous/next block is not text it keeps searching until it can join with one instead of failing.

This has unexpected consequences. One such example would be from the Commands.defaultCommandMap where we have this sequence:

    , ( "joinBackward", transform joinBackward )
    , ( "selectBackward", transform selectBackward )

So if one removes a paragraph which is right after a selectable leaf block, for example, the cursor jumps over the selectable leaf block and lands at the end of the previous paragraph (if there is one). This is because joinBackward succeeds when one would expect it to fail and for selectBackward to succeed instead.

I presume the error is because in both commands we try to findPreviousTextBlock and findNextTextBlock which use findBackwardFromExclusive and findForwardFromExclusive respectively which search until the condition (is text block) is met.

Thank you so much for this package, it is brilliant! One of the best experiences (if not the best) I had building a WYSIWYG editor.

For example:
Editor is using Definitions.markdown and user pastes text from clipboard, that contains single <span> and that results in lose formatting since <span> is not part of the spec.

Can we just ignore unknown tags?

When pasting, loading HTML etc.

See line 140.

See: https://discourse.elm-lang.org/t/a-toolkit-to-create-rich-text-editors-in-elm/5464/5?u=mweiss

I think the most general way of doing this would be to have custom validation expressions in RichText.Model.ElementDefinition that would take in a node and return a list of validation errors if there were any.

validateNode : Node -> List String

But perhaps this is overkill, and creating something more specialized like content expressions would be easier for people to use, or more expressive for other things like defaulting behavior. In that case, perhaps having a content expression type that users implement for each element definition is the way to go.

A really useful feature to have would be input rules similar to ProseMirror:
https://prosemirror.net/docs/ref/#inputrules

This would need to be designed and implemented to fit the toolkit's architecture.

We should add element definitions and commands for tables. Perhaps we can use prosemirror's table spec as a jumping off point.

In terms of UI, there are quite a few good implementations already out there. https://tiptap.scrumpy.io/tables (which is ProseMirror underneath) is a good implementation in terms of features, especially the resizable column feature.

Currently, if you try to copy an inline of block leaf without selecting it as part of a range selection, it will not update the text/html clipboard data. This is because native browser copy-paste behavior will not copy anything for a caret selection. Perhaps can be fixed several ways: by making a selected block leaf or inline leaf a range selection via focus and anchor offsets as boundary points, having a custom clipboard for selected leaf nodes, or by setting the clipboard data on copy events. However I haven't tried any of these methods out to see if they're viable.