Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit ff1ea37

Browse files
authored
svelte: Remove full page scrolling of repo pages (#62024)
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. - Toggling the blame view maintains the current line view (well, almost but that's as best as we can do at the moment). 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.
1 parent a2b170d commit ff1ea37

File tree

13 files changed

+488
-233
lines changed

13 files changed

+488
-233
lines changed

client/web-sveltekit/src/lib/CodeMirrorBlob.svelte

Lines changed: 114 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@
2828
languages: string[]
2929
}
3030
31-
const extensionsCompartment = new Compartment()
32-
3331
const defaultTheme = EditorView.theme({
3432
'&': {
3533
height: '100%',
@@ -42,6 +40,7 @@
4240
lineHeight: '1rem',
4341
fontFamily: 'var(--code-font-family)',
4442
fontSize: 'var(--code-font-size)',
43+
overflow: 'auto',
4544
},
4645
'.cm-content': {
4746
padding: 0,
@@ -106,20 +105,12 @@
106105
defaultTheme,
107106
linkify,
108107
]
109-
110-
function configureSyntaxHighlighting(content: string, lsif: string): Extension {
111-
return lsif ? syntaxHighlight.of({ content, lsif }) : []
112-
}
113-
114-
function configureMiscSettings({ wrapLines }: { wrapLines: boolean }): Extension {
115-
return [wrapLines ? EditorView.lineWrapping : []]
116-
}
117108
</script>
118109

119110
<script lang="ts">
120111
import '$lib/highlight.scss'
121112
122-
import { Compartment, EditorState, type Extension } from '@codemirror/state'
113+
import { EditorState, type Extension } from '@codemirror/state'
123114
import { EditorView } from '@codemirror/view'
124115
import { createEventDispatcher, onMount } from 'svelte'
125116
@@ -136,14 +127,22 @@
136127
linkify,
137128
createCodeIntelExtension,
138129
syncSelection,
139-
temporaryTooltip,
140130
showBlame as showBlameColumn,
141131
blameData as blameDataFacet,
142132
type BlameHunkData,
133+
lockFirstVisibleLine,
134+
temporaryTooltip,
143135
} from '$lib/web'
144136
145137
import BlameDecoration from './blame/BlameDecoration.svelte'
146138
import { type Range, staticHighlights } from './codemirror/static-highlights'
139+
import {
140+
createCompartments,
141+
restoreScrollSnapshot,
142+
type ExtensionType,
143+
type ScrollSnapshot,
144+
getScrollSnapshot as getScrollSnapshot_internal,
145+
} from './codemirror/utils'
147146
import { goToDefinition, openImplementations, openReferences } from './repo/blob'
148147
149148
export let blobInfo: BlobInfo
@@ -152,21 +151,34 @@
152151
export let selectedLines: LineOrPositionOrRange | null = null
153152
export let codeIntelAPI: CodeIntelAPI
154153
export let staticHighlightRanges: Range[] = []
154+
/**
155+
* The initial scroll position when the editor is first mounted.
156+
* Changing the value afterwards has no effect.
157+
*/
158+
export let initialScrollPosition: ScrollSnapshot | null = null
155159
156160
export let showBlame: boolean = false
157161
export let blameData: BlameHunkData | undefined = undefined
158162
163+
export function getScrollSnapshot(): ScrollSnapshot | null {
164+
return view ? getScrollSnapshot_internal(view) : null
165+
}
166+
159167
const dispatch = createEventDispatcher<{ selectline: SelectedLineRange }>()
168+
const extensionsCompartment = createCompartments({
169+
selectableLineNumbers: null,
170+
syntaxHighlighting: null,
171+
lineWrapping: null,
172+
temporaryTooltip,
173+
codeIntelExtension: null,
174+
staticExtensions,
175+
staticHighlightExtension: null,
176+
blameDataExtension: null,
177+
blameColumnExtension: null,
178+
})
160179
161-
let editor: EditorView
162180
let container: HTMLDivElement | null = null
163-
164-
const lineNumbers = selectableLineNumbers({
165-
onSelection(range) {
166-
dispatch('selectline', range)
167-
},
168-
initialSelection: selectedLines?.line === undefined ? null : selectedLines,
169-
})
181+
let view: EditorView | undefined = undefined
170182
171183
$: documentInfo = {
172184
repoName: blobInfo.repoName,
@@ -198,8 +210,8 @@
198210
}
199211
},
200212
})
201-
$: settings = configureMiscSettings({ wrapLines })
202-
$: sh = configureSyntaxHighlighting(blobInfo.content, highlights)
213+
$: lineWrapping = wrapLines ? EditorView.lineWrapping : []
214+
$: syntaxHighlighting = highlights ? syntaxHighlight.of({ content: blobInfo.content, lsif: highlights }) : []
203215
$: staticHighlightExtension = staticHighlights(staticHighlightRanges)
204216
205217
$: blameColumnExtension = showBlame
@@ -216,51 +228,96 @@
216228
: []
217229
$: blameDataExtension = blameDataFacet(blameData)
218230
219-
$: extensions = [
220-
sh,
221-
settings,
222-
lineNumbers,
223-
temporaryTooltip,
224-
codeIntelExtension,
225-
staticExtensions,
226-
staticHighlightExtension,
227-
blameColumnExtension,
228-
blameDataExtension,
229-
]
231+
// Reinitialize the editor when its content changes. Update only the extensions when they change.
232+
$: update(view => {
233+
// blameColumnExtension is omitted here. It's updated separately below because we need to
234+
// apply additional effects when it changes (but only when it changes).
235+
const extensions: Partial<ExtensionType<typeof extensionsCompartment>> = {
236+
codeIntelExtension,
237+
lineWrapping,
238+
syntaxHighlighting,
239+
staticHighlightExtension,
240+
blameDataExtension,
241+
}
242+
if (view.state.sliceDoc() !== blobInfo.content) {
243+
view.setState(createEditorState(blobInfo, extensions))
244+
} else {
245+
extensionsCompartment.update(view, extensions)
246+
}
247+
})
230248
231-
function update(blobInfo: BlobInfo, extensions: Extension, range: LineOrPositionOrRange | null) {
232-
if (editor) {
233-
// TODO(fkling): Find a way to combine this into a single transaction.
234-
if (editor.state.sliceDoc() !== blobInfo.content) {
235-
editor.setState(
236-
EditorState.create({ doc: blobInfo.content, extensions: extensionsCompartment.of(extensions) })
237-
)
238-
} else {
239-
editor.dispatch({ effects: [extensionsCompartment.reconfigure(extensions)] })
240-
}
241-
editor.dispatch({
242-
effects: setSelectedLines.of(range?.line && isValidLineRange(range, editor.state.doc) ? range : null),
243-
})
249+
// Show/hide the blame column and ensure that the style changes do not change the scroll position
250+
$: update(view => {
251+
extensionsCompartment.update(view, { blameColumnExtension }, ...lockFirstVisibleLine(view))
252+
})
244253
245-
if (range) {
246-
syncSelection(editor, range)
247-
}
254+
// Update the selected lines. This will scroll the selected lines into view. Also set the editor's
255+
// selection (essentially the cursor position) to the selected lines. This is necessary in case the
256+
// selected range references a symbol.
257+
$: update(view => {
258+
view.dispatch({
259+
effects: setSelectedLines.of(
260+
selectedLines?.line && isValidLineRange(selectedLines, view.state.doc) ? selectedLines : null
261+
),
262+
})
263+
if (selectedLines) {
264+
syncSelection(view, selectedLines)
248265
}
249-
}
250-
251-
$: update(blobInfo, extensions, selectedLines)
266+
})
252267
253268
onMount(() => {
254269
if (container) {
255-
editor = new EditorView({
256-
state: EditorState.create({ doc: blobInfo.content, extensions: extensionsCompartment.of(extensions) }),
270+
view = new EditorView({
271+
// On first render initialize all extensions
272+
state: createEditorState(blobInfo, {
273+
codeIntelExtension,
274+
lineWrapping,
275+
syntaxHighlighting,
276+
staticHighlightExtension,
277+
blameDataExtension,
278+
blameColumnExtension,
279+
}),
257280
parent: container,
258281
})
259282
if (selectedLines) {
260-
syncSelection(editor, selectedLines)
283+
syncSelection(view, selectedLines)
261284
}
285+
if (initialScrollPosition) {
286+
restoreScrollSnapshot(view, initialScrollPosition)
287+
}
288+
}
289+
return () => {
290+
view?.destroy()
262291
}
263292
})
293+
294+
// Helper function to update the editor state whithout depending on the view variable
295+
// (those updates should only run on subsequent updates)
296+
function update(updater: (view: EditorView) => void) {
297+
if (view) {
298+
updater(view)
299+
}
300+
}
301+
302+
function createEditorState(blobInfo: BlobInfo, extensions: Partial<ExtensionType<typeof extensionsCompartment>>) {
303+
return EditorState.create({
304+
doc: blobInfo.content,
305+
extensions: extensionsCompartment.init({
306+
selectableLineNumbers: selectableLineNumbers({
307+
onSelection(range) {
308+
dispatch('selectline', range)
309+
},
310+
initialSelection: selectedLines?.line === undefined ? null : selectedLines,
311+
// We don't want to scroll the selected line into view when a scroll position is explicitly set.
312+
skipInitialScrollIntoView: initialScrollPosition !== null,
313+
}),
314+
...extensions,
315+
}),
316+
selection: {
317+
anchor: 0,
318+
},
319+
})
320+
}
264321
</script>
265322

266323
{#if browser}
@@ -273,9 +330,10 @@
273330

274331
<style lang="scss">
275332
.root {
276-
display: contents;
277333
--blame-decoration-width: 400px;
278334
--blame-recency-width: 4px;
335+
336+
height: 100%;
279337
}
280338
pre {
281339
margin: 0;

0 commit comments

Comments
 (0)