-
Notifications
You must be signed in to change notification settings - Fork 280
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
Router.onBeforeAction AccountsTemplates.ensureSignedIn #278
Comments
Yeah, I can reproduce this, that's a bug :( Might it be that this line should also check Meteor.loggingIn? |
Glad you were able to reproduce. Thanks! Don |
I'd be glad if you could play a bit with this and create a PR. Tnx! |
I'd like to try. What do I do in order to make the change and how do I get an version of the package to test it in my app? I'm not familiar with working on community projects like this. But, I want to help where I can. Thanks, |
When I click New Pull Request, the green button is grayed out. How to I request client.js to edit this line? |
Hei @doubletaketech, |
I think I've found a little trick to manage this... In any case you'd better add another hook like this before the one using Router.onBeforeAction(function () {
if (Meteor.loggingIn()) {
this.render('loading');
this.renderRegions();
} else {
this.next();
}
}, {
only: ['home']
}); Tnx! |
@splendido's solution works great for me. |
@micahalcorn do you mean the fix committed or the additional |
Thank you. Is this new version available via meteor update? |
Yep! Version 1.7.0 |
I've updated to 1.7.1 and I still see the login screen flash before home page loads. If I add your extra hook, with loading, then it works fine. Thank you! Appreciate your great work! Don |
...ok, I'll try to debug this a little more. Tnx! |
...I've eventually did it: see e9ee6d2 and Content Protection |
I updated to 1.8 and replaced my previous code with
but I get this error ... ^ when attempting to run the app. Thanks, |
mmmm, I've just realized I defined it for the client only... Are you by any chance setting it on the server side? |
my routes.js is available to both. Thank you!
|
I'll move the definition to be available on both sides... Thank you for the heads up! |
May I ask if you could make some animated gifs comparing the before and after the plugin? |
...reasoning a bit more, there's no point to have the same plugin working 'server-side' where Do you think a fake definition on the server-side would be enough to get no complains and permit to define routing on both sides? |
From my learning experience with Meteor last year, they tended to recommend that one routes.js be available to both client and server. Especially since Iron-Router has the "where" to designate something as a server route. So, anything you can do to make it "idiot proof" for people like me, the less issues will be posted. :) If faking doesn't cost you anything, then do it. But, you may be able to just handle this in your docs reminding people that is client-side only plugin. So, you are wanting to compare before and after. Do you mean before, when the login prompt used to appear ... and after, when it no longer appears? Don |
Don't be silly with yourself: you're not an idiot for sure! ;-) You're right about the good practice to declare routes both sides, this is why I think we should fix this issue. At the same time, there's no point for a plugin like this to be executed on the server side, since there's no way to check which user is requesting a particular route. So my proposal is to create an I'll try to publish a patch tomorrow. ...and yep, the request for the comparison was for before/after the plugin ;-) |
I've just answered this question by @fardeemmunir on SO |
My home page is protected with ensureSignedIn. If the user is already logged in and goes to the home page, the login screen flashes before the home loads. How do I prevent this? I don't want to see the login screen at all if the user is already logged in.
The text was updated successfully, but these errors were encountered: