Programming exercises: Remember scrolling position when switching between files in the online code editor

src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts

@@ -42,7 +42,7 @@ import { ArtemisProgrammingManualAssessmentModule } from 'app/exercises/programm
 import { CodeEditorHeaderComponent } from 'app/exercises/programming/shared/code-editor/header/code-editor-header.component';
 import { ArtemisSharedModule } from 'app/shared/shared.module';
 
-type FileSession = { [fileName: string]: { code: string; cursor: EditorPosition; loadingError: boolean } };
+type FileSession = { [fileName: string]: { code: string; cursor: EditorPosition; scrollTop: number; loadingError: boolean } };
 type FeedbackWithLineAndReference = Feedback & { line: number; reference: string };
 export type Annotation = { fileName: string; row: number; column: number; text: string; type: string; timestamp: number; hash?: string };
 @Component({
@@ -157,6 +157,11 @@ export class CodeEditorMonacoComponent implements OnChanges {
             this.editor().reset();
         }
         if ((changes.selectedFile && this.selectedFile()) || editorWasRefreshed) {
+            const previousFileName: string | undefined = changes.selectedFile?.previousValue;
+            // we save the old scrollTop before switching to another file
+            if (previousFileName && this.fileSession()[previousFileName]) {
+                this.fileSession()[previousFileName].scrollTop = this.editor().getScrollTop();
+            }
             await this.selectFileInEditor(this.selectedFile());
             this.setBuildAnnotations(this.annotationsArray);
             this.newFeedbackLines.set([]);
@@ -196,25 +201,37 @@ export class CodeEditorMonacoComponent implements OnChanges {
                     this.onError.emit('loadingFailed');
                 }
             }
-            this.fileSession.set({ ...this.fileSession(), [fileName]: { code: fileContent, loadingError, cursor: { column: 0, lineNumber: 0 } } });
+            this.fileSession.set({
+                ...this.fileSession(),
+                [fileName]: { code: fileContent, loadingError: loadingError, scrollTop: 0, cursor: { column: 0, lineNumber: 0 } },
+            });
         }
 
         const code = this.fileSession()[fileName].code;
         this.binaryFileSelected.set(this.fileTypeService.isBinaryContent(code));
 
         // Since fetching the file may take some time, we need to check if the file is still selected.
         if (!this.binaryFileSelected() && this.selectedFile() === fileName) {
-            this.editor().changeModel(fileName, code);
-            this.editor().setPosition(this.fileSession()[fileName].cursor);
+            this.switchToSelectedFile(fileName, code);
         }
         this.loadingCount.set(this.loadingCount() - 1);
     }
 
+    switchToSelectedFile(selectedFileName: string, code: string): void {
+        this.editor().changeModel(selectedFileName, code);
+        this.editor().setPosition(this.fileSession()[selectedFileName].cursor);
+        this.editor().setScrollTop(this.fileSession()[this.selectedFile()!].scrollTop ?? 0);
+    }
+
     onFileTextChanged(text: string): void {
         if (this.selectedFile() && this.fileSession()[this.selectedFile()!]) {
             const previousText = this.fileSession()[this.selectedFile()!].code;
+            const previousScrollTop = this.fileSession()[this.selectedFile()!].scrollTop;
             if (previousText !== text) {
-                this.fileSession.set({ ...this.fileSession(), [this.selectedFile()!]: { code: text, loadingError: false, cursor: this.editor().getPosition() } });
+                this.fileSession.set({
+                    ...this.fileSession(),
+                    [this.selectedFile()!]: { code: text, loadingError: false, scrollTop: previousScrollTop, cursor: this.editor().getPosition() },
+                });
                 this.onFileContentChange.emit({ file: this.selectedFile()!, fileContent: text });
             }
         }
@@ -405,7 +422,7 @@ export class CodeEditorMonacoComponent implements OnChanges {
             this.fileSession.set(this.fileService.updateFileReferences(this.fileSession(), fileChange));
             this.storeAnnotations([fileChange.fileName]);
         } else if (fileChange instanceof CreateFileChange && fileChange.fileType === FileType.FILE) {
-            this.fileSession.set({ ...this.fileSession(), [fileChange.fileName]: { code: '', cursor: { lineNumber: 0, column: 0 }, loadingError: false } });
+            this.fileSession.set({ ...this.fileSession(), [fileChange.fileName]: { code: '', cursor: { lineNumber: 0, column: 0 }, scrollTop: 0, loadingError: false } });
         }
         this.setBuildAnnotations(this.annotationsArray);
     }

src/main/webapp/app/shared/monaco-editor/monaco-editor.component.ts

@@ -189,6 +189,14 @@ export class MonacoEditorComponent implements OnInit, OnDestroy {
         this._editor.setPosition(position);
     }
 
+    getScrollTop(): number {
+        return this._editor.getScrollTop();
+    }
+
+    setScrollTop(scrollTop: number) {
+        this._editor.setScrollTop(scrollTop);
+    }
+
     setSelection(range: EditorRange): void {
         this._editor.setSelection(range);
     }

src/test/javascript/spec/component/code-editor/code-editor-monaco.component.spec.ts

@@ -117,11 +117,11 @@ describe('CodeEditorMonacoComponent', () => {
         [() => fixture.componentRef.setInput('disableActions', true), true],
         [() => fixture.componentRef.setInput('commitState', CommitState.CONFLICT), true],
         [() => fixture.componentRef.setInput('selectedFile', undefined), true],
-        [() => comp.fileSession.set({ ['file']: { code: '', cursor: { lineNumber: 0, column: 0 }, loadingError: true } }), true], // TODO: convert to signal
+        [() => comp.fileSession.set({ ['file']: { code: '', cursor: { lineNumber: 0, column: 0 }, loadingError: true, scrollTop: 0 } }), true], // TODO: convert to signal
     ])('should correctly lock the editor on changes', (setup: () => void, shouldLock: boolean) => {
         fixture.componentRef.setInput('selectedFile', 'file');
         comp.fileSession.set({
-            [comp.selectedFile()!]: { code: 'some code', cursor: { lineNumber: 0, column: 0 }, loadingError: false },
+            [comp.selectedFile()!]: { code: 'some code', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 },
         });
         fixture.detectChanges();
         setup();
@@ -131,7 +131,7 @@ describe('CodeEditorMonacoComponent', () => {
     it('should update the file session and notify when the file content changes', () => {
         const selectedFile = 'file';
         const fileSession = {
-            [selectedFile]: { code: 'some unchanged code', cursor: { lineNumber: 1, column: 1 }, loadingError: false },
+            [selectedFile]: { code: 'some unchanged code', cursor: { lineNumber: 1, column: 1 }, loadingError: false, scrollTop: 0 },
         };
         const newCode = 'some new code';
         const valueCallbackStub = jest.fn();
@@ -142,7 +142,7 @@ describe('CodeEditorMonacoComponent', () => {
         comp.onFileTextChanged(newCode);
         expect(valueCallbackStub).toHaveBeenCalledExactlyOnceWith({ file: selectedFile, fileContent: newCode });
         expect(comp.fileSession()).toEqual({
-            [selectedFile]: { ...fileSession[selectedFile], code: newCode },
+            [selectedFile]: { ...fileSession[selectedFile], code: newCode, scrollTop: 0 },
         });
     });
 
@@ -154,7 +154,7 @@ describe('CodeEditorMonacoComponent', () => {
         const changeModelStub = jest.spyOn(comp.editor(), 'changeModel').mockImplementation();
         const presentFileName = 'present-file';
         const presentFileSession = {
-            [presentFileName]: { code: 'code\ncode', cursor: { lineNumber: 1, column: 2 }, loadingError: false },
+            [presentFileName]: { code: 'code\ncode', cursor: { lineNumber: 1, column: 2 }, scrollTop: 0, loadingError: false },
         };
         fixture.detectChanges();
         comp.fileSession.set(presentFileSession);
@@ -165,7 +165,7 @@ describe('CodeEditorMonacoComponent', () => {
         expect(loadFileFromRepositoryStub).toHaveBeenCalledExactlyOnceWith(fileToLoad.fileName);
         expect(comp.fileSession()).toEqual({
             ...presentFileSession,
-            [fileToLoad.fileName]: { code: fileToLoad.fileContent, cursor: { column: 0, lineNumber: 0 }, loadingError: false },
+            [fileToLoad.fileName]: { code: fileToLoad.fileContent, cursor: { column: 0, lineNumber: 0 }, scrollTop: 0, loadingError: false },
         });
         expect(setPositionStub).toHaveBeenCalledTimes(2);
         expect(changeModelStub).toHaveBeenCalledTimes(2);
@@ -174,22 +174,24 @@ describe('CodeEditorMonacoComponent', () => {
     it('should load a selected file after a loading error', async () => {
         const fileToLoad = { fileName: 'file-to-load', fileContent: 'some code' };
         // File session after loading fails
-        const fileSession = { [fileToLoad.fileName]: { code: '', loadingError: true, cursor: { lineNumber: 0, column: 0 } } };
+        const fileSession = { [fileToLoad.fileName]: { code: '', loadingError: true, cursor: { lineNumber: 0, column: 0 }, scrollTop: 0 } };
         const loadedFileSubject = new BehaviorSubject(fileToLoad);
         loadFileFromRepositoryStub.mockReturnValue(loadedFileSubject);
         comp.fileSession.set(fileSession);
         fixture.componentRef.setInput('selectedFile', fileToLoad.fileName);
         fixture.detectChanges();
         await new Promise(process.nextTick);
         expect(loadFileFromRepositoryStub).toHaveBeenCalledOnce();
-        expect(comp.fileSession()).toEqual({ [fileToLoad.fileName]: { code: fileToLoad.fileContent, loadingError: false, cursor: { lineNumber: 0, column: 0 } } });
+        expect(comp.fileSession()).toEqual({
+            [fileToLoad.fileName]: { code: fileToLoad.fileContent, loadingError: false, cursor: { lineNumber: 0, column: 0 }, scrollTop: 0 },
+        });
     });
 
     it('should not load binaries into the editor', async () => {
         const changeModelSpy = jest.spyOn(comp.editor(), 'changeModel');
         const fileName = 'file-to-load';
         comp.fileSession.set({
-            [fileName]: { code: '\0\0\0\0 (binary content)', loadingError: false, cursor: { lineNumber: 0, column: 0 } },
+            [fileName]: { code: '\0\0\0\0 (binary content)', loadingError: false, cursor: { lineNumber: 0, column: 0 }, scrollTop: 0 },
         });
         fixture.detectChanges();
         fixture.componentRef.setInput('selectedFile', fileName);
@@ -214,7 +216,9 @@ describe('CodeEditorMonacoComponent', () => {
         await new Promise(process.nextTick);
         expect(loadFileFromRepositoryStub).toHaveBeenCalledOnce();
         expect(errorCallbackStub).toHaveBeenCalledExactlyOnceWith(errorCode);
-        expect(comp.fileSession()).toEqual({ [fileToLoad.fileName]: { code: '', loadingError: true, cursor: { lineNumber: 0, column: 0 } } });
+        expect(comp.fileSession()).toEqual({
+            [fileToLoad.fileName]: { code: '', loadingError: true, cursor: { lineNumber: 0, column: 0 }, scrollTop: 0 },
+        });
     });
 
     it('should discard local changes when the editor is refreshed', async () => {
@@ -224,21 +228,21 @@ describe('CodeEditorMonacoComponent', () => {
         loadFileFromRepositoryStub.mockReturnValue(reloadedFileSubject);
         fixture.componentRef.setInput('selectedFile', fileToReload.fileName);
         comp.fileSession.set({
-            [fileToReload.fileName]: { code: 'some local undiscarded changes', cursor: { lineNumber: 0, column: 0 }, loadingError: false },
+            [fileToReload.fileName]: { code: 'some local undiscarded changes', cursor: { lineNumber: 0, column: 0 }, scrollTop: 0, loadingError: false },
         });
         fixture.componentRef.setInput('editorState', EditorState.CLEAN);
         // Simulate a refresh of the editor.
         await comp.ngOnChanges({ editorState: new SimpleChange(EditorState.REFRESHING, EditorState.CLEAN, false) });
         expect(comp.fileSession()).toEqual({
-            [fileToReload.fileName]: { code: fileToReload.fileContent, cursor: { lineNumber: 0, column: 0 }, loadingError: false },
+            [fileToReload.fileName]: { code: fileToReload.fileContent, cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 },
         });
         expect(editorResetStub).toHaveBeenCalledOnce();
     });
 
     it('should only load the currently selected file', async () => {
         const changeModelSpy = jest.spyOn(comp.editor(), 'changeModel');
         // Occurs when the first file load takes a while, but the user has already selected another file.
-        comp.fileSession.set({ ['file2']: { code: 'code2', cursor: { lineNumber: 0, column: 0 }, loadingError: false } });
+        comp.fileSession.set({ ['file2']: { code: 'code2', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 } });
         fixture.detectChanges();
         fixture.componentRef.setInput('selectedFile', 'file1');
         const longLoadingFileSubject = new Subject();
@@ -258,7 +262,7 @@ describe('CodeEditorMonacoComponent', () => {
         fixture.detectChanges();
         const selectedFile = 'file1';
         const fileSession = {
-            [selectedFile]: { code: 'code\ncode', cursor: { lineNumber: 1, column: 2 }, loadingError: false },
+            [selectedFile]: { code: 'code\ncode', cursor: { lineNumber: 1, column: 2 }, loadingError: false, scrollTop: 0 },
         };
         comp.fileSession.set(fileSession);
         fixture.componentRef.setInput('selectedFile', selectedFile);
@@ -426,8 +430,8 @@ describe('CodeEditorMonacoComponent', () => {
         const newFileName = 'new-file-name';
         const otherFileName = 'other-file';
         const fileSession = {
-            [oldFileName]: { code: 'renamed', cursor: { lineNumber: 0, column: 0 }, loadingError: false },
-            [otherFileName]: { code: 'unrelated', cursor: { lineNumber: 0, column: 0 }, loadingError: false },
+            [oldFileName]: { code: 'renamed', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 },
+            [otherFileName]: { code: 'unrelated', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 },
         };
         fixture.detectChanges();
         comp.fileSession.set({ ...fileSession });
@@ -443,8 +447,8 @@ describe('CodeEditorMonacoComponent', () => {
         const fileToDeleteName = 'file-to-delete';
         const otherFileName = 'other-file';
         const fileSession = {
-            [fileToDeleteName]: { code: 'will be deleted', cursor: { lineNumber: 0, column: 0 }, loadingError: false },
-            [otherFileName]: { code: 'unrelated', cursor: { lineNumber: 0, column: 0 }, loadingError: false },
+            [fileToDeleteName]: { code: 'will be deleted', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 },
+            [otherFileName]: { code: 'unrelated', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 },
         };
         fixture.detectChanges();
         comp.fileSession.set({ ...fileSession });
@@ -459,15 +463,15 @@ describe('CodeEditorMonacoComponent', () => {
         const fileToCreateName = 'file-to-create';
         const otherFileName = 'other-file';
         const fileSession = {
-            [otherFileName]: { code: 'unrelated', cursor: { lineNumber: 0, column: 0 }, loadingError: false },
+            [otherFileName]: { code: 'unrelated', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 },
         };
         fixture.detectChanges();
         comp.fileSession.set({ ...fileSession });
         const createFileChange = new CreateFileChange(FileType.FILE, fileToCreateName);
         await comp.onFileChange(createFileChange);
         expect(comp.fileSession()).toEqual({
             [otherFileName]: fileSession[otherFileName],
-            [fileToCreateName]: { code: '', cursor: { lineNumber: 0, column: 0 }, loadingError: false },
+            [fileToCreateName]: { code: '', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: 0 },
         });
     });
 
@@ -477,4 +481,18 @@ describe('CodeEditorMonacoComponent', () => {
         comp.highlightLines(1, 2);
         expect(highlightStub).toHaveBeenCalledExactlyOnceWith(1, 2, CodeEditorMonacoComponent.CLASS_DIFF_LINE_HIGHLIGHT);
     });
+
+    it('should remember scroll position', async () => {
+        const setScrollTopStub = jest.spyOn(comp.editor(), 'setScrollTop');
+        const scrolledFile = 'file';
+        const scrollTop = 42;
+        const fileSession = {
+            [scrolledFile]: { code: 'unrelated', cursor: { lineNumber: 0, column: 0 }, loadingError: false, scrollTop: scrollTop },
+        };
+        fixture.detectChanges();
+        comp.fileSession.set(fileSession);
+        fixture.componentRef.setInput('selectedFile', scrolledFile);
+        await comp.selectFileInEditor(scrolledFile);
+        expect(setScrollTopStub).toHaveBeenCalledExactlyOnceWith(scrollTop);
+    });
 });