marcelklehr / vdom-virtualize

Virtualize a DOM node
MIT License
131 stars 27 forks source link

Better support for text node and added CI for test cases #29

Open bitinn opened 9 years ago

bitinn commented 9 years ago

This PR supersede #21, main change:

  1. Added phantomjs to provide ci test
  2. Fixed previously failed test cases (basically to get it as closed to html-to-vdom as possible)
  3. like #21, npm run dist build command and .gitignore file.
  4. Nice badges (you need to enabled travis-ci integration)

You should be able to open the test/index.html and run test directly, or use npm test to run tests as phantomjs.

Things still missing:

  1. key support.
  2. if input is var html = ' <div></div>'; what's the expected output? (html-to-vdom throw errors).
marcelklehr commented 9 years ago

Dataset has been removed, since data-* attribs are now supported through the attributes property (see https://github.com/marcelklehr/vdom-virtualize/commit/7c5846b75d30f37a15d24c268f86964c93266b68 and https://github.com/marcelklehr/vdom-virtualize/commit/e7dddc4fc039a702677946786936b51c0e442f70)

Other than that this pr looks good!

bitinn commented 9 years ago

I added it back because html-to-vdom has tests on them, especially for properties.dataset.foobar, see these tests for example:

https://github.com/marcelklehr/vdom-virtualize/pull/29/files#diff-910eb6f57886ca16c136101fb1699231R141

And from my research on this topic (I actually did read the commit and issue you posted), it appears dataset support isn't removed:

https://github.com/Matt-Esch/virtual-dom/blob/88534bbd34c5befbd1689fc71e2373c9145a9807/docs/vnode.md#custom-attributes-data-

Could you take a look as well @TimBeyer ?

bitinn commented 9 years ago

PS: I would like to find a test on dataset in virtual-dom repo, but there doesn't seem to be one, they only test for the patched real dom, so I am not sure if properties.dataset.foobar is valid.

marcelklehr commented 9 years ago

dataset doesn't work in all browsers (i.e. IE8 has problems), so that's why i removed it in favor of adding data-* attributes.

bitinn commented 9 years ago

On one hand I don't have a problem with it because I use attributes, on the other it means we should change test cases (and be slightly incompatible with html-to-vdom, not a big deal).

In the latter case, could you check what else you would like to remove from or add to test cases?

https://github.com/bitinn/vdom-virtualize/blob/ci-test-cases/test/index.js

PS: I would like to remove that dataset special case from code as well, so no more confusion on this topic.

marcelklehr commented 9 years ago

SVG should be tested, as well as the ie8 issues, namely style attribute(already done), data attributes, and img tags, as well as form imputs.

bitinn commented 9 years ago

Sorry for not following up earlier, after spending some time on this I realize the tests are more difficult than I have originally thought, so I ended up writing my own test suite and implementation. Hopefully it's useful to vdom-virtualize as well:

https://github.com/bitinn/vdom-parser

I will keep this branch open for now.