From 4c9cd52d6f604cbaa2796830f8c9bce41ead0156 Mon Sep 17 00:00:00 2001 From: Patrik Zander Date: Mon, 14 Oct 2024 22:02:53 +0200 Subject: [PATCH 01/30] Use signals (CodeEditorMonacoComponent) --- .../header/code-editor-header.component.ts | 2 +- .../monaco/code-editor-monaco.component.html | 36 +-- .../monaco/code-editor-monaco.component.ts | 240 +++++++++--------- 3 files changed, 137 insertions(+), 141 deletions(-) diff --git a/src/main/webapp/app/exercises/programming/shared/code-editor/header/code-editor-header.component.ts b/src/main/webapp/app/exercises/programming/shared/code-editor/header/code-editor-header.component.ts index e340a1dcb4f2..41e987621e50 100644 --- a/src/main/webapp/app/exercises/programming/shared/code-editor/header/code-editor-header.component.ts +++ b/src/main/webapp/app/exercises/programming/shared/code-editor/header/code-editor-header.component.ts @@ -9,7 +9,7 @@ import { MAX_TAB_SIZE } from 'app/shared/monaco-editor/monaco-editor.component'; }) export class CodeEditorHeaderComponent { @Input() - filename: string; + filename: string | undefined; @Input() isLoading: boolean; diff --git a/src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html b/src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html index dc68e75eca8e..53150892f5d8 100644 --- a/src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html +++ b/src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html @@ -1,43 +1,43 @@
- +
- @if (selectedFile) { - @for (feedback of filterFeedbackForSelectedFile(feedbacks); track feedback.id) { + @if (selectedFile()) { + @for (feedback of filterFeedbackForSelectedFile(feedbackInternal()); track feedback.id) { } - @for (line of newFeedbackLines; track line) { + @for (line of newFeedbackLines(); track line) { } - @for (suggestion of filterFeedbackForSelectedFile(feedbackSuggestions); track suggestion.id) { + @for (suggestion of filterFeedbackForSelectedFile(feedbackSuggestionsInternal()); track suggestion.id) { @@ -45,16 +45,16 @@ } - @if (!selectedFile && !loadingCount) { + @if (!selectedFile() && !loadingCount()) {

- } @else if (binaryFileSelected) { + } @else if (binaryFileSelected()) {

}
diff --git a/src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts b/src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts index 3c4a12677e5d..b6f2fa3656ff 100644 --- a/src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts +++ b/src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts @@ -1,4 +1,4 @@ -import { ChangeDetectorRef, Component, EventEmitter, Input, OnChanges, Output, QueryList, SimpleChanges, ViewChild, ViewChildren, ViewEncapsulation } from '@angular/core'; +import { ChangeDetectorRef, Component, OnChanges, SimpleChanges, ViewEncapsulation, computed, effect, inject, input, output, signal, viewChild, viewChildren } from '@angular/core'; import { RepositoryFileService } from 'app/exercises/shared/result/repository.service'; import { CodeEditorRepositoryFileService, ConnectionError } from 'app/exercises/programming/shared/code-editor/service/code-editor-repository.service'; import { CodeEditorFileService } from 'app/exercises/programming/shared/code-editor/service/code-editor-file.service'; @@ -30,113 +30,109 @@ export type Annotation = { fileName: string; row: number; column: number; text: templateUrl: './code-editor-monaco.component.html', styleUrls: ['./code-editor-monaco.component.scss'], encapsulation: ViewEncapsulation.None, - providers: [RepositoryFileService], + providers: [RepositoryFileService], // TODO: standalone }) export class CodeEditorMonacoComponent implements OnChanges { - @ViewChild('editor', { static: true }) - editor: MonacoEditorComponent; - @ViewChildren(CodeEditorTutorAssessmentInlineFeedbackComponent) - inlineFeedbackComponents: QueryList; - @ViewChildren(CodeEditorTutorAssessmentInlineFeedbackSuggestionComponent) - inlineFeedbackSuggestionComponents: QueryList; - @Input() - commitState: CommitState; - @Input() - editorState: EditorState; - @Input() - course?: Course; - @Input() - feedbacks: Feedback[] = []; - @Input() - feedbackSuggestions: Feedback[] = []; - @Input() - readOnlyManualFeedback: boolean; - @Input() - highlightDifferences: boolean; - @Input() - isTutorAssessment = false; - @Input() - disableActions = false; - @Input() - selectedFile?: string; - @Input() - sessionId: number | string; - @Input() - set buildAnnotations(buildAnnotations: Array) { - this.setBuildAnnotations(buildAnnotations); - } - - annotationsArray: Array = []; - - @Output() - onError: EventEmitter = new EventEmitter(); - @Output() - onFileContentChange: EventEmitter<{ file: string; fileContent: string }> = new EventEmitter<{ file: string; fileContent: string }>(); - @Output() - onUpdateFeedback = new EventEmitter(); - @Output() - onFileLoad = new EventEmitter(); - @Output() - onAcceptSuggestion = new EventEmitter(); - @Output() - onDiscardSuggestion = new EventEmitter(); - @Output() - onHighlightLines = new EventEmitter(); - - editorLocked = false; - /** - * The number of currently loading files. If this number is greater than 0, the editor is in a loading state and hides its content. - */ - loadingCount = 0; - - fileSession: FileSession = {}; - newFeedbackLines: number[] = []; - binaryFileSelected = false; - static readonly CLASS_DIFF_LINE_HIGHLIGHT = 'monaco-diff-line-highlight'; static readonly CLASS_FEEDBACK_HOVER_BUTTON = 'monaco-add-feedback-button'; static readonly FILE_TIMEOUT = 10000; - // Expose to template protected readonly Feedback = Feedback; protected readonly CommitState = CommitState; - constructor( - private repositoryFileService: CodeEditorRepositoryFileService, - private fileService: CodeEditorFileService, - protected localStorageService: LocalStorageService, - private changeDetectorRef: ChangeDetectorRef, - private fileTypeService: FileTypeService, - ) {} + private readonly repositoryFileService = inject(CodeEditorRepositoryFileService); + private readonly fileService = inject(CodeEditorFileService); + private readonly localStorageService = inject(LocalStorageService); + private readonly changeDetectorRef = inject(ChangeDetectorRef); + private readonly fileTypeService = inject(FileTypeService); + + readonly editor = viewChild.required('editor'); + readonly inlineFeedbackComponents = viewChildren(CodeEditorTutorAssessmentInlineFeedbackComponent); + readonly inlineFeedbackSuggestionComponents = viewChildren(CodeEditorTutorAssessmentInlineFeedbackSuggestionComponent); + readonly commitState = input.required(); + readonly editorState = input.required(); + readonly course = input(); + readonly feedbacks = input([]); + readonly feedbackSuggestions = input([]); + readonly readOnlyManualFeedback = input(false); + readonly highlightDifferences = input(false); + readonly isTutorAssessment = input(false); + readonly disableActions = input(false); + readonly selectedFile = input(); + readonly sessionId = input.required(); + readonly buildAnnotations = input([]); // TODO: effect + + readonly onError = output(); + readonly onFileContentChange = output<{ file: string; fileContent: string }>(); + readonly onUpdateFeedback = output(); + readonly onFileLoad = output(); + readonly onAcceptSuggestion = output(); + readonly onDiscardSuggestion = output(); + readonly onHighlightLines = output(); + + readonly loadingCount = signal(0); + readonly newFeedbackLines = signal([]); + readonly binaryFileSelected = signal(false); + readonly editorLocked = computed( + () => + this.disableActions() || + this.isTutorAssessment() || + this.commitState() === CommitState.CONFLICT || + !this.selectedFile() || + !!this.fileSession[this.selectedFile()!]?.loadingError, + ); + + readonly feedbackInternal = signal([]); + readonly feedbackSuggestionsInternal = signal([]); + + fileSession: FileSession = {}; + annotationsArray: Array = []; + + constructor() { + effect( + () => { + this.feedbackInternal.set(this.feedbacks()); + }, + { allowSignalWrites: true }, + ); + + effect( + () => { + this.feedbackSuggestionsInternal.set(this.feedbackSuggestions()); + }, + { allowSignalWrites: true }, + ); + + effect(() => { + this.setBuildAnnotations(this.buildAnnotations()); + }); + } async ngOnChanges(changes: SimpleChanges): Promise { - const editorWasRefreshed = changes.editorState && changes.editorState.previousValue === EditorState.REFRESHING && this.editorState === EditorState.CLEAN; - const editorWasReset = changes.commitState && changes.commitState.previousValue !== CommitState.UNDEFINED && this.commitState === CommitState.UNDEFINED; + const editorWasRefreshed = changes.editorState && changes.editorState.previousValue === EditorState.REFRESHING && this.editorState() === EditorState.CLEAN; + const editorWasReset = changes.commitState && changes.commitState.previousValue !== CommitState.UNDEFINED && this.commitState() === CommitState.UNDEFINED; // Refreshing the editor resets any local files. if (editorWasRefreshed || editorWasReset) { this.fileSession = {}; - this.editor.reset(); + this.editor().reset(); } - if ((changes.selectedFile && this.selectedFile) || editorWasRefreshed) { - await this.selectFileInEditor(this.selectedFile); + if ((changes.selectedFile && this.selectedFile()) || editorWasRefreshed) { + await this.selectFileInEditor(this.selectedFile()); this.setBuildAnnotations(this.annotationsArray); - this.newFeedbackLines = []; + this.newFeedbackLines.set([]); this.renderFeedbackWidgets(); - if (this.isTutorAssessment && !this.readOnlyManualFeedback) { + if (this.isTutorAssessment() && !this.readOnlyManualFeedback()) { this.setupAddFeedbackButton(); } - this.onFileLoad.emit(this.selectedFile); + this.onFileLoad.emit(this.selectedFile()!); } if (changes.feedbacks) { - this.newFeedbackLines = []; + this.newFeedbackLines.set([]); this.renderFeedbackWidgets(); } - this.editorLocked = - this.disableActions || this.isTutorAssessment || this.commitState === CommitState.CONFLICT || !this.selectedFile || !!this.fileSession[this.selectedFile]?.loadingError; - - this.editor.layout(); + this.editor().layout(); } async selectFileInEditor(fileName: string | undefined): Promise { @@ -144,7 +140,7 @@ export class CodeEditorMonacoComponent implements OnChanges { // There is nothing to be done, as the editor will be hidden when there is no file. return; } - this.loadingCount++; + this.loadingCount.set(this.loadingCount() + 1); if (!this.fileSession[fileName] || this.fileSession[fileName].loadingError) { let fileContent = ''; let loadingError = false; @@ -164,41 +160,41 @@ export class CodeEditorMonacoComponent implements OnChanges { } const code = this.fileSession[fileName].code; - this.binaryFileSelected = this.fileTypeService.isBinaryContent(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); + if (!this.binaryFileSelected() && this.selectedFile() === fileName) { + this.editor().changeModel(fileName, code); + this.editor().setPosition(this.fileSession[fileName].cursor); } - this.loadingCount--; + this.loadingCount.set(this.loadingCount() - 1); } onFileTextChanged(text: string): void { - if (this.selectedFile && this.fileSession[this.selectedFile]) { - const previousText = this.fileSession[this.selectedFile].code; + if (this.selectedFile() && this.fileSession[this.selectedFile()!]) { + const previousText = this.fileSession[this.selectedFile()!].code; if (previousText !== text) { - this.fileSession[this.selectedFile] = { code: text, loadingError: false, cursor: this.editor.getPosition() }; - this.onFileContentChange.emit({ file: this.selectedFile, fileContent: text }); + this.fileSession[this.selectedFile()!] = { code: text, loadingError: false, cursor: this.editor().getPosition() }; + this.onFileContentChange.emit({ file: this.selectedFile()!, fileContent: text }); } } } getText(): string { - return this.editor.getText(); + return this.editor().getText(); } getNumberOfLines(): number { - return this.editor.getNumberOfLines(); + return this.editor().getNumberOfLines(); } highlightLines(startLine: number, endLine: number) { - this.editor.highlightLines(startLine, endLine, CodeEditorMonacoComponent.CLASS_DIFF_LINE_HIGHLIGHT); + this.editor().highlightLines(startLine, endLine, CodeEditorMonacoComponent.CLASS_DIFF_LINE_HIGHLIGHT); this.onHighlightLines.emit(this.getLineHighlights()); } setupAddFeedbackButton(): void { - this.editor.setLineDecorationsHoverButton(CodeEditorMonacoComponent.CLASS_FEEDBACK_HOVER_BUTTON, (lineNumber) => this.addNewFeedback(lineNumber)); + this.editor().setLineDecorationsHoverButton(CodeEditorMonacoComponent.CLASS_FEEDBACK_HOVER_BUTTON, (lineNumber) => this.addNewFeedback(lineNumber)); } /** @@ -209,7 +205,7 @@ export class CodeEditorMonacoComponent implements OnChanges { // TODO for a follow-up: in the future, there might be multiple feedback items on the same line. const lineNumberZeroBased = lineNumber - 1; if (!this.getInlineFeedbackNode(lineNumberZeroBased)) { - this.newFeedbackLines.push(lineNumberZeroBased); + this.newFeedbackLines().push(lineNumberZeroBased); this.renderFeedbackWidgets(lineNumberZeroBased); } } @@ -220,17 +216,17 @@ export class CodeEditorMonacoComponent implements OnChanges { */ updateFeedback(feedback: Feedback) { const line = Feedback.getReferenceLine(feedback); - const existingFeedbackIndex = this.feedbacks.findIndex((f) => f.reference === feedback.reference); + const existingFeedbackIndex = this.feedbackInternal().findIndex((f) => f.reference === feedback.reference); if (existingFeedbackIndex !== -1) { // Existing feedback -> update only - this.feedbacks[existingFeedbackIndex] = feedback; + this.feedbackInternal()[existingFeedbackIndex] = feedback; } else { // New feedback -> save as actual feedback. - this.feedbacks.push(feedback); - this.newFeedbackLines = this.newFeedbackLines.filter((l) => l !== line); + this.feedbackInternal().push(feedback); + this.newFeedbackLines.set(this.newFeedbackLines().filter((l) => l !== line)); } this.renderFeedbackWidgets(); - this.onUpdateFeedback.emit(this.feedbacks); + this.onUpdateFeedback.emit(this.feedbackInternal()); } /** @@ -239,8 +235,8 @@ export class CodeEditorMonacoComponent implements OnChanges { */ cancelFeedback(line: number) { // We only have to remove new feedback. - if (this.newFeedbackLines.includes(line)) { - this.newFeedbackLines = this.newFeedbackLines.filter((l) => l !== line); + if (this.newFeedbackLines().includes(line)) { + this.newFeedbackLines.set(this.newFeedbackLines().filter((l) => l !== line)); this.renderFeedbackWidgets(); } } @@ -250,8 +246,8 @@ export class CodeEditorMonacoComponent implements OnChanges { * @param feedback The feedback to remove. */ deleteFeedback(feedback: Feedback) { - this.feedbacks = this.feedbacks.filter((f) => !Feedback.areIdentical(f, feedback)); - this.onUpdateFeedback.emit(this.feedbacks); + this.feedbackInternal.set(this.feedbackInternal().filter((f) => !Feedback.areIdentical(f, feedback))); + this.onUpdateFeedback.emit(this.feedbackInternal()); this.renderFeedbackWidgets(); } @@ -260,7 +256,7 @@ export class CodeEditorMonacoComponent implements OnChanges { * @param feedback The feedback item of the feedback suggestion. */ acceptSuggestion(feedback: Feedback): void { - this.feedbackSuggestions = this.feedbackSuggestions.filter((f) => f !== feedback); + this.feedbackSuggestionsInternal.set(this.feedbackSuggestionsInternal().filter((f) => f !== feedback)); feedback.text = (feedback.text ?? FEEDBACK_SUGGESTION_IDENTIFIER).replace(FEEDBACK_SUGGESTION_IDENTIFIER, FEEDBACK_SUGGESTION_ACCEPTED_IDENTIFIER); this.updateFeedback(feedback); this.onAcceptSuggestion.emit(feedback); @@ -271,7 +267,7 @@ export class CodeEditorMonacoComponent implements OnChanges { * @param feedback The feedback item of the feedback suggestion. */ discardSuggestion(feedback: Feedback): void { - this.feedbackSuggestions = this.feedbackSuggestions.filter((f) => f !== feedback); + this.feedbackSuggestionsInternal.set(this.feedbackSuggestionsInternal().filter((f) => f !== feedback)); this.renderFeedbackWidgets(); this.onDiscardSuggestion.emit(feedback); } @@ -285,15 +281,15 @@ export class CodeEditorMonacoComponent implements OnChanges { // Since the feedback widgets rely on the DOM nodes of each feedback item, Angular needs to re-render each node, hence the timeout. this.changeDetectorRef.detectChanges(); setTimeout(() => { - this.editor.disposeWidgets(); - for (const feedback of this.filterFeedbackForSelectedFile([...this.feedbacks, ...this.feedbackSuggestions])) { + this.editor().disposeWidgets(); + for (const feedback of this.filterFeedbackForSelectedFile([...this.feedbackInternal(), ...this.feedbackSuggestionsInternal()])) { this.addLineWidgetWithFeedback(feedback); } // New, unsaved feedback has no associated object yet. - for (const line of this.newFeedbackLines) { + for (const line of this.newFeedbackLines()) { const feedbackNode = this.getInlineFeedbackNodeOrElseThrow(line); - this.editor.addLineWidget(line + 1, 'feedback-new-' + line, feedbackNode); + this.editor().addLineWidget(line + 1, 'feedback-new-' + line, feedbackNode); } // Focus the text area of the widget on the specified line if available. @@ -320,7 +316,7 @@ export class CodeEditorMonacoComponent implements OnChanges { * @param line The line (0-based) for which to retrieve the feedback node. */ getInlineFeedbackNode(line: number): HTMLElement | undefined { - return [...this.inlineFeedbackComponents, ...this.inlineFeedbackSuggestionComponents].find((c) => c.codeLine === line)?.elementRef?.nativeElement; + return [...this.inlineFeedbackComponents(), ...this.inlineFeedbackSuggestionComponents()].find((c) => c.codeLine === line)?.elementRef?.nativeElement; } private addLineWidgetWithFeedback(feedback: Feedback): void { @@ -331,7 +327,7 @@ export class CodeEditorMonacoComponent implements OnChanges { // In the future, there may be more than one feedback node per line. const feedbackNode = this.getInlineFeedbackNodeOrElseThrow(line); // Feedback is stored with 0-based lines, but the lines of the Monaco editor used in Artemis are 1-based. We add 1 to correct this - this.editor.addLineWidget(line + 1, 'feedback-' + feedback.id, feedbackNode); + this.editor().addLineWidget(line + 1, 'feedback-' + feedback.id, feedbackNode); } /** @@ -339,10 +335,10 @@ export class CodeEditorMonacoComponent implements OnChanges { * @param feedbacks The feedbacks to filter. */ filterFeedbackForSelectedFile(feedbacks: Feedback[]): Feedback[] { - if (!this.selectedFile) { + if (!this.selectedFile()) { return []; } - return feedbacks.filter((feedback) => feedback.reference && Feedback.getReferenceFilePath(feedback) === this.selectedFile); + return feedbacks.filter((feedback) => feedback.reference && Feedback.getReferenceFilePath(feedback) === this.selectedFile()); } /** @@ -396,7 +392,7 @@ export class CodeEditorMonacoComponent implements OnChanges { } setBuildAnnotations(buildAnnotations: Annotation[]): void { - if (buildAnnotations.length > 0 && this.selectedFile) { + if (buildAnnotations.length > 0 && this.selectedFile()) { const sessionAnnotations = this.loadAnnotations(); this.annotationsArray = buildAnnotations.map((a) => { const hash = a.fileName + a.row + a.column + a.text; @@ -409,13 +405,13 @@ export class CodeEditorMonacoComponent implements OnChanges { } else { this.annotationsArray = buildAnnotations; } - this.editor.setAnnotations( - buildAnnotations.filter((buildAnnotation) => buildAnnotation.fileName === this.selectedFile), - this.commitState === CommitState.UNCOMMITTED_CHANGES, + this.editor().setAnnotations( + buildAnnotations.filter((buildAnnotation) => buildAnnotation.fileName === this.selectedFile()), + this.commitState() === CommitState.UNCOMMITTED_CHANGES, ); } getLineHighlights(): MonacoEditorLineHighlight[] { - return this.editor.getLineHighlights(); + return this.editor().getLineHighlights(); } } From 6212991d28f8367e9f5ccea502e8a801062af778 Mon Sep 17 00:00:00 2001 From: Patrik Zander Date: Mon, 14 Oct 2024 22:15:01 +0200 Subject: [PATCH 02/30] Change CodeEditorHeaderComponent and CodeEditorMonacoComponent to standalone --- .../manage/build-plan-editor.component.html | 2 +- .../programming-exercise-management.module.ts | 2 ++ .../shared/code-editor/code-editor.module.ts | 6 +---- .../header/code-editor-header.component.html | 6 ++--- .../header/code-editor-header.component.ts | 27 +++++++++---------- .../monaco/code-editor-monaco.component.html | 2 +- .../monaco/code-editor-monaco.component.ts | 25 +++++++++++++++-- 7 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/main/webapp/app/exercises/programming/manage/build-plan-editor.component.html b/src/main/webapp/app/exercises/programming/manage/build-plan-editor.component.html index 2ab8b64cf561..c13fa7dcab16 100644 --- a/src/main/webapp/app/exercises/programming/manage/build-plan-editor.component.html +++ b/src/main/webapp/app/exercises/programming/manage/build-plan-editor.component.html @@ -40,7 +40,7 @@
- +

-  {{ filename }} - @if (isLoading) { +  {{ fileName() }} + @if (isLoading()) { }

- @if (showTabSizeSelector) { + @if (showTabSizeSelector()) {
- +
(); readonly isLoading = input(false); readonly showTabSizeSelector = input(true); - readonly tabSizeChanged = output(); + readonly onValidateTabSize = output(); readonly tabSize = model(4); @@ -33,6 +33,6 @@ export class CodeEditorHeaderComponent { */ validateTabSize(): void { this.tabSize.set(Math.max(1, Math.min(this.tabSize(), MAX_TAB_SIZE))); - this.tabSizeChanged.emit(this.tabSize()); + this.onValidateTabSize.emit(this.tabSize()); } } diff --git a/src/test/javascript/spec/component/code-editor/code-editor-header.component.spec.ts b/src/test/javascript/spec/component/code-editor/code-editor-header.component.spec.ts index 57c55ace07fd..5858695beba7 100644 --- a/src/test/javascript/spec/component/code-editor/code-editor-header.component.spec.ts +++ b/src/test/javascript/spec/component/code-editor/code-editor-header.component.spec.ts @@ -36,8 +36,8 @@ describe('CodeEditorHeaderComponent', () => { expect(comp.tabSize()).toBe(MAX_TAB_SIZE); }); - it('should notify when the tab size changed', fakeAsync(() => { - comp.tabSizeChanged.subscribe((tabSize) => { + it('should notify when the tab size was validated', fakeAsync(() => { + comp.onValidateTabSize.subscribe((tabSize) => { expect(tabSize).toBe(5); });