From 4ec93287b6acd96007f50e8aa77ae0a09a778bfe Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Fri, 27 Sep 2024 20:31:09 +0200 Subject: [PATCH] General: Improve tab navigation on login page --- jest.config.js | 4 +- src/main/webapp/app/app-routing.module.ts | 4 + src/main/webapp/app/app.module.ts | 2 - .../webapp/app/core/auth/account.service.ts | 16 ++-- .../webapp/app/core/auth/auth-jwt.service.ts | 10 +- .../app/core/auth/state-storage.service.ts | 56 +----------- .../core/auth/user-route-access-service.ts | 14 ++- .../webapp/app/core/login/login.service.ts | 12 +-- src/main/webapp/app/home/home.component.html | 28 +++--- src/main/webapp/app/home/home.component.ts | 55 +++++++---- src/main/webapp/app/home/home.module.ts | 12 --- src/main/webapp/app/home/home.route.ts | 11 --- .../home/saml2-login/saml2-login.component.ts | 22 ++--- .../spec/service/account.service.spec.ts | 91 +++++++++++-------- .../service/state-storage.service.spec.ts | 62 ------------- 15 files changed, 143 insertions(+), 256 deletions(-) delete mode 100644 src/main/webapp/app/home/home.module.ts delete mode 100644 src/main/webapp/app/home/home.route.ts diff --git a/jest.config.js b/jest.config.js index 742a965ec87e..41354957ab0d 100644 --- a/jest.config.js +++ b/jest.config.js @@ -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'], diff --git a/src/main/webapp/app/app-routing.module.ts b/src/main/webapp/app/app-routing.module.ts index dc106122148b..b95970e4be38 100644 --- a/src/main/webapp/app/app-routing.module.ts +++ b/src/main/webapp/app/app-routing.module.ts @@ -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), diff --git a/src/main/webapp/app/app.module.ts b/src/main/webapp/app/app.module.ts index 0a415aa50d80..8f90b73a34cb 100644 --- a/src/main/webapp/app/app.module.ts +++ b/src/main/webapp/app/app.module.ts @@ -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'; @@ -37,7 +36,6 @@ import { ScrollingModule } from '@angular/cdk/scrolling'; ServiceWorkerModule.register('ngsw-worker.js', { enabled: true }), ArtemisSharedModule, ArtemisCoreModule, - ArtemisHomeModule, ArtemisAppRoutingModule, GuidedTourModule, ArtemisSystemNotificationModule, diff --git a/src/main/webapp/app/core/auth/account.service.ts b/src/main/webapp/app/core/auth/account.service.ts index ac677cdc6356..93b66699ebba 100644 --- a/src/main/webapp/app/core/auth/account.service.ts +++ b/src/main/webapp/app/core/auth/account.service.ts @@ -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'; @@ -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(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; } diff --git a/src/main/webapp/app/core/auth/auth-jwt.service.ts b/src/main/webapp/app/core/auth/auth-jwt.service.ts index ec7ab3fde9b8..fc6345a38381 100644 --- a/src/main/webapp/app/core/auth/auth-jwt.service.ts +++ b/src/main/webapp/app/core/auth/auth-jwt.service.ts @@ -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'; @@ -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); login(credentials: Credentials): Observable { return this.http.post('api/public/authenticate', credentials); diff --git a/src/main/webapp/app/core/auth/state-storage.service.ts b/src/main/webapp/app/core/auth/state-storage.service.ts index 7e0d77de6e44..281ce78366e0 100644 --- a/src/main/webapp/app/core/auth/state-storage.service.ts +++ b/src/main/webapp/app/core/auth/state-storage.service.ts @@ -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. @@ -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); - } } diff --git a/src/main/webapp/app/core/auth/user-route-access-service.ts b/src/main/webapp/app/core/auth/user-route-access-service.ts index 3fde5337d3e1..f096a174586e 100644 --- a/src/main/webapp/app/core/auth/user-route-access-service.ts +++ b/src/main/webapp/app/core/auth/user-route-access-service.ts @@ -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'; @@ -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); /** * Check if the client can activate a route. diff --git a/src/main/webapp/app/core/login/login.service.ts b/src/main/webapp/app/core/login/login.service.ts index 63f1a97da105..498867b0c1f3 100644 --- a/src/main/webapp/app/core/login/login.service.ts +++ b/src/main/webapp/app/core/login/login.service.ts @@ -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'; @@ -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); /** * Login the user with the given credentials. diff --git a/src/main/webapp/app/home/home.component.html b/src/main/webapp/app/home/home.component.html index 2ac14640fc07..3e736b788c1f 100644 --- a/src/main/webapp/app/home/home.component.html +++ b/src/main/webapp/app/home/home.component.html @@ -31,16 +31,21 @@

- @if (usernameForm.errors && (usernameForm.dirty || usernameForm.touched)) { + @if (usernameForm.errors && (usernameForm.dirty || usernameForm.touched) && usernameTouched) { } @@ -48,13 +53,13 @@

- +
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) { } @@ -85,17 +93,11 @@