search

AmpersandJS / ampersand-select-view

Select field for ampersand-form-views
MIT License
11 stars 26 forks source link

Check for parentNode before invoking removeChild #41

Closed achingbrain closed 9 years ago

achingbrain commented 9 years ago

Brings ampersand-select-view more in line with how ampersand-view does it, otherwise you occasionally get Uncaught TypeError: Cannot read property 'removeChild' of null.

wraithgar commented 9 years ago

So this happens when this.el was never attached to anything, right? Makes sense to me.

Would you be willing to add a test to this to 1) make sure that remove works as expected (removes this.el from a parent) under normal circumstances and 2) doesn't explode if there is no parent)? If not I can get to that a little later.

achingbrain commented 9 years ago

I've added a couple of tests as suggested.

wraithgar commented 9 years ago

+1 looks good thanks for doing this!

achingbrain commented 9 years ago

Hmm, Travis says:

Error: Zuul tried to run tests in saucelabs, however no saucelabs credentials were provided. See the zuul wiki (https://github.com/defunctzombie/zuul/wiki/Cloud-testing) for info on how to setup cloud testing.

Not sure what I can do about that, sorry.

wraithgar commented 9 years ago

Nothing you can do, the saucelabs thing is a known issue w/ the testing for some of these modules. We just have to run tests locally for now.

cdaringe commented 9 years ago

+1, tests pass

cdaringe commented 9 years ago

you guys think we should add the tests to view-conventions additionally/instead?

wraithgar commented 9 years ago

Great idea. May have to make el an actual attached el in those tests though, do proper tests to make sure it got removed or something.

wraithgar commented 9 years ago

released as v3.0.1