Closed rvagg closed 12 years ago
here's a better diff of the most interesting changes: https://github.com/rvagg/bonzo/compare/ce4b15c7...5dd72be9
Added assertions to verify that the first use of an inserted element is not a clone and therefore, all subsequent uses are the clones. This is true for fragments and inline elements. Has allowed insert()
to be simplified somewhat.
my pull requests mature with age, leave them long enough and they ripen
replaceWith()
into this and made it behave the same as the other insertion methods, i.e. it can take all the same types and returns the right thing.For @benvinegar because I know this is important to him, I've done some quick testing of jQuery to see what it does against all these tests to see how close to compatible we are (if you take dommanip_insertions.js and tests.html you can replace all the other scripts except sink with jQuery and do an $.create = $
and the tests will run just fine). Here's the only differences, apart from these, the code behaves the same:
append()
, prepend()
, before()
and after()
, the original element(s) end up in the last use rather than the first as my Bonzo code does. e.g. take X0 and do an after()
to A, B & C and you end up with A-X1, B-X2, C-X0, i.e. the last element, C, gets the original, the others get clones. Arguably it doesn't really matter because a clone should be just as good, but in my opinion, my Bonzo code is more consistent as jQuery does the reverse for appendTo()
etc.replaceWith()
in jQuery is much more simplistic. There's no cloning if you want to use the source element(s) multiple times, it'll only end up in the last place! I guess you're only supposed to use it for 1:1 replacements. So, for example, if you have X and do an $('A,B,C').replaceWith(X)
then A and B will be completely removed and only C will be replaced with X. I'm considering filling a bug with jQuery for this just to see what they say about it; seems like an oversight.The only other thing left is cloneNode()
could be fleshed out a bit to clone data as well, plus tests are needed to make sure events are properly cloned in all cases. But alas, I've run out of time on this and need to get back to real work so I'm not going to get to this until it impacts me or I have some finger-twiddling time.
Darn it, I remembered that html()
needed the same treatment! So each case now has an html()
test too so it now does the same as the other insertion methods and works beautifully.
Interestingly, after running these tests against jQuery it looks like it does the full job with html()
and not the partial job that replaceWith()
does, all the tests pass in jQuery except the cloning again is done on the all but the last use of the inserted element.
Added a closes comment for #67 and #69 because this resolves both of those but goes a few steps further. I dunno if multiple closes in one commit comment will work.
alright. it appears to be harvest season. lets have a look
+1 million
boop
So I was being a boss on my Atari 2600, mastering my newly acquired first-edition paddles and I thought: why don't I put all this awesomeness to work fixing those Bonzo insertion issues instead?
Resolves #67, tho I didn't merge any of @benvinegar's code I did make sure his tests pass against my code (the tests kind of duplicate some what my new tests are doing so I didn't bother pulling them in). That pull can be closed when this is merged.
The first batch of commits are simply reorganising the test code. I pulled the JS out of index.html and split it up into multiple files to make it easier to manage. The real changes start at ce4b15c7. Most of the test code is left as-is (descemiolonified), mostly the new stuff is just in dommanip_insertions.js.
prepend()
,before()
,prependTo()
,insertBefore()
) so they are consistent and work as you would expect--nodes remain in their original order after they are prepended.insert()
(appendTo()
,prependTo()
,insertAfter()
,insertBefore()
) were working mostly OK but the others didn't do any cloning so stuff gets lost when you're doing multiple insertions in one call. I pulled out the cloning code frominsert()
and letnormalize()
call it where required.Tested on all browsers that matter. Didn't test on OS X cause my Mac VM is busted at the moment but that's not really an important target these days anyway is it?