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.

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

General: Improve tab navigation on login page #9373

Merged
merged 1 commit into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,10 @@ module.exports = {
coverageThreshold: {
global: {
// TODO: in the future, the following values should increase to at least 90%
statements: 87.38,
statements: 87.35,
branches: 73.57,
functions: 81.91,
lines: 87.44,
lines: 87.41,
},
},
coverageReporters: ['clover', 'json', 'lcov', 'text-summary'],
Expand Down
4 changes: 4 additions & 0 deletions src/main/webapp/app/app-routing.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ const LAYOUT_ROUTES: Routes = [navbarRoute, ...errorRoute];
RouterModule.forRoot(
[
...LAYOUT_ROUTES,
{
path: '',
loadComponent: () => import('./home/home.component').then((m) => m.HomeComponent),
},
{
path: 'admin',
loadChildren: () => import('./admin/admin.module').then((m) => m.ArtemisAdminModule),
Expand Down
2 changes: 0 additions & 2 deletions src/main/webapp/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { ErrorComponent } from 'app/shared/layouts/error/error.component';
import { ArtemisCoreModule } from 'app/core/core.module';
import { GuidedTourModule } from 'app/guided-tour/guided-tour.module';
import { ArtemisComplaintsModule } from 'app/complaints/complaints.module';
import { ArtemisHomeModule } from 'app/home/home.module';
import { OrionOutdatedComponent } from 'app/shared/orion/outdated-plugin-warning/orion-outdated.component';
import { LoadingNotificationComponent } from 'app/shared/notification/loading-notification/loading-notification.component';
import { NotificationPopupComponent } from 'app/shared/notification/notification-popup/notification-popup.component';
Expand All @@ -37,7 +36,6 @@ import { ScrollingModule } from '@angular/cdk/scrolling';
ServiceWorkerModule.register('ngsw-worker.js', { enabled: true }),
ArtemisSharedModule,
ArtemisCoreModule,
ArtemisHomeModule,
ArtemisAppRoutingModule,
GuidedTourModule,
ArtemisSystemNotificationModule,
Expand Down
16 changes: 7 additions & 9 deletions src/main/webapp/app/core/auth/account.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable } from '@angular/core';
import { Injectable, inject } from '@angular/core';
import { SessionStorageService } from 'ngx-webstorage';
import { HttpClient, HttpParams, HttpResponse } from '@angular/common/http';
import { BehaviorSubject, Observable, lastValueFrom, of } from 'rxjs';
Expand Down Expand Up @@ -35,19 +35,17 @@ export interface IAccountService {

@Injectable({ providedIn: 'root' })
export class AccountService implements IAccountService {
private translateService = inject(TranslateService);
private sessionStorage = inject(SessionStorageService);
private http = inject(HttpClient);
private websocketService = inject(JhiWebsocketService);
private featureToggleService = inject(FeatureToggleService);

private userIdentityValue?: User;
private authenticated = false;
private authenticationState = new BehaviorSubject<User | undefined>(undefined);
private prefilledUsernameValue?: string;

constructor(
private translateService: TranslateService,
private sessionStorage: SessionStorageService,
private http: HttpClient,
private websocketService: JhiWebsocketService,
private featureToggleService: FeatureToggleService,
) {}

get userIdentity() {
return this.userIdentityValue;
}
Expand Down
10 changes: 4 additions & 6 deletions src/main/webapp/app/core/auth/auth-jwt.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable } from '@angular/core';
import { Injectable, inject } from '@angular/core';
import { HttpClient } from '@angular/common/http';
import { Observable, of } from 'rxjs';
import { LocalStorageService, SessionStorageService } from 'ngx-webstorage';
Expand All @@ -20,11 +20,9 @@ export interface IAuthServerProvider {

@Injectable({ providedIn: 'root' })
export class AuthServerProvider implements IAuthServerProvider {
constructor(
private http: HttpClient,
private localStorage: LocalStorageService,
private sessionStorage: SessionStorageService,
) {}
private http = inject(HttpClient);
private localStorage = inject(LocalStorageService);
private sessionStorage = inject(SessionStorageService);

krusche marked this conversation as resolved.
Show resolved Hide resolved
login(credentials: Credentials): Observable<object> {
return this.http.post('api/public/authenticate', credentials);
Expand Down
56 changes: 2 additions & 54 deletions src/main/webapp/app/core/auth/state-storage.service.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,9 @@
import { Injectable } from '@angular/core';
import { Injectable, inject } from '@angular/core';
import { SessionStorageService } from 'ngx-webstorage';

@Injectable({ providedIn: 'root' })
export class StateStorageService {
constructor(private sessionStorage: SessionStorageService) {}

/**
* Get the previous state of the current session.
* @returns the previous state as string or null when there is no previous state
*/
getPreviousState(): string | null {
return this.sessionStorage.retrieve('previousState');
}

/**
* Reset the previous state of the current session.
*/
resetPreviousState(): void {
this.sessionStorage.clear('previousState');
}

/**
* Store a new previous state for the current session.
* @param previousStateName Name of the new previous state
* @param previousStateParams Parameters of the new previous state
*/
storePreviousState(previousStateName: any, previousStateParams: any): void {
const previousState = { name: previousStateName, params: previousStateParams };
this.sessionStorage.store('previousState', previousState);
}

/**
* Get the destination state for the current session.
*/
getDestinationState(): string {
return this.sessionStorage.retrieve('destinationState');
}
private sessionStorage = inject(SessionStorageService);

/**
* Store an url as previousURL in the current session.
Expand All @@ -51,24 +19,4 @@ export class StateStorageService {
getUrl(): string {
return this.sessionStorage.retrieve('previousUrl');
}

/**
* Store a new destination state for the current session.
* @param destinationState Name of the new destination state
* @param destinationStateParams Parameters of the new destination state
* @param fromState the fromState of the new destination state
*/
storeDestinationState(destinationState: any, destinationStateParams: any, fromState: any): void {
const destinationInfo = {
destination: {
name: destinationState.name,
data: destinationState.data,
},
params: destinationStateParams,
from: {
name: fromState.name,
},
};
this.sessionStorage.store('destinationState', destinationInfo);
}
}
14 changes: 6 additions & 8 deletions src/main/webapp/app/core/auth/user-route-access-service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable, isDevMode } from '@angular/core';
import { Injectable, inject, isDevMode } from '@angular/core';
import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from '@angular/router';
import { AccountService } from 'app/core/auth/account.service';
import { StateStorageService } from 'app/core/auth/state-storage.service';
Expand All @@ -10,13 +10,11 @@ import { AlertService } from 'app/core/util/alert.service';

@Injectable({ providedIn: 'root' })
export class UserRouteAccessService implements CanActivate {
constructor(
private router: Router,
private accountService: AccountService,
private alertService: AlertService,
private stateStorageService: StateStorageService,
private orionVersionValidator: OrionVersionValidator,
) {}
private router = inject(Router);
private accountService = inject(AccountService);
private alertService = inject(AlertService);
private stateStorageService = inject(StateStorageService);
private orionVersionValidator = inject(OrionVersionValidator);

krusche marked this conversation as resolved.
Show resolved Hide resolved
/**
* Check if the client can activate a route.
Expand Down
12 changes: 5 additions & 7 deletions src/main/webapp/app/core/login/login.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable } from '@angular/core';
import { Injectable, inject } from '@angular/core';
import { AlertService } from 'app/core/util/alert.service';
import { Router } from '@angular/router';
import { finalize } from 'rxjs/operators';
Expand All @@ -10,12 +10,10 @@ import { AccountService } from 'app/core/auth/account.service';
export class LoginService {
logoutWasForceful = false;

constructor(
private accountService: AccountService,
private authServerProvider: AuthServerProvider,
private router: Router,
private alertService: AlertService,
) {}
private accountService = inject(AccountService);
private authServerProvider = inject(AuthServerProvider);
private router = inject(Router);
private alertService = inject(AlertService);

krusche marked this conversation as resolved.
Show resolved Hide resolved
/**
* Login the user with the given credentials.
Expand Down
28 changes: 15 additions & 13 deletions src/main/webapp/app/home/home.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,39 +31,47 @@ <h1 jhiTranslate="home.title"></h1>
<input
#usernameForm="ngModel"
[(ngModel)]="username"
[ngModelOptions]="{ updateOn: 'blur' }"
[ngModelOptions]="{ updateOn: 'change' }"
krusche marked this conversation as resolved.
Show resolved Hide resolved
[pattern]="usernameRegexPattern"
autocomplete="username"
class="form-control"
id="username"
name="username"
[placeholder]="usernamePlaceholderTranslated"
[minlength]="USERNAME_MIN_LENGTH"
[maxLength]="USERNAME_MAX_LENGTH"
(ngModelChange)="checkFormValidity()"
(blur)="usernameTouched = true"
type="text"
tabindex="1"
/>
@if (usernameForm.errors && (usernameForm.dirty || usernameForm.touched)) {
@if (usernameForm.errors && (usernameForm.dirty || usernameForm.touched) && usernameTouched) {
<span class="mt-1 text-danger small" [jhiTranslate]="errorMessageUsername"></span>
}
</div>
<div class="form-group">
<div class="d-flex mb-2">
<label for="password" jhiTranslate="login.form.password"></label>
<div class="ms-auto">
<a class="text-primary small" jhiTranslate="login.password.forgot" routerLink="account/reset/request"></a>
<a class="text-primary small" jhiTranslate="login.password.forgot" routerLink="account/reset/request" tabindex="4"></a>
krusche marked this conversation as resolved.
Show resolved Hide resolved
</div>
</div>
<input
#passwordForm="ngModel"
[(ngModel)]="password"
[ngModelOptions]="{ updateOn: 'blur' }"
[ngModelOptions]="{ updateOn: 'change' }"
krusche marked this conversation as resolved.
Show resolved Hide resolved
autocomplete="current-password"
class="form-control"
id="password"
name="password"
type="password"
[minlength]="PASSWORD_MIN_LENGTH"
[maxLength]="PASSWORD_MAX_LENGTH"
(ngModelChange)="checkFormValidity()"
(blur)="passwordTouched = true"
tabindex="2"
/>
@if (passwordForm.errors && (passwordForm.dirty || passwordForm.touched)) {
@if (passwordForm.errors && (passwordForm.dirty || passwordForm.touched) && passwordTouched) {
<span class="mt-1 text-danger small" jhiTranslate="home.errors.passwordIncorrect"></span>
}
</div>
Expand All @@ -85,17 +93,11 @@ <h1 jhiTranslate="home.title"></h1>
</div>
<div class="btn-toolbar">
<button
[disabled]="
isSubmittingLogin ||
(!userAcceptedTerms && needsToAcceptTerms) ||
!password ||
password.length < PASSWORD_MIN_LENGTH ||
!username ||
username.length < USERNAME_MIN_LENGTH
"
[disabled]="isSubmittingLogin || (!userAcceptedTerms && needsToAcceptTerms) || !isFormValid"
class="btn btn-primary w-100"
id="login-button"
type="submit"
tabindex="3"
>
@if (isSubmittingLogin) {
<span class="me-1"><fa-icon [icon]="faCircleNotch" animation="spin" /></span>
Expand Down
55 changes: 37 additions & 18 deletions src/main/webapp/app/home/home.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AfterViewChecked, Component, OnInit, Renderer2 } from '@angular/core';
import { AfterViewChecked, Component, OnInit, Renderer2, inject } from '@angular/core';
import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap';
import { ActivatedRoute, Router } from '@angular/router';
import { ActivatedRoute, Router, RouterLink } from '@angular/router';
import { User } from 'app/core/user/user.model';
import { Credentials } from 'app/core/auth/auth-jwt.service';
import { OrionConnectorService } from 'app/shared/orion/orion-connector.service';
Expand All @@ -11,19 +11,40 @@ import { LoginService } from 'app/core/login/login.service';
import { ProfileService } from 'app/shared/layouts/profiles/profile.service';
import { StateStorageService } from 'app/core/auth/state-storage.service';
import { ProfileInfo } from 'app/shared/layouts/profiles/profile-info.model';
import { PASSWORD_MAX_LENGTH, PASSWORD_MIN_LENGTH, USERNAME_MIN_LENGTH } from 'app/app.constants';
import { PASSWORD_MAX_LENGTH, PASSWORD_MIN_LENGTH, USERNAME_MAX_LENGTH, USERNAME_MIN_LENGTH } from 'app/app.constants';
import { EventManager } from 'app/core/util/event-manager.service';
import { AlertService } from 'app/core/util/alert.service';
import { faCircleNotch } from '@fortawesome/free-solid-svg-icons';
import { TranslateService } from '@ngx-translate/core';
import { ArtemisSharedModule } from 'app/shared/shared.module';
import { TranslateDirective } from '../shared/language/translate.directive';
import { FormsModule } from '@angular/forms';
import { FaIconComponent } from '@fortawesome/angular-fontawesome';
import { Saml2LoginComponent } from './saml2-login/saml2-login.component';

@Component({
selector: 'jhi-home',
templateUrl: './home.component.html',
styleUrls: ['home.scss'],
standalone: true,
imports: [TranslateDirective, FormsModule, RouterLink, FaIconComponent, Saml2LoginComponent, ArtemisSharedModule],
})
krusche marked this conversation as resolved.
Show resolved Hide resolved
export class HomeComponent implements OnInit, AfterViewChecked {
private router = inject(Router);
private activatedRoute = inject(ActivatedRoute);
private accountService = inject(AccountService);
private loginService = inject(LoginService);
private stateStorageService = inject(StateStorageService);
private renderer = inject(Renderer2);
private eventManager = inject(EventManager);
private orionConnectorService = inject(OrionConnectorService);
private modalService = inject(NgbModal);
private profileService = inject(ProfileService);
private alertService = inject(AlertService);
private translateService = inject(TranslateService);

krusche marked this conversation as resolved.
Show resolved Hide resolved
USERNAME_MIN_LENGTH = USERNAME_MIN_LENGTH;
USERNAME_MAX_LENGTH = USERNAME_MAX_LENGTH;
PASSWORD_MIN_LENGTH = PASSWORD_MIN_LENGTH;
PASSWORD_MAX_LENGTH = PASSWORD_MAX_LENGTH;
authenticationError = false;
Expand All @@ -50,27 +71,15 @@ export class HomeComponent implements OnInit, AfterViewChecked {

externalUserManagementActive = true;

isFormValid = false;
isSubmittingLogin = false;

profileInfo: ProfileInfo | undefined = undefined;

// Icons
faCircleNotch = faCircleNotch;

constructor(
private router: Router,
private activatedRoute: ActivatedRoute,
private accountService: AccountService,
private loginService: LoginService,
private stateStorageService: StateStorageService,
private renderer: Renderer2,
private eventManager: EventManager,
private orionConnectorService: OrionConnectorService,
private modalService: NgbModal,
private profileService: ProfileService,
private alertService: AlertService,
private translateService: TranslateService,
) {}
usernameTouched = false;
passwordTouched = false;

ngOnInit() {
this.profileService.getProfileInfo().subscribe((profileInfo) => {
Expand Down Expand Up @@ -236,4 +245,14 @@ export class HomeComponent implements OnInit, AfterViewChecked {
this.password = event.target.value;
}
}

checkFormValidity() {
this.isFormValid =
this.username !== undefined &&
this.username.length >= this.USERNAME_MIN_LENGTH &&
this.username.length <= this.USERNAME_MAX_LENGTH &&
this.password !== undefined &&
this.password.length >= this.PASSWORD_MIN_LENGTH &&
this.password.length <= this.PASSWORD_MAX_LENGTH;
}
}
krusche marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 0 additions & 12 deletions src/main/webapp/app/home/home.module.ts

This file was deleted.

Loading
Loading