search

ded / bonzo

library agnostic, extensible DOM utility
Other
1.32k stars 137 forks source link

When cloning elements, clone events from all child elements #70

Closed benvinegar closed 12 years ago

benvinegar commented 12 years ago

Somehow #65 became closed – here's part 2.

Per @rvagg's suggestion, I converted integration.html into a proper sink test suite. Then I added a test case that demonstrates this bug.

rvagg commented 12 years ago

Nice work on the integration tests! We'll have to start coming up with more tests for the cross-over points within the Jeesh, such as the Qwery->Bonzo dependency for $('<>') element creation.

FYI, the font on the test page goes default serif on systems without Helvetica Neue, such as mine, not that this is a problem but it could be reset at the end of that test with $('body').css('font', '').

fat commented 12 years ago

trying to run the integration tests and i get an error : String.prototype.toString is not generic any ideas?

apparently you can't $(el).addClass(0)...

fat commented 12 years ago

fix that stuff up and everything looks good to me!

benvinegar commented 12 years ago

Made those changes.

rvagg commented 12 years ago

did you commit? still seeing only your commits from last month.

benvinegar commented 12 years ago

I amended the commits.

rvagg commented 12 years ago

oh, that's interesting, I thought pushing amended commits to GitHub didn't work, at least they always have for me, I'll have to play with that.

rvagg commented 12 years ago

.. and, I've just noticed that you've modified bonzo.js rather than src/bonzo.js, you may want to change that or your changes will disappear after a make.

benvinegar commented 12 years ago

Fixed. Also, looks like my test wasn't covering the right scenario 100%.

Aside: you're right that GH pull requests don't really care for amending – won't do that again.

ded commented 12 years ago

this wasn't the first PR that changed bonzo.js and not src/bonzo.js — i really need to document this better in the readme

fat commented 12 years ago

This all looks good - @ded any reason not to merge?

ded commented 12 years ago

i'll pull and run the tests.

ded commented 12 years ago

lol. i just tried git pull https://github.com/benvinegar/bonzo clone-events — and it couldn't find the repo because you named it funzo.... right on. trying again.