-
Notifications
You must be signed in to change notification settings - Fork 1
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
Added Redirect Capability #4
Added Redirect Capability #4
Conversation
Added method override to provideAuthService to allow for specifying a redirect and a homepath URL. Added constructor overrides to allow for passthrough of the redirect and homepath URLs a well as the ability to inject the router for the application.
closable: false // Users are required to log in | ||
closable: false, // Users are required to log in | ||
auth: { | ||
redirectUrl: (this.redirectPath == null ? location.origin : location.origin + this.redirectPath), |
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.
What happens if you just say location.origin + this.redirectPath
here?
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 would probably work as well, string concatenation in Typescript should do the same thing either way.
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.
Turns out this doesn't work like that. For example if location.origin
is http:\example.com and this.redirectPath
isn't set, the results of location.origin + this.redirectPath
will end up as http:\example.comundefinded
}); | ||
|
||
constructor(private clientId: string, private domain: string) { | ||
constructor( clientId: string, domain: string); | ||
constructor( clientId: string, domain: string, homePath: string, redirectPath: string, router: Router); |
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 wonder why is this signature necessary?
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.
This seems to be the recommended method of overloading methods in Typescript.
export function provideAuthService(clientId: string, domain: string): Provider; | ||
export function provideAuthService(clientId: string, domain: string, homePath: string, redirectPath: string): Provider; | ||
export function provideAuthService(clientId: string, domain: string, homePath?: string, redirectPath?: string): Provider { | ||
if(redirectPath == null){ |
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.
Should this check be on homePath
? That is the one that requires router
dependency.
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.
It should be, I'll change that.
changed a comparison from redirectPath to homePath, as that is actually what should be checked.
Added method override to provideAuthService to allow for specifying a
redirect and a homepath URL.
Added constructor overrides to allow for passthrough of the redirect and
homepath URLs a well as the ability to inject the router for the
application.