Modeling exercises: Improve the editor experience across creation, submission, and assessment - #13429
Modeling exercises: Improve the editor experience across creation, submission, and assessment#13429FelixTJDietrich wants to merge 33 commits into
Modeling exercises: Improve the editor experience across creation, submission, and assessment#13429Conversation
Modeling exercises: Upgrade Apollon and unify editor workspacesModeling exercises: Improve the editor experience across creation, submission, and assessment
End-to-End Test Results
❌ Failed Tests
Test Strategy: Running all tests (configuration or infrastructure changes detected) Overall: ❌ E2E: real (non-flaky) test failure 🔗 Workflow Run · 📊 Test Report |
Squashed work-in-progress covering the Apollon 5.2 integration follow-ups: the assessment side panel, Athena chrome placement, exam and summary layout fixes, the quiz diagram dialog, the example-solution publication timeline, and the example-submission practice assessment flow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…xercises/apollon-5.2-integration # Conflicts: # src/main/webapp/app/fileupload/manage/file-upload-exercise-timeline/file-upload-exercise-timeline.component.html # src/main/webapp/app/fileupload/manage/file-upload-exercise-timeline/file-upload-exercise-timeline.component.ts # src/main/webapp/app/fileupload/manage/update/file-upload-exercise-update.component.html # src/main/webapp/app/fileupload/manage/update/file-upload-exercise-update.component.spec.ts # src/main/webapp/app/fileupload/manage/update/file-upload-exercise-update.component.ts # src/main/webapp/app/modeling/manage/modeling-exercise-timeline/modeling-exercise-timeline.component.html # src/main/webapp/app/modeling/manage/modeling-exercise-timeline/modeling-exercise-timeline.component.ts # src/main/webapp/app/modeling/manage/modeling-exercise/modeling-exercise.component.spec.ts # src/main/webapp/app/modeling/manage/modeling-exercise/modeling-exercise.component.ts # src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.html # src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.spec.ts # src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts # src/main/webapp/app/text/manage/text-exercise/text-exercise-timeline/text-exercise-timeline.component.html # src/main/webapp/app/text/manage/text-exercise/text-exercise-timeline/text-exercise-timeline.component.ts # src/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.html # src/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.spec.ts # src/main/webapp/app/text/manage/text-exercise/update/text-exercise-update.component.ts
…lop merge Develop moved TUM UI dialogs onto the Angular CDK overlay, whose container is appended to <html> at $zindex-modal + 5 — below the fullscreen editor frame. The help dialog therefore opened behind the canvas. Apollon's popovers still use a Base UI portal under <body>, so both roots need lifting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviewing a result is reading, not editing, and the summary card has width to spare - so the feedback belongs beside the diagram, as it was before the panel moved into the editor's chrome, rather than floating over it. The camera also has to refit once the rail reserves its width. Reserving room is not the same as using it: without a refit the nodes kept their old framing and sat underneath the panel.
…ck panel
The panel floats so it keeps the notch-and-glass look of the editor's other
chrome, which means only its trigger contributes an inset - the camera framed
the diagram behind the open panel. Reserving the panel's measured width as an
explicit inset keeps the float and lays the diagram out clear of it.
Also stops the exam modeling assessment page titling itself "Apollon Diagram
{{ title }}": that key belongs to the quiz diagram editor, and its parameter
was never interpolated. Every other exam exercise type already uses its own
exercise title, as does the course-level modeling assessment route.
Two artefacts appeared under the notch when the explanation took focus. The body outline runs the whole way round, including the stretch the notch sits on, and the seam rect that hides it is 92% opaque glass - so the primary stroke bled through as a faint line under the label. That stretch is now clipped out of the outline, so it is never painted there to begin with. The glow was a filter on the whole backdrop, which makes every child its own silhouette: the two helper rects that fill the seam and square off the corner each grew a halo of their own. The glow now belongs to the outline shapes.
…efacts Apollon 5.2 owns canvas sizing, so the two canvas shells stopped binding resizeOptions - but the input, its derived edges, the size constants, the jhiResizable wiring, the handle markup and its SCSS all stayed behind, computing values nothing read. Also drops 14 debugging scripts and 4 help images that were committed by accident and referenced by nothing.
Apollon renders through React Flow, which does not size itself to its content - its container must have a resolved height. Removing the "dead" resize machinery took the default `height: 600px` with it, on the premise that Apollon 5.2 owns canvas sizing. That holds only for the tile and fullscreen variants, which take their height from a sized parent. Everywhere else - the example solution editor on the exercise create/edit form, which sits in a scrolling page - there is nothing to inherit from, so the canvas collapsed to zero height and the editor rendered as nothing at all. React Flow says so in the console: "The parent container needs a width and a height to render the graph." Only that one usage regressed: three others pass `[tile]`, and the remaining six are read-only and render an SVG through a separately sized `.readonly-diagram`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LuVfUjEN3BSUNFK5TyQ37k
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@FelixTJDietrich The new assessment fullscreen action uses subtree fullscreen even though assessment popups are body-portaled, so enabled feedback popovers disappear while fullscreen. The captured required gate is red because test/quality/e2e jobs were cancelled, and the available E2E report references neither this head nor the failed run, so I could not attribute that CI failure to this change.
|
Just tested the whole flow and practice mode is broken, will need to fix this tomorrow. |
…on model shape `getSubmissionSize` added `elements.length + relationships.length`, which only matches the v4 array form. The keyed v3 form and the v5 nodes/edges form both yield `undefined + undefined`, so the example submission list rendered `NaN`. Count through `countModelElements`, which handles all three shapes; the spec that already covers them now passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196cj1LjAuhYCBBBd4p89u3
…llscreen popup-safe Practice mode opened on the graded assessment instead of the editor. The graded and the practice participation are both `participate/:participationId` on the same route, so switching between them only re-emits the route params and reuses `ModelingSubmissionComponent`. `result`, `assessmentResult` and the surrounding assessment state are assigned only when the freshly loaded submission carries a result, so the practice attempt inherited the graded one. Reset everything submission-scoped before applying the new submission, and dispose the autosave timer and the result websocket subscriptions that the same reuse path re-registered. The fullscreen control on the assessed canvas made the wrapper the fullscreen element. Apollon popovers render through a Base UI portal under `<body>`, and a browser paints nothing outside the fullscreen subtree, so enabled feedback popups disappeared in fullscreen. `ModelingAssessmentComponent` now promotes its frame to `<body>` and takes the document root fullscreen, the same way `ModelingEditorComponent` does; both mark the promoted frame with a shared class the global portal stacking rules key on. `FullscreenComponent`, whose subtree fullscreen was the last consumer of that mechanism, is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196cj1LjAuhYCBBBd4p89u3
|
Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
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 |
…xercises/apollon-5.2-integration # Conflicts: # src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.html # src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.spec.ts # src/main/webapp/app/modeling/manage/update/modeling-exercise-update.component.ts
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@FelixTJDietrich The current head resolves the fullscreen issue: both assessed canvas and editor use document-root fullscreen with a shared promoted-frame marker, keeping body-portaled Apollon popovers and CDK overlays visible. The participation-switch reset and cross-version element counting have focused regression coverage, and I found no remaining blocking issue; the snapshot had no failed checks, although several non-required CI jobs were still running.
… was missing The client-test job is red on the modeling module gate (statements 87.12 / 87.30, branches 69.93 / 72.20, lines 87.11 / 87.30). The gate had not actually run on this branch before: the previous CI run failed at the test step, which short-circuits the coverage check, so the shortfall was masked rather than new. Cover the branch's own least-tested paths: the feedback tone/score/element-name presentation helpers, the unsaved-changes and unload guards, the collaboration-user fallbacks, the complaint-section conditions, and the fullscreen edge cases on the assessed canvas (another element already fullscreen, a refused request, and teardown while it owns the screen). modeling: statements 88.19, branches 72.45, functions 88.95, lines 88.23. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196cj1LjAuhYCBBBd4p89u3
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@FelixTJDietrich The reactive example-assessment score calculation bypasses the canonical structured-grading scorer, causing limited instructions to be overcounted. Codacy Static Code Analysis is red, but its captured evidence contains only a generic complexity summary without a concrete failing rule, so I could not attribute an actionable failure. The captured E2E report predates this head and references neither this head nor the failed run.
…xercises/apollon-5.2-integration
…l grading scorer The reactive score summed raw credits, which ignores `GradingInstruction.usageCount`: a five-point instruction limited to one use scored ten when applied twice, so the header disagreed with the grading semantics the server persists. Route the computed through `StructuredGradingCriterionService.computeAssessmentScore`, which is what the imperative version it replaced already used; it caps and floors the total itself, so the extra clamping is gone. Restores the `usageCount` and score-cap regression tests that were dropped in the move to a computed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196cj1LjAuhYCBBBd4p89u3
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@FelixTJDietrich The structured-grading score fix is correct and restores coverage for usage limits and score capping. However, the shared exercise timeline now rejects valid example-solution publication schedules by comparing the date with the assessment due date. Codacy remains red with only a generic complexity summary; the captured E2E report does not reference this head, while the current non-required E2E job is still running.
| timelineItems.push({ | ||
| kind: 'optional', | ||
| labelStringKey: 'artemisApp.exercise.exampleSolutionPublicationDate', | ||
| date: this.exampleSolutionPublicationDate, |
There was a problem hiding this comment.
@FelixTJDietrich [medium] Appending this item without orderCheckAgainst makes TimelineComponent compare it with every preceding date, including assessmentDueDate. The server explicitly permits publishing an example solution before the assessment due date—and, for exercises excluded from the score, before the due date—so modeling, text, and file-upload forms now mark valid schedules invalid. Restrict this timeline item to the release-date ordering constraint and leave the existing exercise-specific validation to enforce the conditional due-date rule.
🤖 Prompt for AI agents
In src/main/webapp/app/exercise/exercise-timeline/exercise-update-timeline/exercise-update-timeline.component.ts, the example-solution publication item inherits an all-prior-items ordering check and therefore rejects domain-valid dates. Define a named release-date item and set this entry's orderCheckAgainst to only that item, leaving the due-date and included-in-score rule to the existing exercise validation.
There was a problem hiding this comment.
Fixed in 537b3b9 — correct, and the server confirms it. BaseExercise.isValidExampleSolutionPublicationDate bounds the date only by releaseDate and startDate; the dueDate bound is conditional on includedInOverallScore != NOT_INCLUDED, and assessmentDueDate is never a bound (only isValidAssessmentDueDate uses it).
The release and start items are now named and the entry carries orderCheckAgainst: [releaseDateItem, startDateItem], so it is bounded exactly by the server's hard lower bounds and the conditional due-date rule stays with the exercise-specific validation, as you suggested. Regression test in exercise-update-timeline.component.spec.ts asserts the restriction and that it references the rendered items rather than copies.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
WoH
left a comment
There was a problem hiding this comment.
tested locally at 1f9a72b: roles: instructor artemis_test_user_16, tutor _6, students _1/_2, admin.
Exercise authoring (instructor):
- editor fills tile, same border and radius as surrounding content: yes
- diagram type, help, fullscreen, explanation available without covering apollon controls: yes
- diagram type change asks confirmation with a non empty model: yes, cancel keeps the model, confirm discards
- multiline explanation, edit/preview, markdown rendered: yes, but the default 138px surface clips overflowing preview content with overflow hidden, no scrollbar, no indicator. only a manual resize reveals the rest. see inline comment on modeling-markdown-explanation-editor
- resize explanation, open/close minimap, stays aligned: yes
pr-13429-vA5-resize-minimap-tight.mp4
- fullscreen: problem statement open by default, resizable, nothing clips: yes
- resize problem statement, leave fullscreen, editor returns to page width: yes
modeling submission (student):
- editor fills tile, no redundant outer handle, no page scroll: yes
- save status unsaved/saving/saved, untruncated: yes
pr-13429-vB1-savestatus-tight.mp4
- resize problem statement split and browser window, no overlap: yes
pr-13429-vB2-thrash-tight.mp4
- pointer outside editor, browser zoom not captured: partial. pointer outside passes through, but with focus on the explanation resize handle ctrl+= / ctrl+0 / ctrl+- are still captured by apollon, 3/3 reproducible, and the help dialog's own text says shortcuts apply while the canvas has focus
- fullscreen, palette drag preview and element popover visible: yes
pr-13429-vB4-B6-fullscreen-small-tight.mp4
- help dialog and problem statement above the editor, usable: yes
- minimap open/close, exit fullscreen, explanation returns to correct width/position: yes
Example submission setup (instructor):
- edit model vs define assessment mode obvious: yes. but toggling read and confirm / assess correctly on an existing submission and saving from assessment mode silently reverts the toggle: those save paths put the stale object without applying the selection, only the submission mode save applies it. see inline comment
- explanation uses the same responsive surface, markdown edit/preview: yes
- define assessment legend icons, instructions, rationale, additional feedback readable: yes
- resize both split directions, bounds, cursor, indicator: yes
- narrow and wide widths, no overlap with palette/minimap/undo: yes
- empty additional feedback state light and dark: yes
Assessment training (tutor):
- example submission, explanation, instructions, rationale visible without page scroll: yes, but a reproducible typeerror fires on every training mount (3/3, sentry logged, attributed to modeling-explanation-surface.component.ts:62:40, a line that provably cannot throw that error, so the stack is misattributed and needs a real source mapped capture before fixing)
- switching model/assessment keeps the active mode clear: yes
- legend readable, items do not collapse to icon only: yes
- additional feedback uses available width, dark empty state ok: yes
- resizing keeps content accessible, bottom controls visible: yes
note: neither tutor facing screen (training or regular assessment) exposes a fullscreen button, so the fullscreen checks are not reachable there
Regular assessment (tutor):
- explanation and bottom editor controls visible without page scroll: yes
- points outside the editor: yes
- instructions, additional feedback, rationale, private tutor notes in the shared workspace: yes
- resize narrow and wide, no pane overflow, affordances visible: yes. note the 840/560 breakpoints react to the workspace container width, not the window, the sidebar offsets the numbers
- add feedback, save draft, reopen, submit: yes. after submit the canvas stays editable client side: node popover accepts edits and the point tally updates, nothing persists (no save path remains) but the ui misrepresents the lock. apollon 5.3 added setReadonly for this and nothing calls it. pre existing on develop, but this pr rewrote the file. see inline comment
pr-13429-vC03-readonly-flip-attempt-tight.mp4
- open another submission and cancel, context preserved: yes
also, blocking: the complaint / more feedback hint banner is gone for every exercise type. jhi-assessment-complaint-alert was deleted from assessment-layout together with its component and en+de i18n keys, no replacement anywhere, and text, programming, file upload and modeling all share that layout. the same hunk also gates the assessment note behind a new showAssessmentNote input, a second unannounced behavior change. see inline comment
cross cutting: light and dark at wide and 480px done, medium viewport reduced coverage. no control jumping, focus outlines ok in what was covered. training first load recorded clean, no score flash:
pr-13429-vC02-first-load-score-flash-tight.mp4
Exam mode:
- test run create, start, edit model and explanation, save indicator updates: yes, save status island flips to saved after autosave with no further input, checked explicitly
- resize split, enter/leave fullscreen, navigate to another exam exercise and return: no, blocking. entering fullscreen and then switching exercises leaves the old editor pinned fullscreen and click intercepting over everything. the exam switches underneath, only exit fullscreen on the stuck frame recovers. promote() moves the frame to document.body, so the page switcher's [hidden] on the old wrapper can no longer hide it, and nothing restores the presentation on page change. see inline comment
pr-13429-vA18-exam-fullscreen-switch-tight.mp4
- model and explanation restored after navigation, test run submits: yes
- test run assessment: yes, same behavior as regular assessment, including the post submit editable canvas
also checked:
- the red e2e on ci is this pr, seems not to be a flake: 6 hard failures + 1 flaky are the pr's own new modeling specs (ModelingAssessmentWorkspace, ModelingEditorFullscreen x3, ModelingEditorPageScroll, ExamModelingSummary), plus the pre existing ExamResults.spec.ts:197 broken by the rewritten assessment display.
- Not sure about codacity, I ignored that
Blocking for me: the exam fullscreen escape (2 above), the removed complaint banner, and the red e2e from the pr's own specs. the rest is inline.
| this.restoreAnchor = this.document.createComment('fullscreen-presentation-anchor'); | ||
| element.before(this.restoreAnchor); | ||
| this.promotedElement = element; | ||
| this.document.body.append(element); |
There was a problem hiding this comment.
promote() re-parents the frame to document.body, so any ancestor [hidden] stops applying. the exam page switcher relies on exactly that (exam-participation.component.html:77) and nothing restores the presentation on activePageIndex change: the stuck fullscreen repro in the review body. the fix should also cover a readOnly flip while fullscreen, same escape class, different trigger.
There was a problem hiding this comment.
Fixed in 537b3b9. You were right about both escapes and the second one was worse than the first.
The promotion is now self-terminating: promote() takes an onEscape callback and watches two things while it holds the frame — a shallow childList observer on <body> (the frame is a direct child while promoted, so this is cheap) catches Angular tearing the element out on a readOnly flip, and an IntersectionObserver on the anchor's parent catches the original slot losing its box, which is what [hidden] on the exam wrapper does. The intersection is only the trigger; the decision is checkVisibility(), so merely scrolling the page out of view is not treated as an escape. Both ModelingEditorComponent and ModelingAssessmentComponent restore and call exitFullscreen() from the callback.
restore() additionally refuses to re-insert a frame that is no longer connected, so a destroyed editor is not resurrected into the view that dropped it. Three regression tests in fullscreen-presentation.service.spec.ts cover hidden-slot escape, destroyed-element escape, and the no-resurrection rule.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
| @@ -30,9 +30,8 @@ | |||
| > | |||
| <ng-content select="[jhiBannerSlot]" ngProjectAs="[jhiBannerSlot]" /> | |||
| </jhi-assessment-header> | |||
There was a problem hiding this comment.
the complaint/more-feedback hint banner used to sit here and is deleted with its component and i18n keys, no replacement. this layout serves text, programming, file upload and modeling assessment alike. if the removal is intentional it needs an equivalent cue plus a callout in the pr description. the new showAssessmentNote gating in the same hunk is also a behavior change for all four consumers.
There was a problem hiding this comment.
Fixed in 537b3b9. The banner removal was not intentional and is restored — component, template, spec, and the four complaint.hint / moreFeedback.hint keys in en and de — so text, programming, file upload and modeling all get the cue back.
On showAssessmentNote: that one is deliberate and not a behavior change for the other three. It defaults to true, and the only caller passing false is modeling-assessment-editor.component.html, because modeling now renders the note inside its own assessment workspace and the shared layout would otherwise render a second one. I have added that reasoning as a doc comment on the input and called it out in the PR description.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
| this.handleMutationObserver = undefined; | ||
| return; | ||
| } | ||
| this.handleMutationObserver = new MutationObserver(() => this.applyHandleStyles(this.resizableEdges())); |
There was a problem hiding this comment.
this MutationObserver (childList+subtree) fires undebounced and the callback rewrites styles/aria unconditionally plus two getBoundingClientRect calls: measured ~2 forced reflows per keystroke in the explanation markdown editor. code-editor-grid and discussion-section use jhiResizable over equally churny subtrees, so programming editor typing and chat pay the same tax. coalesce via rAF like apollon-chrome-placement does, and diff before write. not a feedback loop (attributeFilter excludes its own writes), just reflow per mutation.
There was a problem hiding this comment.
Fixed in 537b3b9. Handle upkeep is now coalesced into one frame — the observer callback schedules through requestAnimationFrame instead of running per mutation, so a keystroke burst produces one pass — and every style/attribute write is diffed first, since writing an unchanged value still dirties style. The aria upkeep also measures once instead of calling getBoundingClientRect() per axis. The imperative call sites (the input effects) stay synchronous so initial setup is still immediate.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
|
|
||
| this.apollonEditor = new ApollonEditor(this.editorContainer()!.nativeElement, { | ||
| mode: ApollonMode.Assessment, | ||
| readonly: this.readOnly(), |
There was a problem hiding this comment.
readonly is baked in once here and setReadonly() is never called anywhere in the app, so a readOnly() flip on the live instance (e.g. submit sets a fresh result) leaves the canvas editable client side, repro in the review body. pre existing gap, but apollon 5.3 added setReadonly for exactly this and this file grew several new effects in this pr, one more for readOnly would close it.
There was a problem hiding this comment.
Fixed in 537b3b9. An effect now pushes readOnly() onto the live editor via setReadonly, and moves the assessment-selection subscription with it — selection is only reported while read-only, so a flip has to add or drop that subscription too. Regression test asserts both directions on the live instance.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
| } | ||
|
|
||
| const bottomCenter = this.editorBottomCenter()?.nativeElement; | ||
| if (this.bottomCenterMounted && (!bottomCenter || !this.hasEditorBottomCenter())) { |
There was a problem hiding this comment.
releaseRegionElement without re-parenting first detaches the projected rail element from the document entirely (apollon RegionMount cleanup removes it), so the [hidden] binding on it becomes a fiction while out of fullscreen. modeling-assessment.component.ts prepends the element back into the host before releasing, mirror that here.
There was a problem hiding this comment.
Fixed in 537b3b9. Added releaseHostRegion(region, element), which prepends the projected element back into the editor frame before calling releaseRegionElement, mirroring ModelingAssessmentComponent#synchronizeHostRegion. All five release sites now go through it: the three in mountHostChrome, the right-rail release when leaving fullscreen, and the four in destroyApollonEditor.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
|
|
||
| if (palette && paletteRegion === 'left-rail') { | ||
| placementLeft = Math.max(placementLeft, palette.right + chromeGap); | ||
| } else if (palette && paletteRegion === 'right-rail') { |
There was a problem hiding this comment.
nit: the right-rail palette branch of calculateBottomCenterPlacement has no spec, the rest of the file is well covered.
There was a problem hiding this comment.
Fixed in 537b3b9. Added two specs for the right-rail branch: the inline case, where the palette pulls the right bound in from the editor inset, and the elevated case, where it does the same after the zoom/minimap row squeezes the surface out of line. Both expectations are derived from the geometry rather than recorded from a run.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
| } | ||
| </div> | ||
|
|
||
| <div [hidden]="readOnly()" class="modeling-editor__canvas-shell"> |
There was a problem hiding this comment.
nit: [hidden]="readOnly()" is dead inside @if (!readOnly()).
There was a problem hiding this comment.
Fixed in 537b3b9 — removed.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
| # @tumaet/apollon is maintained in the same organization and this integration | ||
| # deliberately consumes its freshly published release. Keep the age-gate | ||
| # exception scoped to that package and remove it once 5.3.0 has aged past 24h. | ||
| minimumReleaseAgeExclude: |
There was a problem hiding this comment.
nit: the exclusion has no expiry, easy to forget once 5.3.0 passes the age cutoff. a dated todo or a follow-up issue would do.
There was a problem hiding this comment.
Fixed in 537b3b9. Added a dated TODO(2026-09-01) noting that 5.3.0 was published on 2026-08-20 and is long past the 24h cutoff, so the block exists only to keep the branch installable while it is open and should be deleted before merge unless another fresh Apollon release is being picked up.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
| if (message == null) { | ||
| return null; | ||
| } | ||
| return message.replaceAll("[\\p{Cntrl}]+", " "); |
There was a problem hiding this comment.
nit: String.replaceAll recompiles the pattern per call, hoist a private static final Pattern.
There was a problem hiding this comment.
Fixed in 537b3b9 — hoisted to a private static final Pattern CONTROL_CHARACTERS and the call site uses matcher(message).replaceAll(" ").
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
| <label | ||
| class="form-control-label" | ||
| for="exampleSolutionPublicationDateEnabled" | ||
| [ngStyle]="!canConfigureExampleSolutionPublication() ? { color: 'grey' } : {}" |
There was a problem hiding this comment.
nit: hardcoded grey via ngStyle in a shared component, no dark mode token for the disabled label state.
There was a problem hiding this comment.
Fixed in 537b3b9 — replaced the ngStyle grey with [class.text-muted-color], which is the semantic token and therefore theme-aware.
Note
🤖 Written by Claude Opus 5 (Claude Code) on my behalf. Not manually verified yet — the code change and its unit tests are in, but I have not re-run the affected flow by hand. Treat the reasoning as a proposal to check, not a confirmation.
…xercises/apollon-5.2-integration
Fullscreen escapes (blocking). `promote()` re-parents the frame to `<body>`, out of reach of everything its original ancestors do to it. The exam page switcher hides the previous exercise with `[hidden]` on a wrapper the frame no longer descends from, so switching exercises left the old editor pinned fullscreen and intercepting every click; a `readOnly` flip tore the frame's `@if` down while the document stayed fullscreen on nothing. The promotion now watches for both — a shallow `childList` watch on `<body>` for destruction, an IntersectionObserver on the original slot for hiding — and calls back so the owner restores and leaves fullscreen. Complaint banner (blocking). `jhi-assessment-complaint-alert` was deleted from the shared assessment layout together with its component and its en/de keys, with no replacement, for text, programming, file upload and modeling alike. Restored. The `showAssessmentNote` input in the same hunk is not a behavior change: it defaults to true and only modeling opts out, because modeling renders the note in its own workspace. E2E (blocking). The five failing modeling specs navigate to `modeling-exercises/8`, `submissions/8` and `example-submissions/1`, none of which the E2E seed provisions — it seeds courses, users and conversations only, and CI's `GET /api/exercise/exercises/8/title` answers 404. They now create their own exercise, submission and example submission and reach the assessment through the dashboard. `ExamResultsPage` still looked for `jhi-unified-feedback` markup that modeling no longer renders; it now matches both. Also from the review: - the assessed canvas pushes `readOnly` flips onto the live Apollon editor via `setReadonly`, so submitting really locks it instead of only appearing to; - the editor re-parents a projected region back into its frame before releasing it, as the assessment already did, so Apollon's cleanup cannot detach it; - the example-submission training toggle is applied on the assessment save paths, so it no longer reverts under a success toast; - the example-solution publication date is ordered only against the release and start dates, matching `BaseExercise.isValidExampleSolutionPublicationDate`, instead of against the assessment due date it was never bounded by; - `jhiResizable` coalesces handle upkeep into one frame and diffs before writing, so a churny subtree no longer forces a reflow per mutation; - the markdown explanation preview scrolls instead of clipping, and the explanation surface no longer hands zoom shortcuts to the canvas; - nits: dead `[hidden]`, hoisted `Pattern` in `HeaderUtil`, a theme token for the disabled timeline label, a dated expiry note on the release-age exclusion, and a spec for the right-rail palette branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196cj1LjAuhYCBBBd4p89u3
…rolls The restored "scroll down to review the complaint" banner pointed at a scroll the modeling assessment page cannot perform: the page is sized to the viewport with `overflow: hidden`, and the shared layout appends both the banner and the complaint form below the assessment. The form was therefore clipped out of reach entirely, and the banner sent the tutor somewhere they could not go. `AssessmentLayoutComponent` gains `showComplaintSection` (default true, so text, programming and file upload are untouched). The modeling assessment page opts out and renders both halves inside its own "Feedback & notes" pane, which is `overflow: auto` and `cdkScrollable` — so the banner's instruction is accurate there and the form is reachable. Banner and form are gated together; a layout that places one must place both. Verified in the running app: exactly one banner, inside the pane and visible without page scrolling, with the form reachable by scrolling the pane (scrollTop 272 of 545) while the page itself stays at scrollY 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196cj1LjAuhYCBBBd4p89u3
Once a modeling submission is assessed, the canvas is the read-only assessment view — there is no editor at all — yet the header still offered Submit, which would only resubmit unchanged work. The shell decided purely on "a participation exists"; the only result-aware guard in `effectiveOnSubmitExercise` is quiz-specific. `ExerciseSubmission` gains an optional `canSubmitExercise` signal, and the split panel withdraws the action when the routed component exposes it as false. Only components that can go read-only opt in, so text, file upload and programming keep the behaviour they had; `ModelingSubmissionComponent` implements it from `shouldShowLiveEditor()`. It also regains its `implements ExerciseSubmission` declaration, which the branch had dropped while keeping the method the duck-type guard looks for. Verified in the running app, both directions: with a manual assessment before the due date the header goes from `[Submit]` to `[]` with no editor rendered, while a fresh participation with no result still shows Submit over a live editor. Note this is deliberately modeling-scoped. Text and file upload have the same defect — text's Submit actually resubmits `answer()`, file upload's is a silent no-op — and are left for a separate change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196cj1LjAuhYCBBBd4p89u3
Selecting an element on an assessed canvas marked nothing in the feedback list, which left the selection ring looking purely decorative — the canvas appeared interactive while nothing came of it. `ModelingAssessmentComponent` subscribed to `subscribeToAssessmentSelection`, which reports the ids of the *assessments*. Every consumer of `selectedElementIdsChanged` matches the emitted ids against `Feedback.referenceId`, i.e. the id of the *element* a feedback references (the `<type>:<elementId>` reference), so the comparison could never match. Apollon's `subscribeToSelectionChange` is the API that reports element ids — its own signature names the parameter `selectedElementIds`. Confirmed against the live editor before changing anything: with a feedback referencing `Class:ccf5edad-…`, the assessment channel emitted unrelated assessment uuids while the selection channel emitted exactly `ccf5edad-…`. After the switch, clicking the assessed element marks its feedback row and clicking an unassessed one marks nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0196cj1LjAuhYCBBBd4p89u3
…xercises/apollon-5.2-integration
Summary
Improves the modeling editor experience for everyone who creates, solves, or assesses a modeling exercise. The editor now uses the same responsive layout and controls across exercise authoring, student submissions, example submissions, assessment training, grading, and exam participation.
Explanations remain visible and resizable, the problem statement stays available in fullscreen, and editor controls no longer overlap or disappear at smaller sizes. The change also integrates the pending Apollon release from ls1intum/Apollon#841, currently validated through a local package snapshot built from Apollon commit
900edf9b, and aligns its colors, spacing, corners, shadows, and interaction states with Artemis.Checklist
General
Client
No routes or authorities are added by this PR.
Motivation and Context
The modeling experience had evolved into several similar but independent layouts. This made the behavior depend on which page a person opened:
This change gives those screens one shared editor experience while preserving the information and actions needed by instructors, students, and tutors.
Description
Consistent editor experience
Authoring and student submissions
Assessment training and grading
Implementation notes for reviewers
Shared components this PR touches beyond modeling
Called out explicitly because
jhi-assessment-layoutandjhiResizableserve text, programming and file upload as well:AssessmentLayoutComponentgains ashowAssessmentNoteinput, defaulting totrue. Only the modeling assessment editor passesfalse, because modeling renders the assessment note inside its own workspace and the shared layout would otherwise render a second one. Text, programming and file upload are unchanged.jhi-assessment-complaint-alert) stays in the shared layout for every exercise type. An earlier revision of this branch removed it; that was not intentional and has been restored together with itscomplaint.hint/moreFeedback.hintkeys in en and de.jhiResizablenow coalesces its handle upkeep into one animation frame and diffs before writing styles and aria attributes. This is a performance change for every consumer (modeling explanation surface, code-editor grid, discussion section); behaviour is unchanged.FullscreenPresentationService.promote()takes anonEscapecallback and gives up the screen when the promoted frame is hidden by an ancestor or destroyed. Without it the exam page switcher could leave an editor pinned fullscreen over the whole app.Steps for Testing
Test data and accounts
Prepare the following before starting:
If the course does not yet contain suitable example submissions, create them during the instructor flow below before testing the tutor flow.
Exercise authoring as an instructor or editor
Navigation:
Verify:
Modeling submission as a student
Navigation:
Verify:
Example submission setup as an instructor or editor
Navigation:
Verify:
Assessment training as a tutor
Navigation:
Verify:
Regular assessment as a tutor or instructor
Navigation:
Verify:
Cross-cutting visual checks
Repeat the authoring, student, example-submission, and regular-assessment views with:
Confirm that controls do not jump when content expands, focus outlines follow the complete control shape, shadows remain consistent, and no editor content is clipped.
Exam Mode Testing
Test-run submission:
Test-run assessment:
Merge Dependency
develop; the integration conflicts are resolved.@tumaet/apollon@5.3.0.5.3.0registry artifact and its integrity hash; the temporaryfile:../.apollon-localdependency is removed.minimumReleaseAgeExcludeis still scoped to@tumaet/apollon. 5.3.0 is long past the 24h cutoff, so the block now only keeps the branch installable while it is open; it carries a dated TODO and should be deleted before merge unless another freshly published Apollon release is picked up. All third-party dependencies remain protected by the age gate.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 this PR, and trigger the deployment.
Test-server deployment and cross-role manual verification are pending while this PR is a draft.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Local Validation
Validation after merging current
developand installing the published@tumaet/apollon@5.3.0artifact:pnpm install --lockfile-only(1,585 lockfile entries pass supply-chain verification)pnpm install --frozen-lockfilepnpm run compile:testspnpm run webapp:buildpnpm run tum-ui:lint.artemisApp.modelingEditor.apollonexemptedgit diff --checkLatest validation on
537b3b99ab:tsc --noEmit(client) andpnpm run lint:playwrightmodeling(statements 88.18, branches 72.57, functions 88.68, lines 88.29)./gradlew spotlessCheck checkstyleMainrun-e2e-tests-local-fast.shcould not start its Postgres. They are fixed against the diagnosed root cause (see below) and need a green CI E2E run to confirm.Why the modeling E2E specs were red
Five of the six modeling failures had the same cause and were never able to pass in CI:
ModelingAssessmentWorkspace,ModelingEditorPageScrolland threeModelingEditorFullscreentests navigated to hardcodedmodeling-exercises/8,submissions/8andexample-submissions/1. The E2E seed provisions only courses, users, conversations and posts — no exercises — so CI answeredGET /api/exercise/exercises/8/titlewith 404 and the pages rendered nothing. They now create their own exercise, student submission and example submission throughExerciseAPIRequestsand reach the assessment through the dashboard.The sixth,
ExamResults › Check modeling exercise results, was a stale selector:ExamResultsPagelooked forjhi-unified-feedbackmarkup that the modeling submission view no longer renders. It now matches both markups, since text and programming still use the old one.Test Coverage
Client
Server
Last updated: 2026-08-25 11:17:37 UTC
Screenshots
Pending test-server deployment. Before marking this PR ready for review, add captures for: