svelte: Remove full page scrolling of repo pages #62024
Conversation
| const extensionsCompartment = createCompartments({ | ||
| selectableLineNumbers: null, | ||
| syntaxHighlighting: null, | ||
| lineWrapping: null, | ||
| temporaryTooltip, | ||
| codeIntelExtension: null, | ||
| staticExtensions, | ||
| staticHighlightExtension: null, | ||
| blameDataExtension: null, | ||
| blameColumnExtension: null, | ||
| }) |
There was a problem hiding this comment.
I had introduced this helper function when refactoring the search input and it seems to make sense to use it here too to get more fine grained control over updating individual extensions.
There was a problem hiding this comment.
The changes in this file make it possible to set the initial selected line without scrolling it into view. It was necessary to do this otherwise the selected line would be scrolled into view after restoring the scroll position (most likely because requestAnimationFrame schedules the update to be run after the scroll restoration).
|
I noticed horizontal scroll for blob view scrolls the whole codemirror blob. I think this is unrelated to this PR (also see it on s2), but wanted to point it out in case it was an easy fix with the new changes |
| const lineWrap = writable<boolean>(false) | ||
| let cmblob: CodeMirrorBlob | null = null | ||
| let blob: Awaited<PageData['blob']> | null = null |
Looks like that only happens when the blame view is enabled. I think we "unfix" the line number gutter when it's enabled, so it scrolls, but I don't know anymore why we do this. |
This commit removes the full page scrolling behavior for repo root, directory and file pages. The implementation had several issues. In the new implementation the scroll container is CodeMirror itself. It provides the following behavior: - Show the top of the file when navigating to a new file. - Scroll the selected line into view when opening a URL containing a position. - Do not scroll when selecting a line (currently broken). - Restore previous scroll position when navigating backward or forward in the browser history. Additional changes in this commit: - Removed logic to enable/disable full page scrolling. - Remove CSS for making elements `sticky`. - Update "scroll selected line into view" logic to prevent scrolling when restoring the previous scroll position. - Update `codemirror/view` to include a recent fix for a scroll related bug.
fkling
force-pushed
the
fkling/sk/remove-full-page-scrolling
branch
from
5cb4a4e to
8c08d08
Compare
This commit removes the full page scrolling behavior for repo root, directory and file pages. The implementation had several issues.
In the new implementation the scroll container is CodeMirror itself. It provides the following behavior:
Additional changes in this commit:
sticky.codemirror/viewto include a recent fix for a scroll related bug.NOTE: Due to these changes we lost scroll restoration for repo root and directory pages. This will be fixed in a separate PR.
Test plan
pnpm testpassesManual testing:
Open file page in new tab -> file is scrolled to the top
Open file page with selected line in new tab -> selected line is scrolled into view
Navigate to new file via file tree -> new file is scrolled to the top
Scroll file, navigate to new file and back -> scroll position is restored
Scroll file, navigate back, navigate forward -> scroll position is restored
Preview panel appears to be working (can scroll and navigate between matches)
Directory page is scrollable
Repo root page is scrollable