HubSpot / draft-convert

Extensibly serialize & deserialize Draft.js ContentState with HTML.
Apache License 2.0
483 stars 94 forks source link

convertFromHTML img tags don't add content to editor #48

Open jisaacks opened 7 years ago

jisaacks commented 7 years ago

In my editor, when you add an image. It adds placeholder text like {{image}} and applies an immutable entity to that with the url set to the image URL.

I am converting to HTML like so:

entityToHTML(entity, originalText) {
    if (entity.type === 'IMG') {
      return <span><img src={entity.data.url} /></span>
    }
    return originalText
  }

Converting to HTML works great.

However when I convert back FROM html it is not adding the original text back:

htmlToEntity(nodeName, node) {
    if (nodeName === 'img') {
      return Entity.create(
        'IMG',
        'IMMUTABLE',
        { url: node.src }
      )
    }
  }

It seems like htmlToEntity gets back the entityID and then applies to the the content of the node that produced it. However bc the img is a void element and has not content, no content gets added back to the editor to apply the entity to.

Maybe it might make sense that if htmlToEntity returns an object with key and originalText it uses the key as the entity key and the originalText as the content instead of the content of the node.

benbriggs commented 7 years ago

@jisaacks is your entity within an atomic block? if so this case should be covering your issue.

fdidron commented 7 years ago

@benbriggs I'm facing the same issue, but I'm not sure I understand your answer can you elaborate ?

Thanks !

benbriggs commented 7 years ago

Sorry, subsequent changes to the file moved the highlighted area, here's a permanent link to what I meant to highlight.

When using atomic blocks and an entity is within the block convertFromHTML will add a character to the text content of the block so that there's a character on which to attach the entity. Using the atomic block type gets you other nice selection interactions for things like images so I recommend using it in general.

Additionally in versions of Draft since its initial release block metadata has been added which is a cleaner and more stable way of adding things like images to your content without relying on adding a character that shouldn't really exist. draft-convert supports returning a { type, data } object from htmlToBlock to parse out metadata from HTML into content state.

benbriggs commented 7 years ago

As a side note I hope to in the somewhat near future write more documentation about specific patterns like dealing with atomic blocks as well as some more in-depth best practices.

fdidron commented 7 years ago

@benbriggs thanks for the update.

I tried using htmlToBlock but to no avail.

const contentState = convertFromHTML({
      htmlToBlock: (nodeName, node) => {
        if (nodeName === 'img') {
          return {
            type: 'image',
            data: { src: node.src, caption: node.alt }
          };
        }
      }
    })(html);

I tested with the following html payload

Block of type 'image' is created. Success :) !

<img src="https://cdn.pixabay.com/photo/2016/03/28/12/35/cat-1285634_960_720.png" alt="Some kind of caption" />

Block of type 'undefined' is created. Fail :( !

<div><img src="https://cdn.pixabay.com/photo/2016/03/28/12/35/cat-1285634_960_720.png" alt="Some kind of caption" /></div>
nghuuphuoc commented 7 years ago

Is there any workaround for this issue? I tried both htmlToEntity and htmlToBlock without success.

benbriggs commented 6 years ago

i'd recommend instead of using different block types, instead using an atomic block type for anything that isn't editable via traditional typing (besides when deleting the block) and using something like an atomicType property in the block metadata to determine what type of block it is (image, video, etc).

both draft-js and draft-convert have special casing around the atomic block type to better handle interacting with it and converting it appropriately. in the case of draft-convert that special case covers that exact difference with the wrapping <div>, so i'd recommend leveraging that if possible.