Enable remove animation for empty view

AmpersandJS / ampersand-collection-view

Renders a collection with one view per model within an element in a way that cleans up and unbinds all views when removed.

MIT License

12 stars 8 forks source link

Enable remove animation for empty view #46

Closed fyockm closed 7 years ago

fyockm commented 8 years ago

Calling _removeView on the empty view will allow for animation in the transition to collection views.

A couple other simplifications:

_insertViewAtIndex position calculation
move this.views.length === 0 checks into _renderEmptyView because DRY.

dhritzkiv commented 7 years ago

LGTM.

I've also checked this out, tested it, and used it in one of my projects. Everything looks good.

fyockm commented 7 years ago

Doesn't seem like there's anything I can do via the PR to fix CI tests. Something needs to be updated in Zuul/saucelabs.

From Travis: Zuul tried to run tests in saucelabs, however no saucelabs credentials were provided.

cdaringe commented 7 years ago

@wraithgar had worked on this some time ago. it was so long ago that I don't recall the issue! by this point there's surely a way to get all our CI alive again :$

cdaringe commented 7 years ago

fyockm commented 7 years ago

Any arguments against merging, sans proper CI? not sure has access on this repo? @lukekarrys @latentflip

dhritzkiv commented 7 years ago

I'm okay to merge, but I don't have access to npm release

fyockm commented 7 years ago

funny thing- looks like I actually collaborator privileges on this package in npm, but not here. so @dhritzkiv if you merge, I will publish.

dhritzkiv commented 7 years ago

Merged!

fyockm commented 7 years ago

thanks @dhritzkiv, could you also run $ npm version patch on your local machine and push that commit?

I can do it locally, but I don't want github and npm to be out of sync.

dhritzkiv commented 7 years ago

Done

fyockm commented 7 years ago

Published- nice teamwork! ✋