Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev-branch: Named yields do not unload #174

Closed
Strosch opened this issue Oct 3, 2013 · 13 comments
Closed

dev-branch: Named yields do not unload #174

Strosch opened this issue Oct 3, 2013 · 13 comments
Assignees
Milestone

Comments

@Strosch
Copy link

Strosch commented Oct 3, 2013

Hi guys!

Just a side note: Thanks to the help of Chris I managed to implement named yields into my loading template (in the dev-branch): #138

My current problem with the dev branch version is something different though: Named yields do not unload anymore.

Please see my current version here: http://questfultest.meteor.com/
When you surf directly onto this url, everything is fine (no green header navbar). If you then click "Get started" and go into a event (for example: http://questfultest.meteor.com/Eiaiv) and then click the back button, the top bar with the tabs stays, although it shouldn't! If you click back again to get to the landing page, both the navbar and the tabbar stay (which is obviously also wrong!).

Is there some major difference in the devbranch where I have to tell some named yields to "unload" or what am I doing wrong?!

cheers,
P

@Strosch
Copy link
Author

Strosch commented Oct 3, 2013

Here's the code I am using for this particular route (error also occurs when I switch to another route and then go back to this one or another one):

       this.route('groupsPage', {
            path: '/:ehash',
            action: function () {
                var onWait = function() {
                    this.render('loading');
                    this.render('groupsHeader', {to: 'header'});
                };
                var onReady = function() {
                    this.render();
                    this.render('groupsHeader', {to: 'header'});
                    this.render('groupsTabs', {to: 'tabs'});
                };
                var subs = [ Meteor.subscribe('events'), Meteor.subscribe('groups', this.params.ehash) ];
                this.wait(subs, onReady, onWait);
            },
            data: function() { return [ singleevent = Events.find({ehash: this.params.ehash}).fetch()[0], foo = 123 ]; },
            before: function() { Session.set('ehash', this.params.ehash) }
        });

As of said, the groupsHeader and the groupsTabs stay loaded even when I switch for example to the landing page which should neither have the header navbar nor the tabs.
The code for the landing page route looks like this:

this.route('landingPage', {path: '/home'});

@cmather
Copy link
Contributor

cmather commented Oct 4, 2013

I still need to add a clear method so you can clear previous yield regions. You can hack it in there now on the dev branch using Router.setTemplate('region', '') but it should be a controller method too.

We were trying to be smarter about how we clear regions automatically if you don't render into it again. But since each region responds reactively it's not clear when we should do that magic. I haven't had time quite yet to circle back and think about it more.

@Strosch
Copy link
Author

Strosch commented Oct 4, 2013

Hmm I am not sure if I quite understand .... I am not a pro or anything but why don't you just empty all named yields on route change? I can't see a reason for keeping the loaded templates in there since the named yields will be filled with the new templates of the new route anyway?

Anyways how exactly do I fix this? I tried to do Router.setTemplate('groupsTabs', '') but instead of unloading the tabs it just switches to another route and the tabs stay in their yield. I am a bit confused. Is there a way I could use the bit of code of the stable branch which is responsible for unloading? Because there I didn't have this problem, although I need the dev branch for having my named yields in the loadingTemplate (see issue #138).

@cmather
Copy link
Contributor

cmather commented Oct 4, 2013

Ooops sorry I got the setTemplate method signature confused. We need to add a clearYield method to Router, and then to RouteController in client. We might also have a clearAllYields. If you have time to take a stab at it on the dev branch go for it. I'll get to it soon.

@tmeasday
Copy link
Contributor

tmeasday commented Oct 5, 2013

@Questful The underlying reason is that people often don't want the template to change and although, sure you can render the same template again, it leads to flickering, and other re-rendering side effects.

Hopefully this problem will go away when the UI branch of Meteor comes out, but until then, we have this (IMO incorrect) behaviour of not clearing templates.

In the meantime, if Chris' technique doesn't work, perhaps you can set the yieldTemplate to a empty template as a workaround?

@Strosch
Copy link
Author

Strosch commented Oct 5, 2013

Thanks Tom for clarifying. That makes total sense. But why not have another option for each named yield (besides the "to:") such as "autounload" or sth which then always unloads this named yield on route change?! That way you could achieve both things as the coder can decide which named yields should stay loaded (like now) and which should always unload (like before in the stable branch). Is sth like this achieveable?

About the workaround: makes sense! Will try that! :)

@cmather
Copy link
Contributor

cmather commented Oct 6, 2013

That may be a good idea. We will have a solution to this before releasing, as it's currently driving me nuts on the EventMind site :-).

@Strosch
Copy link
Author

Strosch commented Oct 7, 2013

Haha alright perfect! Sounds good! :) Just as a side note: Loading empty templates into the named yields works as a workaround!
Just one small thing (not sure if it's a feature or a bug haha - but it doesn't matter anyway since this is only a workaround): I can't load the same template into two different named yields! Is this intended behaviour?
I tried loading the same empty template (<template name="empty"></template>) into my "header" and my "tabs" yield but didn't work. I had to make two empty templates with different names to achieve the intended unloading! No big deal, just wanted to mention it! :)

@cmather
Copy link
Contributor

cmather commented Oct 19, 2013

Ugh, as I go through the issues, this one is always tricky and I get a sinking feeling :-). I ended up solving the problem by making sure all yield regions always had templates. But obviously this is annoying and should be unnecessary. I'm thinking we could put a default after hook that does the clearing. The original issue is that the entire run is reactive. So you might run the first time and you don't call render at all. Then you run again and you call render. So when is the right time to do the clearing for unused regions? Maybe it's in an after hook. Thinking....

@ghost ghost assigned cmather Oct 19, 2013
@tmeasday
Copy link
Contributor

An after hook would make sense. Then if we named the hooks and could disable them, you achieve the (IMO broken) current behaviour if you wanted it.

@cmather
Copy link
Contributor

cmather commented Oct 19, 2013

I think there's a few things needed here which I'll work on today:

  1. A clearYields method that resets all yield regions.
  2. A clearYield (maybe same method as above) that clears a specific yield.
  3. When you call render on a controller instance it tracks which templates you called render on
  4. A clearUnusedYields on Router that takes a list of yields you've used and clears ones that aren't used
  5. An after hook that calls the above method

EDIT: Not a fan yet of naming hooks and enabling/disabling. I feel like the use case isn't strong enough for that yet. I don't have a good reference point for something else that works this way so my feeling is if you don't want a hook just don't add it (rather than trying to configure it after the fact). With 'out of box' hooks we add, we could provide an option not to add them, or always require the user to add the ones they want manually. Might necessitate allowing them to append to the end of the list or the beginning.

@cmather
Copy link
Contributor

cmather commented Oct 21, 2013

Implemented in an after hook. Still testing.

@cmather cmather closed this as completed Oct 21, 2013
@Strosch
Copy link
Author

Strosch commented Nov 18, 2013

Hi guys!

So I finally found some time again to work on Questful! :)
I just upgraded to the latest dev branch and my old solution for this problem here seems not to work any more.

Any "normal route" still works perfectly fine!
Example:

        this.route('Pricing', {
            template: 'pricingPage',
            path: '/pricing',
            waitOn: function() { return Meteor.subscribe('analytics'); },
            yieldTemplates: {
                'Header': {to: 'header'},
                'emptyBox': {to: 'eventbox'},
                'Footer': {to: 'footer'}
            },
            data: function() { return [ singleevent = null ] }
        });

The ones where I want to have the header displayed during loading use a custom action like this:

        this.route('Event', {
            template: 'eventHomePage',
            path: '/:ehash',
            action: function () {
                var onWait = function() {
                    this.render('loading');
                    this.render('Header', {to: 'header'});
                };
                var onReady = function() {
                    this.render();
                    this.render('Header', {to: 'header'});
                    this.render('eventBox', {to: 'eventbox'});
                    this.render('Footer', {to: 'footer'});
                };
                var subs = [ Meteor.subscribe('events') ];
                this.wait(subs, onReady, onWait);
            },
            data: function() { return [ singleevent = Events.find({ehash: this.params.ehash}).fetch()[0], foo = 123 ]; },
            before: function() { Session.set('ehash', this.params.ehash) }
        });

All routes like this one do not display anything anymore! All yields including the main yield just stay empty. No errors whatsoever.
I checked the current docs but couldn't find any changes which would break my code!
Any ideas?!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants