Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Programming exercises: Change the solution entry and testwise coverage editors to Monaco #9173

Merged
merged 21 commits into from
Aug 11, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ <h4 class="modal-title" jhiTranslate="artemisApp.programmingExerciseSolutionEntr
<div class="form-group">
<label class="label-narrow" for="field_path" jhiTranslate="artemisApp.programmingExerciseSolutionEntry.file"></label>
@if (solutionRepositoryFileNames?.length) {
<select class="form-select" required name="path" [(ngModel)]="this.solutionEntry.filePath" id="field_path">
<select class="form-select" required name="path" [(ngModel)]="this.solutionEntry.filePath" (change)="onUpdateFilePath()" id="field_path">
@for (fileName of solutionRepositoryFileNames; track fileName) {
<option [ngValue]="fileName">{{ fileName }}</option>
}
</select>
}
</div>
<div class="d-flex flex-row">
<jhi-solution-entry class="flex-grow-1" [solutionEntry]="solutionEntry" [enableEditing]="true" />
<div class="d-flex flex-column align-items-end gap-3">
<jhi-solution-entry #solutionEntryComponent class="w-100" [solutionEntry]="solutionEntry" [enableEditing]="true" />
<div class="ps-2 flex-grow-0">
<button
type="button"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import { Component, EventEmitter, OnDestroy, OnInit } from '@angular/core';
import { Component, EventEmitter, OnDestroy, OnInit, ViewChild } from '@angular/core';
import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
import { CodeHintService } from 'app/exercises/shared/exercise-hint/services/code-hint.service';
import { Subject } from 'rxjs';
import { ProgrammingExerciseTestCase } from 'app/entities/programming-exercise-test-case.model';
import { ProgrammingExerciseSolutionEntry } from 'app/entities/hestia/programming-exercise-solution-entry.model';
import { ProgrammingExerciseService } from 'app/exercises/programming/manage/services/programming-exercise.service';
import { CodeHint } from 'app/entities/hestia/code-hint-model';
import { ProgrammingExerciseSolutionEntryService } from 'app/exercises/shared/exercise-hint/services/programming-exercise-solution-entry.service';
import { SolutionEntryComponent } from 'app/exercises/shared/exercise-hint/shared/solution-entry.component';

@Component({
selector: 'jhi-manual-solution-entry-creation-modal',
templateUrl: './manual-solution-entry-creation-modal.component.html',
})
export class ManualSolutionEntryCreationModalComponent implements OnInit, OnDestroy {
@ViewChild('solutionEntryComponent', { static: false }) solutionEntryComponent: SolutionEntryComponent;

solutionEntry = new ProgrammingExerciseSolutionEntry();

exerciseId: number;
Expand All @@ -27,7 +29,6 @@ export class ManualSolutionEntryCreationModalComponent implements OnInit, OnDest

constructor(
private activeModal: NgbActiveModal,
private service: CodeHintService,
private exerciseService: ProgrammingExerciseService,
private solutionEntryService: ProgrammingExerciseSolutionEntryService,
) {
Expand Down Expand Up @@ -58,6 +59,14 @@ export class ManualSolutionEntryCreationModalComponent implements OnInit, OnDest
});
}

/**
* Updates the solution entry editor to refer to the correct file path.
* In particular, this updates the syntax highlighting of the editor.
*/
onUpdateFilePath(): void {
this.solutionEntryComponent.setupEditor();
}

clear() {
this.activeModal.close();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ <h4 class="modal-title" jhiTranslate="artemisApp.programmingExerciseSolutionEntr
>Note: Changes apply to the fragments in existing code hints that contain this fragment.</span
>
</div>
<div class="d-flex flex-row">
<jhi-solution-entry class="flex-grow-1" [solutionEntry]="solutionEntry" [enableEditing]="isEditable" />
<div class="d-flex flex-column align-items-end gap-3">
<jhi-solution-entry class="w-100" [solutionEntry]="solutionEntry" [enableEditing]="isEditable" />
@if (isEditable) {
<div class="ps-2 flex-grow-0">
<button type="button" class="btn btn-primary" jhiTranslate="entity.action.save" (click)="saveSolutionEntry()"></button>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
@for (solutionEntry of sortedSolutionEntries; track solutionEntry) {
<div>
<div class="d-flex flex-row mb-3">
<jhi-solution-entry class="flex-grow-1" [solutionEntry]="solutionEntry" [enableEditing]="enableEditing" (onRemoveEntry)="removeEntryFromCodeHint(solutionEntry.id!)" />
<div class="d-flex flex-column gap-1 mb-4">
<jhi-solution-entry [solutionEntry]="solutionEntry" [enableEditing]="enableEditing" (onRemoveEntry)="removeEntryFromCodeHint(solutionEntry.id!)" />
@if (enableEditing) {
<div class="ps-2 flex-grow-0">
<div>
<button
jhiDeleteButton
[entityTitle]="solutionEntry.id!.toString()"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { NgModule } from '@angular/core';
import { ArtemisSharedModule } from 'app/shared/shared.module';
import { CastToCodeHintPipe } from 'app/exercises/shared/exercise-hint/services/code-hint-cast.pipe';
import { SolutionEntryComponent } from 'app/exercises/shared/exercise-hint/shared/solution-entry.component';
import { AceEditorModule } from 'app/shared/markdown-editor/ace-editor/ace-editor.module';
import { CodeHintContainerComponent } from 'app/exercises/shared/exercise-hint/shared/code-hint-container.component';
import { ArtemisMarkdownModule } from 'app/shared/markdown.module';
import { MonacoEditorModule } from 'app/shared/monaco-editor/monaco-editor.module';

@NgModule({
imports: [ArtemisSharedModule, AceEditorModule, ArtemisMarkdownModule],
imports: [ArtemisSharedModule, ArtemisMarkdownModule, MonacoEditorModule],
declarations: [SolutionEntryComponent, CodeHintContainerComponent, CastToCodeHintPipe],
exports: [SolutionEntryComponent, CodeHintContainerComponent, CastToCodeHintPipe],
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<div class="col-6">
<h6>{{ solutionEntry.filePath }}</h6>
</div>
<jhi-ace-editor
#editor
[mode]="'java'"
[readOnly]="!enableEditing"
[hidden]="false"
[autoUpdateContent]="true"
(textChange)="onEditorContentChange($event)"
class="jhi-solution-entry-editor"
/>
<div class="solution-entry-editor" [style.height.px]="editorHeight">
<jhi-monaco-editor
#editor
[readOnly]="!enableEditing"
[shrinkToFit]="false"
(contentHeightChanged)="onContentSizeChange($event)"
(textChanged)="onEditorContentChange($event)"
/>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.solution-entry-editor {
border: 1px solid var(--border-color);
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import { Component, EventEmitter, Input, OnInit, Output, ViewChild } from '@angular/core';
import { ActivatedRoute } from '@angular/router';

import { AceEditorComponent } from 'app/shared/markdown-editor/ace-editor/ace-editor.component';
import { faTimes } from '@fortawesome/free-solid-svg-icons';
import { ProgrammingExerciseSolutionEntry } from 'app/entities/hestia/programming-exercise-solution-entry.model';
import { MonacoEditorComponent } from 'app/shared/monaco-editor/monaco-editor.component';

@Component({
selector: 'jhi-solution-entry',
templateUrl: './solution-entry.component.html',
styleUrls: ['./solution-entry.component.scss'],
})
export class SolutionEntryComponent implements OnInit {
@ViewChild('editor', { static: true })
editor: AceEditorComponent;
editor: MonacoEditorComponent;

@Input()
solutionEntry: ProgrammingExerciseSolutionEntry;
Expand All @@ -21,40 +20,28 @@ export class SolutionEntryComponent implements OnInit {
@Output()
onRemoveEntry: EventEmitter<void> = new EventEmitter();

faTimes = faTimes;
editorHeight = 20;

constructor(protected route: ActivatedRoute) {}
protected readonly faTimes = faTimes;

ngOnInit() {
ngOnInit(): void {
this.setupEditor();
}

emitRemovalEvent() {
this.onRemoveEntry.emit();
onEditorContentChange(value: string): void {
this.solutionEntry.code = value;
}

onEditorContentChange(value: any) {
this.solutionEntry.code = value;
onContentSizeChange(contentHeight: number): void {
this.editorHeight = contentHeight;
}

private setupEditor() {
const line = this.solutionEntry.line;

this.editor.getEditor().setOptions({
animatedScroll: true,
maxLines: Infinity,
showPrintMargin: false,
});
// Ensure that the line counter is according to the solution entry
this.editor.getEditor().session.gutterRenderer = {
getWidth(session: any, lastLineNumber: number, config: any) {
return this.getText(session, lastLineNumber).toString().length * config.characterWidth;
},
getText(session: any, row: number): string | number {
return !line ? '' : row + line;
},
};
this.editor.getEditor().getSession().setValue(this.solutionEntry.code);
this.editor.getEditor().resize();
setupEditor(): void {
const startLine = this.solutionEntry.line ?? 1;
this.editor.setStartLineNumber(startLine);
this.editor.changeModel(this.solutionEntry.filePath ?? 'file', this.solutionEntry.code ?? '');
this.editor.layout();
// We manually fetch the initial content height, as the editor does not provide it immediately
this.editorHeight = this.editor.getContentHeight();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ export class MonacoEditorComponent implements OnInit, OnDestroy {
@Output()
textChanged = new EventEmitter<string>();

@Output()
contentHeightChanged = new EventEmitter<number>();

private contentHeightListener?: monaco.IDisposable;
private textChangedListener?: monaco.IDisposable;
private textChangedEmitTimeout?: NodeJS.Timeout;

ngOnInit(): void {
Expand All @@ -104,17 +109,25 @@ export class MonacoEditorComponent implements OnInit, OnDestroy {
});
resizeObserver.observe(this.monacoEditorContainerElement);

this._editor.onDidChangeModelContent(() => {
this.textChangedListener = this._editor.onDidChangeModelContent(() => {
this.emitTextChangeEvent();
}, this);

this.contentHeightListener = this._editor.onDidContentSizeChange((event) => {
if (event.contentHeightChanged) {
this.contentHeightChanged.emit(event.contentHeight + this._editor.getOption(monaco.editor.EditorOption.lineHeight));
}
});

this.themeSubscription = this.themeService.getCurrentThemeObservable().subscribe((theme) => this.changeTheme(theme));
}

ngOnDestroy() {
this.reset();
this._editor.dispose();
this.themeSubscription?.unsubscribe();
this.textChangedListener?.dispose();
this.contentHeightListener?.dispose();
}

private emitTextChangeEvent() {
Expand Down Expand Up @@ -150,6 +163,10 @@ export class MonacoEditorComponent implements OnInit, OnDestroy {
return this._editor.getValue();
}

getContentHeight(): number {
return this._editor.getContentHeight() + this._editor.getOption(monaco.editor.EditorOption.lineHeight);
}

setText(text: string): void {
if (this.getText() !== text) {
this._editor.setValue(text);
Expand Down Expand Up @@ -364,4 +381,14 @@ export class MonacoEditorComponent implements OnInit, OnDestroy {
wordWrap: value ? 'on' : 'off',
});
}

/**
* Sets the line number from which the editor should start counting.
* @param startLineNumber The line number to start counting from (starting at 1).
*/
setStartLineNumber(startLineNumber: number): void {
this._editor.updateOptions({
lineNumbers: (number) => `${startLineNumber + number - 1}`,
});
}
pzdr7 marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import { ArtemisTestModule } from '../../../test.module';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { AceEditorComponent } from 'app/shared/markdown-editor/ace-editor/ace-editor.component';
import { ArtemisTranslatePipe } from 'app/shared/pipes/artemis-translate.pipe';
import { MockPipe } from 'ng-mocks';
import { MockComponent, MockPipe } from 'ng-mocks';
import { SolutionEntryComponent } from 'app/exercises/shared/exercise-hint/shared/solution-entry.component';
import { ProgrammingExerciseSolutionEntry } from 'app/entities/hestia/programming-exercise-solution-entry.model';
import { MonacoEditorComponent } from 'app/shared/monaco-editor/monaco-editor.component';

describe('Solution Entry Component', () => {
let comp: SolutionEntryComponent;
let fixture: ComponentFixture<SolutionEntryComponent>;
let solutionEntry: ProgrammingExerciseSolutionEntry;

beforeEach(() => {
TestBed.configureTestingModule({
imports: [ArtemisTestModule],
declarations: [SolutionEntryComponent, AceEditorComponent, MockPipe(ArtemisTranslatePipe)],
declarations: [SolutionEntryComponent, MockComponent(MonacoEditorComponent), MockPipe(ArtemisTranslatePipe)],
}).compileComponents();
fixture = TestBed.createComponent(SolutionEntryComponent);
comp = fixture.componentInstance;
Expand All @@ -23,35 +24,32 @@ describe('Solution Entry Component', () => {
comp.solutionEntry.filePath = '/src/de/Test.java';
comp.solutionEntry.line = 1;
comp.solutionEntry.code = 'ABC';

solutionEntry = comp.solutionEntry;
});

afterEach(() => {
jest.restoreAllMocks();
});

it('should setup editors', () => {
jest.spyOn(comp.editor.getEditor(), 'setOptions');
jest.spyOn(comp.editor.getEditor().getSession(), 'setValue');

comp.ngOnInit();
it('should correctly setup editors', () => {
const setStartLineNumberStub = jest.spyOn(comp.editor, 'setStartLineNumber').mockImplementation();
const changeModelStub = jest.spyOn(comp.editor, 'changeModel').mockImplementation();
fixture.detectChanges();

expect(comp.editor.getEditor().setOptions).toHaveBeenCalledOnce();
expect(comp.editor.getEditor().setOptions).toHaveBeenCalledWith({
animatedScroll: true,
maxLines: Infinity,
showPrintMargin: false,
});

expect(comp.editor.getEditor().getSession().setValue).toHaveBeenCalledOnce();
expect(comp.editor.getEditor().getSession().setValue).toHaveBeenCalledWith('ABC');
expect(setStartLineNumberStub).toHaveBeenCalledExactlyOnceWith(solutionEntry.line);
expect(changeModelStub).toHaveBeenCalledExactlyOnceWith(solutionEntry.filePath, solutionEntry.code);
});

it('should give correct line for gutter', () => {
comp.ngOnInit();
const gutterRendererNow = comp.editor.getEditor().session.gutterRenderer;
const config = { characterWidth: 1 };
it('should update the editor height', () => {
const contentHeight = 123;
comp.onContentSizeChange(contentHeight);
expect(comp.editorHeight).toEqual(contentHeight);
});

expect(gutterRendererNow.getText(null, 2)).toBe(3);
expect(gutterRendererNow.getWidth(null, 2, config)).toBe(1);
it('should update the solution entry code', () => {
const newCode = 'DEF';
comp.onEditorContentChange(newCode);
expect(comp.solutionEntry.code).toEqual(newCode);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { ProgrammingExerciseTestCase } from 'app/entities/programming-exercise-t
import { of } from 'rxjs';
import { ProgrammingExerciseSolutionEntry } from 'app/entities/hestia/programming-exercise-solution-entry.model';
import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap';
import { SolutionEntryComponent } from 'app/exercises/shared/exercise-hint/shared/solution-entry.component';
import { MockComponent } from 'ng-mocks';

describe('ManualSolutionEntryCreationModal Component', () => {
let comp: ManualSolutionEntryCreationModalComponent;
Expand All @@ -25,6 +27,7 @@ describe('ManualSolutionEntryCreationModal Component', () => {
beforeEach(() => {
TestBed.configureTestingModule({
imports: [ArtemisTestModule],
declarations: [ManualSolutionEntryCreationModalComponent, MockComponent(SolutionEntryComponent)],
}).compileComponents();
fixture = TestBed.createComponent(ManualSolutionEntryCreationModalComponent);
comp = fixture.componentInstance;
Expand Down Expand Up @@ -85,4 +88,10 @@ describe('ManualSolutionEntryCreationModal Component', () => {
comp.clear();
expect(closedSpy).toHaveBeenCalledOnce();
});

it('should setup editor when file path changes', () => {
comp.solutionEntryComponent = { setupEditor: jest.fn() } as unknown as SolutionEntryComponent;
comp.onUpdateFilePath();
expect(comp.solutionEntryComponent.setupEditor).toHaveBeenCalledOnce();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,15 @@ describe('MonacoEditorComponent', () => {
expect(modelDisposeSpy).toHaveBeenCalledOnce();
expect(model.isDisposed()).toBeTrue();
});

it('should correctly set the start line number', () => {
fixture.detectChanges();
comp.changeModel('file', multiLineText);
comp.setStartLineNumber(5);
// Ensure that the editor is large enough to display all lines.
comp.layoutWithFixedSize(400, 400);
const lineNumbers = fixture.debugElement.nativeElement.querySelectorAll('.line-numbers');
expect(lineNumbers).toHaveLength(5);
expect([...lineNumbers].map((elem: HTMLElement) => elem.textContent)).toContainAllValues(['5', '6', '7', '8', '9']);
});
pzdr7 marked this conversation as resolved.
Show resolved Hide resolved
});
Loading