-
Notifications
You must be signed in to change notification settings - Fork 0
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
#652 login #127
#652 login #127
Conversation
{ | ||
id: GoogleLoginProvider.PROVIDER_ID, | ||
provider: new GoogleLoginProvider( | ||
'257845957103-qdi3hno6v1bf4lli44km2av2gijrs758.apps.googleusercontent.com' |
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.
is it possible that this not be hard coded?
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'll see
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 think it's possible, but I think it's maybe too much work for this pr? I can add a todo, create a ticket, and tackle this kind of refactor later?
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'll see how much effort this is, maybe i can do it now, but i have a feeling for deployment, it will need some work to inject these variables in somehow
} | ||
|
||
async loginWithGoogle(): Promise<void> { | ||
await this.socialAuthService.signIn(GoogleLoginProvider.PROVIDER_ID); // TODO:: try option {ux_mode: 'redirect'} |
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.
Is this TODO still valid?
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.
yea, i may want to try this, i need a hand from patrick though, will confirm by next pr
} else { | ||
return { | ||
headers: { | ||
idToken, |
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.
small detail, should the header be id_token like we've done in onpoint/jetstream?
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.
nah
<span class="title"><b>UDMI</b> Device Management Console</span> | ||
<span class="spacer"></span> | ||
<nav role="navigation" aria-label="Main navigation"> | ||
<a mat-button routerLink="/devices" routerLinkActive="active">Devices</a> | ||
</nav> | ||
<button mat-button [matMenuTriggerFor]="userMenu" aria-label="User menu"> | ||
<span>Hi Marc!</span> | ||
<span>{{ displayName | async }}</span> |
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.
Optional - We could also show the google user image here instead of the displayName.
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's an option, maybe i can try that at some point and show you to see how it looks
No description provided.