AmpersandJS / ampersand-view-switcher

A utility for swapping out views inside a container element.
MIT License
23 stars 13 forks source link

Conform view-switcher to view conventions so that it can be used as subview. #37

Closed RickButler closed 7 years ago

RickButler commented 8 years ago

Added render method and _rendered attribute so that render is only called once, otherwise subsequent calls to render would appended to the view multiple times. Added an autoRender attribute to config that is set to true by default, this is opposite of ampersand-view but retains backwards compatibility.

I tried to make as few changes as possible to implement this.

RickButler commented 8 years ago

@kamilogorek, I will take a look at fixing those. I'm also going to make some additional changes, so that it passes tests in https://github.com/AmpersandJS/ampersand-view-conventions

RickButler commented 7 years ago

@kamilogorek I finally got around to making changes (a year later lol). I still want to make a real example before you merge this, I made the commit to get feedback before I get too far along.

I had to change the constructor, ampersand-view-conventions requires the el be passed in options. So this introduces breaking changes and would be a major version bump.

RickButler commented 7 years ago

I swapped out view switcher in a basic ampersand-cli generated project and it looks good: https://github.com/RickButler/view-switcher-example

I just need to make your suggested changes.

Also, I may start working on adding extend method after this. That ties into my making this work cleaner as a subview. I want to be able to extend view-switcher with some base behavior, and then invoke that instead of having to redefine config values each time.

RickButler commented 7 years ago

@kamilogorek and @dhritzkiv, I made the your requested changes. Also there is a link in my previous comment to a sample project pointing to my git rep for these changes to test with. It's looks ready to me.

dhritzkiv commented 7 years ago

All looks good to me. I'll let @kamilogorek approve the changes and merge the PR

kamilogorek commented 7 years ago

@dhritzkiv I believe I still don't have NPM access after all that time 😅 Could you bump a major version and release it in your spare time?

kamilogorek commented 7 years ago

And thanks @RickButler!