Skip to content

Preserve scroll position when opening/closing the relationship modal - #4732

Open
Wout-atmire wants to merge 17 commits into
DSpace:mainfrom
atmire:w2p-133535_fix-preserve-scroll-position_contribute-9.x
Open

Preserve scroll position when opening/closing the relationship modal#4732
Wout-atmire wants to merge 17 commits into
DSpace:mainfrom
atmire:w2p-133535_fix-preserve-scroll-position_contribute-9.x

Conversation

@Wout-atmire

@Wout-atmire Wout-atmire commented Sep 22, 2025 •
edited by alexandrevryghem
Loading

Copy link
Copy Markdown
Contributor

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
@alexandrevryghem @Wout-atmire
133535: Navigate to a non-existing ID to prevent the browser from scr…
1599633 
…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
@Wout-atmire
Merge branch 'w2p-133535_fix-preserve-scroll-position_contribute-7.6'…
011df62 
… 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 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

github-actions Bot commented Nov 10, 2025

Copy link
Copy Markdown

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

@jlipka

jlipka commented Mar 11, 2026

Copy link
Copy Markdown
Contributor

I just tested the behaviour, and the scroll position seems to be preserved while using Chrome and Edge browser, but using Firefox the ui still jumps to the top of the page. (Linux / ubuntu)

@kanasznagyzoltan

kanasznagyzoltan commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

For me is working: #prevent-scroll is appearing in the background and the mainform behind the modal is not jumping back to the top.

pr_4732.mp4

nathanmlf
nathanmlf reviewed May 15, 2026
View reviewed changes

@nathanmlf nathanmlf left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Wout-atmire! I've tested it on Chrome just like @kanasznagyzoltan and on Firefox like @jlipka . I had the same problem in Firefox, but it works as expected in Chrome.

Just to add to the investigation, my tests were on Windows 11 (I don't think the Firefox issue is specific to Linux). I've attached both videos below.

Firefox (Version 150.0.3 - 64 bits)

pr_4732_firefox.mp4

Google Chrome (Version 148.0.7778.97 - 64 bits)

pr_4732_chrome.mp4

@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown

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

@lgeggleston lgeggleston moved this to 👀 Under Review in DSpace 11.0 Release Jun 2, 2026
@lgeggleston lgeggleston added port to dspace-10_x This PR needs to be ported to `dspace-10_x` branch for next bug-fix release and removed port to dspace-7_x labels Jun 2, 2026
@Wout-atmire

Wout-atmire commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Given #6108, scroll position retention in submission may be unnecessary, so I’m putting this PR on hold until that is merged and we can confirm whether anything is still needed here.

Credits to @alexandrevryghem for pointing this out

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

Reviewers

1 more reviewer
@nathanmlf nathanmlf nathanmlf left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@Wout-atmire Wout-atmire

Labels

bug component: configurable entities related to configurable entities component: submission merge conflict 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 port to dspace-10_x This PR needs to be ported to `dspace-10_x` branch for next bug-fix release

Projects

DSpace 11.0 Release
Status: 👀 Under Review

Milestone

No milestone

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

7 participants

@Wout-atmire @jlipka @kanasznagyzoltan @nathanmlf @tdonohue @alexandrevryghem @lgeggleston