Skip to content
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.

Sign up for GitHub

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

Jump to bottom

WIP: feat: add option to hide connected user panel #483

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

WIP: feat: add option to hide connected user panel #483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9d4e1c1
feat: add option to hide connected user panel
blackholegalaxy Mar 24, 2020
8ebe378
refac: code style changes
blackholegalaxy Mar 25, 2020
56e0ba2
feat: add loading event
blackholegalaxy Mar 25, 2020
57d0967
code review changes
blackholegalaxy Mar 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
build:
docker:
# specify the version you desire here
- image: circleci/node:8.9.4
- image: circleci/node:10

# Specify service dependencies here if necessary
# CircleCI maintains a library of pre-built images
Expand Down
2 changes: 1 addition & 1 deletion .nvmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
11
12
32 changes: 15 additions & 17 deletions projects/ngx-auth-firebaseui/src/lib/components/ngx-auth-firebaseui/auth.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
- we arrived from the guard after trying to access a protected route even though we are connected
-->
<div
*ngIf="(config.guardProtectedRoutesUntilEmailIsVerified && !user.emailVerified) || (authProcess.emailConfirmationSent && !user.emailVerified); else signedInUser"
*ngIf="isEmailConfirmationScreenVisible(user)"
fxLayout="row" fxLayoutAlign="center center">
<ngx-auth-firebaseui-email-confirmation
(signOut)="signOut()"
Expand All @@ -21,23 +21,21 @@
</ngx-auth-firebaseui-email-confirmation>
</div>

<ng-template #signedInUser>
<div class="signed-in-container" fxLayout="column" fxLayoutAlign="center center">
<img *ngIf="user?.photoURL; else noPhoto" [src]="user?.photoURL" class="account-circle">
<ng-template #noPhoto>
<mat-icon class="account-circle">account_circle</mat-icon>
</ng-template>
<div class="user-display-name mat-title">{{ user?.displayName }}</div>
<div class="user-email mat-body-2">{{ user?.email }}</div>
<div class="actions">
<mat-progress-bar *ngIf="isLoading" mode="indeterminate"></mat-progress-bar>
<a [routerLink]="goBackURL" class="go-back-button action-button" color="primary"
mat-stroked-button>{{ verifyEmailGoBackText }}</a>
<button (click)="signOut()" class="sign-out-button action-button" color="warn"
mat-stroked-button>{{ signOutText }}</button>
</div>
<div *ngIf="isUserProfileScreenVisible(user)" class="signed-in-container" fxLayout="column" fxLayoutAlign="center center" *ngIf="">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can u please provide more info about the implementation and why the logic should change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In current implementation, only two "alternatives" are covered:

  • If the user is signed In: we display the profile or if the email is not verified (and guard exists) then we display the panel telling the user must validate its email.
  • If the user is not signed in: we display the form

My proposal is to offer a third option (implementation detail: I moved the condition in a method so the HTML is more readable).

To allow this third option i removed the if ; else syntax, thus removing the ng-template. After my change we basically have 3 options:

  • If the user is not connected: display the form
  • If the user is connected and doesn't have email validated (and a guard exists) we display the email validation screen
  • If the user is connected and have email validated but for a reason I don't want, where I placed the form, to display the "profile" data (picture, logout button etc), then we add a flag to allow not to display this profile. This "profile" view replacing the form after login may disturb the design.

Since everything has parameters in ngx-auth-firebaseui, it's nice to allow developper not to display something and allowing him to use the new ngx-auth-firebaseui with signin/register/email validation guard without forcing display of profile after login.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will perform more tests to find if there is another way to achieve this :)

<img *ngIf="user?.photoURL; else noPhoto" [src]="user?.photoURL" class="account-circle">
<ng-template #noPhoto>
<mat-icon class="account-circle">account_circle</mat-icon>
</ng-template>
<div class="user-display-name mat-title">{{ user?.displayName }}</div>
<div class="user-email mat-body-2">{{ user?.email }}</div>
<div class="actions">
<mat-progress-bar *ngIf="isLoading" mode="indeterminate"></mat-progress-bar>
<a [routerLink]="goBackURL" class="go-back-button action-button" color="primary"
mat-stroked-button>{{ verifyEmailGoBackText }}</a>
<button (click)="signOut()" class="sign-out-button action-button" color="warn"
mat-stroked-button>{{ signOutText }}</button>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above for detail :)

</div>
</ng-template>
</div>

</ng-container>

Expand Down
20 changes: 19 additions & 1 deletion projects/ngx-auth-firebaseui/src/lib/components/ngx-auth-firebaseui/auth.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export class AuthComponent implements OnInit, AfterViewInit, OnChanges, OnDestro
@Input() tabIndex: number | null;
@Input() registrationEnabled = true;
@Input() resetPasswordEnabled = true;
@Input() connectedUserScreenEnabled = true;
@Input() guestEnabled = true;
@Input() tosUrl: string;
@Input() privacyPolicyUrl: string;
Expand All @@ -85,6 +86,7 @@ export class AuthComponent implements OnInit, AfterViewInit, OnChanges, OnDestro
// tslint:disable-next-line:no-output-on-prefix
@Output() onError: any;
@Output() selectedTabChange: EventEmitter<MatTabChangeEvent> = new EventEmitter();
@Output() loading: EventEmitter<boolean> = new EventEmitter();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need a new event emitter ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new event emitter was added in case we want to alert the container component something inside is loading. Can be useful to display a loader over the whole container. It doesn't disturb the flow. I can remove it though


// Password strength api
@Input() enableLengthRule = true;
Expand Down Expand Up @@ -192,6 +194,15 @@ export class AuthComponent implements OnInit, AfterViewInit, OnChanges, OnDestro
return this.authenticationError ? 'warn' : 'primary';
}

public isEmailConfirmationScreenVisible(user: firebase.User): boolean {
return (this.config.guardProtectedRoutesUntilEmailIsVerified && !user.emailVerified)
|| (this.authProcess.emailConfirmationSent && !user.emailVerified);
}

public isUserProfileScreenVisible(user: firebase.User): boolean {
return !this.isEmailConfirmationScreenVisible(user) && this.connectedUserScreenEnabled;
}

public ngOnInit(): void {
if (isPlatformBrowser(this.platformId)) {
this.onErrorSubscription = this.onError.subscribe(() => this.authenticationError = true);
Expand Down Expand Up @@ -245,10 +256,12 @@ export class AuthComponent implements OnInit, AfterViewInit, OnChanges, OnDestro
async signOut() {
try {
this.isLoading = true;
this.loading.emit(true);
this.changeDetectorRef.markForCheck();
await this.authProcess.signOut();
} finally {
this.isLoading = false;
this.loading.emit(false);
this.changeDetectorRef.markForCheck();
}
}
Expand All @@ -259,13 +272,15 @@ export class AuthComponent implements OnInit, AfterViewInit, OnChanges, OnDestro
}
try {
this.isLoading = true;
this.loading.emit(true);
this.changeDetectorRef.markForCheck();
await this.authProcess.signInWith(this.authProviders.EmailAndPassword, {
email: this.signInFormGroup.value.email,
password: this.signInFormGroup.value.password
});
} finally {
this.isLoading = false;
this.loading.emit(false);
this.changeDetectorRef.markForCheck();
}
}
Expand Down Expand Up @@ -304,6 +319,7 @@ export class AuthComponent implements OnInit, AfterViewInit, OnChanges, OnDestro
async signUp() {
try {
this.isLoading = true;
this.loading.emit(true);
this.changeDetectorRef.markForCheck();
return await this.authProcess.signUp(
this.signUpFormGroup.value.name,
Expand All @@ -314,22 +330,24 @@ export class AuthComponent implements OnInit, AfterViewInit, OnChanges, OnDestro
);
} finally {
this.isLoading = false;
this.loading.emit(false);
this.changeDetectorRef.markForCheck();
}
}

async signUpAnonymously() {
try {
this.isLoading = true;
this.loading.emit(true);
this.changeDetectorRef.markForCheck();
await this.authProcess.signInWith(this.authProvider.ANONYMOUS);
} finally {
this.isLoading = false;
this.loading.emit(false);
this.changeDetectorRef.markForCheck();
}
}


resetPassword() {
this.authProcess.resetPassword(this.resetPasswordEmailFormControl.value)
.then(() => {
Expand Down