Skip to content

fix: use replaceState to prevent scroll jump on modal close#200

Merged
rdmueller merged 1 commit intoLLM-Coding:mainfrom
raifdmueller:fix/modal-close-scroll
Mar 15, 2026
Merged

fix: use replaceState to prevent scroll jump on modal close#200
rdmueller merged 1 commit intoLLM-Coding:mainfrom
raifdmueller:fix/modal-close-scroll

Conversation

@raifdmueller
Copy link
Contributor

@raifdmueller raifdmueller commented Mar 15, 2026

Summary

Closing the anchor modal caused the page to scroll to top because window.location.hash = ... triggered a hashchange event, which re-rendered the page.

Fix: Use history.replaceState to update the URL silently — the modal hides, the page stays exactly where it was.

Test plan

  • Open anchor modal from doc page → close → page stays at same scroll position
  • URL updates correctly after modal close

🤖 Generated with Claude Code

Summary by CodeRabbit

Versionshinweise

  • Fehlerbehebungen
    • Optimiertes Schließen von Modalen zur Vermeidung unnötiger Seitenaktualisierungen und verbesserten Navigation.

Using history.replaceState instead of setting window.location.hash
avoids triggering hashchange, which would re-render the page and
scroll to top.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 15, 2026

Überblick

Die closeModal-Funktion in der Ankermanager-Komponente wurde umgestaltet, um history.replaceState() statt Hash-Manipulation für die Navigation zu verwenden. Dies vermeidet das Auslösen von hashchange-Ereignissen und mögliche Re-Renders. Die Scroll-Position-Wiederherstellungslogik wurde entfernt.

Änderungen

Cohort / File(s) Zusammenfassung
Modal Navigation Logic
website/src/components/anchor-modal.js
Refaktorisierung von closeModal: Wechsel von Hash-Manipulation zu history.replaceState() zur Vermeidung von hashchange-Ereignissen; Entfernung der Scroll-Position-Wiederherstellungslogik.

Geschätzter Code Review-Aufwand

🎯 2 (Einfach) | ⏱️ ~10 Minuten

Möglicherweise verwandte PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Der Titel beschreibt präzise die Hauptänderung: Verwendung von replaceState statt direkter Hash-Manipulation, um ein Scroll-Springen beim Schließen des Modals zu verhindern.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
website/src/components/anchor-modal.js (1)

3-3: ⚠️ Potential issue | 🟡 Minor

Unbenutzten Import entfernen

getScrollBeforeModal wird hier nicht verwendet und sollte aus dem Import entfernt werden.

Diff-Vorschlag
-import { getRouteBeforeModal, getScrollBeforeModal } from '../utils/router.js'
+import { getRouteBeforeModal } from '../utils/router.js'

As per coding guidelines website/src/**/*.js: Check for: security issues, unused variables, proper error handling.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/src/components/anchor-modal.js` at line 3, Remove the unused import
getScrollBeforeModal from the import statement in
website/src/components/anchor-modal.js (leave getRouteBeforeModal), search the
file for any remaining references to getScrollBeforeModal and remove or replace
them if present, and run the linter/ESLint to confirm no unused-import warnings
remain and that the module still builds correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@website/src/components/anchor-modal.js`:
- Line 3: Remove the unused import getScrollBeforeModal from the import
statement in website/src/components/anchor-modal.js (leave getRouteBeforeModal),
search the file for any remaining references to getScrollBeforeModal and remove
or replace them if present, and run the linter/ESLint to confirm no
unused-import warnings remain and that the module still builds correctly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 7837bb60-3109-407d-93aa-11f1a556be91

📥 Commits

Reviewing files that changed from the base of the PR and between dd7339b and 58c648b.

📒 Files selected for processing (1)
  • website/src/components/anchor-modal.js

@rdmueller rdmueller merged commit f353fe7 into LLM-Coding:main Mar 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants