search

google / incremental-dom

An in-place DOM diffing library
http://google.github.io/incremental-dom/
Apache License 2.0
3.54k stars 180 forks source link

contenteditable elements #293

Open jezell opened 8 years ago

jezell commented 8 years ago

How should these be handled. incremental-dom seems to just overwrite contenteditable node's content every time a render happens and it's not apparent from the docs how this type of situation should be handled. If I could disable the diff on the element contents that would do the trick, but not sure that's possible or if there is a better way.

sparhami commented 8 years ago

For now, you can do:

IncrementalDom.elementOpen('div', null, null, 'contenteditable', true);
  IncrementalDom.skip();
IncrementalDom.elementClose('div');

Not quite sure what the right thing to do is here. I think it depends on whether you want to update some application state to match the state of the contenteditable or if you want to just let it do it's own thing.

jezell commented 8 years ago

Awesome, this does the trick. Thanks! I think saving the state could be challenging for contenteditable since you don't really want to have dom mutation listeners trying to sync the subtree back. Is skip not being in the docs an intentional omission?

jezell commented 8 years ago

One note, setting key of parent element seems to be required for this to work properly in practice. Diffs break down with sequential skip elements otherwise and you end up with wrong node being created.

jezell commented 8 years ago

Maybe a console.warn with if you have multiple skip elements in a row (with same tag name?) should suggest adding key?

sparhami commented 8 years ago

Awesome, this does the trick. Thanks! I think saving the state could be challenging for contenteditable since you don't really want to have dom mutation listeners trying to sync the subtree back. Is skip not being in the docs an intentional omission?

One note, setting key of parent element seems to be required for this to work properly in practice. Diffs break down with sequential skip elements otherwise and you end up with wrong node being created.

This is basically the reason it hasn't been documented yet. While it does exactly what you tell it to do, it isn't quite what you expect.

Maybe a console.warn with if you have multiple skip elements in a row (with same tag name?) should suggest adding key?

It can also be confusing if you have an element with a skip next to a non-skipped one, where one of those is in a conditional branch. The key suggestion is a good one, but there are some use cases where you wouldn't need a key and we don't want to send confusing messaging. For example, if you always have a custom tag name and it always uses skip, then there really isn't a problem. This technique could be used by a library that builds on top of Incremental DOM, so warning the consumer of the higher level library would be confusing, since they don't know what is going on.

The library would not be able to safely assign a key. The typeId and open / close functions will be able to better support this use case. When those are supported, I think adding a warning for this + documenting the feature makes sense.