General: Fix scroll to section when clicking status bar in exercise u…

…pdate view (#9243)

src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts

@@ -1,4 +1,4 @@
-import { AfterViewInit, ChangeDetectionStrategy, Component, OnDestroy, OnInit, ViewChild } from '@angular/core';
+import { AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, Component, OnDestroy, OnInit, ViewChild } from '@angular/core';
 import { ActivatedRoute, Router } from '@angular/router';
 import { HttpErrorResponse } from '@angular/common/http';
 import { ModelingExercise } from 'app/entities/modeling-exercise.model';
@@ -93,6 +93,7 @@ export class ModelingExerciseUpdateComponent implements AfterViewInit, OnDestroy
         private activatedRoute: ActivatedRoute,
         private router: Router,
         private navigationUtilService: ArtemisNavigationUtilService,
+        private changeDetectorRef: ChangeDetectorRef,
     ) {}
 
     get editType(): EditType {
@@ -243,6 +244,9 @@ export class ModelingExerciseUpdateComponent implements AfterViewInit, OnDestroy
                         !this.modelingExercise.releaseDate?.isValid()),
             },
         ];
+
+        // otherwise the change detection does not work on the initial load
+        this.changeDetectorRef.detectChanges();
     }
 
     /**

src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html

@@ -1,20 +1,19 @@
-<div class="form-status-bar-container pt-2 pb-1">
-    <hr class="form-status-line" />
-    <div class="d-flex justify-content-between">
-        @for (formStatusSection of formStatusSections; track formStatusSection.title) {
-            <div class="d-flex flex-column align-items-center">
-                <div
-                    class="d-flex align-items-center justify-content-center border rounded-circle border-secondary form-status-circle"
-                    (click)="scrollToHeadline(formStatusSection.title)"
-                >
-                    <jhi-checklist-check
-                        [checkAttribute]="formStatusSection.valid"
-                        [iconColor]="formStatusSection.empty && formStatusSection.valid ? 'var(--bs-gray)' : undefined"
-                        class="fa-sm"
-                    />
+@if (formStatusSections) {
+    <div #statusBar class="form-status-bar-container pt-2 pb-1">
+        <hr class="form-status-line" />
+        <div class="d-flex justify-content-between">
+            @for (formStatusSection of formStatusSections; track formStatusSection.title; let index = $index) {
+                <div class="d-flex flex-column align-items-center" id="status-bar-section-item-{{ index }}">
+                    <div class="form-status-circle border rounded-circle" (click)="scrollToHeadline(formStatusSection.title)">
+                        <jhi-checklist-check
+                            [checkAttribute]="formStatusSection.valid"
+                            [iconColor]="formStatusSection.empty && formStatusSection.valid ? 'var(--bs-gray)' : undefined"
+                            class="fa-sm position-absolute top-50 start-50 translate-middle"
+                        />
+                    </div>
+                    <a (click)="scrollToHeadline(formStatusSection.title)" class="d-none d-sm-inline link-primary">{{ formStatusSection.title | artemisTranslate }}</a>
                 </div>
-                <a (click)="scrollToHeadline(formStatusSection.title)" class="d-none d-sm-inline link-primary">{{ formStatusSection.title | artemisTranslate }}</a>
-            </div>
-        }
+            }
+        </div>
     </div>
-</div>
+}

src/main/webapp/app/forms/form-status-bar/form-status-bar.component.scss

@@ -16,8 +16,8 @@
 .form-status-circle {
     position: relative;
     z-index: 8;
-    height: 28px;
-    width: 28px;
+    height: 1.5rem;
+    width: 1.5rem;
     background-color: var(--bs-card-bg);
     cursor: pointer;
 }

src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts

@@ -1,4 +1,5 @@
-import { AfterViewInit, Component, HostListener, Input } from '@angular/core';
+import { AfterViewInit, Component, ElementRef, HostListener, Input, ViewChild } from '@angular/core';
+import { updateHeaderHeight } from 'app/shared/util/navbar.util';
 
 export type FormSectionStatus = {
     title: string;
@@ -15,23 +16,28 @@ export class FormStatusBarComponent implements AfterViewInit {
     @Input()
     formStatusSections: FormSectionStatus[];
 
+    @ViewChild('statusBar', { static: false }) statusBar?: ElementRef;
+
     @HostListener('window:resize')
-    onResize() {
-        setTimeout(() => {
-            const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight;
-            document.documentElement.style.setProperty('--header-height', `${headerHeight}px`);
-        });
+    onResizeAddDistanceFromStatusBarToNavbar() {
+        updateHeaderHeight();
     }
 
     ngAfterViewInit() {
-        this.onResize();
+        this.onResizeAddDistanceFromStatusBarToNavbar();
     }
 
     scrollToHeadline(id: string) {
         const element = document.getElementById(id);
         if (element) {
-            element.style.scrollMarginTop = 'calc(2rem + 78px)';
-            element.scrollIntoView();
+            const navbarHeight = (document.querySelector('jhi-navbar') as HTMLElement)?.getBoundingClientRect().height;
+            const statusBarHeight = this.statusBar?.nativeElement.getBoundingClientRect().height;
+
+            /** Needs to be applied to the scrollMarginTop to ensure that the scroll to element is not hidden behind header elements */
+            const scrollOffsetInPx = navbarHeight + statusBarHeight;
+
+            element.style.scrollMarginTop = `${scrollOffsetInPx}px`;
+            element.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'start' });
         }
     }
 }

src/main/webapp/app/shared/components/checklist-check/checklist-check.component.html

@@ -0,0 +1,6 @@
+@if (checkAttribute) {
+    <fa-icon [icon]="faCheckCircle" [size]="size" [style]="{ color: iconColor ?? 'var(--green)' }" />
+} @else {
+    <fa-icon [icon]="faTimes" [size]="size" class="fs-5" [style]="{ color: iconColor ?? 'var(--markdown-preview-red)' }" />
+}
+&nbsp;

src/main/webapp/app/shared/components/checklist-check.component.ts → src/main/webapp/app/shared/components/checklist-check/checklist-check.component.ts

@@ -5,14 +5,12 @@ import { faCheckCircle, faTimes } from '@fortawesome/free-solid-svg-icons';
 @Component({
     selector: 'jhi-checklist-check',
     templateUrl: './checklist-check.component.html',
-    styleUrls: ['./checklist-check.component.scss'],
 })
 export class ChecklistCheckComponent {
+    protected readonly faTimes = faTimes;
+    protected readonly faCheckCircle = faCheckCircle;
+
     @Input() checkAttribute: boolean | undefined = false;
     @Input() iconColor?: string;
     @Input() size?: SizeProp;
-
-    // Icons
-    faTimes = faTimes;
-    faCheckCircle = faCheckCircle;
 }

src/main/webapp/app/shared/components/shared-component.module.ts

@@ -1,5 +1,5 @@
 import { NgModule } from '@angular/core';
-import { ChecklistCheckComponent } from 'app/shared/components/checklist-check.component';
+import { ChecklistCheckComponent } from 'app/shared/components/checklist-check/checklist-check.component';
 import { ArtemisSharedModule } from 'app/shared/shared.module';
 import { FeatureToggleModule } from 'app/shared/feature-toggle/feature-toggle.module';
 import { ConfirmAutofocusModalComponent } from 'app/shared/components/confirm-autofocus-modal.component';

src/main/webapp/app/shared/detail-overview-navigation-bar/detail-overview-navigation-bar.component.ts

@@ -1,4 +1,5 @@
 import { AfterViewInit, Component, HostListener, Input } from '@angular/core';
+import { updateHeaderHeight } from 'app/shared/util/navbar.util';
 
 @Component({
     selector: 'jhi-detail-overview-navigation-bar',
@@ -10,15 +11,12 @@ export class DetailOverviewNavigationBarComponent implements AfterViewInit {
     sectionHeadlines: { id: string; translationKey: string }[];
 
     @HostListener('window:resize')
-    onResize() {
-        setTimeout(() => {
-            const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight;
-            document.documentElement.style.setProperty('--header-height', `${headerHeight - 2}px`);
-        });
+    onResizeAddDistanceFromStatusBarToNavbar() {
+        updateHeaderHeight();
     }
 
     ngAfterViewInit() {
-        this.onResize();
+        this.onResizeAddDistanceFromStatusBarToNavbar();
     }
 
     scrollToView(id: string) {

src/main/webapp/app/shared/util/navbar.util.ts

@@ -0,0 +1,16 @@
+/**
+ * Update the header height SCSS variable based on the navbar height.
+ *
+ * The navbar height can change based on the screen size and the content of the navbar
+ * (e.g. long breadcrumbs due to longs exercise names)
+ */
+export function updateHeaderHeight() {
+    setTimeout(() => {
+        const navbar = document.querySelector('jhi-navbar');
+        if (navbar) {
+            // do not use navbar.offsetHeight, this might not be defined in Safari!
+            const headerHeight = navbar.getBoundingClientRect().height;
+            document.documentElement.style.setProperty('--header-height', `${headerHeight}px`);
+        }
+    });
+}

src/test/javascript/spec/component/exam/manage/exams/exam-checklist-exercisegroup-table.component.spec.ts

@@ -4,7 +4,7 @@ import { RouterTestingModule } from '@angular/router/testing';
 import { ArtemisDatePipe } from 'app/shared/pipes/artemis-date.pipe';
 import { Component } from '@angular/core';
 import { HasAnyAuthorityDirective } from 'app/shared/auth/has-any-authority.directive';
-import { ChecklistCheckComponent } from 'app/shared/components/checklist-check.component';
+import { ChecklistCheckComponent } from 'app/shared/components/checklist-check/checklist-check.component';
 import { FaIconComponent } from '@fortawesome/angular-fontawesome';
 import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { ProgressBarComponent } from 'app/shared/dashboards/tutor-participation-graph/progress-bar/progress-bar.component';

src/test/javascript/spec/component/exam/manage/exams/exam-detail.component.spec.ts

@@ -9,7 +9,7 @@ import { FaIconComponent } from '@fortawesome/angular-fontawesome';
 import { AccountService } from 'app/core/auth/account.service';
 import { Course } from 'app/entities/course.model';
 import { Exam } from 'app/entities/exam/exam.model';
-import { ChecklistCheckComponent } from 'app/shared/components/checklist-check.component';
+import { ChecklistCheckComponent } from 'app/shared/components/checklist-check/checklist-check.component';
 import { ExamChecklistExerciseGroupTableComponent } from 'app/exam/manage/exams/exam-checklist-component/exam-checklist-exercisegroup-table/exam-checklist-exercisegroup-table.component';
 import { ExamChecklistComponent } from 'app/exam/manage/exams/exam-checklist-component/exam-checklist.component';
 import { ExamDetailComponent } from 'app/exam/manage/exams/exam-detail.component';

src/test/javascript/spec/component/forms/form-status-bar.component.spec.ts

@@ -30,10 +30,6 @@ describe('FormStatusBarComponent', () => {
         jest.restoreAllMocks();
     });
 
-    it('should initializes', () => {
-        expect(comp).toBeDefined();
-    });
-
     it('should scroll to correct headline', () => {
         const mockDOMElement = { scrollIntoView: jest.fn(), style: {} };
         const getElementSpy = jest.spyOn(document, 'getElementById').mockReturnValue(mockDOMElement as any as HTMLElement);

src/test/javascript/spec/component/tutorial-groups/tutorial-groups-management/tutorial-groups/tutorial-groups-management/tutorial-groups-checklist.component.spec.ts

@@ -1,7 +1,7 @@
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { TutorialGroupsChecklistComponent } from 'app/course/tutorial-groups/tutorial-groups-management/tutorial-groups-checklist/tutorial-groups-checklist.component';
 import { CourseManagementService } from 'app/course/manage/course-management.service';
-import { ChecklistCheckComponent } from 'app/shared/components/checklist-check.component';
+import { ChecklistCheckComponent } from 'app/shared/components/checklist-check/checklist-check.component';
 import { MockRouter } from '../../../../../helpers/mocks/mock-router';
 import { MockComponent, MockPipe, MockProvider } from 'ng-mocks';
 import { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe';

src/test/playwright/e2e/course/CourseManagement.spec.ts

@@ -2,10 +2,9 @@ import { test } from '../../support/fixtures';
 import dayjs from 'dayjs';
 import { Course } from 'app/entities/course.model';
 import { admin, studentOne } from '../../support/users';
-import { convertBooleanToCheckIconClass, dayjsToString, generateUUID, trimDate } from '../../support/utils';
+import { base64StringToBlob, convertBooleanToCheckIconClass, dayjsToString, generateUUID, trimDate } from '../../support/utils';
 import { expect } from '@playwright/test';
 import { Fixtures } from '../../fixtures/fixtures';
-import { base64StringToBlob } from '../../support/utils';
 
 // Common primitives
 const courseData = {

src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts

@@ -33,6 +33,36 @@ test.describe('Programming Exercise Management', () => {
             await expect(courseManagementExercises.getExerciseTitle(exerciseTitle)).toBeVisible();
             await page.waitForURL(`**/programming-exercises/${exercise.id}**`);
         });
+
+        test('FormStatusBar scrolls to the correct section with headline visible after scroll', async ({
+            login,
+            page,
+            navigationBar,
+            courseManagement,
+            courseManagementExercises,
+            programmingExerciseCreation,
+        }) => {
+            await login(admin, '/');
+            await navigationBar.openCourseManagement();
+            await courseManagement.openExercisesOfCourse(course.id!);
+            await courseManagementExercises.createProgrammingExercise();
+            await page.waitForURL('**/programming-exercises/new**');
+
+            const firstSectionHeadline = 'General';
+            const firstSectionStatusBarId = 0;
+            const fourthSectionHeadline = 'Problem';
+            const fourthSectionStatusBarId = 3;
+
+            // scroll down
+            await programmingExerciseCreation.clickFormStatusBarSection(fourthSectionStatusBarId);
+            await programmingExerciseCreation.checkIsHeadlineVisibleToUser(firstSectionHeadline, false);
+            await programmingExerciseCreation.checkIsHeadlineVisibleToUser(fourthSectionHeadline, true);
+
+            // scroll up
+            await programmingExerciseCreation.clickFormStatusBarSection(firstSectionStatusBarId);
+            await programmingExerciseCreation.checkIsHeadlineVisibleToUser(firstSectionHeadline, true);
+            await programmingExerciseCreation.checkIsHeadlineVisibleToUser(fourthSectionHeadline, false);
+        });
     });
 
     test.describe('Programming exercise deletion', () => {

src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts

@@ -1,4 +1,4 @@
-import { Page, expect } from '@playwright/test';
+import { Locator, Page, expect } from '@playwright/test';
 import { PROGRAMMING_EXERCISE_BASE, ProgrammingLanguage } from '../../../constants';
 import { Dayjs } from 'dayjs';
 
@@ -73,4 +73,62 @@ export class ProgrammingExerciseCreationPage {
         }
         await expect(this.page.locator('.owl-dt-popup')).not.toBeVisible();
     }
+
+    /**
+     * Uses an element that is affected by the scrolling of the page as reference to determine if the page is still scrolling
+     *
+     * Here we are using the headline of the page as reference
+     */
+    async waitForPageToFinishScrolling(maxTimeout: number = 5000) {
+        const elementOnPageAffectedByScroll = this.page.locator('h2');
+        let isScrolling = true;
+        const startTime = Date.now();
+
+        while (isScrolling) {
+            const initialPosition = await elementOnPageAffectedByScroll.boundingBox();
+            await this.page.waitForTimeout(100); // give the page a short time to scroll
+            const newPosition = await elementOnPageAffectedByScroll.boundingBox();
+
+            isScrolling = initialPosition?.y !== newPosition?.y;
+
+            const isWaitingForScrollExceedingTimeout = Date.now() - startTime > maxTimeout;
+            if (isWaitingForScrollExceedingTimeout) {
+                throw new Error(`Aborting waiting for scroll end - page is still scrolling after ${maxTimeout}ms`);
+            }
+        }
+    }
+
+    async clickFormStatusBarSection(sectionId: number) {
+        const searchedSectionId = `#status-bar-section-item-${sectionId}`;
+        const sectionStatusBarLocator: Locator = this.page.locator(searchedSectionId);
+        expect(await sectionStatusBarLocator.isVisible()).toBeTruthy();
+        await sectionStatusBarLocator.click();
+        await this.waitForPageToFinishScrolling();
+    }
+
+    /**
+     * Verifies that the locator is visible in the viewport and not hidden by another element
+     * (e.g. could be hidden by StatusBar / Navbar)
+     *
+     * {@link toBeHidden} and {@link toBeVisible} do not solve this problem
+     * @param locator
+     */
+    private async verifyLocatorIsVisible(locator: Locator) {
+        const initialPosition = await locator.boundingBox();
+        await locator.click(); // scrolls to the locator if needed (e.g. if hidden by another element)
+        const newPosition = await locator.boundingBox();
+        expect(initialPosition).toEqual(newPosition);
+    }
+
+    async checkIsHeadlineVisibleToUser(searchedHeadlineDisplayText: string, expected: boolean) {
+        const headlineLocator = this.page.getByRole('heading', { name: searchedHeadlineDisplayText }).first();
+
+        if (expected) {
+            await expect(headlineLocator).toBeInViewport({ ratio: 1 });
+            /** additional check because {@link toBeInViewport} is too inaccurate */
+            await this.verifyLocatorIsVisible(headlineLocator);
+        } else {
+            await expect(headlineLocator).not.toBeInViewport();
+        }
+    }
 }

src/test/playwright/support/utils.ts

@@ -87,10 +87,12 @@ export function titleLowercase(title: string) {
 /**
  * Converts a boolean value to its related icon class.
  * @param boolean - The boolean value to be converted.
- * @returns The corresponding ".checked" or ".unchecked" string.
+ * @returns The corresponding icon class
  */
 export function convertBooleanToCheckIconClass(boolean: boolean) {
-    return boolean ? '.checked' : '.unchecked';
+    const sectionInvalidIcon = '.fa-xmark';
+    const sectionValidIcon = '.fa-circle-check';
+    return boolean ? sectionValidIcon : sectionInvalidIcon;
 }
 
 /**