From 0e1a93ce60961ec11b0aa48e00ffe0c47bd28cf3 Mon Sep 17 00:00:00 2001 From: Johannes Wiest Date: Wed, 16 Oct 2024 15:02:24 +0200 Subject: [PATCH 1/4] introduce loading spinner for navigation to previous and successor learning object --- .../learning-path-student-nav.component.html | 50 +++-- .../learning-path-student-nav.component.scss | 4 + .../learning-path-student-nav.component.ts | 48 ++-- .../learning-path-navigation.service.ts | 14 +- .../learning-path-nav.component.spec.ts | 208 ++++++------------ 5 files changed, 148 insertions(+), 176 deletions(-) diff --git a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.html b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.html index e788ee5ebe01..f367d7acaa4e 100644 --- a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.html +++ b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.html @@ -4,7 +4,15 @@
@if (predecessorLearningObject(); as predecessorLearningObject) {
-
@@ -13,36 +21,48 @@
@if (successorLearningObject(); as successorLearningObject) { - {{ successorLearningObject.name }} + {{ successorLearningObject.name }}
-
} @else if (currentLearningObject() && !successorLearningObject()) {
- diff --git a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.scss b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.scss index 42df02050a00..e62ea7e9d2f0 100644 --- a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.scss +++ b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.scss @@ -11,3 +11,7 @@ width: 500px; box-shadow: 0 8px 12px 0 var(--lecture-unit-card-shadow); } + +.loading-icon-container { + width: 15px; +} diff --git a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts index 90eb7bb2b43e..5b8b77c84e83 100644 --- a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts +++ b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts @@ -1,9 +1,9 @@ -import { Component, InputSignal, Signal, WritableSignal, computed, effect, inject, input, signal } from '@angular/core'; +import { Component, computed, effect, inject, input, signal, untracked } from '@angular/core'; import { LearningPathNavigationObjectDTO } from 'app/entities/competency/learning-path.model'; import { CommonModule } from '@angular/common'; import { NgbAccordionModule, NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap'; import { FontAwesomeModule } from '@fortawesome/angular-fontawesome'; -import { IconDefinition, faCheckCircle, faChevronDown, faFlag } from '@fortawesome/free-solid-svg-icons'; +import { faCheckCircle, faChevronDown, faChevronLeft, faChevronRight, faFlag, faSpinner } from '@fortawesome/free-solid-svg-icons'; import { LearningPathNavOverviewComponent } from 'app/course/learning-paths/components/learning-path-nav-overview/learning-path-nav-overview.component'; import { ArtemisSharedModule } from 'app/shared/shared.module'; import { LearningPathNavigationService } from 'app/course/learning-paths/services/learning-path-navigation.service'; @@ -16,33 +16,43 @@ import { LearningPathNavigationService } from 'app/course/learning-paths/service styleUrl: './learning-path-student-nav.component.scss', }) export class LearningPathNavComponent { - protected readonly faChevronDown: IconDefinition = faChevronDown; - protected readonly faCheckCircle: IconDefinition = faCheckCircle; - protected readonly faFlag: IconDefinition = faFlag; + protected readonly faChevronDown = faChevronDown; + protected readonly faCheckCircle = faCheckCircle; + protected readonly faFlag = faFlag; + protected readonly faSpinner = faSpinner; + protected readonly faChevronLeft = faChevronLeft; + protected readonly faChevronRight = faChevronRight; - private learningPathNavigationService: LearningPathNavigationService = inject(LearningPathNavigationService); + private learningPathNavigationService = inject(LearningPathNavigationService); - readonly learningPathId: InputSignal = input.required(); + readonly learningPathId = input.required(); - readonly isLoading: WritableSignal = this.learningPathNavigationService.isLoading; + readonly isLoading = this.learningPathNavigationService.isLoading; + readonly isLoadingPredecessor = signal(false); + readonly isLoadingSuccessor = signal(false); - readonly learningPathProgress: Signal = computed(() => this.learningPathNavigationService.learningPathNavigation()?.progress ?? 0); - readonly predecessorLearningObject: Signal = computed( - () => this.learningPathNavigationService.learningPathNavigation()?.predecessorLearningObject, - ); - readonly currentLearningObject: Signal = computed(() => this.learningPathNavigationService.currentLearningObject()); - readonly successorLearningObject: Signal = computed( - () => this.learningPathNavigationService.learningPathNavigation()?.successorLearningObject, - ); + private readonly learningPathNavigation = this.learningPathNavigationService.learningPathNavigation; + readonly learningPathProgress = computed(() => this.learningPathNavigation()?.progress ?? 0); + readonly predecessorLearningObject = computed(() => this.learningPathNavigation()?.predecessorLearningObject); + readonly currentLearningObject = computed(() => this.learningPathNavigation()?.currentLearningObject); + readonly successorLearningObject = computed(() => this.learningPathNavigation()?.successorLearningObject); - readonly isDropdownOpen: WritableSignal = signal(false); + readonly isDropdownOpen = signal(false); constructor() { - effect(async () => await this.learningPathNavigationService.loadLearningPathNavigation(this.learningPathId()), { allowSignalWrites: true }); + effect( + () => { + const learningPathId = this.learningPathId(); + untracked(() => this.learningPathNavigationService.loadLearningPathNavigation(learningPathId)); + }, + { allowSignalWrites: true }, + ); } - async selectLearningObject(selectedLearningObject: LearningPathNavigationObjectDTO): Promise { + async selectLearningObject(selectedLearningObject: LearningPathNavigationObjectDTO, isSuccessor: boolean): Promise { + isSuccessor ? this.isLoadingSuccessor.set(true) : this.isLoadingPredecessor.set(true); await this.learningPathNavigationService.loadRelativeLearningPathNavigation(this.learningPathId(), selectedLearningObject); + isSuccessor ? this.isLoadingSuccessor.set(false) : this.isLoadingPredecessor.set(false); } completeLearningPath(): void { diff --git a/src/main/webapp/app/course/learning-paths/services/learning-path-navigation.service.ts b/src/main/webapp/app/course/learning-paths/services/learning-path-navigation.service.ts index 7a7e3fb1dba3..1ecbc1033c33 100644 --- a/src/main/webapp/app/course/learning-paths/services/learning-path-navigation.service.ts +++ b/src/main/webapp/app/course/learning-paths/services/learning-path-navigation.service.ts @@ -1,19 +1,19 @@ -import { Injectable, Signal, WritableSignal, computed, inject, signal } from '@angular/core'; +import { computed, inject, Injectable, signal } from '@angular/core'; import { LearningPathNavigationDTO, LearningPathNavigationObjectDTO } from 'app/entities/competency/learning-path.model'; import { AlertService } from 'app/core/util/alert.service'; import { LearningPathApiService } from 'app/course/learning-paths/services/learning-path-api.service'; @Injectable({ providedIn: 'root' }) export class LearningPathNavigationService { - private readonly learningPathApiService: LearningPathApiService = inject(LearningPathApiService); - private readonly alertService: AlertService = inject(AlertService); + private readonly learningPathApiService = inject(LearningPathApiService); + private readonly alertService = inject(AlertService); - readonly isLoading: WritableSignal = signal(false); + readonly isLoading = signal(false); - readonly learningPathNavigation: WritableSignal = signal(undefined); - readonly currentLearningObject: Signal = computed(() => this.learningPathNavigation()?.currentLearningObject); + readonly learningPathNavigation = signal(undefined); + readonly currentLearningObject = computed(() => this.learningPathNavigation()?.currentLearningObject); - readonly isCurrentLearningObjectCompleted: WritableSignal = signal(false); + readonly isCurrentLearningObjectCompleted = signal(false); async loadLearningPathNavigation(learningPathId: number): Promise { try { diff --git a/src/test/javascript/spec/component/learning-paths/components/learning-path-nav.component.spec.ts b/src/test/javascript/spec/component/learning-paths/components/learning-path-nav.component.spec.ts index 6706a6b833af..03c61898b620 100644 --- a/src/test/javascript/spec/component/learning-paths/components/learning-path-nav.component.spec.ts +++ b/src/test/javascript/spec/component/learning-paths/components/learning-path-nav.component.spec.ts @@ -1,19 +1,17 @@ -import { provideHttpClient } from '@angular/common/http'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { TranslateService } from '@ngx-translate/core'; import { LearningPathNavComponent } from 'app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component'; import { LearningObjectType, LearningPathNavigationDTO } from 'app/entities/competency/learning-path.model'; -import { By } from '@angular/platform-browser'; import { LearningPathNavOverviewComponent } from 'app/course/learning-paths/components/learning-path-nav-overview/learning-path-nav-overview.component'; import { MockTranslateService } from '../../../helpers/mocks/service/mock-translate.service'; -import { LearningPathApiService } from 'app/course/learning-paths/services/learning-path-api.service'; +import { LearningPathNavigationService } from 'app/course/learning-paths/services/learning-path-navigation.service'; +import { MockComponent } from 'ng-mocks'; describe('LearningPathStudentNavComponent', () => { let component: LearningPathNavComponent; let fixture: ComponentFixture; - let learningPathApiService: LearningPathApiService; - let getLearningPathNavigationSpy: jest.SpyInstance; - let getRelativeLearningPathNavigationSpy: jest.SpyInstance; + let learningPathNavigationService: LearningPathNavigationService; + let learningPathNavigationSpy: jest.SpyInstance; const navigationDto: LearningPathNavigationDTO = { predecessorLearningObject: { @@ -47,25 +45,30 @@ describe('LearningPathStudentNavComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ - imports: [LearningPathNavComponent], + imports: [LearningPathNavComponent, MockComponent(LearningPathNavOverviewComponent)], providers: [ - provideHttpClient(), { provide: TranslateService, useClass: MockTranslateService, }, + { + provide: LearningPathNavigationService, + useValue: { + isLoading: jest.fn(), + learningPathNavigation: jest.fn(), + currentLearningObject: jest.fn(), + loadLearningPathNavigation: jest.fn(), + loadRelativeLearningPathNavigation: jest.fn(), + completeLearningPath: jest.fn(), + }, + }, ], }) - .overrideComponent(LearningPathNavComponent, { - add: { - imports: [LearningPathNavOverviewComponent], - }, - }) + .compileComponents(); - learningPathApiService = TestBed.inject(LearningPathApiService); - getLearningPathNavigationSpy = jest.spyOn(learningPathApiService, 'getLearningPathNavigation').mockResolvedValue(navigationDto); - getRelativeLearningPathNavigationSpy = jest.spyOn(learningPathApiService, 'getRelativeLearningPathNavigation'); + learningPathNavigationService = TestBed.inject(LearningPathNavigationService); + learningPathNavigationSpy = jest.spyOn(learningPathNavigationService, 'learningPathNavigation'); fixture = TestBed.createComponent(LearningPathNavComponent); component = fixture.componentInstance; @@ -76,163 +79,98 @@ describe('LearningPathStudentNavComponent', () => { jest.restoreAllMocks(); }); - it('should initialize', async () => { + it('should load initial learning path navigation', async () => { + const loadLearningPathNavigationSpy = jest.spyOn(learningPathNavigationService, 'loadLearningPathNavigation'); + learningPathNavigationSpy.mockReturnValue(navigationDto); + fixture.detectChanges(); - expect(component).toBeTruthy(); - expect(component.learningPathId()).toBe(learningPathId); + expect(loadLearningPathNavigationSpy).toHaveBeenCalledExactlyOnceWith(learningPathId); }); it('should show progress bar percentage', async () => { - fixture.detectChanges(); - await fixture.whenStable(); - fixture.detectChanges(); - - const progressBar = fixture.debugElement.query(By.css('.progress-bar')); - expect(progressBar.nativeElement.style.width).toBe('50%'); - }); + learningPathNavigationSpy.mockReturnValue(navigationDto); - it('should navigate with next and previous button', async () => { - fixture.detectChanges(); - await fixture.whenStable(); fixture.detectChanges(); - expect(component.predecessorLearningObject()).toEqual(navigationDto.predecessorLearningObject); - expect(component.currentLearningObject()).toEqual(navigationDto.currentLearningObject); - expect(component.successorLearningObject()).toEqual(navigationDto.successorLearningObject); - const previousButton = fixture.debugElement.query(By.css('#previous-button')); - expect(previousButton).toBeTruthy(); - const nextButton = fixture.debugElement.query(By.css('#next-button')); - expect(nextButton).toBeTruthy(); + const progressBar = fixture.nativeElement.querySelector('.progress-bar'); + + expect(progressBar.style.width).toBe('50%'); }); - it('should set current to previous unit on complete button', async () => { - const navigationDto = { - predecessorLearningObject: { - id: 1, - name: 'Exercise', - type: LearningObjectType.EXERCISE, - completed: true, - }, - currentLearningObject: { - id: 2, - name: 'Lecture', - type: LearningObjectType.LECTURE, - completed: false, - }, - progress: 95, - }; - getLearningPathNavigationSpy.mockResolvedValue(navigationDto); + it('should set learningPathProgress correctly', () => { + learningPathNavigationSpy.mockReturnValue(navigationDto); fixture.detectChanges(); - await fixture.whenStable(); - fixture.detectChanges(); - - const completeButton = fixture.debugElement.query(By.css('#complete-button')); - completeButton.nativeElement.click(); - expect(component.predecessorLearningObject()).toBe(navigationDto.currentLearningObject); - expect(component.currentLearningObject()).toBeUndefined(); - expect(component.learningPathProgress()).toBe(100); + expect(component.learningPathProgress()).toBe(50); }); - it('should show navigation with previous and complete button', async () => { - const navigationDto = { - predecessorLearningObject: { - id: 1, - name: 'Exercise', - type: LearningObjectType.EXERCISE, - completed: true, - }, - currentLearningObject: { - id: 2, - name: 'Lecture', - type: LearningObjectType.LECTURE, - completed: false, - }, - progress: 95, - }; - getLearningPathNavigationSpy.mockResolvedValue(navigationDto); + it('should set learning objects correctly', async () => { + learningPathNavigationSpy.mockReturnValue(navigationDto); - fixture.detectChanges(); - await fixture.whenStable(); fixture.detectChanges(); expect(component.predecessorLearningObject()).toEqual(navigationDto.predecessorLearningObject); expect(component.currentLearningObject()).toEqual(navigationDto.currentLearningObject); - expect(component.successorLearningObject()).toBeUndefined(); - - const previousButton = fixture.debugElement.query(By.css('#previous-button')); - expect(previousButton).toBeTruthy(); - - const nextButton = fixture.debugElement.query(By.css('#next-button')); - expect(nextButton).toBeFalsy(); - - const completeButton = fixture.debugElement.query(By.css('#complete-button')); - expect(completeButton).toBeTruthy(); + expect(component.successorLearningObject()).toEqual(navigationDto.successorLearningObject); }); - it('should show navigation with only next button', async () => { - const navigationDto = { - currentLearningObject: { - id: 2, - name: 'Lecture', - type: LearningObjectType.LECTURE, - completed: false, - }, - successorLearningObject: { - id: 3, - name: 'Exercise', - type: LearningObjectType.EXERCISE, - completed: false, - }, - progress: 0, - }; - getLearningPathNavigationSpy.mockResolvedValue(navigationDto); - - fixture.detectChanges(); - await fixture.whenStable(); + it('should navigate with next button', async () => { + learningPathNavigationSpy.mockReturnValue(navigationDto); + const loadRelativeLearningPathNavigationSpy = jest.spyOn(learningPathNavigationService, 'loadRelativeLearningPathNavigation'); + const isLoadingSuccessor = jest.spyOn(component.isLoadingSuccessor, 'set'); fixture.detectChanges(); - expect(component.predecessorLearningObject()).toBeUndefined(); - expect(component.currentLearningObject()).toEqual(navigationDto.currentLearningObject); - expect(component.successorLearningObject()).toEqual(navigationDto.successorLearningObject); + const nextButton = fixture.nativeElement.querySelector('#next-button'); + nextButton.click(); - const previousButton = fixture.debugElement.query(By.css('#previous-button')); - expect(previousButton).toBeFalsy(); + await fixture.whenStable(); + fixture.detectChanges(); - const nextButton = fixture.debugElement.query(By.css('#next-button')); - expect(nextButton).toBeTruthy(); + expect(loadRelativeLearningPathNavigationSpy).toHaveBeenCalledExactlyOnceWith(learningPathId, navigationDto.successorLearningObject); + expect(isLoadingSuccessor).toHaveBeenNthCalledWith(1, true); + expect(isLoadingSuccessor).toHaveBeenNthCalledWith(2, false); }); - it('should show navigation overview on click', async () => { - const setIsDropdownOpen = jest.spyOn(component, 'setIsDropdownOpen'); + it('should navigate with previous button', async () => { + learningPathNavigationSpy.mockReturnValue(navigationDto); + const loadRelativeLearningPathNavigationSpy = jest.spyOn(learningPathNavigationService, 'loadRelativeLearningPathNavigation'); + const isLoadingSuccessor = jest.spyOn(component.isLoadingPredecessor, 'set'); fixture.detectChanges(); + + const nextButton = fixture.nativeElement.querySelector('#previous-button'); + nextButton.click(); + await fixture.whenStable(); fixture.detectChanges(); - const navOverviewButton = fixture.debugElement.query(By.css('#navigation-overview')); - navOverviewButton.nativeElement.click(); - fixture.detectChanges(); - const navOverview = fixture.debugElement.query(By.directive(LearningPathNavOverviewComponent)); - expect(navOverview).toBeTruthy(); - expect(setIsDropdownOpen).toHaveBeenCalledWith(true); + expect(loadRelativeLearningPathNavigationSpy).toHaveBeenCalledExactlyOnceWith(learningPathId, navigationDto.predecessorLearningObject); + expect(isLoadingSuccessor).toHaveBeenNthCalledWith(1, true); + expect(isLoadingSuccessor).toHaveBeenNthCalledWith(2, false); }); - it('should call select learning object on previous click', async () => { - const selectLearningObjectSpy = jest.spyOn(component, 'selectLearningObject'); + it('should set current to previous unit on complete button', async () => { + const completeLearningPathSpy = jest.spyOn(learningPathNavigationService, 'completeLearningPath'); + learningPathNavigationSpy.mockReturnValue({ + predecessorLearningObject: { ...navigationDto.predecessorLearningObject }, + currentLearningObject: { ...navigationDto.currentLearningObject }, + progress: 95, + }); - fixture.detectChanges(); - await fixture.whenStable(); fixture.detectChanges(); - const previousButton = fixture.debugElement.query(By.css('#previous-button')); - previousButton.nativeElement.click(); + const completeButton = fixture.nativeElement.querySelector('#complete-button'); + completeButton.click(); - fixture.detectChanges(); + expect(completeLearningPathSpy).toHaveBeenCalledOnce(); + }); + + it('should show navigation overview on click', async () => { + const setIsDropdownOpen = jest.spyOn(component.isDropdownOpen, 'set'); + + component.setIsDropdownOpen(false); - expect(getLearningPathNavigationSpy).toHaveBeenCalledOnce(); - expect(getRelativeLearningPathNavigationSpy).toHaveBeenCalledOnce(); - expect(selectLearningObjectSpy).toHaveBeenCalledWith(navigationDto.predecessorLearningObject); + expect(setIsDropdownOpen).toHaveBeenNthCalledWith(1, false); }); }); From ea6aa3ac4f854f8bb9f8f9247324bf5e137d9b22 Mon Sep 17 00:00:00 2001 From: Johannes Wiest Date: Mon, 21 Oct 2024 08:02:23 +0200 Subject: [PATCH 2/4] fix client style --- .../learning-path-student-nav.component.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts index 5b8b77c84e83..d243c506179b 100644 --- a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts +++ b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.ts @@ -50,9 +50,10 @@ export class LearningPathNavComponent { } async selectLearningObject(selectedLearningObject: LearningPathNavigationObjectDTO, isSuccessor: boolean): Promise { - isSuccessor ? this.isLoadingSuccessor.set(true) : this.isLoadingPredecessor.set(true); + const loadingSpinner = isSuccessor ? this.isLoadingSuccessor : this.isLoadingPredecessor; + loadingSpinner.set(true); await this.learningPathNavigationService.loadRelativeLearningPathNavigation(this.learningPathId(), selectedLearningObject); - isSuccessor ? this.isLoadingSuccessor.set(false) : this.isLoadingPredecessor.set(false); + loadingSpinner.set(false); } completeLearningPath(): void { From 854f28ebfc726eef485efa0e14b8283a5a6ce5ed Mon Sep 17 00:00:00 2001 From: Johannes Wiest Date: Mon, 21 Oct 2024 08:19:55 +0200 Subject: [PATCH 3/4] sort imports --- .../learning-paths/services/learning-path-navigation.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/app/course/learning-paths/services/learning-path-navigation.service.ts b/src/main/webapp/app/course/learning-paths/services/learning-path-navigation.service.ts index 1ecbc1033c33..c07f8dbcee62 100644 --- a/src/main/webapp/app/course/learning-paths/services/learning-path-navigation.service.ts +++ b/src/main/webapp/app/course/learning-paths/services/learning-path-navigation.service.ts @@ -1,4 +1,4 @@ -import { computed, inject, Injectable, signal } from '@angular/core'; +import { Injectable, computed, inject, signal } from '@angular/core'; import { LearningPathNavigationDTO, LearningPathNavigationObjectDTO } from 'app/entities/competency/learning-path.model'; import { AlertService } from 'app/core/util/alert.service'; import { LearningPathApiService } from 'app/course/learning-paths/services/learning-path-api.service'; From c1002964fead389972c7218ec92eb46d5b83038d Mon Sep 17 00:00:00 2001 From: Johannes Wiest Date: Mon, 21 Oct 2024 08:23:45 +0200 Subject: [PATCH 4/4] reformat template with prettier --- .../learning-path-student-nav.component.html | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.html b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.html index f367d7acaa4e..ccf9b94629c5 100644 --- a/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.html +++ b/src/main/webapp/app/course/learning-paths/components/learning-path-student-nav/learning-path-student-nav.component.html @@ -4,8 +4,12 @@
@if (predecessorLearningObject(); as predecessorLearningObject) {
-