Replace mat-chip Sign in button with Sign in with Google button#2413
Replace mat-chip Sign in button with Sign in with Google button#2413sidd190 wants to merge 10 commits intogoogle:masterfrom
Conversation
|
@gino-m @rfontanarosa This is also ready for review, please take a look whenever available. Thanks. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2413 +/- ##
==========================================
- Coverage 59.13% 58.98% -0.15%
==========================================
Files 111 111
Lines 2748 2748
Branches 408 408
==========================================
- Hits 1625 1621 -4
- Misses 1065 1069 +4
Partials 58 58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
hi @sidd190 , thank you for your contribution. I've just left some comments. |
Umm hi again, Sorry but where are these comments? I don't see them anywhere. :( Might be glitch or a miss on my side. 
|
| Sign in to continue | ||
| </p> | ||
| <div class="buttons"> | ||
| <button mat-flat-button color="primary" (click)="onGoogleSignIn()"> |
There was a problem hiding this comment.
Hi @sidd190, instead of hardcoding the SVG paths, I'd suggest using the official asset. I've found it here: https://developers.google.com/identity/branding-guidelines.
<img src="assets/img/web_light_rd_na.svg" alt="" />
I've already updated the file to have a transparent background (fill="none") and removed the border, so the hover state on the button will work correctly without being blocked by a white box. This should also help you clean up some CSS.
| align-items: center; | ||
| justify-content: center; | ||
| gap: 12px; | ||
| padding: 12px 24px; |
| } | ||
| } | ||
|
|
||
| .google-logo { |
There was a problem hiding this comment.
remove this .google-logo
| class="google-signin-btn" | ||
| (click)="onGoogleSignIn()" | ||
| type="button" | ||
| aria-label="Sign in with Google"> |
There was a problem hiding this comment.
can you please remove the aria-label? we already have "Sign in with Google" inside.
|
hi @sidd190, can you please check now? |
|
hi @sidd190 , do you have time to complete this? |
3 participants
Fixes #2368