ractivejs / ractive

Next-generation DOM manipulation
http://ractive.js.org
MIT License
5.94k stars 396 forks source link

noIntro should apply to child components #1853

Closed Rich-Harris closed 7 years ago

Rich-Harris commented 9 years ago

Demo: http://jsfiddle.net/rich_harris/rhdaezxm/

martypdx commented 9 years ago

Why not set option on that component? http://jsfiddle.net/rhdaezxm/3/

Madgvox commented 9 years ago

Yeah, I second @martypdx. There could be situations where you would want to preserve child intros.

EDIT: That said, I guess you could make it opt-out by specifying noIntro: false on child components. I guess it depends on which behavior makes more sense as the default.

martypdx commented 9 years ago

I think, lize lazy and twoway, it would be helpful to have element-level attributes. Though the name might need to change because <div noIntro='true' intro='fade'>. is a head scratcher. While we're at it, I've needed a noOutro as well...

Madgvox commented 9 years ago

It seems like this should be a larger discussion than just intros. It would be nice to address the nested transitions issue from #1397 as well.

martypdx commented 9 years ago

Wanted to see if you could dynamicatlly control noIntro from the parent (you can), so leaving this link here.

Rich-Harris commented 9 years ago

@Madgvox is right, there is a need for a larger discussion about transitions. Their main virtue at the moment is their simplicity (and the fact that they're not restricted to CSS transitions), but there is a cost associated with that in terms of expressiveness. Handling nested transitions is one part of that discussion; so are...

...and probably some other stuff I've forgotten.

Just for clarity on the noIntro thing, my current use case is that I'm rendering pages on the server, then re-rendering them on the client once JS kicks in (no progressive enhancement yet! hopefully not before too long...), and in that situation I want the noIntro setting of the top level new Ractive() call to be respected by the entire tree, otherwise it's visually rather jarring - I don't want components to be able to make their own decisions about whether to use transitions or not. As usual though, this conversation shows there's all sorts of nuance... man, good APIs are hard.

evs-chris commented 9 years ago

I have no real useful input on the subject at hand, but I'd like to throw in that it'd be pretty nice to be able to trigger animations at arbitrary points not necessarily related to transitioning. To my severe inexperience, it feels like that, transition tracking, and flexibility for type of transition are closely related.

This is probably a stupid question, but would it make sense to pull transitions out into a separate project and replace them with some (very carefully crafted) animation hooks?

Rich-Harris commented 9 years ago

but would it make sense to pull transitions out into a separate project and replace them with some (very carefully crafted) animation hooks?

I suspect it would, in an ideal world. I've resisted that so far because of the complexity around controlling when elements can be removed from the DOM following an outro, which is something I believe most libraries/frameworks struggle with. If there's a good way to offload some of this tricky (and opinionated) stuff to an external package, then :+1:

The one thing to be wary of, I think, is making everything so modular that plugins have their own plugins, since that's confusing. I'm a fan of 'batteries included'. But in any case, figuring out what those hooks would look like would make for better internal APIs.

martypdx commented 9 years ago

I think if ractive supplied the template hooks and api - and people could easily plug in their favorite animation library, that would be ideal.

in that situation I want the noIntro setting of the top level new Ractive() call to be respected by the entire tree, otherwise it's visually rather jarring - I don't want components to be able to make their own decisions about whether to use transitions or not

Ractive.prototype.noIntro = true;

Seems like a nice API to me. ;)

Rich-Harris commented 9 years ago

Almost... it doesn't quite capture the same intent though! http://jsfiddle.net/rich_harris/ekq2ufma/

martypdx commented 9 years ago

Ah, what you want is this: http://jsfiddle.net/ekq2ufma/3/

Which shows two things:

martypdx commented 9 years ago

Actually, it looks like along the way in cleaning things up for 0.7 we lost the ability to use functions for basic options. I think for a case like this, it would be handy to write:

    noIntro: function(){
        return !this.parent.rendered;
    }
Rich-Harris commented 9 years ago

Oops, that's almost certainly my bad. Something something test suite something...

That would work, certainly, but it still feels like kind of a hack. I guess the thing is I'm expecting noIntro to apply to the whole tree when used as a new Ractive(...) option, rather than just the top-level instance. It's one of those rare cases where something that makes sense as an instantiation option doesn't really make sense as a Ractive.extend(...) option, or at least means something different.

I suppose what noIntro really means (in my head anyway) is something that sidesteps talk of trees and instances altogether:

// I'm imagining this...
var ractive = new Ractive({..., noIntro: true });

// ...to be equivalent to this:
var enabled = Ractive.transitionsEnabled; // global flag. not actually a thing
Ractive.transitionsEnabled = false; 

var ractive = new Ractive(...);

Ractive.transitionsEnabled = enabled;

Ractive.prototype.transitionsEnabled is close (http://jsfiddle.net/rich_harris/cqdj9bpm/), but it still allows components to override the setting (http://jsfiddle.net/rich_harris/gr709Lq4/) which is exactly what needs to be avoided on first render in this case.

Whoever first said 'the devil is in the details' was pretty smart.

martypdx commented 9 years ago

Oops, that's almost certainly my bad. Something something test suite something...

I just went back through commits and it's been missing awhile. Easy fix, plus a test :)

I suppose what noIntro really means (in my head anyway) is something that sidesteps talk of trees and instances altogether

When I've been using it I've been going the other way, wanting element specific directives because it's not granular enough, and I end up bailing out of ractive intro/outro and use class add/remove hacks with css animations.

Rich-Harris commented 9 years ago

Easy fix, plus a test :)

Having said that do we need it? No-one missed it while it was away! Maybe it's better to leave that stuff in the hands of onconstruct - that way we'll never run into e.g. problems where someone innocently defines a method with the same name as an option they didn't know about, and gets confused about why the method executes on initialisation.

I end up bailing out of ractive intro/outro and use class add/remove hacks with css animations.

That's a sure sign that the whole system needs a bit of a rethink. Not something I desperately wanted us to get bogged down in for 0.8... but I suppose that's how it goes.

martypdx commented 9 years ago

Having said that do we need it? No-one missed it while it was away! Maybe it's better to leave that stuff in the hands of onconstruct

Fair enough, though it would be onconfig in this case.

That's a sure sign that the whole system needs a bit of a rethink. Not something I desperately wanted us to get bogged down in for 0.8... but I suppose that's how it goes.

Well let's keep in mind how the other half lives. I showed someone the basic intro/outro support the other day and their jaw dropped with how simple it is. I think 0.9 for transition/animations would be fine.

martypdx commented 9 years ago

hat way we'll never run into e.g. problems where someone innocently defines a method with the same name as an option they didn't know about, and gets confused about why the method executes on initialisation.

Should we test for that? none of the standard options are function. If they did do a function it would be truthy and could cause unexpected results

Rich-Harris commented 9 years ago

Should we test for that?

Yeah, maybe. I'm basically in favour of doing whatever we can to guide people away from that sort of accident, as long as it doesn't unduly impact performance.

I think 0.9 for transition/animations would be fine.

Makes sense. Will give this the appropriate milestone and we can come back to it.