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

Redirect to sub-route in same group does not trigger reactive update of .getRouteName() #314

Closed
derwaldgeist opened this issue Sep 8, 2015 · 12 comments

Comments

@derwaldgeist
Copy link

Scenario:

I have set up a group route containing a teaser page at /group and a hidden page at /group/page. The idea is to show the teaser page if the user is not logged in, and replace it by the hidden page the user has logged in. Here is my route configuration for this:

var myGroupRoutes = FlowRouter.group({
  prefix: '/group'
});

myGroupRoutes.route('/', {
  // If user is logged in, redirect to sub page
  triggersEnter: [function (context, redirect) {
    if (Meteor.user()) {
      redirect('myGroupPage');
    }
  }],
  // If user is not logged in, show teaser page
  action: function () {
    BlazeLayout.render('layout', {
      mainCanvas: 'teaserPage'
    });
  },
  name: 'myGroup'
});

myGroupRoutes.route('/page', {
  // If user is not logged in, redirect to teaser page
  triggersEnter: [function (context, redirect) {
    if (!Meteor.user()) {
      redirect('myGroup');
    }
  }],
  action: function () {
    BlazeLayout.render('layout', {
      mainCanvas: 'subPage'
    });
  },
  name: 'myGroupPage'
});

I am also using a navigation bar with two nav items: one for the app home (called root, with route path /) and a second item for the group route, which shall be highlighted for for both /group and /group/page. I am using zimme:meteor-activeroute to do this, with this configuration:

<li class="{{isActiveRoute 'root'}}"><a href="{{pathFor 'root'}}">Home</a></li>
<li class="{{isActiveRoute regex='^myGroup'}}"><a href="{{pathFor 'myGroup'}}">Menu Entry</a></li>

The switch between the nav items works fine if the user is not logged in. However, once the user is logged in, clicking on the second nav item will not cause a switch of the nav item highlight (i.e. the isActiveRoute helpers won't update). After debugging and writing FlowRouter's route state to the console, I found out that FlowRouter's reactive functions like .getRouteName() won't reactively update if a redirect to a sub-route of a route group takes place. The reactive updates only work if the redirect target is the root of the route group. I tested this by placing a dummy helper in my template that just reports .getRouteName() to the console.

@arunoda
Copy link
Contributor

arunoda commented Sep 12, 2015

Could you send me that sample repo?

@arunoda arunoda added the bug label Sep 12, 2015
@mbernath
Copy link

I experienced the same problem as @derwaldgeist.

The problem was that currentContext.oldRoute.registerRouteClose() was only run for the intermediate route and not the original route.

I think I could fix it. I'll add a pull request.

@derwaldgeist
Copy link
Author

Hey, that sounds great. Was out of office the last days and did not find the time to create a test repo. Le'ts see if it's still necessary.

@arunoda
Copy link
Contributor

arunoda commented Sep 14, 2015

@mbernath That's great. Looking forward to that.

mbernath added a commit to mbernath/flow-router that referenced this issue Sep 14, 2015
@mbernath
Copy link

Here you go! That change fixes the bug for me...

@mbernath
Copy link

Hmmm... The change seems to break the tests. (Sorry I couldn't run them locally - didn't work)

I've added a check if context.oldRoute exists, which should fix the problem. You want another pull request?

@arunoda
Copy link
Contributor

arunoda commented Sep 14, 2015

No. Just update your branch with the new code.
To run tests do this:

  • Go to the local repo
  • run meteor test-packages ./ --port 9000

@mbernath
Copy link

Thanks! I see the tests pass now...

mbernath added a commit to mbernath/flow-router that referenced this issue Sep 14, 2015
@arunoda
Copy link
Contributor

arunoda commented Sep 16, 2015

Hey, release a new version for this issue in 2.26.1. Could you check this.

@arunoda arunoda mentioned this issue Sep 16, 2015
@mbernath
Copy link

@arunoda: Thank you!

I checked and I can confirm that it works for me :-)

@arunoda
Copy link
Contributor

arunoda commented Sep 17, 2015

Great.

@derwaldgeist
Copy link
Author

I can also confirm it is working now. Great, thanks a lot!

@arunoda arunoda added feature and removed bug labels Sep 28, 2015
@arunoda arunoda closed this as completed Sep 28, 2015
@arunoda arunoda removed the feature label Sep 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants