Quiz exercises: Fix code blocks not rendering in markdown editor
#11706
Conversation
github-actions
bot
added
client
WalkthroughRefactors short-answer question text processing to produce both plain and HTML tokenizations and updates all call sites to use the new structured return. UI styling for short-answer display was adjusted (inline/block swaps, baseline alignment, code-block responsive rules). Tests and components were updated to consume the new { plain, html } shape. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts (1)
304-310: The indentation detection will break with HTML-wrapped content.The concern is valid. When
htmlForMarkdownconverts markdown, it wraps content in HTML tags like<p>,<b>, etc. For example, plain text" some text"becomes"<p> some text</p>".The issue: The
getIndentationmethod looks for spaces at the string start usingwhile (text[index] === ' '). When leading spaces are inside HTML tags, this fails. For input"<p> text</p>", it returns''instead of detecting the 4 spaces.The same problem affects
getFirstWord, which usestext.trim().split(' ')and will incorrectly parse HTML tag content.At line 378 in
addIndentationToTextParts, the code callsgetIndentation(originalTextParts[i][0])expecting plain text, butoriginalTextPartsnow contains HTML from the modifieddivideQuestionTextIntoTextParts. The existing tests only cover plain text inputs, so this regression is not caught.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.scss(2 hunks)src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: magaupp
Repo: ls1intum/Artemis PR: 8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-10-08T15:35:42.972Z
Learning: Code inside the `exercise` directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
Learnt from: magaupp
Repo: ls1intum/Artemis PR: 8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-08-05T00:11:50.650Z
Learning: Code inside the `exercise` directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
📚 Learning: 2025-08-19T20:07:18.114Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts:76-78
Timestamp: 2025-08-19T20:07:18.114Z
Learning: In ShortAnswerQuestion components (src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts), the text, hint, and explanation properties are required fields for short answer questions and do not need null safety checks when accessing them from shortAnswerQuestion().
Applied to files:
src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.scsssrc/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts
📚 Learning: 2025-08-19T19:56:34.228Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts:40-40
Timestamp: 2025-08-19T19:56:34.228Z
Learning: In the ShortAnswerQuestionComponent (src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts), the fnOnSubmittedTextUpdate input should remain typed as "any" rather than being changed to a more specific function type like "(() => void) | undefined". This is an intentional design decision.
Applied to files:
src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.scsssrc/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts
📚 Learning: 2025-08-19T19:56:07.618Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/multiple-choice-question/visual-question/multiple-choice-visual-question.component.ts:84-86
Timestamp: 2025-08-19T19:56:07.618Z
Learning: In Angular signals migration for MultipleChoiceVisualQuestionComponent, when adding new answer options, it's preferred to create a copy of the answerOptions array rather than directly mutating it, maintaining immutability principles while working with signal-returned objects.
Applied to files:
src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.scss
📚 Learning: 2025-08-21T17:30:20.530Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/drag-and-drop-question/drag-and-drop-question.component.spec.ts:34-34
Timestamp: 2025-08-21T17:30:20.530Z
Learning: FitTextDirective in src/main/webapp/app/quiz/shared/fit-text/fit-text.directive.ts is a standalone directive marked with standalone: true, so it should be imported in TestBed imports array, not declarations array.
Applied to files:
src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.scsssrc/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts
🧬 Code graph analysis (1)
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts (1)
src/main/webapp/app/shared/util/markdown.conversion.util.ts (1)
htmlForMarkdown(44-97)
⏰ 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). (6)
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Analyse
🔇 Additional comments (2)
src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.scss (2)
14-31: Well-designed responsive handling for code blocks.The CSS changes properly support the new HTML structure from the service layer:
align-items: baselineensures proper alignment between text and input fieldsinline-blockwith nestedinlinedivs allows text to flow naturally within the flex container- The
:has(pre)selector cleverly handles code blocks by giving them full width (flex-basis: 100%,width: 100%) and making nested divs block-level for proper formattingThis approach ensures code blocks render on their own line with proper spacing while keeping regular text inline.
Note: The
:has()pseudo-class is well-supported in modern browsers as of 2025, so compatibility should not be a concern.
43-43: Better vertical alignment approach.Changing from
margin-bottomtovertical-align: middleis a more semantic way to align inline-block input fields with surrounding text content, especially given the newalign-items: baselineon the parent container.
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts
Outdated
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
|
||||||||||||||||||
MarcosOlivaKaczmarek
left a comment
MarcosOlivaKaczmarek
left a comment
There was a problem hiding this comment.
Tested on TS2. Everything worked as far as I can tell.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts (2)
95-108: Hardcoded HTML expectations may be fragile.The test at lines 104-107 expects specific HTML structure including
<p>tags wrapping words like<p>Thisandquestion.</p>. This couples the test to markdown-it's specific rendering behavior. Consider using a more flexible assertion similar to what's done in later test cases:- const expectedTextParts = [ - ['<p>This', 'is', 'a', '[-spot 12]', 'regarding', 'this', 'question.</p>'], - ['<p>Another', '[-spot 8]', 'is', 'in', 'the', 'line', 'above</p>'], - ]; - expect(component.textParts).toEqual(expectedTextParts); + // Verify spot tags are preserved + const allSpots = component.textParts.flat().filter((part) => part && part.includes('[-spot')); + expect(allSpots).toContain('[-spot 12]'); + expect(allSpots).toContain('[-spot 8]'); + // Verify content is preserved + const allText = component.textParts.flat().join(' '); + expect(allText).toContain('regarding this question'); + expect(allText).toContain('Another');This would make the test more resilient to minor markdown rendering changes while still verifying the core functionality.
540-546: Consider flexible assertion for consistency.Similar to the earlier comment, this test has hardcoded HTML structure expectations. While it currently works, consider using flexible assertions:
- const splitString = ['<p>This', 'is', 'a', 'text', 'for', 'a', 'test</p>']; - expect(component.textParts.pop()).toEqual(splitString); + const lastPart = component.textParts.pop()!; + const joinedText = lastPart.join(' '); + expect(joinedText).toContain('This is a text for a test');This is a minor suggestion for test maintainability.
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.spec.ts (1)
210-223: Tests could better verify indentation application.The tests pass
{ plain, html }objects but expect the exacthtmlPartsarrays as output. SinceaddIndentationToTextPartsmodifiesformattedTextPartsin place (adding for indentation), these tests work by coincidence because the test data doesn't have significant indentation differences.Consider adding a test that verifies indentation is actually applied:
it('should apply indentation from plain text to HTML', () => { const plainParts = [[' indented text']]; const htmlParts = [['indented text']]; const result = service.transformTextPartsIntoHTML({ plain: plainParts, html: htmlParts }); expect(result[0][0]).toContain(' '); expect(result[0][0]).toContain('indented text'); });This would ensure the indentation preservation logic is explicitly tested.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts(9 hunks)src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts(3 hunks)src/main/webapp/app/quiz/manage/statistics/short-answer-question-statistic/short-answer-question-statistic.component.ts(1 hunks)src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts(1 hunks)src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.spec.ts(1 hunks)src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/**/*.ts
⚙️ CodeRabbit configuration file
Files:
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.tssrc/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.tssrc/main/webapp/app/quiz/shared/service/short-answer-question-util.service.spec.tssrc/main/webapp/app/quiz/manage/statistics/short-answer-question-statistic/short-answer-question-statistic.component.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: magaupp
Repo: ls1intum/Artemis PR: 8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-10-08T15:35:42.972Z
Learning: Code inside the `exercise` directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
Learnt from: magaupp
Repo: ls1intum/Artemis PR: 8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-08-05T00:11:50.650Z
Learning: Code inside the `exercise` directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts:76-78
Timestamp: 2025-08-19T20:07:18.114Z
Learning: In ShortAnswerQuestion components (src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts), the text, hint, and explanation properties are required fields for short answer questions and do not need null safety checks when accessing them from shortAnswerQuestion().
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts:40-40
Timestamp: 2025-08-19T19:56:34.228Z
Learning: In the ShortAnswerQuestionComponent (src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts), the fnOnSubmittedTextUpdate input should remain typed as "any" rather than being changed to a more specific function type like "(() => void) | undefined". This is an intentional design decision.
📚 Learning: 2025-08-19T19:56:34.228Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts:40-40
Timestamp: 2025-08-19T19:56:34.228Z
Learning: In the ShortAnswerQuestionComponent (src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts), the fnOnSubmittedTextUpdate input should remain typed as "any" rather than being changed to a more specific function type like "(() => void) | undefined". This is an intentional design decision.
Applied to files:
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.tssrc/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.tssrc/main/webapp/app/quiz/shared/service/short-answer-question-util.service.spec.tssrc/main/webapp/app/quiz/manage/statistics/short-answer-question-statistic/short-answer-question-statistic.component.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts
📚 Learning: 2025-08-19T20:07:18.114Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts:76-78
Timestamp: 2025-08-19T20:07:18.114Z
Learning: In ShortAnswerQuestion components (src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts), the text, hint, and explanation properties are required fields for short answer questions and do not need null safety checks when accessing them from shortAnswerQuestion().
Applied to files:
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.tssrc/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.tssrc/main/webapp/app/quiz/shared/service/short-answer-question-util.service.spec.tssrc/main/webapp/app/quiz/manage/statistics/short-answer-question-statistic/short-answer-question-statistic.component.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts
📚 Learning: 2025-08-19T19:56:07.618Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/multiple-choice-question/visual-question/multiple-choice-visual-question.component.ts:84-86
Timestamp: 2025-08-19T19:56:07.618Z
Learning: In Angular signals migration for MultipleChoiceVisualQuestionComponent, when adding new answer options, it's preferred to create a copy of the answerOptions array rather than directly mutating it, maintaining immutability principles while working with signal-returned objects.
Applied to files:
src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.tssrc/main/webapp/app/quiz/manage/statistics/short-answer-question-statistic/short-answer-question-statistic.component.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts
📚 Learning: 2024-10-13T12:03:02.430Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9463
File: src/test/javascript/spec/component/shared/monaco-editor/monaco-editor.service.spec.ts:50-55
Timestamp: 2024-10-13T12:03:02.430Z
Learning: In `src/test/javascript/spec/component/shared/monaco-editor/monaco-editor.service.spec.ts`, when a function is called multiple times in a test, use `toHaveBeenCalledTimes` and `toHaveBeenNthCalledWith` assertions instead of `toHaveBeenCalledExactlyOnceWith`.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.tssrc/main/webapp/app/quiz/shared/service/short-answer-question-util.service.spec.ts
📚 Learning: 2024-10-10T11:42:23.069Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9443
File: src/test/javascript/spec/component/hestia/git-diff-report/git-diff-modal.component.spec.ts:55-60
Timestamp: 2024-10-10T11:42:23.069Z
Learning: In `git-diff-report-modal.component.spec.ts`, using `fakeAsync` and `tick` does not work for handling asynchronous operations in the tests; alternative methods are needed.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts
📚 Learning: 2025-08-21T17:30:20.530Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/drag-and-drop-question/drag-and-drop-question.component.spec.ts:34-34
Timestamp: 2025-08-21T17:30:20.530Z
Learning: FitTextDirective in src/main/webapp/app/quiz/shared/fit-text/fit-text.directive.ts is a standalone directive marked with standalone: true, so it should be imported in TestBed imports array, not declarations array.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.tssrc/main/webapp/app/quiz/shared/service/short-answer-question-util.service.spec.tssrc/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts
📚 Learning: 2025-09-01T10:20:40.706Z
Learnt from: Michael-Breu-UIbk
Repo: ls1intum/Artemis PR: 10989
File: src/main/webapp/app/programming/manage/detail/programming-exercise-detail.component.with-sharing.spec.ts:132-149
Timestamp: 2025-09-01T10:20:40.706Z
Learning: In the Artemis codebase, Angular component test files for ProgrammingExerciseDetailComponent follow a pattern where the component is imported but not explicitly declared in TestBed.configureTestingModule(), yet TestBed.createComponent() still works successfully. This pattern is consistently used across test files like programming-exercise-detail.component.spec.ts and programming-exercise-detail.component.with-sharing.spec.ts.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts
📚 Learning: 2024-10-20T22:00:52.335Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9505
File: src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts:179-181
Timestamp: 2024-10-20T22:00:52.335Z
Learning: In `src/test/javascript/spec/integration/code-editor/code-editor-container.integration.spec.ts`, `ResizeObserver` is mocked within the `beforeEach` block.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts
📚 Learning: 2025-02-16T16:00:38.131Z
Learnt from: iyannsch
Repo: ls1intum/Artemis PR: 10344
File: src/test/javascript/spec/component/shared/code-button.component.spec.ts:0-0
Timestamp: 2025-02-16T16:00:38.131Z
Learning: In test cases, prefer removing unnecessary type assertions over adding mock properties that aren't relevant to what's being tested. This keeps tests focused and reduces maintenance burden.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts
📚 Learning: 2024-10-08T15:35:52.595Z
Learnt from: pzdr7
Repo: ls1intum/Artemis PR: 9407
File: src/test/javascript/spec/component/programming-exercise/programming-exercise-custom-aeolus-build-plan.component.spec.ts:172-175
Timestamp: 2024-10-08T15:35:52.595Z
Learning: When testing components that use `MonacoEditorComponent`, it's acceptable not to test `MonacoEditorComponent` functionality within those tests, as dedicated tests already exist for it.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts
📚 Learning: 2024-07-09T19:09:49.666Z
Learnt from: florian-glombik
Repo: ls1intum/Artemis PR: 8858
File: src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts:88-89
Timestamp: 2024-07-09T19:09:49.666Z
Learning: For the PR ls1intum/Artemis#8858, avoid suggesting to change `expect(component.expandAll).toHaveBeenCalledOnce()` to `expect(component.expandAll).toHaveBeenCalledTimes(1)`.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts
📚 Learning: 2024-07-09T19:09:58.572Z
Learnt from: florian-glombik
Repo: ls1intum/Artemis PR: 8858
File: src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts:115-115
Timestamp: 2024-07-09T19:09:58.572Z
Learning: For the PR ls1intum/Artemis#8858, avoid suggesting to change `expect(component.setStoredCollapseState).toHaveBeenCalledOnce()` to `expect(component.setStoredCollapseState).toHaveBeenCalledTimes(1)`.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts
📚 Learning: 2025-08-19T19:53:56.451Z
Learnt from: MoritzSpengler
Repo: ls1intum/Artemis PR: 11297
File: src/main/webapp/app/quiz/shared/questions/multiple-choice-question/multiple-choice-question.component.ts:26-37
Timestamp: 2025-08-19T19:53:56.451Z
Learning: In the MultipleChoiceQuestionComponent (src/main/webapp/app/quiz/shared/questions/multiple-choice-question/multiple-choice-question.component.ts), the fnOnSelection input should remain typed as "any" rather than being changed to a more specific function type like "(() => void) | undefined". This is an intentional design decision.
Applied to files:
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts
🧬 Code graph analysis (1)
src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts (1)
src/main/webapp/app/shared/util/markdown.conversion.util.ts (1)
htmlForMarkdown(44-97)
⏰ 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). (8)
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: client-style
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: Analyse
🔇 Additional comments (10)
src/main/webapp/app/quiz/manage/statistics/short-answer-question-statistic/short-answer-question-statistic.component.ts (1)
60-63: LGTM!The adaptation to the new
divideQuestionTextIntoTextPartsAPI is correct. The structured object is properly passed totransformTextPartsIntoHTML, which returns the HTML-formattedstring[][]for assignment tothis.textParts.src/main/webapp/app/quiz/shared/questions/short-answer-question/short-answer-question.component.ts (1)
82-91: LGTM!The
watchCollectionmethod correctly uses the new structured return fromdivideQuestionTextIntoTextPartsand passes it totransformTextPartsIntoHTML. This is consistent with changes in other components. Based on learnings, the non-null assertion ontextis acceptable as it's a required field for short answer questions.src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.ts (2)
289-320: LGTM! Good fix for the code blocks rendering issue.The refactored approach correctly separates concerns:
plainTextPartspreserves original whitespace for indentation detectionhtmlTextPartsuseshtmlForMarkdownto render markdown features like code blocks with syntax highlightingThis addresses the prior review feedback about indentation logic being broken when HTML was passed for both parameters.
351-361: Clean implementation with clear documentation.The updated method properly delegates indentation detection to plain text while preserving HTML formatting in the output. The JSDoc comments accurately describe the new behavior.
src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.spec.ts (1)
379-383: LGTM!The mock correctly returns the new structured object format expected by
divideQuestionTextIntoTextParts.src/main/webapp/app/quiz/shared/service/short-answer-question-util.service.spec.ts (1)
188-208: Good test coverage for the new API structure.The test properly verifies:
- The new return structure with
plainandhtmlproperties- HTML conversion of markdown (e.g.,
**bold**→<strong>bold</strong>)- Spot tag preservation in the HTML output
- Paragraph wrapping behavior from markdown-it
The assertions correctly access
textPartsData.htmlto verify the rendered output.src/main/webapp/app/quiz/manage/short-answer-question/short-answer-question-edit.component.ts (4)
170-195: LGTM - Correct use of the new API for HTML rendering context.The change correctly accesses
.html[0]from the new structured return type, which is appropriate since this method parses text for visual display purposes.
431-433: LGTM - Correct use of plain text for position-based manipulation.Using
.plainhere is appropriate since the subsequent substring operations (lines 432-433) require raw text positions that would be incorrect if HTML entities were present.
437-438: LGTM - Proper integration with refactored utility API.The pattern of capturing the full data object and passing it to
transformTextPartsIntoHTMLcorrectly follows the new API design that separates plain and HTML representations.
605-606: LGTM - Consistent pattern with the other call sites.The implementation correctly mirrors the pattern used in
addSpotAtCursorVisualMode, maintaining consistency across the component. Based on learnings, the non-null assertion ontextis acceptable since it's a required field.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
Checklist
General
Client
Motivation and Context
Fixes an issue where markdown code blocks (using triple backticks ```) were not rendering correctly in quiz short answer questions. Code blocks were displaying as plain text instead of being properly formatted with syntax highlighting. This issue only affected short answer questions, while multiple choice and drag-and-drop questions rendered code blocks correctly.
Description
This PR fixes the code block rendering issue in quiz short answer questions by refactoring the markdown processing pipeline:
Steps for 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
Before (Code block not rendering):
After (Fixed):
Summary by CodeRabbit
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.