marionettejs / backbone.marionette

The Backbone Framework
https://marionettejs.com
Other
7.06k stars 1.26k forks source link

Allowing smooth transition in region when changing layout #320

Closed jniemin closed 10 years ago

jniemin commented 12 years ago

I have a single page app that have multiple pages. The main view consist of header, content and footer regions. My different pages that are inside content region are defined as layouts. To provide smooth transition between different layouts normal region.show() and then override open() is not enough. As in this case the previous layout will be closed before showing the new one. What I created was custom swap() function. Which first prepends layout to background and then I use transition to change old view to new one. This allows smooth transition between views. I can imagine that this is something maybe other could be use. I'll provide my code here, but I know that it is not perfect and for example transition should be defined maybe as a function parameter. Also not sure if this closes current view correctly.

var ContentRegion = Backbone.Marionette.Region.extend({
  el: "#content",
  swap : function(view){
    if(this.currentView === undefined){
      this.show(view);
    }else{
      view.render();
      this.$el.prepend(view.el);
      view.trigger("show");
      var that = this;
      this.currentView.$el.fadeOut(400, function(){
        that.currentView.close();
        that.trigger("show");
        that.currentView = view;
     });
   }
}
mxriverlynn commented 12 years ago

You're on the right track. There are some subtleties and nuances that are hard to catch when building a region that does jquery animations like this, though. The way jQuery does animations uses a timer in the background, which means it allows other code to run in between it's animation frames. This can wreak havoc on your app when you show two views in one region, in rapid succession.

But I've been working on this exact problem with a client for a few weeks now, and last week we finally got what we think is the last bit of this in place. I'll get a copy of the code from him and paste it here. Hopefully that solution will make it in to Marionette, as well.

mxriverlynn commented 12 years ago

Here's the code from the work with the client: https://gist.github.com/3947145

var FadeTransitionRegion = Backbone.Marionette.Region.extend({

  show: function(view){
    this.ensureEl();
    view.render();

    this.close(function() {
      if (this.currentView && this.currentView !== view) { return; }
      this.currentView = view;

      this.open(view, function(){
        if (view.onShow){view.onShow();}
        view.trigger("show");

        if (this.onShow) { this.onShow(view); }
        this.trigger("view:show", view);
      });
    });

  },

  close: function(cb){
    var view = this.currentView;
    delete this.currentView;

    if (!view){
      if (cb){ cb.call(this); }
      return; 
    }

    var that = this;
    view.fadeOut(function(){
      if (view.close) { view.close(); }
      that.trigger("view:closed", view);
      if (cb){ cb.call(that); }
    });

  },

  open: function(view, callback){
    var that = this;
    this.$el.html(view.$el.hide());
    view.fadeIn(function(){
      callback.call(that);
    });
  }

});

Note that in this code, he has added a fadeOut and fadeIn method directly to his views. You can replace view.fadeOut and view.fadeIn with standard jQuery calls, though.

jniemin commented 12 years ago

Nice! I'll try to incorporate that to my code base. From Marionette's perspective probably it is better that it has an separate function for transition change. Or possible to pass transition as parameter to show/open function. I hope this functionality ends up to Marionette as I can imagine that there are other people with same use case

eschwartz commented 12 years ago

@derickbailey - I'm glad to see this is something you're working on. I've been trying to figure out this issue of how to run a transition animation before a view is closed. The code you posted - and the new ViewSwapper component - looks like it deals with a closing animation on an entire region -- my question is what if you want closing animations on individual views within a region.

I've been using something like this:

_.extend(Backbone.Marionette.View.prototype, {
    close: function(callback) {
        callback = (callback && _.isFunction(callback))? callback : function() {};

        if (this.beforeClose) {

            // if beforeClose returns false, wait for beforeClose to resolve before closing
            var dfd = $.Deferred(), close = dfd.resolve, self = this;
            if(this.beforeClose(close) === false) {
                dfd.done(function() {
                    self._closeView();
                    callback.call(self);
                });
                return true;
            }
        }

        // Run close immediately if beforeClose does not return false
        this._closeView();
        callback.call(this);
    },

    _closeView: function() {
        this.remove();

        if (this.onClose) { this.onClose(); }
        this.trigger('close');
        this.unbindAll();
        this.unbind();      
    }
});

Which let's me do something like this:

var MyView = Backbone.Marionette.ItemView.extend({
    //...
    beforeClose: function(resolveClose) {
        this.$el.slideUp(resolveClose);
        return false;
    }
    //...
});

This works well in some situations - for example, if I reset a collection on a CollectionView, I guess a nice closing animation on each item view element. The issue I'm running into today is if I'm swapping out a collection view to be shown in a Region, the region won't wait for all of the item views to close before showing the new view.

I would appreciate your thoughts on this. Thanks!

jsoverson commented 11 years ago

I've had to deal with this a few ways so far but have mostly dealt with them on the view level. I started to implement a Region type like @derickbailey but it didn't feel very clean by the end. I ended up extending the views I needed to implement transitions with and dealing with deferreds at the Region level.

Maybe regions should inherently support promises as return values and, if received, accomodate the deferred close/open. This could be a different region but, since we're throwing away the return value of close() now, we could wedge it in.

The problem with expecting a return value other than the view is that we break the chainable convention that backbone views encourage, but I'm not sure we're losing anything by potentially breaking the chain on close().

Any thoughts, @eschwartz, @derickbailey? Do either of you consistently deal with chainability in your implementing code (return this;)?

feng92f commented 11 years ago

I like this way of transition http://codepen.io/somethingkindawierd/pen/cpiEw

laurentdebricon commented 11 years ago

@feng92f the link to Marionnetejs lib was outdatted in your nice demo : http://codepen.io/anon/pen/nDmgp

paynecodes commented 11 years ago

Any developments on a new View Swapper implementation?

brett-shwom commented 10 years ago

I have a Marionette.ItemView which is being rendered inside of a Marionette.CollectionView. I'd like to add a fade out transition to the ItemView when its model gets removed from the collection it's part of.

Would the following code properly delay the closing of a Marionette.ItemView by 2 seconds (enough time to process an animation)?

MyItemView = Backbone.Marionette.ItemView.extend
 onBeforeClose : ->
    if @_closing
      return
    setTimeout =>
      @_closing=true
      @close()
    , 2000
    false
eschwartz commented 10 years ago

I just so happened to be messing around with view transitions again today.

A couple of things that worked out well for me:

onRender: function() {
 this.$el.hide();

 _.defer(_.bind(this.transitionIn_, this)); 
},

transitionIn_: function() {
 this.$el.slideUp
}

If you're not familiar with UnderscoreJS:

For whatever reason, using defer prevented choppy animation behavior.

And for a transitionOut animation, I would suggest override Backbone's remove method:

remove: function() {
 var parent_remove = _.bind(function() {
  Backbone.View.prototype.remove.call(this);
 }, this);

 // Calls parent's `view` method after animation completes
 this.$el.slideUp(400, parent_remove);
}

This remove method get's called by ItemView#close.

@brett-shwom Using a closing flag may work, and it would prevent stacking of multiple close attempts. My only concern is that you end up guessing on our timeout.

jamesplease commented 10 years ago

Related to #1085 and #1128

jasonLaster commented 10 years ago

Here's an animated region that I worked on the otherday: https://gist.github.com/jasonLaster/9794836

jasonLaster commented 10 years ago

Thanks @eschwartz, @derickbailey, @jniemin, @feng92f for the examples. I've changed this issues label to recipe so that we can consolidate these ideas into a couple recipes when we turn our attention to the cookbook in the near future.

jamesplease commented 10 years ago

I'm also going to add enhancement to this. Right now it's way too difficult to create an animated region. I'd love to see the region code changed in a way that makes this easier, if possible.

In my mind you should be able to rewrite 2 functions (open and close) to add this functionality. The code above you posted above works, @jasonLaster, but it requires so much hacking in my mind to make it work – you're basically completely ignoring all of the code currently in the region.

jptaylor commented 10 years ago

Would love to see official support for region transitions - even if it as simple as adding a "before:close" event and deferring the close method. Resorting to overwriting the close method definitely feels hacky.

jamesplease commented 10 years ago

hey @jptaylor, thanks for the input! I agree that pomises are certainly the most logical way to approach this issue...I'm just worried about the ramifications of making just one Marionette function asynchronous. The next logical conclusion as I see it would be making them all asynchronous. Hmmm...

samccone commented 10 years ago

Been thinking about the solution to this problem for a few days. I think the correct solution is going to be a custom regionManager that people can opt into using via an external dependency

Marionette.animatedRegion

and then will be able to use within their layout instances https://github.com/marionettejs/backbone.marionette/issues/1210

the animated regions then can use syntax like this to add animations

REGION.show(fooView, {slide: 'top'}) REGION.show(fooView, customAnimationMethod(view1, view2, region))

paynecodes commented 10 years ago

I don't have much to add to where this conversation is headed. I like the idea of promises. I don't have a ton of direct experience with region manager, but it seems like it could be quite flexible.

samccone commented 10 years ago

@jpdesigndev no one really does, it is poorly abstracted from layouts ATM so no one uses it.

1210 will fix this and basically make the regionManager a first class citizen within the marionette ecosystem.

paynecodes commented 10 years ago

@samccone It seems that many of us, including myself were thinking the way to go would be to extend region. As I'm not deeply familiar with what region manager really is apart from what the docs say about it, what advantages does this possibility bring over a new regionType?

Note: I'm certainly not questioning the premise. I'm just curious about the benefits of this approach


Giant aside: After listening to this podcast with Ember Core Member, Tom Dale, it seems we've got a ton of work to do in the Transition arena to build the next generation of web apps for mobile and desktop. It seems Ember gleaned some insight by looking at some Cocoa.

Something that triggered my interest from this conversation:

  1. Deferred objects are used to queue (pause/replay/cancel transitions) animations. This could be useful for several purposes. What if a user is filling out a form, they click a button to init a new transition, but we want to interrupt that transition to let the user know they are about to lose data they haven't submitted. The transition object would be paused, and can later be replayed or canceled. Perhaps when a user is clicking through the app faster than the transitionEnd event. Perhaps the user clicks the back button quickly many times. Check out the 20:00 minute mark in the linked podcast if this interests you.
samccone commented 10 years ago

Oh you are right that this will impact the region type as well. Basically going at it from the regionManager down will give us an additional layer of abstraction to play with. I will draw up a diagram of the logic and we can go from there.

jamesplease commented 10 years ago

note @samccone @thejameskyle was prototyping ways of actually removing RegionManager altogether hah, or at least making it more useful.

I think we all agree something needs to be done about RegionManager. We should make that the focus of a weekly meeting sometime. Maybe after v2 would be a good time.

jptaylor commented 10 years ago

@samccone having an external dependancy works, although it is slightly reminiscent of how angular does things with ng-animate. Considering that Marionette is fundamentally already a dependancy of Backbone, it'd be nice if this was integrated into the core package somehow. I also feel that it is becoming increasingly rare that an app won't have some kind of view transition when it moves beyond MVP / prototype, especially when working with mobile. For me personally, the aesthetic benefits are one of the main draws to JS SPA. Respect this is obviously just my opinion :)

paynecodes commented 10 years ago

@jptaylor I agree with you mostly given the premise that this implementation won't be all encompassing. I'd like this to become a rather in-depth implementation. See https://github.com/marionettejs/backbone.marionette/issues/320#issuecomment-40957068 That being the case, for me, I wouldn't mind this being an external dependency so core can remain concise and to the point. Those who don't need transitions could keep a smaller codebase, which is still important when considering debugging, network speeds on 3G/4G, etc. Transitions are a huge part of every SPA's I am interested in building, but keeping in external would most likely benefit Marionette core team while allowing those of us that aren't familiar with a large part of Marionette's internals to contribute. This, too, is simply my opinion.

samccone commented 10 years ago

+1 @jptaylor I agree

these are the two major points that I think really sell splitting it off for me.

jamesplease commented 10 years ago

@jptaylor I also agree. My two concerns are:

  1. Animated regions should not be core
  2. It should be incredibly simple to add animated regions

Right now the problem I see is that the second thing isn't true. I'm not entirely sure how we will go about making it easier for people to override, but it definitely will be!

JanMesaric commented 10 years ago

Would there be any implementation of similar functionality as in backbone pageslider, https://github.com/ccoenraets/PageSlider, could someone describe how could I implement such library with marionette?

paynecodes commented 10 years ago

@Janckk You could take a look at my MarionetteTransition library. You can use that as is. Bare in mind, this implementation isn't really production ready.

JSteunou commented 10 years ago

+1 @jmeas

Do not bother to add animation into core, but please make hooking with region manager easier so we can add transition ourselves.

At this time we did our own like @derickbailey demonstrate above, but we are not so please with this hackish way.

jamesplease commented 10 years ago

So I came up with a way to support animated regions that I'm pretty pleased with.

Check out a live example here.

Code here

Details:

The new region delegates the task of animating to your view. To animate in, simply add an animationIn method on your view. Then, within that method, trigger this.trigger('animationIn'); once the animation is complete.

The same applies for animating out.

I can see something like this landing in core. I'd like to hear your thoughts @jasonLaster @samccone @thejameskyle @ahumphreys87 @jpdesigndev

JSteunou commented 10 years ago

Very nice @jmeas

I just have mixed though about triggering animateIn / Out when done with animation. Could it be easier to make a parent.prototype.animateIn.call(this)?

Today we have a lot of parents triggering events and children implementing method on<Event>, it's kind of bizarre to see the opposite, but it's in the right way to do it though, I guess it's just me having trouble to deal with this way of "bubbling" events.

jamiebuilds commented 10 years ago

I like the solution @jmeas came up with, however I'd like to look at different use cases that we'd want to accommodate and make sure those are solved before we commit to this to avoid backing ourselves into a corner. These are two things I can think of now:

jamesplease commented 10 years ago

@JSteunou – hrm...views don't get a handle on their parent region, which is for the better, I think. I'd rather use events to communicate up the chain, which I think is what we do in other situations, than get a handle of the region in the view.

Today we have a lot of parents triggering events and children implementing method on, it's kind of bizarre to see the opposite

You think? I think of it as being the exact opposite. I see what you mean by parents calling events on their children, like with collectionViews triggering show and such, but that's because the parent knows more than the child in that situation. It's not only controlling its API directly, by calling render and such on it, but it's also more knowledgeable about what's happening to the view than the view itself. That's the reason the parent triggers the methods directly.

I guess it's just me having trouble to deal with this way of "bubbling" events.

I would go so far as to say that this way of bubbling events is best practices. It's all over the place in view-model relationships:

myView.listenTo(myView.model, 'change', myView.onChange);

I think of the region-view relationship as no different.

myRegion.listenToOnce(myRegion.currentView, 'animateIn', myRegion._onAnimateIn);

In both cases you have a temporary parent storing a child object. I like this pattern more than the child sometimes having a reference to a parent object that changes.

jamesplease commented 10 years ago

@thejameskyle those are good considerations! Here are some thoughts on tackling those concerns:

on page load: we could expose animation options region.show. Then it's up to the user to determine when to use them. Possible options:

  1. animateIn: whether or not to respect a view's animateIn methods
  2. animateOut: whether or not to respect a view's animateOut methods
  3. animateFirst: whether or not to animate the first time it shows a region
  4. animateEmpty: whether or not to animate anytime it goes from being empty to full (not view-to-view)
  5. animateTransition: whether or not to animate anytime it goes from view-to-view

I think that set of options, or something like them, would give users complete control over their animated regions to handle the case you mentioned, and all similar cases. But we might not need them all.

transitioning at the same time - the one solution I can think of involves adding a bit more code here.

In the case of animating both we need to do a mix of those two conditions...we want to run the animation but continue executing show synchronously. So something like

if (animateSync) {
  this.currentView.animateOut();
  this._onTransitionOut();
}

then what we can do is move the destruction of the old view to the end of the animations. So instead of it being here

it would go here.

This slight reordering of things might break BC in some pretty unique cases, but I think for the most part we'll be good + still passing the unit tests.

JSteunou commented 10 years ago

@jmeas I was already sold and you achieve to convince me ;)

@thejameskyle good points! With some options it could be easy to handle your second point, in order to let the user decide.

jamiebuilds commented 10 years ago

We should also take time and try to work out different animations. I think that we should have enough hooks to use any animation library, CSS or JS based.

For now, I'd like this to stay a separate library that people can use in their applications if they'd like (I'd like to try this in my own apps), but not part of Marionette itself until we have a rock-solid API.

jamesplease commented 10 years ago

Added to #1976. v3 is the earliest this will land.

jamiebuilds commented 10 years ago

1796 **

marcin-krysiak commented 9 years ago

I've written the following marionette plugin that adds 4 kind of transitions. There can be easily added more transition types.

https://github.com/marcinkrysiak1979/marionette.showAnimated

jasonLaster commented 9 years ago

Thanks @marcinkrysiak1979