From 15996338ff1a47c538bf5806a4f0784c8013762b Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Mon, 25 Aug 2025 11:35:07 +0200 Subject: [PATCH 1/6] 133535: Navigate to a non-existing ID to prevent the browser from scrolling Fix scroll position being lost in selected relationships & imported from external sources tabs 133535: Minor fixes for cleaner diff 133535: fix specs 133535: add scroll-service to specs 133535: add top margin on the submission forms 133535: fix clear pagination 133535: Small fixes 133535: retainScrollPosition for search facet filters & labels 133535: add retainScrollPosition to view-mode-switch.component 133535: use retainScrollPosition for search-switch-configuration.component 133535: use retainScrollPosition for search-filters 133535: preserveFragment when filter reset 133535: fragment in pagination state 133535: do not override input value in themed wrapper 133535: add comments 133535: fix flaky search-form.component specs 133535: fix cannot read ".page" of undefined 133535: keep fragment wen changing page 133535: add fragment to query search 133535: add retainScrollPosition to RPP 133535: add fragment to facetValue 133535: add retainScrollPosition to search components for sort order 133535: add retainScrollPosition to search components 133535: Prevent opening and closing of relationship modal from resetting the scroll position --- .../pagination/pagination.service.spec.ts | 10 +++-- src/app/core/pagination/pagination.service.ts | 41 +++++++++++++------ src/app/core/scroll/scroll.service.ts | 38 +++++++++++++++++ .../core/shared/search/search.service.spec.ts | 14 ++++++- src/app/core/shared/search/search.service.ts | 4 +- ...namic-lookup-relation-modal.component.html | 3 ++ ...dynamic-lookup-relation-modal.component.ts | 27 ++++++------ ...elation-external-source-tab.component.html | 4 +- ...-relation-external-source-tab.component.ts | 7 ++++ ...l-source-entry-import-modal.component.html | 1 + ...nal-source-entry-import-modal.component.ts | 17 +++++--- ...-relation-external-source-tab.component.ts | 16 +++++++- ...-lookup-relation-search-tab.component.html | 1 + ...ic-lookup-relation-search-tab.component.ts | 7 +++- ...ic-lookup-relation-search-tab.component.ts | 6 ++- ...okup-relation-selection-tab.component.html | 3 +- ...lookup-relation-selection-tab.component.ts | 7 +++- .../object-collection.component.html | 1 + .../object-collection.component.ts | 5 +++ .../object-list/object-list.component.html | 1 + .../object-list/object-list.component.ts | 5 +++ .../themed-object-list.component.ts | 6 +++ .../page-size-selector.component.ts | 10 ++++- .../search-form/search-form.component.spec.ts | 30 ++++++++------ .../search-form/search-form.component.ts | 10 +++-- .../themed-search-form.component.ts | 3 ++ .../search-authority-filter.component.html | 4 +- .../search-boolean-filter.component.html | 4 +- .../search-facet-option.component.html | 1 + .../search-facet-option.component.ts | 5 +++ .../search-facet-range-option.component.html | 1 + .../search-facet-range-option.component.ts | 5 +++ ...earch-facet-selected-option.component.html | 1 + .../search-facet-selected-option.component.ts | 5 +++ .../search-facet-filter-wrapper.component.ts | 9 +++- .../search-facet-filter.component.spec.ts | 13 +++++- .../search-facet-filter.component.ts | 22 +++++----- .../search-filter.component.html | 3 +- .../search-filter/search-filter.component.ts | 5 +++ .../search-hierarchy-filter.component.html | 4 +- .../search-hierarchy-filter.component.spec.ts | 7 +++- .../search-hierarchy-filter.component.ts | 21 ++++------ .../search-range-filter.component.html | 2 +- .../search-range-filter.component.spec.ts | 11 +++-- .../search-range-filter.component.ts | 17 ++++---- .../search-text-filter.component.html | 4 +- .../search-filters.component.html | 4 +- .../search-filters.component.ts | 5 +++ .../themed-search-filters.component.ts | 9 +++- .../search-label/search-label.component.html | 1 + .../search-label/search-label.component.ts | 1 + .../search-labels.component.html | 2 +- .../search-labels/search-labels.component.ts | 5 +++ .../search-results.component.html | 1 + .../search-results.component.ts | 5 +++ .../themed-search-results.component.ts | 4 +- .../search-settings.component.html | 2 +- .../search-settings.component.ts | 8 +++- .../themed-search-settings.component.ts | 3 +- .../search-sidebar.component.html | 9 ++-- .../search-sidebar.component.ts | 5 +++ .../themed-search-sidebar.component.ts | 5 ++- ...rch-switch-configuration.component.spec.ts | 1 + .../search-switch-configuration.component.ts | 5 +++ src/app/shared/search/search.component.html | 6 ++- src/app/shared/search/search.component.ts | 5 +++ .../shared/search/themed-search.component.ts | 4 +- .../sidebar/page-with-sidebar.component.ts | 3 ++ src/app/shared/testing/router.stub.ts | 2 +- src/app/shared/testing/scroll-service.stub.ts | 10 +++++ .../view-mode-switch.component.ts | 7 +++- 71 files changed, 403 insertions(+), 130 deletions(-) create mode 100644 src/app/core/scroll/scroll.service.ts create mode 100644 src/app/shared/testing/scroll-service.stub.ts diff --git a/src/app/core/pagination/pagination.service.spec.ts b/src/app/core/pagination/pagination.service.spec.ts index da508d45cda..e19db1fc0a8 100644 --- a/src/app/core/pagination/pagination.service.spec.ts +++ b/src/app/core/pagination/pagination.service.spec.ts @@ -4,12 +4,15 @@ import { of as observableOf } from 'rxjs'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { SortDirection, SortOptions } from '../cache/models/sort-options.model'; import { FindListOptions } from '../data/find-list-options.model'; +import { ScrollService } from '../scroll/scroll.service'; +import { ScrollServiceStub } from '../../shared/testing/scroll-service.stub'; describe('PaginationService', () => { let service: PaginationService; let router; let routeService; + let scrollService: ScrollService; const defaultPagination = new PaginationComponentOptions(); const defaultSort = new SortOptions('dc.title', SortDirection.ASC); @@ -35,8 +38,9 @@ describe('PaginationService', () => { return observableOf(value); } }; + scrollService = new ScrollServiceStub() as any; - service = new PaginationService(routeService, router); + service = new PaginationService(routeService, router, scrollService); }); describe('getCurrentPagination', () => { @@ -69,7 +73,7 @@ describe('PaginationService', () => { return observableOf(value); } }; - service = new PaginationService(routeService, router); + service = new PaginationService(routeService, router, scrollService); service.getCurrentSort('test-id', defaultSort).subscribe((currentSort) => { expect(currentSort).toEqual(defaultSort); @@ -93,7 +97,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); }); }); diff --git a/src/app/core/pagination/pagination.service.ts b/src/app/core/pagination/pagination.service.ts index ce4c3adc3c8..e92a58ffdc5 100644 --- a/src/app/core/pagination/pagination.service.ts +++ b/src/app/core/pagination/pagination.service.ts @@ -1,14 +1,17 @@ -import { Injectable } from '@angular/core'; +import { Injectable, InjectionToken } from '@angular/core'; import { NavigationExtras, Router } from '@angular/router'; import { RouteService } from '../services/route.service'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; -import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; +import { combineLatest as observableCombineLatest, Observable, BehaviorSubject } from 'rxjs'; import { filter, map, take } from 'rxjs/operators'; import { SortDirection, SortOptions } from '../cache/models/sort-options.model'; import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; import { difference } from '../../shared/object.util'; import { FindListOptions } from '../data/find-list-options.model'; import { isNumeric } from '../../shared/numeric.util'; +import { ScrollService } from '../scroll/scroll.service'; + +export const RETAIN_SCROLL_POSITION: InjectionToken> = new InjectionToken('retainScrollPosition'); @Injectable({ providedIn: 'root', @@ -33,7 +36,8 @@ export class PaginationService { private clearParams = {}; constructor(protected routeService: RouteService, - protected router: Router + protected router: Router, + protected scrollService: ScrollService, ) { } @@ -105,9 +109,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 - Scroll to the pagination component after updating the route instead of the top of the page */ - resetPage(paginationId: string) { - this.updateRoute(paginationId, {page: 1}); + resetPage(paginationId: string, retainScrollPosition?: boolean): void { + this.updateRoute(paginationId, { page: 1 }, undefined, retainScrollPosition); } @@ -141,7 +146,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 - Scroll to the active fragment after updating the route instead of the top of the page * @param navigationExtras - Extra parameters to pass on to `router.navigate`. Can be used to override values set by this service. */ updateRouteWithUrl( @@ -161,16 +166,26 @@ 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) { + // By navigating to a non-existing ID, like "prevent-scroll", the browser won't perform any scroll operations + const fragment: string = this.scrollService.activeFragment ?? 'prevent-scroll'; + this.scrollService.setFragment(fragment); this.router.navigate(url, { queryParams: queryParams, queryParamsHandling: 'merge', - fragment: `p-${paginationId}`, + fragment: fragment, ...navigationExtras, + }).then((success: boolean) => { + setTimeout(() => { + if (success) { + this.scrollService.scrollToActiveFragment(); + } + }); }); } else { + this.scrollService.setFragment(null); this.router.navigate(url, { queryParams: queryParams, queryParamsHandling: 'merge', @@ -226,16 +241,16 @@ export class PaginationService { sortDirection?: SortDirection }) { 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..96eaf7388ff --- /dev/null +++ b/src/app/core/scroll/scroll.service.ts @@ -0,0 +1,38 @@ +import { Inject, Injectable } from '@angular/core'; +import { DOCUMENT } from '@angular/common'; + +/** + * 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', + }); + } + } +} diff --git a/src/app/core/shared/search/search.service.spec.ts b/src/app/core/shared/search/search.service.spec.ts index fe5b495ab0c..c6746cc5119 100644 --- a/src/app/core/shared/search/search.service.spec.ts +++ b/src/app/core/shared/search/search.service.spec.ts @@ -141,13 +141,23 @@ describe('SearchService', () => { it('should call the navigate method on the Router with view mode list parameter as a parameter when setViewMode is called', () => { searchService.setViewMode(ViewMode.ListElement); - expect(paginationService.updateRouteWithUrl).toHaveBeenCalledWith('page-id', ['/search'], { page: 1 }, { view: ViewMode.ListElement } + expect(paginationService.updateRouteWithUrl).toHaveBeenCalledWith( + 'page-id', + ['/search'], + { page: 1 }, + { view: ViewMode.ListElement }, + undefined, ); }); it('should call the navigate method on the Router with view mode grid parameter as a parameter when setViewMode is called', () => { searchService.setViewMode(ViewMode.GridElement); - expect(paginationService.updateRouteWithUrl).toHaveBeenCalledWith('page-id', ['/search'], { page: 1 }, { view: ViewMode.GridElement } + expect(paginationService.updateRouteWithUrl).toHaveBeenCalledWith( + 'page-id', + ['/search'], + { page: 1 }, + { view: ViewMode.GridElement }, + undefined, ); }); diff --git a/src/app/core/shared/search/search.service.ts b/src/app/core/shared/search/search.service.ts index a88a8b0d16b..f7851ef3697 100644 --- a/src/app/core/shared/search/search.service.ts +++ b/src/app/core/shared/search/search.service.ts @@ -315,7 +315,7 @@ export class SearchService implements OnDestroy { * @param {ViewMode} viewMode Mode to switch to * @param {string[]} searchLinkParts */ - setViewMode(viewMode: ViewMode, searchLinkParts?: string[]) { + setViewMode(viewMode: ViewMode, searchLinkParts?: string[], retainScrollPosition?: boolean) { this.paginationService.getCurrentPagination(this.searchConfigurationService.paginationID, new PaginationComponentOptions()).pipe(take(1)) .subscribe((config) => { let pageParams = { page: 1 }; @@ -325,7 +325,7 @@ export class SearchService implements OnDestroy { } else if (config.pageSize === 1) { pageParams = Object.assign(pageParams, { pageSize: 10 }); } - this.paginationService.updateRouteWithUrl(this.searchConfigurationService.paginationID, hasValue(searchLinkParts) ? searchLinkParts : [this.getSearchLink()], pageParams, queryParams); + this.paginationService.updateRouteWithUrl(this.searchConfigurationService.paginationID, hasValue(searchLinkParts) ? searchLinkParts : [this.getSearchLink()], pageParams, queryParams, retainScrollPosition); }); } 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 b72a8722aec..0a2e6d1bf04 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 @@ -24,6 +24,7 @@