Skip to content

Conversation

@Wout-atmire
Copy link
Contributor

@Wout-atmire Wout-atmire commented Sep 22, 2025

References

Description

Fixes the issue of being redirected to the top of the page after opening a modal, by adding a fragment to the URI of the original position were the modal was opened.

Instructions for Reviewers

List of changes in this PR:

  • Components that may call this.router.navigate when opening a modal are given @Input() retainScrollPosition = false;
    • For example starting from dynamic-lookup-relation-search-tab.component.html
    • This is passed down to more general child-components eg. ds-search-sidebar
    • The idea is to make it so that the child-components know if it is fine to reload to the top of the page or not.
  • Introduced scroll.service.ts allows to scroll to a specific fragment using JS.
    • On navigation angular automatically redirects to the fragment defined in the URI, but Angular doesn't keep the scroll-margin-top which prevents the sticky header from overlapping our fragment in mind.
    • To select a target fragment ScrollService.setFragment is used.
  • If no custom fragment has been set, a fragment of a non-existing id is added: "prevent-scroll"
    • This is to prevent any scrolling to be performed.

Testing

  1. As submitter navigate to the submission forms on a "Publication" collection.
  2. Scroll down and open a modal (eg. for an Author)
    image (1)
  3. When openning the modal the scroll position should not be reset to the top of the page.
  4. Inside the modal, changing pagination options that edit the URL shouldn't remove #prevent-scroll fragment from the URL. And the scroll position should not be reset to the top of the page.
    • Adding search filters
    • Reseting search filters
    • Changing page to another page
    • Changing search order
    • Changing amount of page results

Checklist

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using npm run lint
  • My PR doesn't introduce circular dependencies (verified via npm run check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR aligns with Accessibility guidelines if it makes changes to the user interface.
  • My PR uses i18n (internationalization) keys instead of hardcoded English text, to allow for translations.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

alexandrevryghem and others added 2 commits September 18, 2025 15:44
…olling

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
… into w2p-133535_fix-preserve-scroll-position_contribute-9.x-v2
@Wout-atmire Wout-atmire force-pushed the w2p-133535_fix-preserve-scroll-position_contribute-9.x branch from 9fd42d5 to 6dd25fa Compare September 22, 2025 11:55
@alexandrevryghem alexandrevryghem added component: submission component: configurable entities related to configurable entities port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release labels Sep 22, 2025
@tdonohue tdonohue moved this to 🙋 Needs Reviewers Assigned in DSpace 10.0 Release Sep 22, 2025
@tdonohue tdonohue added the bug label Sep 22, 2025
@alexandrevryghem alexandrevryghem changed the title W2p 133535 fix preserve scroll position contribute 9.x Preserve scroll position when opening/closing the relationship modal Sep 23, 2025
@Wout-atmire Wout-atmire force-pushed the w2p-133535_fix-preserve-scroll-position_contribute-9.x branch from 8a2d708 to fd80742 Compare November 6, 2025 12:51
@alexandrevryghem alexandrevryghem force-pushed the w2p-133535_fix-preserve-scroll-position_contribute-9.x branch from fd80742 to 672ab40 Compare November 7, 2025 10:17
@github-actions
Copy link

Hi @Wout-atmire,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug component: configurable entities related to configurable entities component: submission port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release

Projects

Status: 🙋 Needs Reviewers Assigned

Development

Successfully merging this pull request may close these issues.

Opening a Lookup Relation Modal always brings you to the top of the Submission Form

3 participants