Preserve scroll position when opening/closing the relationship modal

.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 }}

src/app/core/pagination/pagination.service.spec.ts

@@ -1,10 +1,12 @@
+import { ScrollServiceStub } from '@dspace/core/testing/scroll-service.stub';
 import { of } from 'rxjs';
 
 import {
   SortDirection,
   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', () => {

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<BehaviorSubject<any>> = new InjectionToken<boolean>('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;

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',
+      });
+    });
+  }
+}

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 {

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 {
+  }
+
+}

src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.html

@@ -26,6 +26,7 @@ <h4 class="modal-title" id="modal-title">{{ ('submission.sections.describe.relat
             [item]="item"
             [isEditRelationship]="isEditRelationship"
             [toRemove]="toRemove"
+            [retainScrollPosition]="true"
             (selectObject)="select($event)"
             (deselectObject)="deselect($event)"
             (resultFound)="setTotalInternals($event.pageInfo.totalElements)"
@@ -47,6 +48,7 @@ <h4 class="modal-title" id="modal-title">{{ ('submission.sections.describe.relat
               [context]="context"
               [query]="query"
               [externalSource]="source"
+              [retainScrollPosition]="true"
               (importedObject)="imported($event)"
               class="d-block pt-3">
             </ds-dynamic-lookup-relation-external-source-tab>
@@ -62,6 +64,7 @@ <h4 class="modal-title" id="modal-title">{{ ('submission.sections.describe.relat
               [listId]="listId"
               [relationshipType]="relationshipOptions.relationshipType"
               [repeatable]="repeatable"
+              [retainScrollPosition]="true"
               [context]="context"
               (selectObject)="select($event)"
               (deselectObject)="deselect($event)"

src/app/shared/form/builder/ds-dynamic-form-ui/relation-lookup-modal/dynamic-lookup-relation-modal.component.ts

@@ -13,6 +13,7 @@ import { RequestParam } from '@dspace/core/cache/models/request-param.model';
 import { ExternalSourceDataService } from '@dspace/core/data/external-source-data.service';
 import { FindListOptions } from '@dspace/core/data/find-list-options.model';
 import { PaginatedList } from '@dspace/core/data/paginated-list.model';
+import { PaginationService } from '@dspace/core/pagination/pagination.service';
 import { Context } from '@dspace/core/shared/context.model';
 import { DSpaceObject } from '@dspace/core/shared/dspace-object.model';
 import { ExternalSource } from '@dspace/core/shared/external-source.model';
@@ -218,6 +219,7 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy
     private zone: NgZone,
     private store: Store<AppState>,
     private router: Router,
+    protected paginationService: PaginationService,
   ) {
 
   }
@@ -366,7 +368,10 @@ export class DsDynamicLookupRelationModalComponent implements OnInit, OnDestroy
   }
 
   ngOnDestroy() {
-    this.router.navigate([], {});
+    this.paginationService.clearPagination(this.searchConfigService.paginationID);
+    this.paginationService.updateRoute(this.searchConfigService.paginationID, undefined, undefined, true, {
+      queryParamsHandling: '',
+    });
     Object.values(this.subMap).forEach((subscription) => subscription.unsubscribe());
   }