diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4b1e66a4466..0cbf8aa8e7f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -113,7 +113,7 @@ jobs: # so that it can be shared with the 'codecov' job (see below) # NOTE: Angular CLI only supports code coverage for specs. See https://github.com/angular/angular-cli/issues/6286 - name: Upload code coverage report to Artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 if: matrix.node-version == '18.x' with: name: coverage-report-${{ matrix.node-version }} @@ -155,7 +155,7 @@ jobs: # Cypress always creates a video of all e2e tests (whether they succeeded or failed) # Save those in an Artifact - name: Upload e2e test videos to Artifacts - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 if: always() with: name: e2e-test-videos-${{ matrix.node-version }} @@ -164,7 +164,7 @@ jobs: # If e2e tests fail, Cypress creates a screenshot of what happened # Save those in an Artifact - name: Upload e2e test failure screenshots to Artifacts - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 if: failure() with: name: e2e-test-screenshots-${{ matrix.node-version }} diff --git a/src/app/core/pagination/pagination.service.spec.ts b/src/app/core/pagination/pagination.service.spec.ts index cb9a7fb7319..a47431e0353 100644 --- a/src/app/core/pagination/pagination.service.spec.ts +++ b/src/app/core/pagination/pagination.service.spec.ts @@ -1,3 +1,4 @@ +import { ScrollServiceStub } from '@dspace/core/testing/scroll-service.stub'; import { of } from 'rxjs'; import { @@ -5,6 +6,7 @@ import { SortOptions, } from '../cache/models/sort-options.model'; import { FindListOptions } from '../data/find-list-options.model'; +import { ScrollService } from '../scroll/scroll.service'; import { RouterStub } from '../testing/router.stub'; import { PaginationService } from './pagination.service'; import { PaginationComponentOptions } from './pagination-component-options.model'; @@ -14,6 +16,7 @@ describe('PaginationService', () => { let service: PaginationService; let router; let routeService; + let scrollService: ScrollService; const defaultPagination = new PaginationComponentOptions(); const defaultSort = new SortOptions('dc.title', SortDirection.ASC); @@ -39,8 +42,9 @@ describe('PaginationService', () => { return of(value); }, }; + scrollService = new ScrollServiceStub() as any; - service = new PaginationService(routeService, router); + service = new PaginationService(routeService, router, scrollService); }); describe('getCurrentPagination', () => { @@ -73,7 +77,7 @@ describe('PaginationService', () => { return of(value); }, }; - service = new PaginationService(routeService, router); + service = new PaginationService(routeService, router, scrollService); service.getCurrentSort('test-id', defaultSort).subscribe((currentSort) => { expect(currentSort).toEqual(defaultSort); @@ -97,7 +101,7 @@ describe('PaginationService', () => { spyOn(service, 'updateRoute'); service.resetPage('test'); - expect(service.updateRoute).toHaveBeenCalledWith('test', { page: 1 }); + expect(service.updateRoute).toHaveBeenCalledWith('test', { page: 1 }, undefined, undefined); }); }); @@ -135,6 +139,25 @@ describe('PaginationService', () => { expect(router.navigate).toHaveBeenCalledWith([], { queryParams: navigateParams, queryParamsHandling: 'preserve', replaceUrl: true, preserveFragment: true }); }); + it('should restore the scroll position when retainScrollPosition is true', async () => { + spyOn(scrollService, 'getScrollPosition').and.returnValue([10, 250]); + spyOn(scrollService, 'restoreScrollPosition'); + + service.updateRoute('test', { page: 2 }, undefined, true); + + const navigateParams = {}; + navigateParams[`test.page`] = `2`; + navigateParams[`test.rpp`] = `10`; + navigateParams[`test.sf`] = `score`; + navigateParams[`test.sd`] = `ASC`; + + expect(router.navigate).toHaveBeenCalledWith([], { + queryParams: navigateParams, + queryParamsHandling: 'merge', + }); + await Promise.resolve(); + expect(scrollService.restoreScrollPosition).toHaveBeenCalledWith([10, 250]); + }); }); describe('updateRouteWithUrl', () => { it('should update the route with the provided page params and url', () => { diff --git a/src/app/core/pagination/pagination.service.ts b/src/app/core/pagination/pagination.service.ts index 0590031af45..a166d55e871 100644 --- a/src/app/core/pagination/pagination.service.ts +++ b/src/app/core/pagination/pagination.service.ts @@ -1,4 +1,7 @@ -import { Injectable } from '@angular/core'; +import { + Injectable, + InjectionToken, +} from '@angular/core'; import { NavigationExtras, Router, @@ -11,6 +14,7 @@ import { import { isNumeric } from '@dspace/shared/utils/numeric.util'; import { difference } from '@dspace/shared/utils/object.util'; import { + BehaviorSubject, combineLatest as observableCombineLatest, Observable, } from 'rxjs'; @@ -25,10 +29,13 @@ import { SortOptions, } from '../cache/models/sort-options.model'; import { FindListOptions } from '../data/find-list-options.model'; +import { ScrollService } from '../scroll/scroll.service'; import { RouteService } from '../services/route.service'; import { PaginationComponentOptions } from './pagination-component-options.model'; import { PaginationRouteParams } from './pagination-route-params.interface'; +export const RETAIN_SCROLL_POSITION: InjectionToken> = new InjectionToken('retainScrollPosition'); + @Injectable({ providedIn: 'root', }) @@ -53,6 +60,7 @@ export class PaginationService { constructor(protected routeService: RouteService, protected router: Router, + protected scrollService: ScrollService, ) { } @@ -124,9 +132,10 @@ export class PaginationService { /** * Reset the current page for the provided pagination ID to 1. * @param paginationId - The pagination id for which to reset the page + * @param retainScrollPosition - Keep the current scroll position after updating the route instead of jumping to the top */ - resetPage(paginationId: string) { - this.updateRoute(paginationId, { page: 1 }); + resetPage(paginationId: string, retainScrollPosition?: boolean): void { + this.updateRoute(paginationId, { page: 1 }, undefined, retainScrollPosition); } @@ -135,7 +144,7 @@ export class PaginationService { * @param paginationId - The pagination ID for which to update the route with info * @param params - The page related params to update in the route * @param extraParams - Addition params unrelated to the pagination that need to be added to the route - * @param retainScrollPosition - Scroll to the pagination component after updating the route instead of the top of the page + * @param retainScrollPosition - Keep the current scroll position after updating the route instead of jumping to the top * @param navigationExtras - Extra parameters to pass on to `router.navigate`. Can be used to override values set by this service. */ updateRoute( @@ -155,7 +164,7 @@ export class PaginationService { * @param url - The url to navigate to * @param params - The page related params to update in the route * @param extraParams - Addition params unrelated to the pagination that need to be added to the route - * @param retainScrollPosition - Scroll to the pagination component after updating the route instead of the top of the page + * @param retainScrollPosition - Keep the current scroll position (or scroll to the active fragment) after updating the route * @param navigationExtras - Extra parameters to pass on to `router.navigate`. Can be used to override values set by this service. */ updateRouteWithUrl( @@ -170,16 +179,31 @@ export class PaginationService { const currentParametersWithIdName = this.getParametersWithIdName(paginationId, currentFindListOptions); const parametersWithIdName = this.getParametersWithIdName(paginationId, params); if (isNotEmpty(difference(parametersWithIdName, currentParametersWithIdName)) || isNotEmpty(extraParams) || isNotEmpty(this.clearParams)) { - const queryParams = Object.assign({}, this.clearParams, currentParametersWithIdName, - parametersWithIdName, extraParams); + const queryParams = Object.assign({}, currentParametersWithIdName, + parametersWithIdName, extraParams, this.clearParams); if (retainScrollPosition) { + // Capture and restore the scroll position instead. + const fragment = this.scrollService.activeFragment; + const scrollPosition = this.scrollService.getScrollPosition(); + this.scrollService.setFragment(fragment); this.router.navigate(url, { queryParams: queryParams, queryParamsHandling: 'merge', - fragment: `p-${paginationId}`, + ...(fragment ? { fragment } : {}), ...navigationExtras, + }).then((success: boolean) => { + if (!success) { + return; + } + + if (fragment) { + this.scrollService.scrollToActiveFragment(); + } else { + this.scrollService.restoreScrollPosition(scrollPosition); + } }); } else { + this.scrollService.setFragment(null); this.router.navigate(url, { queryParams: queryParams, queryParamsHandling: 'merge', @@ -230,16 +254,16 @@ export class PaginationService { private getParametersWithIdName(paginationId: string, params: PaginationRouteParams) { const paramsWithIdName = {}; - if (hasValue(params.page)) { + if (hasValue(params?.page)) { paramsWithIdName[`${paginationId}.page`] = `${params.page}`; } - if (hasValue(params.pageSize)) { + if (hasValue(params?.pageSize)) { paramsWithIdName[`${paginationId}.rpp`] = `${params.pageSize}`; } - if (hasValue(params.sortField)) { + if (hasValue(params?.sortField)) { paramsWithIdName[`${paginationId}.sf`] = `${params.sortField}`; } - if (hasValue(params.sortDirection)) { + if (hasValue(params?.sortDirection)) { paramsWithIdName[`${paginationId}.sd`] = `${params.sortDirection}`; } return paramsWithIdName; diff --git a/src/app/core/scroll/scroll.service.ts b/src/app/core/scroll/scroll.service.ts new file mode 100644 index 00000000000..fe64ed148c3 --- /dev/null +++ b/src/app/core/scroll/scroll.service.ts @@ -0,0 +1,73 @@ +import { DOCUMENT } from '@angular/common'; +import { + Inject, + Injectable, +} from '@angular/core'; + +/** + * Service used to scroll to a specific fragment/ID on the page + */ +@Injectable({ + providedIn: 'root', +}) +export class ScrollService { + + activeFragment: string | null = null; + + constructor( + @Inject(DOCUMENT) protected document: Document, + ) { + } + + /** + * Sets the fragment/ID that the user should jump to when the route is refreshed + * + * @param fragment The fragment/ID + */ + setFragment(fragment: string): void { + this.activeFragment = fragment; + } + + /** + * Scrolls to the active fragment/ID if it exists + */ + scrollToActiveFragment(): void { + if (this.activeFragment) { + this.document.getElementById(this.activeFragment)?.scrollIntoView({ + block: 'start', + }); + } + } + + /** + * Returns the current window scroll position as [x, y] + */ + getScrollPosition(): [number, number] { + return [ + this.document.defaultView?.scrollX ?? 0, + this.document.defaultView?.scrollY ?? 0, + ]; + } + + /** + * Restores the window scroll position instantly (no smooth scroll). + * + * @param position The [x, y] scroll coordinates to restore + */ + restoreScrollPosition([x, y]: [number, number]): void { + const window = this.document.defaultView; + + if (!window) { + return; + } + + // this timeout runs after the scroll-to-top. + setTimeout(() => { + window.scrollTo({ + left: x, + top: y, + behavior: 'instant', + }); + }); + } +} diff --git a/src/app/core/testing/router.stub.ts b/src/app/core/testing/router.stub.ts index 3fe64c481c8..e0e3f3654a1 100644 --- a/src/app/core/testing/router.stub.ts +++ b/src/app/core/testing/router.stub.ts @@ -4,7 +4,7 @@ export class RouterStub { url: string; routeReuseStrategy = { shouldReuseRoute: {} }; //noinspection TypeScriptUnresolvedFunction - navigate = jasmine.createSpy('navigate'); + navigate = jasmine.createSpy('navigate').and.returnValue(Promise.resolve(true)); parseUrl = jasmine.createSpy('parseUrl'); events = of({}); navigateByUrl(url): void { diff --git a/src/app/core/testing/scroll-service.stub.ts b/src/app/core/testing/scroll-service.stub.ts new file mode 100644 index 00000000000..ade5f179e85 --- /dev/null +++ b/src/app/core/testing/scroll-service.stub.ts @@ -0,0 +1,20 @@ +/* eslint-disable no-empty, @typescript-eslint/no-empty-function */ +export class ScrollServiceStub { + + activeFragment: string | null = null; + + setFragment(fragment: string): void { + this.activeFragment = fragment; + } + + scrollToActiveFragment(): void { + } + + getScrollPosition(): [number, number] { + return [0, 0]; + } + + restoreScrollPosition(_position: [number, number]): void { + } + +} diff --git a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html index 9e2a8fe5137..4dd29754ea5 100644 --- a/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html +++ b/src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html @@ -26,6 +26,7 @@