Improve visibility and display toggling about barba

[luruke]

Yo'!
Will add this in the next release!

Thank you

[luruke]

Hi @ZachSaucier ,
Thank you for your observation!

In the transition, when .done is called the oldContainer is removed from the DOM:

done: function() {
    this.oldContainer.parentNode.removeChild(this.oldContainer);
    this.deferred.resolve();
}

I can add visibility: visible in BaseTransition.done, but actually should be the custom Transition responsible to make to hide the old Container and show the new one.
So, will change nothing, right?

In visibility: hidden, instead of display: none you can access to some property like height/width of the elements, and it's super useful;

You may start a slider or do other JS operation when the newContainer is attached to the DOM and the transition is not yet finished (event onEnter in Views or newPageReadyin Barba.Dispatcher).

Also, with visibility: hidden the browser (at least Chrome) starts preloading the images, so, it's quite handy.

Hope it reply your answer, if yes, feel free to close the issue.

[ZachSaucier]

I understand your points and see the value in how it currently runs, but my original points still remain I think 😛

It's not a big deal either way, just what I see as an improvement

[luruke]

So, basically, your idea is to make the .donefunction like this:

done: function() {
    this.oldContainer.parentNode.removeChild(this.oldContainer);
    this.newContainer.style.visibility = 'visible';
    this.deferred.resolve();
}

Am I right?

If so, the default Transition HideShowTransition can be rewritten like:

var HideShowTransition = BaseTransition.extend({
  start: function() {
    this.newContainerLoading.then(this.finish.bind(this));
  },

  finish: function() {
    document.body.scrollTop = 0;
    this.done();
  }
});

[ZachSaucier]

Yes, that is the main idea I propose. It keeps things simpler