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
refactor!: set up the login page as a starting page (DSP-1292) #370
Conversation
@@ -1,3 +1,4 @@ | |||
<div class="login-page"> | |||
<dsp-login-form (loginSuccess)="refresh($event)"></dsp-login-form> | |||
<dsp-login-form (loginSuccess)="refresh()"></dsp-login-form> |
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.
The LoginComponent
emits a boolean value in loginSuccess
. I do not understand why you removed it?
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.
reversed in 27bc05c
if (this._session.getSession()) { | ||
this._router.navigate(['dashboard']); | ||
} |
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.
After successful login the user will always be redirected to the dashboard page, right?
Why not keeping this._router.navigate([this.returnUrl]);
? In case a user bookmarked his project page e.g. admin.dasch.swiss/project/1111/info
but (s)he's no longer logged-in, then it would be nice to be redirected to this page after logging-in.
But it's just a detail... maybe not important 😉
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.
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.
haha...yes true. The root path /
was handled in the main component before. This component showed the landing page or the dashboard. Yes, as I said "it's just a little detail". I can live with it.
Another solution would have been to integrate the login page instead of the landing page. I.e. that still the main component handles the redirection.
But it's fine 😉
refresh(status: boolean) { | ||
refresh() { | ||
// check if a session is active | ||
if (this._session.getSession()) { |
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.
could be if(status) {
, because you get the status from login-form
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.
refactored in 34e4acf
@kilchenmann could you explain: |
I think this is what @mdelez implemented. It resolved DSP-331 p.s. for dsp-admin you can comment it out. |
resolves DSP-1292