Skip to content

Bugfix: Fix markdown in modeling exercises example solutions #11653

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
+10 −3
Open

Bugfix: Fix markdown in modeling exercises example solutions #11653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76578f2
Fix html mardkown pipe
m4rl0ne Nov 9, 2025
cea5f7f
Found workaround without altering modeling-explanation-editor.compone…
m4rl0ne Nov 16, 2025
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
8 changes: 7 additions & 1 deletion src/main/webapp/app/modeling/shared/modeling-editor/modeling-editor.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ <h4 class="modal-title" jhiTranslate="artemisApp.modelingEditor.helpModal.title"
}

@if (withExplanation) {
<jhi-modeling-explanation-editor [explanation]="explanation" (explanationChange)="onExplanationInput($event)" [readOnly]="readOnly" />
<!-- <jhi-modeling-explanation-editor [explanation]="explanation" (explanationChange)="onExplanationInput($event)" [readOnly]="readOnly" /> -->

<!-- workaround until further investigation into example solution rendering -->
<label for="explanationText" jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
<div class="p-3 border markdown-preview">
<p id="explanationText" [innerHTML]="explanation | htmlForMarkdown"></p>
Comment on lines +122 to +124
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Incorrect semantic HTML: label pointing to non-form element.

The <label for="explanationText"> points to a <p> element, which is not a form control. The for attribute should only reference form elements like <input>, <textarea>, or <select>. This creates invalid HTML and doesn't provide the expected accessibility benefits.

Apply this diff to fix the semantic issue:

-<label for="explanationText" jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
+<p class="form-label mb-2" jhiTranslate="artemisApp.modelingSubmission.explanationText"></p>
 <div class="p-3 border markdown-preview">
-    <p id="explanationText" [innerHTML]="explanation | htmlForMarkdown"></p>
+    <div [innerHTML]="explanation | htmlForMarkdown"></div>
 </div>

Alternatively, if you want to maintain label semantics for styling purposes, remove the for attribute:

-<label for="explanationText" jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
+<label jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
 <div class="p-3 border markdown-preview">
     <p id="explanationText" [innerHTML]="explanation | htmlForMarkdown"></p>
 </div>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<label for="explanationText" jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
<div class="p-3 border markdown-preview">
<p id="explanationText" [innerHTML]="explanation | htmlForMarkdown"></p>
<p class="form-label mb-2" jhiTranslate="artemisApp.modelingSubmission.explanationText"></p>
<div class="p-3 border markdown-preview">
<div [innerHTML]="explanation | htmlForMarkdown"></div>
🤖 Prompt for AI Agents 
In
src/main/webapp/app/modeling/shared/modeling-editor/modeling-editor.component.html
around lines 122 to 124, the <label for="explanationText"> targets a <p>
(non-form) element which is invalid; replace the label with an appropriate
non-form container (e.g., <h3> or <span class="label-text">) or remove the for
attribute so it’s purely presentational, and keep the existing id on the <p> for
reference if needed for styling or anchors; ensure the resulting element conveys
the same text and accessibility semantics (use a heading level if it’s a section
title).

</div>
}
Comment on lines 118 to 126
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: Verify instructor/admin edit workflow is not broken.

The new implementation shows the same read-only markdown display for all users when withExplanation is true, without checking the readOnly flag. The original ModelingExplanationEditorComponent accepted [readOnly]="readOnly" to conditionally show an editable interface for instructors/admins versus a read-only view for students.

Unless instructors/admins have another interface for editing the explanation, or withExplanation is only true in student contexts, this change breaks the editing capability.

Consider wrapping the new display in a conditional check:

 @if (withExplanation) {
-    <!-- <jhi-modeling-explanation-editor [explanation]="explanation" (explanationChange)="onExplanationInput($event)" [readOnly]="readOnly" /> -->
-
-    <!-- workaround until further investigation into example solution rendering -->
-    <label for="explanationText" jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
-    <div class="p-3 border markdown-preview">
-        <p id="explanationText" [innerHTML]="explanation | htmlForMarkdown"></p>
-    </div>
+    @if (readOnly) {
+        <!-- Student view: read-only markdown display -->
+        <label for="explanationText" jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
+        <div class="p-3 border markdown-preview">
+            <p id="explanationText" [innerHTML]="explanation | htmlForMarkdown"></p>
+        </div>
+    } @else {
+        <!-- Instructor/admin view: editable interface -->
+        <jhi-modeling-explanation-editor [explanation]="explanation" (explanationChange)="onExplanationInput($event)" [readOnly]="false" />
+    }
 }

Alternatively, verify that one of the following is true:

  1. Instructors/admins never reach this component with withExplanation=true
  2. There's another dedicated editing interface elsewhere
  3. The readOnly flag is always true when this block is shown

Run this script to understand when withExplanation is set:

🏁 Script executed:

#!/bin/bash
# Description: Find usages of the withExplanation input to understand when it's enabled

# Search for withExplanation property bindings
rg -n "withExplanation" --type=html --type=ts -C3

Length of output: 9848

The web search returned generic results. I need to search the codebase directly for the onExplanationInput method. Let me run a targeted script:


Let me correct this - I need to run the script on the codebase:

🏁 Script executed:

#!/bin/bash
# Search for onExplanationInput method in modeling-editor component
rg -n "onExplanationInput" --type=ts --type=html -C5

Length of output: 6659

🏁 Script executed:

#!/bin/bash
# Search for onExplanationInput method in modeling-editor component
rg -n "onExplanationInput" --type=ts --type=html -C5

Length of output: 6659

Critical: Editing capability broken for instructors and students in multiple contexts.

Verification confirms the original concern. The readOnly flag is ignored in the new implementation, breaking explanation editing where it should be available:

  1. example-modeling-submission.component.html (line 148): Instructors editing example solutions with [readOnly]="readOnly" — can now only view, not edit.
  2. modeling-submission.component.html (line 104): Students submitting solutions with [readOnly]="!isOwnerOfParticipation" — owners can now only view, not edit.
  3. modeling-exam-submission.component.html (line 33): Exam submissions with [readOnly]="readonly()" — editable exams now read-only.

The onExplanationInput method (modeling-editor.component.ts:217) still exists and has passing tests, but is unreachable since the component that calls it is commented out.

Restore the conditional logic to handle both read-only and editable modes:

 @if (withExplanation) {
-    <!-- <jhi-modeling-explanation-editor [explanation]="explanation" (explanationChange)="onExplanationInput($event)" [readOnly]="readOnly" /> -->
-
-    <!-- workaround until further investigation into example solution rendering -->
-    <label for="explanationText" jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
-    <div class="p-3 border markdown-preview">
-        <p id="explanationText" [innerHTML]="explanation | htmlForMarkdown"></p>
-    </div>
+    @if (readOnly) {
+        <label for="explanationText" jhiTranslate="artemisApp.modelingSubmission.explanationText"></label>
+        <div class="p-3 border markdown-preview">
+            <p id="explanationText" [innerHTML]="explanation | htmlForMarkdown"></p>
+        </div>
+    } @else {
+        <jhi-modeling-explanation-editor [explanation]="explanation" (explanationChange)="onExplanationInput($event)" [readOnly]="false" />
+    }
 }

</div>
5 changes: 3 additions & 2 deletions src/main/webapp/app/modeling/shared/modeling-editor/modeling-editor.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ import { Patch } from '@ls1intum/apollon';
import { TranslateDirective } from 'app/shared/language/translate.directive';
import { FaIconComponent } from '@fortawesome/angular-fontawesome';
import { NgClass, NgStyle } from '@angular/common';
import { ModelingExplanationEditorComponent } from '../modeling-explanation-editor/modeling-explanation-editor.component';
import { captureException } from '@sentry/angular';
import { FormsModule } from '@angular/forms';
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unnecessary FormsModule import.

The FormsModule import appears unnecessary. The new template code (lines 118-126 in the HTML file) doesn't use any forms-related directives like ngModel, ngForm, or form controls. This import was likely needed by the now-commented-out ModelingExplanationEditorComponent, but the replacement code doesn't require it.

Apply this diff to remove the unnecessary import:

-import { FormsModule } from '@angular/forms';
 import { HtmlForMarkdownPipe } from 'app/shared/pipes/html-for-markdown.pipe';
-    imports: [TranslateDirective, FaIconComponent, NgStyle, NgClass, FormsModule, HtmlForMarkdownPipe],
+    imports: [TranslateDirective, FaIconComponent, NgStyle, NgClass, HtmlForMarkdownPipe],

Also applies to: 22-22

🤖 Prompt for AI Agents 
In
src/main/webapp/app/modeling/shared/modeling-editor/modeling-editor.component.ts
around lines 14 and 22, remove the unused FormsModule reference: delete the
top-level import statement for FormsModule and remove it from the
component/module imports array (or any place it was added) since the template no
longer uses forms directives; ensure no other code depends on ngModel/ngForm
before removing.

import { HtmlForMarkdownPipe } from 'app/shared/pipes/html-for-markdown.pipe';

@Component({
selector: 'jhi-modeling-editor',
templateUrl: './modeling-editor.component.html',
styleUrls: ['./modeling-editor.component.scss'],
encapsulation: ViewEncapsulation.None,
imports: [TranslateDirective, FaIconComponent, NgStyle, NgClass, ModelingExplanationEditorComponent],
imports: [TranslateDirective, FaIconComponent, NgStyle, NgClass, FormsModule, HtmlForMarkdownPipe],
})
export class ModelingEditorComponent extends ModelingComponent implements AfterViewInit, OnDestroy, OnChanges {
protected readonly faCheck = faCheck;
Expand Down