-
Notifications
You must be signed in to change notification settings - Fork 61
Auth guards #282
base: master
Are you sure you want to change the base?
Auth guards #282
Conversation
…into auth-guards
@Sanji515 Please take a look |
@@ -35,6 +35,8 @@ import {HostAnalyticsComponent} from './components/analytics/host-analytics/host | |||
import {ResetPasswordComponent} from './components/auth/reset-password/reset-password.component'; | |||
import {ResetPasswordConfirmComponent} from './components/auth/reset-password-confirm/reset-password-confirm.component'; | |||
|
|||
import { AuthGuard } from './guards/auth-guard.service'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a better name: Authentication/authorization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is normal convention
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return true; | ||
} | ||
|
||
const tree: UrlTree = this.router.parseUrl(routePath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how this will help. I think UrlTree
was introduced as return type from Angular 7 version.
Isn't it we can do this as well simply:
return isAuthenticated
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't because it will behave the same way as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out the GIF in the PR description. You will see the difference
Fixes # #271
Changes proposed in this pull request:
Previously:
Currently: