From 296a4948c6f583325e8119fffab36a27c9ee761e Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 22 Aug 2024 16:29:47 +0200 Subject: [PATCH 01/27] Fixing scroll so headline is displayed --- .../form-status-bar.component.ts | 21 +++++-------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index 0e41d2f84710..cdbd91367e8c 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -1,4 +1,4 @@ -import { AfterViewInit, Component, HostListener, Input } from '@angular/core'; +import { Component, Input } from '@angular/core'; export type FormSectionStatus = { title: string; @@ -11,27 +11,16 @@ export type FormSectionStatus = { templateUrl: './form-status-bar.component.html', styleUrl: './form-status-bar.component.scss', }) -export class FormStatusBarComponent implements AfterViewInit { +export class FormStatusBarComponent { @Input() formStatusSections: FormSectionStatus[]; - @HostListener('window:resize') - onResize() { - setTimeout(() => { - const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight; - document.documentElement.style.setProperty('--header-height', `${headerHeight}px`); - }); - } - - ngAfterViewInit() { - this.onResize(); - } - scrollToHeadline(id: string) { const element = document.getElementById(id); + if (element) { - element.style.scrollMarginTop = 'calc(2rem + 78px)'; - element.scrollIntoView(); + element.style.scrollMarginTop = 'calc(3rem + 85px)'; + element.scrollIntoView({ behavior: 'smooth', block: 'start' }); } } } From 8e0d6ee2d8119fc9fd9ca032f4384aa5fe4d15f8 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 22 Aug 2024 16:30:50 +0200 Subject: [PATCH 02/27] Removing template test --- .../spec/component/forms/form-status-bar.component.spec.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/javascript/spec/component/forms/form-status-bar.component.spec.ts b/src/test/javascript/spec/component/forms/form-status-bar.component.spec.ts index 102a97560a30..7c7e5dbbdac3 100644 --- a/src/test/javascript/spec/component/forms/form-status-bar.component.spec.ts +++ b/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); From 5f89b1bc67f1e010b9359ab5d2742b9ba65a821b Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 00:21:52 +0200 Subject: [PATCH 03/27] Adding e2e test for scrolling --- .../form-status-bar.component.html | 6 +++--- .../ProgrammingExerciseManagement.spec.ts | 20 +++++++++++++++++++ .../ProgrammingExerciseCreationPage.ts | 19 +++++++++++++++++- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html index a2497a318085..7b21c38adb6e 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html @@ -1,8 +1,8 @@
- @for (formStatusSection of formStatusSections; track formStatusSection.title) { -
+ @for (formStatusSection of formStatusSections; track formStatusSection.title; let i = $index) { + }
diff --git a/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts b/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts index 51e6c7bb0bc2..169bb78c3c61 100644 --- a/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts +++ b/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts @@ -33,6 +33,26 @@ 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', 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 firstScrollToSectionHeadline = 'Problem'; + const secondScrollToSectionHeadline = 'General'; + // scroll down + await programmingExerciseCreation.clickFormStatusBarSection(3); + await programmingExerciseCreation.getHeadlineLocator(firstScrollToSectionHeadline, true); + await programmingExerciseCreation.getHeadlineLocator(secondScrollToSectionHeadline, false); + + // scroll up + // await programmingExerciseCreation.clickFormStatusBarSection(1); + // await programmingExerciseCreation.getHeadlineLocator(firstScrollToSectionHeadline, false); + // await programmingExerciseCreation.getHeadlineLocator(secondScrollToSectionHeadline, true); + }); }); test.describe('Programming exercise deletion', () => { diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index a3e2cf7b0553..4cb32af5f762 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/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,21 @@ export class ProgrammingExerciseCreationPage { } await expect(this.page.locator('.owl-dt-popup')).not.toBeVisible(); } + + async clickFormStatusBarSection(sectionId: number) { + const searchedSectionId = `#section-status-bar-item-${sectionId}`; + const sectionStatusBarLocator: Locator = this.page.locator(searchedSectionId); + expect(await sectionStatusBarLocator.isVisible()).toBeTruthy(); + await sectionStatusBarLocator.click(); + } + + async getHeadlineLocator(searchedHeadlineDisplayText: string, expected: boolean = true) { + const headlineLocator = this.page.getByRole('heading', { name: searchedHeadlineDisplayText }).first(); + await this.page.waitForTimeout(2000); // wait for the scroll animation to finish + if (expected) { + await expect(headlineLocator).toBeInViewport(); + } else { + await expect(headlineLocator).not.toBeInViewport(); + } + } } From c68588212659dbe52a14e7b9564fc1d6d57f827b Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 00:56:41 +0200 Subject: [PATCH 04/27] Calculating header height differently --- .../app/forms/form-status-bar/form-status-bar.component.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index cdbd91367e8c..74ee93128703 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -19,8 +19,9 @@ export class FormStatusBarComponent { const element = document.getElementById(id); if (element) { - element.style.scrollMarginTop = 'calc(3rem + 85px)'; - element.scrollIntoView({ behavior: 'smooth', block: 'start' }); + const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight; + element.style.scrollMarginTop = `calc(3rem + ${headerHeight}px)`; + element.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'start' }); } } } From c8767e49fb3f218b4209bfda07a3b2fecde67ce0 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 01:15:49 +0200 Subject: [PATCH 05/27] Fixing scroll for safari --- .../app/forms/form-status-bar/form-status-bar.component.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index 74ee93128703..b37577cb9352 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -20,7 +20,11 @@ export class FormStatusBarComponent { if (element) { const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight; - element.style.scrollMarginTop = `calc(3rem + ${headerHeight}px)`; + + const isSafari = /^((?!chrome|android).)*safari/i.test(navigator.userAgent); + const offset = isSafari ? 8 : 4; + + element.style.scrollMarginTop = `calc(${offset}rem + ${headerHeight}px)`; element.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'start' }); } } From dfc7149237c624141d66516673c4236f12ef6aed Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 01:29:07 +0200 Subject: [PATCH 06/27] Adjusting e2e test --- .../ProgrammingExerciseManagement.spec.ts | 15 ++++++++------- .../ProgrammingExerciseCreationPage.ts | 4 ++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts b/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts index 169bb78c3c61..48b891edce79 100644 --- a/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts +++ b/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts @@ -41,17 +41,18 @@ test.describe('Programming Exercise Management', () => { await courseManagementExercises.createProgrammingExercise(); await page.waitForURL('**/programming-exercises/new**'); - const firstScrollToSectionHeadline = 'Problem'; - const secondScrollToSectionHeadline = 'General'; + const firstSectionHeadline = 'General'; + const thirdSectionHeadline = 'Problem'; + // scroll down await programmingExerciseCreation.clickFormStatusBarSection(3); - await programmingExerciseCreation.getHeadlineLocator(firstScrollToSectionHeadline, true); - await programmingExerciseCreation.getHeadlineLocator(secondScrollToSectionHeadline, false); + await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(firstSectionHeadline, false); + await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(thirdSectionHeadline, true); // scroll up - // await programmingExerciseCreation.clickFormStatusBarSection(1); - // await programmingExerciseCreation.getHeadlineLocator(firstScrollToSectionHeadline, false); - // await programmingExerciseCreation.getHeadlineLocator(secondScrollToSectionHeadline, true); + await programmingExerciseCreation.clickFormStatusBarSection(0); + await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(firstSectionHeadline, true); + await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(thirdSectionHeadline, false); }); }); diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index 4cb32af5f762..4e40cc9ea215 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts @@ -81,9 +81,9 @@ export class ProgrammingExerciseCreationPage { await sectionStatusBarLocator.click(); } - async getHeadlineLocator(searchedHeadlineDisplayText: string, expected: boolean = true) { + async checkIsHeadlineLocatorInViewport(searchedHeadlineDisplayText: string, expected: boolean) { const headlineLocator = this.page.getByRole('heading', { name: searchedHeadlineDisplayText }).first(); - await this.page.waitForTimeout(2000); // wait for the scroll animation to finish + if (expected) { await expect(headlineLocator).toBeInViewport(); } else { From c77cbabdfeb89f08af9535559ea55cd4c159eba9 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 02:18:49 +0200 Subject: [PATCH 07/27] Make e2e test more reliable --- .../ProgrammingExerciseCreationPage.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index 4e40cc9ea215..062c7da35c6d 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts @@ -79,13 +79,29 @@ export class ProgrammingExerciseCreationPage { const sectionStatusBarLocator: Locator = this.page.locator(searchedSectionId); expect(await sectionStatusBarLocator.isVisible()).toBeTruthy(); await sectionStatusBarLocator.click(); + await this.page.waitForTimeout(1000); // wait for scroll to finish + } + + /** + * Verifies that the locator is visible in the viewport and not hidden by another element + * + * {@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 checkIsHeadlineLocatorInViewport(searchedHeadlineDisplayText: string, expected: boolean) { const headlineLocator = this.page.getByRole('heading', { name: searchedHeadlineDisplayText }).first(); if (expected) { - await expect(headlineLocator).toBeInViewport(); + await expect(headlineLocator).toBeInViewport({ ratio: 1 }); + // additional check because toBeInViewport is to inaccurate + await this.verifyLocatorIsVisible(headlineLocator); } else { await expect(headlineLocator).not.toBeInViewport(); } From 58dc138298fa22dc0bcad3f28e13266ed02546ee Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 03:02:17 +0200 Subject: [PATCH 08/27] Minor refactorings --- .../form-status-bar.component.html | 4 ++-- .../ProgrammingExerciseManagement.spec.ts | 21 +++++++++++++------ .../ProgrammingExerciseCreationPage.ts | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html index 7b21c38adb6e..bc504597a917 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html @@ -1,8 +1,8 @@
- @for (formStatusSection of formStatusSections; track formStatusSection.title; let i = $index) { -
+ @for (formStatusSection of formStatusSections; track formStatusSection.title; let index = $index) { +
{ await page.waitForURL(`**/programming-exercises/${exercise.id}**`); }); - test('FormStatusBar scrolls to the correct section', async ({ login, page, navigationBar, courseManagement, courseManagementExercises, programmingExerciseCreation }) => { + 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!); @@ -42,17 +49,19 @@ test.describe('Programming Exercise Management', () => { await page.waitForURL('**/programming-exercises/new**'); const firstSectionHeadline = 'General'; - const thirdSectionHeadline = 'Problem'; + const firstSectionStatusBarId = 0; + const fourthSectionHeadline = 'Problem'; + const fourthSectionStatusBarId = 3; // scroll down - await programmingExerciseCreation.clickFormStatusBarSection(3); + await programmingExerciseCreation.clickFormStatusBarSection(fourthSectionStatusBarId); await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(firstSectionHeadline, false); - await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(thirdSectionHeadline, true); + await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(fourthSectionHeadline, true); // scroll up - await programmingExerciseCreation.clickFormStatusBarSection(0); + await programmingExerciseCreation.clickFormStatusBarSection(firstSectionStatusBarId); await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(firstSectionHeadline, true); - await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(thirdSectionHeadline, false); + await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(fourthSectionHeadline, false); }); }); diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index 062c7da35c6d..623f4dd36f2e 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts @@ -75,7 +75,7 @@ export class ProgrammingExerciseCreationPage { } async clickFormStatusBarSection(sectionId: number) { - const searchedSectionId = `#section-status-bar-item-${sectionId}`; + const searchedSectionId = `#status-bar-section-item-${sectionId}`; const sectionStatusBarLocator: Locator = this.page.locator(searchedSectionId); expect(await sectionStatusBarLocator.isVisible()).toBeTruthy(); await sectionStatusBarLocator.click(); From 6a4c64348070387241b091f83e45d27b5c5432d4 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 03:16:56 +0200 Subject: [PATCH 09/27] Fixing typo in comment --- .../exercises/programming/ProgrammingExerciseCreationPage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index 623f4dd36f2e..863a98be0f2b 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts @@ -100,7 +100,7 @@ export class ProgrammingExerciseCreationPage { if (expected) { await expect(headlineLocator).toBeInViewport({ ratio: 1 }); - // additional check because toBeInViewport is to inaccurate + /** additional check because {@link toBeInViewport} is too inaccurate */ await this.verifyLocatorIsVisible(headlineLocator); } else { await expect(headlineLocator).not.toBeInViewport(); From 019a2f2ff52bbd655c489d953039f24530bc4f4c Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 13:39:52 +0200 Subject: [PATCH 10/27] Fixing client test --- .../app/forms/form-status-bar/form-status-bar.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index b37577cb9352..4baaa0e14fa9 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -19,7 +19,7 @@ export class FormStatusBarComponent { const element = document.getElementById(id); if (element) { - const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight; + const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement)?.offsetHeight; const isSafari = /^((?!chrome|android).)*safari/i.test(navigator.userAgent); const offset = isSafari ? 8 : 4; From 6b57ff6b183f586272fbba93bbeb1a2f3eab66fe Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 14:31:16 +0200 Subject: [PATCH 11/27] Use a smarter way than a timeout to wait for the scroll to finish --- .../ProgrammingExerciseCreationPage.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index 863a98be0f2b..1a025329a64c 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts @@ -74,12 +74,32 @@ 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() { + const elementOnPageAffectedByScroll = this.page.locator('h2'); + let isScrolling = true; + + while (isScrolling) { + const initialPosition = await elementOnPageAffectedByScroll.boundingBox(); + await this.page.waitForTimeout(100); + const newPosition = await elementOnPageAffectedByScroll.boundingBox(); + console.log(initialPosition); + console.log(newPosition); + + isScrolling = initialPosition?.y !== newPosition?.y; + } + } + 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.page.waitForTimeout(1000); // wait for scroll to finish + await this.waitForPageToFinishScrolling(); } /** From e58c8f86dd2f64d4e76965eaa751d4696d1b75f3 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 14:35:16 +0200 Subject: [PATCH 12/27] Adding timeout to custom function --- .../programming/ProgrammingExerciseCreationPage.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index 1a025329a64c..980595913c1e 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts @@ -79,18 +79,22 @@ export class ProgrammingExerciseCreationPage { * * Here we are using the headline of the page as reference */ - async waitForPageToFinishScrolling() { + 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); + await this.page.waitForTimeout(100); // give the page a short time to scroll const newPosition = await elementOnPageAffectedByScroll.boundingBox(); - console.log(initialPosition); - console.log(newPosition); 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`); + } } } From b320b61ccdb5a3c5e9ec96b7c176eb4131f2581f Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 14:37:28 +0200 Subject: [PATCH 13/27] Removing unintended space --- .../app/forms/form-status-bar/form-status-bar.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html index bc504597a917..65ccbc1b0770 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html @@ -13,7 +13,7 @@ class="fa-sm" />
- {{ formStatusSection.title | artemisTranslate }} + {{ formStatusSection.title | artemisTranslate }}
}
From 212be98f236ee89316e616ccf195cddce9e4d2e0 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 14:39:01 +0200 Subject: [PATCH 14/27] Improve documentation --- .../exercises/programming/ProgrammingExerciseCreationPage.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index 980595913c1e..fc368fce768b 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts @@ -108,6 +108,7 @@ export class ProgrammingExerciseCreationPage { /** * 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 From 7315efc74aeac6016b073f0adad5433f072e9d1b Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 14:40:08 +0200 Subject: [PATCH 15/27] Improve naming of helper methods --- .../programming/ProgrammingExerciseManagement.spec.ts | 8 ++++---- .../programming/ProgrammingExerciseCreationPage.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts b/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts index 775c76bd70b8..184b10e17640 100644 --- a/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts +++ b/src/test/playwright/e2e/exercise/programming/ProgrammingExerciseManagement.spec.ts @@ -55,13 +55,13 @@ test.describe('Programming Exercise Management', () => { // scroll down await programmingExerciseCreation.clickFormStatusBarSection(fourthSectionStatusBarId); - await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(firstSectionHeadline, false); - await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(fourthSectionHeadline, true); + await programmingExerciseCreation.checkIsHeadlineVisibleToUser(firstSectionHeadline, false); + await programmingExerciseCreation.checkIsHeadlineVisibleToUser(fourthSectionHeadline, true); // scroll up await programmingExerciseCreation.clickFormStatusBarSection(firstSectionStatusBarId); - await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(firstSectionHeadline, true); - await programmingExerciseCreation.checkIsHeadlineLocatorInViewport(fourthSectionHeadline, false); + await programmingExerciseCreation.checkIsHeadlineVisibleToUser(firstSectionHeadline, true); + await programmingExerciseCreation.checkIsHeadlineVisibleToUser(fourthSectionHeadline, false); }); }); diff --git a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts index fc368fce768b..a59342550bbf 100644 --- a/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts +++ b/src/test/playwright/support/pageobjects/exercises/programming/ProgrammingExerciseCreationPage.ts @@ -120,7 +120,7 @@ export class ProgrammingExerciseCreationPage { expect(initialPosition).toEqual(newPosition); } - async checkIsHeadlineLocatorInViewport(searchedHeadlineDisplayText: string, expected: boolean) { + async checkIsHeadlineVisibleToUser(searchedHeadlineDisplayText: string, expected: boolean) { const headlineLocator = this.page.getByRole('heading', { name: searchedHeadlineDisplayText }).first(); if (expected) { From 9d7e02774079bb7619e0ec23d1d5012c79cb9be7 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 14:44:28 +0200 Subject: [PATCH 16/27] Removing magic numbers --- .../app/forms/form-status-bar/form-status-bar.component.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index 4baaa0e14fa9..c8cfcedc5df6 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -17,12 +17,13 @@ export class FormStatusBarComponent { scrollToHeadline(id: string) { const element = document.getElementById(id); - if (element) { const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement)?.offsetHeight; const isSafari = /^((?!chrome|android).)*safari/i.test(navigator.userAgent); - const offset = isSafari ? 8 : 4; + const SAFARI_HEADLINE_OFFSET = 8; + const CHROME_HEADLINE_OFFSET = 4; + const offset = isSafari ? SAFARI_HEADLINE_OFFSET : CHROME_HEADLINE_OFFSET; element.style.scrollMarginTop = `calc(${offset}rem + ${headerHeight}px)`; element.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'start' }); From 5115456ee733c79fcc9f36b97f0a928eef151134 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 16:23:48 +0200 Subject: [PATCH 17/27] Addressing Ramonas feedback --- .../form-status-bar.component.html | 7 ++----- .../form-status-bar.component.scss | 4 ++-- .../form-status-bar/form-status-bar.component.ts | 16 ++++++++++++++-- .../checklist-check.component.html | 5 ++--- .../checklist-check.component.scss | 1 + .../checklist-check.component.ts | 7 +++---- .../shared/components/shared-component.module.ts | 2 +- ...ecklist-exercisegroup-table.component.spec.ts | 2 +- .../manage/exams/exam-detail.component.spec.ts | 2 +- .../tutorial-groups-checklist.component.spec.ts | 2 +- 10 files changed, 28 insertions(+), 20 deletions(-) rename src/main/webapp/app/shared/components/{ => checklist-check}/checklist-check.component.html (71%) rename src/main/webapp/app/shared/components/{ => checklist-check}/checklist-check.component.scss (81%) rename src/main/webapp/app/shared/components/{ => checklist-check}/checklist-check.component.ts (84%) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html index 65ccbc1b0770..3a41a6753710 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html @@ -3,14 +3,11 @@
@for (formStatusSection of formStatusSections; track formStatusSection.title; let index = $index) {
-
+
{{ formStatusSection.title | artemisTranslate }} diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.scss b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.scss index c4049cd93601..8a2c32f9716f 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.scss +++ b/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: 24px; + width: 24px; background-color: var(--bs-card-bg); cursor: pointer; } diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index c8cfcedc5df6..336d6ee90db3 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -1,4 +1,4 @@ -import { Component, Input } from '@angular/core'; +import { AfterViewInit, Component, HostListener, Input } from '@angular/core'; export type FormSectionStatus = { title: string; @@ -11,10 +11,22 @@ export type FormSectionStatus = { templateUrl: './form-status-bar.component.html', styleUrl: './form-status-bar.component.scss', }) -export class FormStatusBarComponent { +export class FormStatusBarComponent implements AfterViewInit { @Input() formStatusSections: FormSectionStatus[]; + @HostListener('window:resize') + onResize() { + setTimeout(function addDistanceFromStatusBarToNavbar() { + const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight; + document.documentElement.style.setProperty('--header-height', `${headerHeight}px`); + }); + } + + ngAfterViewInit() { + this.onResize(); + } + scrollToHeadline(id: string) { const element = document.getElementById(id); if (element) { diff --git a/src/main/webapp/app/shared/components/checklist-check.component.html b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.html similarity index 71% rename from src/main/webapp/app/shared/components/checklist-check.component.html rename to src/main/webapp/app/shared/components/checklist-check/checklist-check.component.html index 3dacf1868569..66ff6169ca2d 100644 --- a/src/main/webapp/app/shared/components/checklist-check.component.html +++ b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.html @@ -1,7 +1,6 @@ @if (checkAttribute) { -} -@if (!checkAttribute) { - +} @else { + }   diff --git a/src/main/webapp/app/shared/components/checklist-check.component.scss b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.scss similarity index 81% rename from src/main/webapp/app/shared/components/checklist-check.component.scss rename to src/main/webapp/app/shared/components/checklist-check/checklist-check.component.scss index c9e2a2e6dbd7..25b07610ad7c 100644 --- a/src/main/webapp/app/shared/components/checklist-check.component.scss +++ b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.scss @@ -4,4 +4,5 @@ .unchecked { color: var(--markdown-preview-red); + font-size: 1.5em; } diff --git a/src/main/webapp/app/shared/components/checklist-check.component.ts b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.ts similarity index 84% rename from src/main/webapp/app/shared/components/checklist-check.component.ts rename to src/main/webapp/app/shared/components/checklist-check/checklist-check.component.ts index ce4e07ac614b..870b8aaff46d 100644 --- a/src/main/webapp/app/shared/components/checklist-check.component.ts +++ b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.ts @@ -8,11 +8,10 @@ import { faCheckCircle, faTimes } from '@fortawesome/free-solid-svg-icons'; 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; } diff --git a/src/main/webapp/app/shared/components/shared-component.module.ts b/src/main/webapp/app/shared/components/shared-component.module.ts index a12237b224e7..090ebcf99f3d 100644 --- a/src/main/webapp/app/shared/components/shared-component.module.ts +++ b/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'; diff --git a/src/test/javascript/spec/component/exam/manage/exams/exam-checklist-exercisegroup-table.component.spec.ts b/src/test/javascript/spec/component/exam/manage/exams/exam-checklist-exercisegroup-table.component.spec.ts index 9c934e9c8817..da54d7079565 100644 --- a/src/test/javascript/spec/component/exam/manage/exams/exam-checklist-exercisegroup-table.component.spec.ts +++ b/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'; diff --git a/src/test/javascript/spec/component/exam/manage/exams/exam-detail.component.spec.ts b/src/test/javascript/spec/component/exam/manage/exams/exam-detail.component.spec.ts index 211c90093b8b..1db106be0c5e 100644 --- a/src/test/javascript/spec/component/exam/manage/exams/exam-detail.component.spec.ts +++ b/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.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'; diff --git a/src/test/javascript/spec/component/tutorial-groups/tutorial-groups-management/tutorial-groups/tutorial-groups-management/tutorial-groups-checklist.component.spec.ts b/src/test/javascript/spec/component/tutorial-groups/tutorial-groups-management/tutorial-groups/tutorial-groups-management/tutorial-groups-checklist.component.spec.ts index e71f5029fb9b..b8e4462c8074 100644 --- a/src/test/javascript/spec/component/tutorial-groups/tutorial-groups-management/tutorial-groups/tutorial-groups-management/tutorial-groups-checklist.component.spec.ts +++ b/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'; From f1ed7a4321cf4fdc22b489637b00d54db685879d Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 16:45:54 +0200 Subject: [PATCH 18/27] Improving readability of form-status-bar.component --- .../forms/form-status-bar/form-status-bar.component.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index 336d6ee90db3..82b346eb1a7f 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -6,6 +6,10 @@ export type FormSectionStatus = { empty?: boolean; }; +const SAFARI_USER_AGENT_REGEX: RegExp = /^((?!chrome|android).)*safari/i; +const SAFARI_HEADLINE_OFFSET = 8; +const CHROME_HEADLINE_OFFSET = 4; + @Component({ selector: 'jhi-form-status-bar', templateUrl: './form-status-bar.component.html', @@ -32,9 +36,7 @@ export class FormStatusBarComponent implements AfterViewInit { if (element) { const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement)?.offsetHeight; - const isSafari = /^((?!chrome|android).)*safari/i.test(navigator.userAgent); - const SAFARI_HEADLINE_OFFSET = 8; - const CHROME_HEADLINE_OFFSET = 4; + const isSafari = SAFARI_USER_AGENT_REGEX.test(navigator.userAgent); const offset = isSafari ? SAFARI_HEADLINE_OFFSET : CHROME_HEADLINE_OFFSET; element.style.scrollMarginTop = `calc(${offset}rem + ${headerHeight}px)`; From 175896e90b1405760d5a55c314072a93f1d23f7f Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 16:52:52 +0200 Subject: [PATCH 19/27] Re-adding fix for Safari --- .../form-status-bar/form-status-bar.component.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index 82b346eb1a7f..c73fc52e925f 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -21,6 +21,13 @@ export class FormStatusBarComponent implements AfterViewInit { @HostListener('window:resize') onResize() { + const addingDistanceToNavbarWouldBreakStatusBar = this.isSafari(); + if (addingDistanceToNavbarWouldBreakStatusBar) { + // on Safari the FormStatusBar disappears when the following code is executed, so we skip it + // the only negative consequence is that the FormStatusBar is a bit closer to the navbar than in Chrome/Firefox + return; + } + setTimeout(function addDistanceFromStatusBarToNavbar() { const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight; document.documentElement.style.setProperty('--header-height', `${headerHeight}px`); @@ -31,13 +38,16 @@ export class FormStatusBarComponent implements AfterViewInit { this.onResize(); } + private isSafari() { + return SAFARI_USER_AGENT_REGEX.test(navigator.userAgent); + } + scrollToHeadline(id: string) { const element = document.getElementById(id); if (element) { const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement)?.offsetHeight; - const isSafari = SAFARI_USER_AGENT_REGEX.test(navigator.userAgent); - const offset = isSafari ? SAFARI_HEADLINE_OFFSET : CHROME_HEADLINE_OFFSET; + const offset = this.isSafari() ? SAFARI_HEADLINE_OFFSET : CHROME_HEADLINE_OFFSET; element.style.scrollMarginTop = `calc(${offset}rem + ${headerHeight}px)`; element.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'start' }); From 2260168d5ad0d4a303658c1e754b2d00e4458611 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 23 Aug 2024 20:54:38 +0200 Subject: [PATCH 20/27] Fixing navbar distance for Safari --- .../form-status-bar.component.ts | 17 ++++------------- .../detail-overview-navigation-bar.component.ts | 10 ++++------ src/main/webapp/app/shared/util/navbar.util.ts | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 19 deletions(-) create mode 100644 src/main/webapp/app/shared/util/navbar.util.ts diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index c73fc52e925f..39c38cf07d9f 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/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 { updateHeaderHeightScssVariableBasedOnNavbar } from 'app/shared/util/navbar.util'; export type FormSectionStatus = { title: string; @@ -20,22 +21,12 @@ export class FormStatusBarComponent implements AfterViewInit { formStatusSections: FormSectionStatus[]; @HostListener('window:resize') - onResize() { - const addingDistanceToNavbarWouldBreakStatusBar = this.isSafari(); - if (addingDistanceToNavbarWouldBreakStatusBar) { - // on Safari the FormStatusBar disappears when the following code is executed, so we skip it - // the only negative consequence is that the FormStatusBar is a bit closer to the navbar than in Chrome/Firefox - return; - } - - setTimeout(function addDistanceFromStatusBarToNavbar() { - const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement).offsetHeight; - document.documentElement.style.setProperty('--header-height', `${headerHeight}px`); - }); + onResizeAddDistanceFromStatusBarToNavbar() { + updateHeaderHeightScssVariableBasedOnNavbar(); } ngAfterViewInit() { - this.onResize(); + this.onResizeAddDistanceFromStatusBarToNavbar(); } private isSafari() { diff --git a/src/main/webapp/app/shared/detail-overview-navigation-bar/detail-overview-navigation-bar.component.ts b/src/main/webapp/app/shared/detail-overview-navigation-bar/detail-overview-navigation-bar.component.ts index 78d79942cada..90f63455d514 100644 --- a/src/main/webapp/app/shared/detail-overview-navigation-bar/detail-overview-navigation-bar.component.ts +++ b/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 { updateHeaderHeightScssVariableBasedOnNavbar } 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() { + updateHeaderHeightScssVariableBasedOnNavbar(); } ngAfterViewInit() { - this.onResize(); + this.onResizeAddDistanceFromStatusBarToNavbar(); } scrollToView(id: string) { diff --git a/src/main/webapp/app/shared/util/navbar.util.ts b/src/main/webapp/app/shared/util/navbar.util.ts new file mode 100644 index 000000000000..616e9dada57d --- /dev/null +++ b/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 updateHeaderHeightScssVariableBasedOnNavbar() { + 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`); + } + }); +} From a1d4751a04363ca2569343253a9a6cc587cb22b1 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 29 Aug 2024 11:46:37 +0200 Subject: [PATCH 21/27] Using more robust solution to retrieve scroll offset --- .../form-status-bar.component.html | 2 +- .../form-status-bar.component.scss | 4 ++-- .../form-status-bar.component.ts | 23 +++++++++---------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html index 3a41a6753710..a8fe342b5523 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html @@ -1,4 +1,4 @@ -
+
@for (formStatusSection of formStatusSections; track formStatusSection.title; let index = $index) { diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.scss b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.scss index 8a2c32f9716f..43b8b2720cec 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.scss +++ b/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: 24px; - width: 24px; + height: 1.5rem; + width: 1.5rem; background-color: var(--bs-card-bg); cursor: pointer; } diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index 39c38cf07d9f..10ae9c5077bd 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -1,4 +1,4 @@ -import { AfterViewInit, Component, HostListener, Input } from '@angular/core'; +import { AfterViewInit, Component, ElementRef, HostListener, Input, Renderer2, ViewChild } from '@angular/core'; import { updateHeaderHeightScssVariableBasedOnNavbar } from 'app/shared/util/navbar.util'; export type FormSectionStatus = { @@ -7,10 +7,6 @@ export type FormSectionStatus = { empty?: boolean; }; -const SAFARI_USER_AGENT_REGEX: RegExp = /^((?!chrome|android).)*safari/i; -const SAFARI_HEADLINE_OFFSET = 8; -const CHROME_HEADLINE_OFFSET = 4; - @Component({ selector: 'jhi-form-status-bar', templateUrl: './form-status-bar.component.html', @@ -20,6 +16,10 @@ export class FormStatusBarComponent implements AfterViewInit { @Input() formStatusSections: FormSectionStatus[]; + @ViewChild('statusBar', { static: false }) statusBar: ElementRef; + + constructor(private renderer: Renderer2) {} + @HostListener('window:resize') onResizeAddDistanceFromStatusBarToNavbar() { updateHeaderHeightScssVariableBasedOnNavbar(); @@ -29,18 +29,17 @@ export class FormStatusBarComponent implements AfterViewInit { this.onResizeAddDistanceFromStatusBarToNavbar(); } - private isSafari() { - return SAFARI_USER_AGENT_REGEX.test(navigator.userAgent); - } - scrollToHeadline(id: string) { const element = document.getElementById(id); if (element) { - const headerHeight = (document.querySelector('jhi-navbar') as HTMLElement)?.offsetHeight; + const navbarHeight = (this.renderer.selectRootElement('jhi-navbar', true) as HTMLElement)?.getBoundingClientRect().height; + const breadcrumbContainerHeight = (this.renderer.selectRootElement('.breadcrumb-container', true) as HTMLElement)?.getBoundingClientRect().height; + const statusBarHeight = this.statusBar.nativeElement.getBoundingClientRect().height; - const offset = this.isSafari() ? SAFARI_HEADLINE_OFFSET : CHROME_HEADLINE_OFFSET; + /** Needs to be applied to the scrollMarginTop to ensure that the scroll to element is not hidden behind header elements */ + const scrollOffsetInPx = navbarHeight + breadcrumbContainerHeight + statusBarHeight; - element.style.scrollMarginTop = `calc(${offset}rem + ${headerHeight}px)`; + element.style.scrollMarginTop = `${scrollOffsetInPx}px`; element.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'start' }); } } From 2b6be1559fcd3f05955353f5d27a62ca413d3a11 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Thu, 29 Aug 2024 12:44:19 +0200 Subject: [PATCH 22/27] Fix broken states for formStatusSections --- .../form-status-bar.component.html | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html index a8fe342b5523..edd4cffc59e1 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.html @@ -1,17 +1,19 @@ -
-
-
- @for (formStatusSection of formStatusSections; track formStatusSection.title; let index = $index) { -
-
- +@if (formStatusSections) { +
+
+
+ @for (formStatusSection of formStatusSections; track formStatusSection.title; let index = $index) { + - {{ formStatusSection.title | artemisTranslate }} -
- } + } +
-
+} From 495a83fa307e3a39480453122538a3be03113fd5 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 30 Aug 2024 12:31:13 +0200 Subject: [PATCH 23/27] Fixing client tests by not using renderer2 --- .../forms/form-status-bar/form-status-bar.component.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index 10ae9c5077bd..3f39aabc38b3 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -1,4 +1,4 @@ -import { AfterViewInit, Component, ElementRef, HostListener, Input, Renderer2, ViewChild } from '@angular/core'; +import { AfterViewInit, Component, ElementRef, HostListener, Input, ViewChild } from '@angular/core'; import { updateHeaderHeightScssVariableBasedOnNavbar } from 'app/shared/util/navbar.util'; export type FormSectionStatus = { @@ -18,8 +18,6 @@ export class FormStatusBarComponent implements AfterViewInit { @ViewChild('statusBar', { static: false }) statusBar: ElementRef; - constructor(private renderer: Renderer2) {} - @HostListener('window:resize') onResizeAddDistanceFromStatusBarToNavbar() { updateHeaderHeightScssVariableBasedOnNavbar(); @@ -32,9 +30,9 @@ export class FormStatusBarComponent implements AfterViewInit { scrollToHeadline(id: string) { const element = document.getElementById(id); if (element) { - const navbarHeight = (this.renderer.selectRootElement('jhi-navbar', true) as HTMLElement)?.getBoundingClientRect().height; - const breadcrumbContainerHeight = (this.renderer.selectRootElement('.breadcrumb-container', true) as HTMLElement)?.getBoundingClientRect().height; - const statusBarHeight = this.statusBar.nativeElement.getBoundingClientRect().height; + const navbarHeight = (document.querySelector('jhi-navbar') as HTMLElement)?.getBoundingClientRect().height; + const breadcrumbContainerHeight = (document.querySelector('.breadcrumb-container') 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 + breadcrumbContainerHeight + statusBarHeight; From fc4fd47e9d5cec7daed6fde098e839d7f3bceaa2 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 30 Aug 2024 12:36:03 +0200 Subject: [PATCH 24/27] Not counding breadcrumb container twice --- .../app/forms/form-status-bar/form-status-bar.component.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index 3f39aabc38b3..bb3db0c8bbfb 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -16,7 +16,7 @@ export class FormStatusBarComponent implements AfterViewInit { @Input() formStatusSections: FormSectionStatus[]; - @ViewChild('statusBar', { static: false }) statusBar: ElementRef; + @ViewChild('statusBar', { static: false }) statusBar?: ElementRef; @HostListener('window:resize') onResizeAddDistanceFromStatusBarToNavbar() { @@ -31,11 +31,10 @@ export class FormStatusBarComponent implements AfterViewInit { const element = document.getElementById(id); if (element) { const navbarHeight = (document.querySelector('jhi-navbar') as HTMLElement)?.getBoundingClientRect().height; - const breadcrumbContainerHeight = (document.querySelector('.breadcrumb-container') 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 + breadcrumbContainerHeight + statusBarHeight; + const scrollOffsetInPx = navbarHeight + statusBarHeight; element.style.scrollMarginTop = `${scrollOffsetInPx}px`; element.scrollIntoView({ behavior: 'smooth', block: 'start', inline: 'start' }); From 009509da02fe9eb63fe89ba2ec560eaac9c174cc Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 30 Aug 2024 18:42:52 +0200 Subject: [PATCH 25/27] Address Ramonas feedback --- .../forms/form-status-bar/form-status-bar.component.ts | 4 ++-- .../checklist-check/checklist-check.component.html | 4 ++-- .../checklist-check/checklist-check.component.scss | 8 -------- .../checklist-check/checklist-check.component.ts | 1 - .../detail-overview-navigation-bar.component.ts | 4 ++-- src/main/webapp/app/shared/util/navbar.util.ts | 2 +- 6 files changed, 7 insertions(+), 16 deletions(-) delete mode 100644 src/main/webapp/app/shared/components/checklist-check/checklist-check.component.scss diff --git a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts index bb3db0c8bbfb..ff512756dd83 100644 --- a/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts +++ b/src/main/webapp/app/forms/form-status-bar/form-status-bar.component.ts @@ -1,5 +1,5 @@ import { AfterViewInit, Component, ElementRef, HostListener, Input, ViewChild } from '@angular/core'; -import { updateHeaderHeightScssVariableBasedOnNavbar } from 'app/shared/util/navbar.util'; +import { updateHeaderHeight } from 'app/shared/util/navbar.util'; export type FormSectionStatus = { title: string; @@ -20,7 +20,7 @@ export class FormStatusBarComponent implements AfterViewInit { @HostListener('window:resize') onResizeAddDistanceFromStatusBarToNavbar() { - updateHeaderHeightScssVariableBasedOnNavbar(); + updateHeaderHeight(); } ngAfterViewInit() { diff --git a/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.html b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.html index 66ff6169ca2d..cae4278c4ea8 100644 --- a/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.html +++ b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.html @@ -1,6 +1,6 @@ @if (checkAttribute) { - + } @else { - + }   diff --git a/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.scss b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.scss deleted file mode 100644 index 25b07610ad7c..000000000000 --- a/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.scss +++ /dev/null @@ -1,8 +0,0 @@ -.checked { - color: var(--green); -} - -.unchecked { - color: var(--markdown-preview-red); - font-size: 1.5em; -} diff --git a/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.ts b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.ts index 870b8aaff46d..3718e0f73adf 100644 --- a/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.ts +++ b/src/main/webapp/app/shared/components/checklist-check/checklist-check.component.ts @@ -5,7 +5,6 @@ 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; diff --git a/src/main/webapp/app/shared/detail-overview-navigation-bar/detail-overview-navigation-bar.component.ts b/src/main/webapp/app/shared/detail-overview-navigation-bar/detail-overview-navigation-bar.component.ts index 90f63455d514..e9eb25918015 100644 --- a/src/main/webapp/app/shared/detail-overview-navigation-bar/detail-overview-navigation-bar.component.ts +++ b/src/main/webapp/app/shared/detail-overview-navigation-bar/detail-overview-navigation-bar.component.ts @@ -1,5 +1,5 @@ import { AfterViewInit, Component, HostListener, Input } from '@angular/core'; -import { updateHeaderHeightScssVariableBasedOnNavbar } from 'app/shared/util/navbar.util'; +import { updateHeaderHeight } from 'app/shared/util/navbar.util'; @Component({ selector: 'jhi-detail-overview-navigation-bar', @@ -12,7 +12,7 @@ export class DetailOverviewNavigationBarComponent implements AfterViewInit { @HostListener('window:resize') onResizeAddDistanceFromStatusBarToNavbar() { - updateHeaderHeightScssVariableBasedOnNavbar(); + updateHeaderHeight(); } ngAfterViewInit() { diff --git a/src/main/webapp/app/shared/util/navbar.util.ts b/src/main/webapp/app/shared/util/navbar.util.ts index 616e9dada57d..7d5a0067c4a8 100644 --- a/src/main/webapp/app/shared/util/navbar.util.ts +++ b/src/main/webapp/app/shared/util/navbar.util.ts @@ -4,7 +4,7 @@ * 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 updateHeaderHeightScssVariableBasedOnNavbar() { +export function updateHeaderHeight() { setTimeout(() => { const navbar = document.querySelector('jhi-navbar'); if (navbar) { From 9d814b6f28a4f4b958bc0138667608cf0e2ea821 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Fri, 30 Aug 2024 20:09:31 +0200 Subject: [PATCH 26/27] Fixing e2e test --- src/test/playwright/e2e/course/CourseManagement.spec.ts | 3 +-- src/test/playwright/support/utils.ts | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/playwright/e2e/course/CourseManagement.spec.ts b/src/test/playwright/e2e/course/CourseManagement.spec.ts index 3e308d1b4431..8a7eacab4135 100644 --- a/src/test/playwright/e2e/course/CourseManagement.spec.ts +++ b/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 = { diff --git a/src/test/playwright/support/utils.ts b/src/test/playwright/support/utils.ts index 9d135bbc7282..bc464f447717 100644 --- a/src/test/playwright/support/utils.ts +++ b/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; } /** From 6bd6400ed438dcd8d90e47d6c329c14f68f786a0 Mon Sep 17 00:00:00 2001 From: Florian Glombik Date: Mon, 2 Sep 2024 14:17:12 +0200 Subject: [PATCH 27/27] Fixing change detection for modeling exercise --- .../modeling/manage/modeling-exercise-update.component.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts b/src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts index 4a4488812450..ff7875cf23c6 100644 --- a/src/main/webapp/app/exercises/modeling/manage/modeling-exercise-update.component.ts +++ b/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(); } /**