-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
General
: Replace the remaining markdown editors with Monaco
#9230
Conversation
…-editor-grading-instructions.integration.spec.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
...pp/app/shared/monaco-editor/model/actions/grading-criteria/monaco-grading-feedback.action.ts
Show resolved
Hide resolved
...d-grading-criterion/grading-instructions-details/grading-instructions-details.component.html
Show resolved
Hide resolved
...red-grading-criterion/grading-instructions-details/grading-instructions-details.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good in general just two small (optional) comments
...n/webapp/app/admin/standardized-competencies/standardized-competency-management.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exercises/file-upload/manage/file-upload-exercise-update.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range comments (1)
src/main/webapp/app/exercises/shared/structured-grading-criterion/grading-instructions-details/grading-instructions-details.component.ts (1)
Line range hint
136-153
: Consider using template literals for string concatenation.Using template literals can improve readability and maintainability.
- let markdownText = ''; - markdownText = - MonacoGradingInstructionAction.IDENTIFIER + - '\n' + - '\t' + - this.generateCreditsText(instruction) + - '\n' + - '\t' + - this.generateGradingScaleText(instruction) + - '\n' + - '\t' + - this.generateInstructionDescriptionText(instruction) + - '\n' + - '\t' + - this.generateInstructionFeedback(instruction) + - '\n' + - '\t' + - this.generateUsageCount(instruction) + - '\n' + - '\n'; + let markdownText = `${MonacoGradingInstructionAction.IDENTIFIER} + \t${this.generateCreditsText(instruction)} + \t${this.generateGradingScaleText(instruction)} + \t${this.generateInstructionDescriptionText(instruction)} + \t${this.generateInstructionFeedback(instruction)} + \t${this.generateUsageCount(instruction)} + \n`;
...t/spec/component/shared/monaco-editor/monaco-editor-grading-instructions.integration.spec.ts
Show resolved
Hide resolved
...red-grading-criterion/grading-instructions-details/grading-instructions-details.component.ts
Show resolved
Hide resolved
...n/webapp/app/admin/standardized-competencies/standardized-competency-management.component.ts
Show resolved
Hide resolved
...app/shared/monaco-editor/model/actions/grading-criteria/monaco-grading-description.action.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM, thank you for the quick changes 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested markdown editors in imprint, exam and quiz exercises on TS3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested in testing session on ts3, works as expected 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested markdown editors in code of conduct, competencies/prerequesites, Standardized Competency on TS3
Checklist
General
Client
Motivation and Context
We want to remove Ace from Artemis. However, the Markdown Editor (except for communication and programming exercise problem statements) still uses Ace.
Description
Replaced every usage of the old markdown editor with Monaco:
Added editor actions for the grading instructions (criterion, instruction, credits, description, scale, feedback, and usage count) and multiple choice / dnd answers (wrong, correct, hint, explanation). These generally follow the same logic of the original markdown editor.
Note: I will remove the remaining references to Ace / Commands / EditorMode ... in a follow-up PR where I remove Ace from Artemis.
Steps for Testing
Default markdown editor
Prerequisites:
Test the markdown editors. In particular, verify that your changes to the markdown text are saved and that you can see your changes in the preview.
Exercises
Update/Create...
Administrator
Modify...
Exam
Courses and competencies
Special markdown editors
Grading instructions
Prerequisites:
Quiz exercises (DnD; MC)
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Client
Screenshots
Examples below
Standardized competencies
Exam live announcements
Grading instructions
Quiz MC
Quiz DND
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests