Programming exercises: Add problem statement as a selectable file in the editor with live preview
#11348
Conversation
HawKhiem
added
feature
client
github-actions
bot
added
exam
HawKhiem
changed the title
Programming Exercises: Add problem statement as a selectable file in the editor with live previewProgramming exercises: Add problem statement as a selectable file in the editor with live preview
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
|
Caution Review failedThe pull request is closed. WalkthroughIntroduce a Problem Statement pseudo-file type and UI node, prevent repository operations on it, add repository-view flag to the code-editor container, conditionally render instructions (with debounced preview) for the Problem Statement, update file-browser behavior/styles/tests, filter PS updates from repository saves, and adjust layout/resizing for the center area. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant FileBrowser
participant EditorContainer as CodeEditorContainer
participant Instructions as Instructions (center)
participant Monaco as Monaco Editor
User->>FileBrowser: click node (path or __problem_statement__)
FileBrowser-->>EditorContainer: set selectedFile(path)
EditorContainer->>EditorContainer: evaluate selectedFile === problemStatementIdentifier && showEditorInstructions && !forRepositoryView
alt Problem Statement selected (instructions shown)
EditorContainer-->>Instructions: render (isEditor: false, disableCollapse: true)
else Regular file or repository view
EditorContainer-->>Monaco: render Monaco editor with sessionId and file events
end
sequenceDiagram
autonumber
participant User
participant RepoService as CodeEditorRepositoryFileService
participant Server
User->>RepoService: updateFiles(fileUpdates[])
RepoService->>RepoService: filtered = fileUpdates.filter(u => !u.fileName.includes(PROBLEM_STATEMENT_IDENTIFIER))
RepoService->>Server: PUT /api/repository/files with filtered
Server-->>RepoService: 200 OK
RepoService-->>User: complete
Note over RepoService: Problem Statement pseudo-file updates excluded from save
sequenceDiagram
autonumber
participant Editor as InstructorBase
participant User
participant Preview as Instructions Preview
User->>Editor: edit problemStatement markdown
Editor->>Editor: onInstructionChanged(markdown) -> push to subject
Editor->>Editor: debounce 200ms, map to void, shareReplay
Editor-->>Preview: previewEvents$ triggers preview render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.html (1)
124-131: Fix: pass correct parent folder to create-nodeWhen creating inside a folder, [folder] should be the folder path, not empty. Otherwise the create component cannot derive the correct target path.
Apply this diff:
- [folder]="''" + [folder]="item.value"src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.spec.ts (2)
274-279: Fix unsupported Jest matcher
toHaveBeenCalledExactlyOnceWithis not standard in Jest/Jest-Extended and will likely fail CI. Use a combination oftoHaveBeenCalledTimes(1)andtoHaveBeenCalledWith.- expect(transformTreeToTreeViewItemStub).toHaveBeenCalledExactlyOnceWith([treeNode]); + expect(transformTreeToTreeViewItemStub).toHaveBeenCalledTimes(1); + expect(transformTreeToTreeViewItemStub).toHaveBeenCalledWith([treeNode]);
807-837: Test bug: renaming a folder is flagged as FILEThe setup uses
FileType.FILEfor a folder rename. This can mask logic errors and reduce test fidelity.- comp.renamingFile = [folderName, folderName, FileType.FILE]; + comp.renamingFile = [folderName, folderName, FileType.FOLDER];src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.ts (1)
642-651: Block destructive/invalid operations on the Problem Statement node.Prevent rename/delete/create using the reserved identifier; otherwise backend calls will fail or corrupt state.
openDeleteFileModal(item: TreeViewItem<string>) { const { value: filePath } = item; const fileType = this.repositoryFiles[filePath]; - if (filePath) { + if (filePath && fileType !== FileType.PROBLEM_STATEMENT) { const modalRef = this.modalService.open(CodeEditorFileBrowserDeleteComponent, { keyboard: true, size: 'lg' }); modalRef.componentInstance.parent = this; modalRef.componentInstance.fileNameToDelete = filePath; modalRef.componentInstance.fileType = fileType; } } @@ setRenamingFile(item: TreeViewItem<string>) { - this.renamingFile = [item.value, item.text, this.repositoryFiles[item.value]]; + if (this.repositoryFiles[item.value] === FileType.PROBLEM_STATEMENT) { + return; + } + this.renamingFile = [item.value, item.text, this.repositoryFiles[item.value]]; } @@ onRenameFile(event: any) { const newFileNamePath = event as string; @@ - const [filePath, , fileType] = this.renamingFile; + const [filePath, , fileType] = this.renamingFile; + if (this.isProblemStatement(filePath) || newFileName === this.PROBLEM_STATEMENT_IDENTIFIER) { + this.onError.emit('unsupportedFile'); + return; + } @@ onCreateFile(fileName: string) { @@ - const file = folderPath ? `${folderPath}/${fileName}` : fileName; + const file = folderPath ? `${folderPath}/${fileName}` : fileName; + if (this.isProblemStatement(file)) { + this.onError.emit('unsupportedFile'); + return; + }Also applies to: 535-537, 523-530, 549-582
🧹 Nitpick comments (20)
src/main/webapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.ts (1)
56-66: Type the event and simplify stopPropagation callUse the base Event type and optional chaining for stopPropagation; keeps the API tighter without changing behavior.
- toggleEditorCollapse(event: any) { + toggleEditorCollapse(event: Event) { // make instructions monaco editor in the main editor not collapsible if (this.disableCollapse) { - if (event?.stopPropagation) { - event.stopPropagation(); - } + event?.stopPropagation?.(); return; }src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-base-container.component.ts (3)
22-24: Consolidate RxJS importsMinor style: merge duplicate rxjs/operators imports into the existing lines to reduce import churn.
82-86: Debounce is good; add distinctUntilChanged to avoid redundant rendersAvoid issuing preview ticks when the markdown text hasn’t changed (helps with frequent cursor movements or identical saves).
- previewEvents$ = this.problemStatementChanges$.pipe( - debounceTime(200), // TODO: consider to prune to 300–500ms for PlantUML-heavy content - map(() => void 0), // Observable<void> - shareReplay({ bufferSize: 1, refCount: true }), // replay latest for late subscribers - ); + previewEvents$ = this.problemStatementChanges$.pipe( + debounceTime(200), // TODO: consider 300–500ms for PlantUML-heavy content + distinctUntilChanged(), + map(() => void 0), + shareReplay({ bufferSize: 1, refCount: true }), + );Note: add
distinctUntilChangedimport from rxjs/operators.
159-162: Public bridge method is clear; consider completing the Subject on destroyCompleting the Subject is a tiny safety to release references after teardown.
ngOnDestroy() { + this.problemStatementChanges$.complete(); if (this.domainChangeSubscription) { this.domainChangeSubscription.unsubscribe(); } if (this.paramSub) { this.paramSub.unsubscribe(); } }src/main/webapp/app/programming/manage/instructions-editor/programming-exercise-editable-instruction.scss (3)
8-11: Make tooltip width responsive instead of fixed 500px.Use max-width and allow wrapping to avoid overflow on small screens.
- .tooltip-inner { - width: 500px; - } + .tooltip-inner { + max-width: min(500px, 90vw); + white-space: normal; + word-wrap: break-word; + }
31-35: Avoid !important if possible.If this is to beat overly-scoped third-party styles, consider a slightly more specific selector instead of !important.
- .markdown-editor-wrapper { - /* override scoped styling */ - background-color: var(--markdown-preview-editor-background) !important; + .editable-instruction-container .markdown-editor-wrapper { + /* override scoped styling without !important */ + background-color: var(--markdown-preview-editor-background); }
81-83: Use theme border variable instead of hard-coded rgba.Improves dark-mode/theming consistency.
- border-left: 1px solid rgba(0, 0, 0, 0.125); + border-left: 1px solid var(--border-color);src/main/webapp/app/programming/shared/code-editor/layout/code-editor-grid/code-editor-grid.scss (2)
279-283: Stylelint no-descending-specificity notice: consider placement or annotate.Your
.editor-center .instructions-wrapper__contentcomes after a more specific right-pane selector targeting the same class; while they don't overlap, the linter warns. Either move the center block above the right-pane block or locally disable the rule.-/* allow all layers in the center to shrink instead of pushing the right pane */ -.editor-center .instructions-wrapper, +/* allow all layers in the center to shrink instead of pushing the right pane */ +/* stylelint-disable-next-line no-descending-specificity */ +.editor-center .instructions-wrapper, -/* contain the markdown editor’s inner flow (it defaults to overflow: visible) */ -.editor-center jhi-markdown-editor-monaco .markdown-editor-wrapper { +/* contain the markdown editor’s inner flow (it defaults to overflow: visible) */ +/* stylelint-disable-next-line no-descending-specificity */ +.editor-center jhi-markdown-editor-monaco .markdown-editor-wrapper {Also applies to: 297-302
97-143: Consider extracting shared “collapsed width” into a CSS variable.Avoids magic 50px in multiple places and eases future changes.
+ :root { + --editor-collapsed-width: 50px; + } - width: 50px !important; + width: var(--editor-collapsed-width) !important; ... - width: 50px; + width: var(--editor-collapsed-width);Also applies to: 170-227
src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.scss (1)
109-115: Fix stylelint “rule-empty-line-before” noticesCodacy flagged missing empty lines before rules. Insert blank lines to satisfy the rule.
.problem-statement-icon { color: var(--bs-primary); } + .list-group-item__fileName { color: var(--bs-primary); font-weight: 600; } @@ - &.node-selected .problem-statement-icon, + + &.node-selected .problem-statement-icon, &.node-selected .list-group-item__fileName { color: inherit; font-weight: inherit; }Also applies to: 120-124
src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.html (2)
89-105: Avoid calling methods from templates (perf + guideline)[getFolderBadges(item)] is invoked per CD cycle. Our guidelines state methods_in_html:false. Precompute badges per folder in the TS (e.g., cache on collapse/expand) or use a pure pipe.
83-86: Event payload: prefer forwarding $eventMinor simplification: let the child emit the selected node and forward it directly instead of closing over item.
- <jhi-code-editor-file-browser-problem-statement [item]="item" (onNodeSelect)="handleNodeSelected(item)" /> + <jhi-code-editor-file-browser-problem-statement [item]="item" (onNodeSelect)="handleNodeSelected($event)" />src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.ts (2)
58-60: Two ViewChilds for instructions may be ambiguousYou now have both
@ViewChild('rightInstructions') rightInstructionsand@ViewChild(CodeEditorInstructionsComponent) instructions. With two instances in the template, the type-based query can bind to an unintended one. Either remove the type-based query or scope both with template refs for determinism.
58-58: Deduplicate PROBLEM_STATEMENT_IDENTIFIERThis magic string is defined in multiple components. Extract a shared constant (e.g., code-editor.constants.ts) and import it to avoid drift.
src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.html (2)
83-92: Hide right-side instructions while editing Problem Statement in centerWhen the center shows instructions (PS selected), also rendering the right panel duplicates content and reduces editor space. Gate the right panel on PS not being selected.
- @if (showEditorInstructions) { + @if (showEditorInstructions && selectedFile !== PROBLEM_STATEMENT_IDENTIFIER) {
64-66: Avoid methods in template (perf guideline)
[feedbacks]="feedbackForSubmission()"violates methods_in_html:false and can trigger extra work per CD. Expose a cached property updated on relevant inputs (participation/result changes) instead.src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.spec.ts (1)
538-587: Add an assertion to verify correct parent folder is passed to create-nodeTo prevent regressions (e.g., binding the folder as ''), assert that the create-node receives the expected
folderinput when creating inside a folder.I can extend this test to query the component instance and check its
folderinput equals'folder2'.src/main/webapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.html (1)
12-17: Deduplicate the two conditional label blocks using a single dynamic translation.This avoids repetition and keeps the collapsed/expanded headers consistent.
- @if (!isEditor) { - <span class="header" jhiTranslate="artemisApp.exercise.problemStatement"></span> - } - @if (isEditor) { - <span class="header" jhiTranslate="artemisApp.exercise.problemStatementEditor"></span> - } + <span class="header"> + {{ (isEditor ? 'artemisApp.exercise.problemStatementEditor' : 'artemisApp.exercise.problemStatement') | artemisTranslate }} + </span>Apply the same change in the collapsed header block. Ensure ArtemisTranslatePipe is available to this component.
Also applies to: 31-36
src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.ts (2)
306-311: Showing PS on load error is good; also suppress file ops for PS.Follow-up: ensure PS cannot be renamed/deleted/opened via repository service in error states.
377-388: Top-pinning sort is fine; exclude PS from consumer-facing file lists.Some consumers of onFileChange and similar APIs expect real repository files only. Filter PS out when emitting file lists to avoid accidental commit/status side-effects.
- this.onFileChange.emit([Object.keys(this.repositoryFiles), fileChange]); + const realFiles = Object.keys(this.repositoryFiles).filter((k) => k !== this.PROBLEM_STATEMENT_IDENTIFIER); + this.onFileChange.emit([realFiles, fileChange]);Apply in any other places where a raw key list is emitted.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (22)
src/main/webapp/app/exam/overview/exercises/programming/programming-exam-submission.component.html(1 hunks)src/main/webapp/app/programming/manage/assess/code-editor-tutor-assessment-container/code-editor-tutor-assessment-container.component.html(1 hunks)src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.html(1 hunks)src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.ts(2 hunks)src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.html(2 hunks)src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.spec.ts(10 hunks)src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.ts(10 hunks)src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.scss(1 hunks)src/main/webapp/app/programming/manage/code-editor/file-browser/problem-statement/code-editor-file-browser-problem-statement.component.html(1 hunks)src/main/webapp/app/programming/manage/code-editor/file-browser/problem-statement/code-editor-file-browser-problem-statement.component.ts(1 hunks)src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-and-editor-container.component.html(1 hunks)src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-and-editor-container.component.ts(2 hunks)src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-base-container.component.ts(5 hunks)src/main/webapp/app/programming/manage/instructions-editor/programming-exercise-editable-instruction.component.html(1 hunks)src/main/webapp/app/programming/manage/instructions-editor/programming-exercise-editable-instruction.scss(3 hunks)src/main/webapp/app/programming/overview/code-editor-student-container/code-editor-student-container.component.html(1 hunks)src/main/webapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.html(2 hunks)src/main/webapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.ts(2 hunks)src/main/webapp/app/programming/shared/code-editor/layout/code-editor-grid/code-editor-grid.scss(8 hunks)src/main/webapp/app/programming/shared/code-editor/model/code-editor.model.ts(1 hunks)src/main/webapp/i18n/de/exercise.json(1 hunks)src/main/webapp/i18n/en/exercise.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/main/webapp/**/*.html
⚙️ CodeRabbit configuration file
@if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
Files:
src/main/webapp/app/programming/manage/code-editor/file-browser/problem-statement/code-editor-file-browser-problem-statement.component.htmlsrc/main/webapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.htmlsrc/main/webapp/app/programming/manage/assess/code-editor-tutor-assessment-container/code-editor-tutor-assessment-container.component.htmlsrc/main/webapp/app/programming/overview/code-editor-student-container/code-editor-student-container.component.htmlsrc/main/webapp/app/exam/overview/exercises/programming/programming-exam-submission.component.htmlsrc/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.htmlsrc/main/webapp/app/programming/manage/instructions-editor/programming-exercise-editable-instruction.component.htmlsrc/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.htmlsrc/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-and-editor-container.component.html
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/programming/manage/code-editor/file-browser/problem-statement/code-editor-file-browser-problem-statement.component.tssrc/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-and-editor-container.component.tssrc/main/webapp/app/programming/shared/code-editor/model/code-editor.model.tssrc/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-base-container.component.tssrc/main/webapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.tssrc/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.spec.tssrc/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.tssrc/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.ts
src/main/webapp/i18n/de/**/*.json
⚙️ CodeRabbit configuration file
German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
Files:
src/main/webapp/i18n/de/exercise.json
🧠 Learnings (6)
📚 Learning: 2024-10-14T10:27:58.500Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/i18n/de/student-dashboard.json:36-37
Timestamp: 2024-10-14T10:27:58.500Z
Learning: Avoid suggesting the translation change for "noExercisesFoundWithAppliedFilter" in the PR ls1intum/Artemis#8858. The preferred translation is "Keine Aufgaben passen zu den gewählten Filtern."
Applied to files:
src/main/webapp/i18n/de/exercise.json
📚 Learning: 2024-10-08T15:35:52.595Z
Learnt from: florian-glombik
PR: ls1intum/Artemis#8858
File: src/main/webapp/i18n/de/student-dashboard.json:0-0
Timestamp: 2024-10-08T15:35:52.595Z
Learning: Avoid suggesting the phrase "Keine Aufgaben passen zu den gewählten Filtern" for the translation key `noExercisesFoundWithAppliedFilter` in the PR ls1intum/Artemis#8858.
Applied to files:
src/main/webapp/i18n/de/exercise.json
📚 Learning: 2024-10-08T15:35:52.595Z
Learnt from: pzdr7
PR: ls1intum/Artemis#9407
File: src/main/webapp/app/shared/monaco-editor/monaco-editor.service.ts:66-66
Timestamp: 2024-10-08T15:35:52.595Z
Learning: In the `MonacoDiffEditorComponent` (`src/main/webapp/app/shared/monaco-editor/monaco-diff-editor.component.ts`), the diff editor is always set to read-only (`readOnly: true`), and there are currently no use cases that require editing in the diff view.
Applied to files:
src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.html
📚 Learning: 2024-10-20T21:59:11.630Z
Learnt from: pzdr7
PR: ls1intum/Artemis#9505
File: src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html:9-9
Timestamp: 2024-10-20T21:59:11.630Z
Learning: In Angular templates within the Artemis project (e.g., `src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html`), properties like `selectedFile()`, `readOnlyManualFeedback()`, `highlightDifferences()`, and `course()` are signals. It is appropriate to call these signals directly in the template.
Applied to files:
src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.htmlsrc/main/webapp/app/programming/manage/instructions-editor/programming-exercise-editable-instruction.component.htmlsrc/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-and-editor-container.component.html
📚 Learning: 2025-02-11T15:46:35.616Z
Learnt from: Wallenstein61
PR: ls1intum/Artemis#9909
File: src/main/webapp/app/sharing/search-result-dto.model.ts:50-55
Timestamp: 2025-02-11T15:46:35.616Z
Learning: The `IExerciseType` enum in `src/main/webapp/app/sharing/search-result-dto.model.ts` must maintain its current naming (with 'I' prefix) and lowercase string values to ensure compatibility with the external sharing platform connector interface. This is an intentional exception to our TypeScript naming conventions.
Applied to files:
src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-base-container.component.ts
📚 Learning: 2024-09-30T08:34:49.363Z
Learnt from: iyannsch
PR: ls1intum/Artemis#9379
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:143-170
Timestamp: 2024-09-30T08:34:49.363Z
Learning: In `code-button.component.ts`, `this.exercise.id` is checked before use, so it's acceptable to use the non-null assertion operator `!` on `this.exercise.id!`.
Applied to files:
src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-base-container.component.ts
🧬 Code graph analysis (3)
src/main/webapp/app/programming/manage/code-editor/file-browser/problem-statement/code-editor-file-browser-problem-statement.component.ts (2)
src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.ts (1)
Component(36-343)src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.ts (1)
Component(65-702)
src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-base-container.component.ts (1)
src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.ts (1)
markdown(165-168)
src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.ts (1)
src/main/webapp/app/programming/shared/code-editor/model/code-editor.model.ts (1)
FileBadge(133-138)
🪛 GitHub Check: Codacy Static Code Analysis
src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.scss
[notice] 111-111: src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.scss#L111
Expected empty line before rule (rule-empty-line-before)
[notice] 120-120: src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.scss#L120
Expected empty line before rule (rule-empty-line-before)
src/main/webapp/app/programming/shared/code-editor/layout/code-editor-grid/code-editor-grid.scss
[notice] 279-279: src/main/webapp/app/programming/shared/code-editor/layout/code-editor-grid/code-editor-grid.scss#L279
Expected selector ".editor-center .instructions-wrapper__content" to come before selector ".editor-wrapper .editor-main .editor-sidebar-right .instructions-wrapper__content", at line 59 (no-descending-specificity)
[notice] 297-297: src/main/webapp/app/programming/shared/code-editor/layout/code-editor-grid/code-editor-grid.scss#L297
Expected selector ".editor-center .instructions-wrapper__content" to come before selector ".editor-wrapper .editor-main .editor-sidebar-right .instructions-wrapper__content", at line 59 (no-descending-specificity)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (28)
src/main/webapp/i18n/en/exercise.json (1)
61-61: LGTM: added translation key“problemStatementEditor” reads well and matches the intended mode switch in the UI.
src/main/webapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.ts (1)
23-29: Inputs for editor/preview behavior look gooddisableCollapse and isEditor defaults are sensible; matches the template’s conditional behavior.
src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-base-container.component.ts (3)
49-51: Good: scoped Subject for PS changesPrivate Subject is appropriate; no external writes.
93-97: Initial emissions look fine; ensure no double initial tickYou already guard with
!= null. With distinctUntilChanged (above), duplicate identical initial emissions won’t re-render.Also applies to: 111-115
80-86: previewEvents$ and onInstructionChanged wiring verified
Everything is correctly bound—(instructionChange)="onInstructionChanged($event)"and[generateHtmlEvents]="previewEvents$"are in place.src/main/webapp/app/programming/manage/instructions-editor/programming-exercise-editable-instruction.scss (3)
31-41: Confirm z-index for sticky .nav is sufficient.z-index: 1 might fall under other sticky headers/overlays (e.g., Monaco). Bump to 2–3 if you notice overlap.
- .nav { - position: sticky; - top: 0; - z-index: 1; + .nav { + position: sticky; + top: 0; + z-index: 3; background-color: var(--markdown-preview-editor-background); }
60-71: Status grid changes look good; min-width: 0 prevents overflow.The 1fr 2fr split and min-width: 0 fixes are solid for shrink behavior.
Also applies to: 73-89
84-87: Re-evaluate forced center alignment in status cells.Centering all direct children may hurt readability of multi-line texts. Consider left-align for textual content.
- > * { - justify-self: center; - text-align: center; - } + > * { + justify-self: center; + } + > *:where(p, span, .text, .label) { + text-align: left; + }src/main/webapp/app/programming/shared/code-editor/layout/code-editor-grid/code-editor-grid.scss (3)
27-31: Good addition of min-width: 0 to allow center strip to shrink.Prevents right pane from being pushed; matches the PR goal.
Also applies to: 44-51
255-267: Resizer UX improvements look right.Padding + user-select/touch-action tweaks help grabbing without side effects.
169-227: Right-pane collapsed state hides status; verify a11y.Hiding with display: none is fine; ensure keyboard focus isn’t trapped on hidden elements and aria-expanded on the toggle reflects state.
src/main/webapp/i18n/de/exercise.json (2)
61-62: German key added is fine and informal requirement is respected.“Aufgabenstellung (Editor)” introduces no formal address; consistent with the style guide.
61-62: Cross-check EN translation and usage.Ensure the corresponding English key exists (“Problem Statement (Editor)”) and all templates switch between problemStatement and problemStatementEditor as intended.
src/main/webapp/app/exam/overview/exercises/programming/programming-exam-submission.component.html (2)
19-20: Binding forRepositoryView=true aligns with “hide PS in repository-only views”.Looks correct for exam online editor; confirm that JhiCodeEditorContainer exposes this @input and defaults remain backward compatible elsewhere.
61-69: Ensure instructions sidebar respects repository-only mode.Verify that when forRepositoryView=true, the Problem Statement pseudo-file isn’t injected/pinned in the file tree, and the sidebar still renders exercise instructions as before.
src/main/webapp/app/programming/overview/code-editor-student-container/code-editor-student-container.component.html (2)
23-24: forRepositoryView=true is appropriate for student editor view.Confirms repository-only presentation; ensure other usages of jhi-code-editor-container were updated consistently (e.g., tutor/instructor variants).
82-89: Check class stacking with new center shrink styles.With the center shrink rules, ensure the
.instructions-wrapper__content card-body p-0still clips overflowing content as intended.src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.scss (1)
89-127: Solid UX accent for the Problem Statement entryThe left accent bar + primary color treatment reads well and matches the tree’s selection model. No functional concerns.
src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-and-editor-container.component.ts (1)
8-8: LGTM: exposes render component to the templateImport + registration look correct for the standalone render component.
Also applies to: 37-37
src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.html (1)
110-112: LGTM: null-safe defaults for change indicators and badgesUsing optional chaining with nullish coalescing prevents template errors during load and keeps the UI consistent.
src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.ts (1)
116-125: LGTM: getter/setter wrapper for selectedFileEncapsulating
selectedFilebehind accessors prepares for side effects without changing bindings.src/main/webapp/app/programming/manage/code-editor/container/code-editor-container.component.html (1)
48-57: LGTM: conditional center-pane rendering for PSThe switch to an instructions center when PS is selected is clean and keeps normal Monaco path intact.
src/main/webapp/app/programming/manage/code-editor/instructor-and-editor-container/code-editor-instructor-and-editor-container.component.html (2)
151-159: LGTM: move editable instructions to center + emit instructionChangeCentering the editable instructions and emitting
instructionChangealigns with the new authoring flow.
161-169: LGTM: add preview-only instructions on the rightRight-hand preview wired to
previewEvents$matches the debounced live-preview objective.src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.spec.ts (1)
175-199: LGTM: repository-empty still renders PS nodeGood coverage for the empty-repo case with PS presence.
src/main/webapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.html (1)
51-53: Confirm layout/resize semantics after wrapper change.Switching from display: contents to a wrapper class can affect the resizable container’s height calculations. Please verify RG/resizer behavior and overflow in narrow viewports.
src/main/webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.ts (2)
287-289: Guard after backend load to avoid overriding a real file named like the PS.initializeRepositoryFiles() should not overwrite a real file; the earlier fix covers it. Ensure tests cover this collision case.
30-31: Icon/component wiring LGTM.Problem Statement component import and faListAlt usage look correct.
Also applies to: 77-81, 172-172
...code-editor-tutor-assessment-container/code-editor-tutor-assessment-container.component.html
Show resolved
Hide resolved
...webapp/app/programming/manage/code-editor/file-browser/code-editor-file-browser.component.ts
Outdated
Show resolved
Hide resolved
...tor/file-browser/problem-statement/code-editor-file-browser-problem-statement.component.html
Show resolved
Hide resolved
...ditor/file-browser/problem-statement/code-editor-file-browser-problem-statement.component.ts
Show resolved
Hide resolved
...gramming/manage/instructions-editor/programming-exercise-editable-instruction.component.html
Show resolved
Hide resolved
...bapp/app/programming/shared/code-editor/instructions/code-editor-instructions.component.html
Outdated
Show resolved
Hide resolved
src/main/webapp/app/programming/shared/code-editor/model/code-editor.model.ts
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
There was a problem hiding this comment.
Code looks good now, thanks for the changes!
Retested on TS2 and it works as expected.
- Problem statement editor is now in the middle and has the problem statement toolbar
- Problem statement live updates responsively after changes in the editor without performance issue thanks to the debouncing
The "Go to repository" view is also unchanged:
Thanks for the changes!
|
@HawKhiem to check client test and see what to do with test coverage: |
HawKhiem
dismissed stale reviews from Elfari1028 and FelixTJDietrich
via
5b422d8
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
krusche
dismissed stale reviews from ShuaiweiYu, coderabbitai[bot], LeZhen1105, ekayandan, KonstiAnon, FelixTJDietrich, and Elfari1028
via
fa94aed
krusche
deleted the
feature/programming-exercises/problem-statement-file-tree
branch
github-project-automation
bot
moved this from Ready For Review
to Merged
in Artemis Development
Checklist
General
Client
Changes affecting Programming Exercises
Motivation and Context
Instructors editing programming exercises previously had to switch contexts to modify the problem statement. This PR addresses the issue #10968 and integrates the Problem Statement directly into the editor’s file tree as a selectable item, enabling a unified editing experience in Monaco with a live preview.
Description
Steps for Testing
Prerequisites:
I. Test 1:
II. Test 2:
Exam Mode Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Performance Review
Code Review
Manual Tests
Test Coverage
Screenshots
Problem statement entry should not be visible when assessing students' submissions
All of the above requirements should be applicable for programming exercise in exam
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests
Chores