Slate: Paste HTML example is broken.

Created on 9 Nov 2017  Â·  10Comments  Â·  Source: ianstormtaylor/slate

Bug

What's the current behavior?

The Paste HTML example doesn't work, and neither does the code in my own project. Pasting a HTML string simply renders that string as-is into the editor.

To reproduce:

What's the expected behavior?

Pasting HTML should convert that HTML into proper rendered markup. Pasting the above should display "Test", which should be correctly wrapped in paragraph tags in the DOM.

bug examples ♥ help

Most helpful comment

@gggdomi to my knowledge, transfer.type is not under slate's control. When your perform copy in other apps, say Notepad, Word, PhotoShop...selected content is loaded into clipboard with its transfer.type. Different upstream marks different types of contents. Copying in Notepad results in 'text' type, in Word and browsers you get 'html', in Finder you get 'file'.

By default slate handles 'html' as plain text, the "Paste HTML" example only demonstrates its capability to convert 'html' type into slate's value with relatively little effort. In the case of plain HTML string, a common case may be copying HTML code from text editor like vim, sublime, etc. Do you expect the HTML code block parsed into rich text?

IMHO you can implement this behavior by parsing HTML string into DOM nodes, putting them into slate-html-serializer, then calling change.insertFragment and it should work. While I'm not sure this behavior is necessary for this common example.

All 10 comments

Pasting in images, either an image from the clipboard or an image URL, into the images example is broken, too. It does nothing. I wonder if the two are related.

It seems that the HTML will render only if the pasted content has been copied from a webpage.
Ex: copy your message from this page and paste it in the example. It should be displayed with format. At least it works on my version of Firefox for OSX.

I suppose it is because of this handler:

  onPaste = (event, change) => {
    const transfer = getEventTransfer(event)
    if (transfer.type != 'html') 
      return  // <- triggered if you just paste '<strong>Text</strong>', 
              // because transfer.type === 'text'
    const { document } = serializer.deserialize(transfer.html)
    change.insertFragment(document)
    return true
  }

With this slightly different version, your use case seems to work as expected:

  onPaste = (event, change) => {
    const transfer = getEventTransfer(event)
    const { document } = serializer.deserialize(transfer.text)
    change.insertFragment(document)
    return true
  }

Not sure what the expected behaviour of the example is exactly. Should it convert both HTML copied from some page and strings containing HTML, or just the former ?

@gggdomi in similar editors like ProseMirror and Draft.js, pasting plain <p>Text</p> is also handled as plain text, so does old editors like TinyMCE. I'm not sure I know the scenario for this case. i.e., why we need to parse string as html iftransfer.type has already been set to text?

@doodlewind I don't suggest converting plain text containing HTML to HTML to be the default behaviour.
However, for the sake of demonstration we may expect this example named Paste HTML to do so. At least, it seems @benoneal did (or I misunderstood the meaning of this issue), and it would have made sense for me too.

I suggested a fix for the use case of @benoneal, although it may be a better practice to set it upstream through transfer.type, I'm not familiar enough with Slate to tell it yet.

But as I said, I don't know if it should be the expected behaviour _for this example_. If it's just "handle plain text containing HTML as plain text", then there is no reason for this issue, right ?

@jonstokes in the images example, it seems that:

  1. b462c2ce195f25416507f33d3c5d79c6339646a0 breaks onDrop and onPaste handlers by replacing the functions with onDropOrPaste, but forgetting to update Editor props (easily fixed)
  2. Once fixed, getEventRange always returns null (whether pasting or dropping), which terminates handler early in:
onDropOrPaste = (event, change, editor) => {
    console.log(event)
    const target = getEventRange(event)
    if (!target) return
    ...
}

Not sure why getEventRange always returns null.

@gggdomi to my knowledge, transfer.type is not under slate's control. When your perform copy in other apps, say Notepad, Word, PhotoShop...selected content is loaded into clipboard with its transfer.type. Different upstream marks different types of contents. Copying in Notepad results in 'text' type, in Word and browsers you get 'html', in Finder you get 'file'.

By default slate handles 'html' as plain text, the "Paste HTML" example only demonstrates its capability to convert 'html' type into slate's value with relatively little effort. In the case of plain HTML string, a common case may be copying HTML code from text editor like vim, sublime, etc. Do you expect the HTML code block parsed into rich text?

IMHO you can implement this behavior by parsing HTML string into DOM nodes, putting them into slate-html-serializer, then calling change.insertFragment and it should work. While I'm not sure this behavior is necessary for this common example.

I think we need to add some notes to this example. I've seen this bug reported at least 3x, including once when I reported it. Every time to my knowledge the example has worked, but users misunderstand what the example means to demonstrate.

I was also confused by this example. I thought that we could paste HTML content (i.e. <p>hello</p>) into the editor and it would automatically convert to the rendered markup. I now realize that it works by copying the rendered content directly into the editor rather than copying over HTML syntax.

Yes its behavior could be clarified. Ideally by modifying the default text of the editor on that example.

If someone wants to create a pull request that would be very appreciated, thanks.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bunterWolf picture bunterWolf  Â·  3Comments

chriserickson picture chriserickson  Â·  3Comments

gorillatron picture gorillatron  Â·  3Comments

chrpeter picture chrpeter  Â·  3Comments

ianstormtaylor picture ianstormtaylor  Â·  3Comments