Development: Refactor Markdown editor actions with the adapter pattern

[pzdr7]

Checklist

General

• I tested all changes and their related features with all corresponding user types on a test server.
• I chose a title conforming to the naming conventions for pull requests.

Client

• Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
• I strictly followed the client coding and design guidelines.
• I documented the TypeScript code using JSDoc style.

Motivation and Context

Like the old command hierarchy, the MonacoEditorAction hierarchy generally used the monaco-editor API directly to interact with the editor, e.g. by interacting with the text within, focusing, layouting...

This created a lot of imports and dependencies on the monaco-editor API across several files that didn't necessarily need to have them, potentially making the system harder to maintain and extend (e.g. with different implementations of editors).

Description

• New interface TextEditor that defines the common building blocks for Actions to use when interacting with the Monaco editor.
• New adapter MonacoTextEditorAdapter and helper classes that do not depend on Monaco. The adapter translates the helper classes and TextEditor interface methods into calls to the Monaco editor instance it contains.
• Renamed MonacoEditorAction to TextEditorAction and the subclasses accordingly
• Removed imports of monaco-editor from the Action hierarchy.

The most important changes are in TextEditorAction, the TextEditor interface, and the MonacoTextEditorAdapter. Other files generally only contain renames or small refactorings.

Steps for Testing

1. Navigate to any of the markdown editors, e.g. editing a programming exercise or the communication view.
2. Check that the actions in the action bar above the editor still work (and that the keyboard shortcuts still work. Windows/Linux: Ctrl+B, Ctrl+U, Ctrl+I and MacOS: Cmd+B, Cmd+U, Cmd+I).

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

• Code Review 1
• Code Review 2

Manual Tests

• Test 1
• Test 2

Test Coverage

Client

Class/File	Line Coverage	Confirmation (assert/expect)
monaco-text-editor.adapter.ts	97.26%	✅
text-editor-completion-item.model.ts	100%	✅
text-editor-keybinding.model.ts	100%	✅
text-editor-range.model.ts	100%	✅
monaco-editor.util.ts	100%	✅
monaco-editor-option.helper.ts	100%	✅
monaco-editor.component.ts	96.21%	✅

Screenshots

Unchanged

Summary by CodeRabbit

• New Features

  • Introduced new action classes for enhanced text editing capabilities, including FullscreenAction, OrderedListAction, and various grading-related actions.
  • Added a MonacoTextEditorAdapter to bridge the Monaco editor with a custom text editor interface.

• Refactor

  • Renamed numerous action classes to simplify and standardize naming conventions, improving clarity and maintainability.
  • Updated existing components and tests to utilize the new action class names and enhanced type definitions.

• Bug Fixes

  • Improved functionality of formula actions and quiz-related actions by updating their instantiation and usage across various components.

[coderabbitai]

Walkthrough

The pull request introduces a comprehensive refactoring of action imports across various components in the codebase. It primarily involves renaming action classes to remove the "Monaco" prefix and replacing specific action types with more generalized alternatives. This restructuring aims to enhance code clarity, maintainability, and consistency throughout the application.

Changes

Files	Change Summary
src/main/webapp/app/exam/manage/exams/exam-checklist-component/...	Refactored action imports in ExamLiveAnnouncementCreateModalComponent, renamed action classes for clarity.
src/main/webapp/app/exercises/file-upload/manage/...	Replaced MonacoFormulaAction with FormulaAction in FileUploadExerciseUpdateComponent.
src/main/webapp/app/exercises/modeling/manage/...	Replaced MonacoFormulaAction with FormulaAction in ModelingExerciseUpdateComponent.
src/main/webapp/app/exercises/programming/manage/...	Refactored action imports in ProgrammingExerciseEditableInstructionComponent, updated various action classes.
src/main/webapp/app/exercises/quiz/manage/...	Renamed quiz-related action classes in multiple files including drag-and-drop-question-edit.component.ts and multiple-choice-question-edit.component.ts.
src/main/webapp/app/exercises/shared/exercise-hint/manage/...	Updated ExerciseHintUpdateComponent to replace MonacoFormulaAction with FormulaAction.
src/main/webapp/app/shared/markdown-editor/monaco/...	Comprehensive refactoring in MarkdownEditorMonacoComponent, updating action imports and types.
src/main/webapp/app/shared/monaco-editor/model/actions/...	Refactored various action classes to remove the "Monaco" prefix and transition to a generalized text editor action framework.
src/test/javascript/spec/component/...	Refactored action imports in multiple spec files to reflect the changes made in the component files, ensuring consistency in naming conventions.

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 12

Outside diff range comments (3)

src/main/webapp/app/shared/monaco-editor/model/monaco-editor-glyph-margin-widget.model.ts (1)

Line range hint 1-36: Class Modifications Approved

The modifications to the MonacoEditorGlyphMarginWidget class enhance clarity and maintainability. The use of a more specific editor interface and the direct use of monaco.editor.GlyphMarginLane improve the explicitness of type usage. The adoption of the makeEditorRange utility function in the getPosition method is a positive change towards better code reuse and flexibility.

Enhancement Suggestion: Consider adding error handling for cases where the editor might not support certain operations, ensuring robustness in all scenarios.

src/main/webapp/app/shared/monaco-editor/model/actions/communication/exercise-reference.action.ts (1)

Line range hint 28-49: Method register: Review and Suggestions.

The register method sets up a completion provider for exercise references. Here are some observations and suggestions:

1. Type Usage: The method correctly uses TextEditor and TextEditorRange, aligning with the new architecture. This is a positive change from the previous monaco specific types.
2. Completion Item Creation: The creation of TextEditorCompletionItem is well-structured, encapsulating the completion item details effectively. However, consider adding more comments to explain the parameters and their roles in the completion process.
3. Error Handling: Ensure that error handling is in place for cases where the exercise fetching fails or the editor instance is not correctly initialized.

Overall, the method is functionally correct but could benefit from additional comments and error handling measures.

src/main/webapp/app/exercises/shared/structured-grading-criterion/grading-instructions-details/grading-instructions-details.component.ts (1)

Line range hint 133-150: Refactor Suggestion: Use template literals for string concatenation.

To improve readability and maintainability, consider refactoring the string concatenation in generateInstructionText to use template literals. This change aligns with modern JavaScript practices and enhances the clarity of the code.

- GradingInstructionAction.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';
+ `${GradingInstructionAction.IDENTIFIER}
+ \t${this.generateCreditsText(instruction)}
+ \t${this.generateGradingScaleText(instruction)}
+ \t${this.generateInstructionDescriptionText(instruction)}
+ \t${this.generateInstructionFeedback(instruction)}
+ \t${this.generateUsageCount(instruction)}
+ 
+ `;

[coderabbitai]

Actionable comments posted: 11

Outside diff range comments (2)

src/main/webapp/app/shared/monaco-editor/model/actions/communication/user-mention.action.ts (1)

Line range hint 17-52: Refactored class for user mentions is well-implemented.

The UserMentionAction class effectively handles user mentions with a clear and concise implementation. The refactoring from MonacoUserMentionAction to UserMentionAction aligns with the PR's goals of reducing dependencies on specific editor APIs.

• Constructor: Initializes the action with necessary services and sets up the label and icon correctly.
• register Method: Properly sets up a completion provider to suggest user mentions based on input, using the new TextEditor interface.
• run Method: Inserts the default text and focuses the editor, triggering the completion provider.

Consider adding error handling for cases where the user mention data might not be available or the network request fails.

Would you like suggestions on implementing error handling for network requests in this context?

src/main/webapp/app/shared/monaco-editor/model/actions/communication/exercise-reference.action.ts (1)

Line range hint 13-59: Refactored class for exercise references is well-implemented.

The ExerciseReferenceAction class effectively handles exercise references with a clear and concise implementation. The refactoring from MonacoExerciseReferenceAction to ExerciseReferenceAction aligns with the PR's goals of reducing dependencies on specific editor APIs.

• Constructor: Initializes the action with necessary services and sets up the label and icon correctly.
• register Method: Properly sets up a completion provider to suggest exercise references based on input, using the new TextEditor interface.
• run Method: Inserts the default text and focuses the editor, triggering the completion provider.

Consider adding error handling for cases where the exercise data might not be available or the network request fails.

Would you like suggestions on implementing error handling for network requests in this context?

[coderabbitai]

Actionable comments posted: 18

Outside diff range comments (5)

src/main/webapp/app/shared/monaco-editor/model/actions/color.action.ts (1)

Line range hint 13-33: Class implementation aligns with refactor objectives.

The ColorAction class is well-refactored to extend TextEditorAction, enhancing its flexibility across different text editors. The method run is appropriately updated to use the TextEditor interface. Consider adding more detailed documentation for the run method to explain the changes in context and expected behavior.

src/main/webapp/app/shared/monaco-editor/model/actions/url.action.ts (1)

Line range hint 13-36: Class implementation aligns with refactor objectives.

The UrlAction class is well-refactored to extend TextEditorAction, enhancing its flexibility across different text editors. The method run is appropriately updated to use the TextEditor interface. Consider adding more detailed documentation for the run method to explain the changes in context and expected behavior.

src/main/webapp/app/shared/monaco-editor/model/actions/communication/lecture-attachment-reference.action.ts (1)

Line range hint 74-127: Method signature updates for broader compatibility.

The updates to method signatures to accept a TextEditor type broaden the action's applicability. Ensure that these changes are correctly implemented and that the methods function as expected with the new interface.

Tools

Biome

[error] 128-128: Forbidden non-null assertion.

Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator

(lint/style/noNonNullAssertion)

src/main/webapp/app/shared/monaco-editor/model/actions/text-editor-action.model.ts (1)

Line range hint 11-108: Comprehensive Review of TextEditorAction

The TextEditorAction class is a crucial part of the new adapter pattern and serves as the base class for all editor actions. Here are some specific observations and suggestions:

1. Constructor and Properties (Lines 37-80): The constructor is well-designed, taking all necessary parameters to fully configure an action. Properties are correctly typed and optional where appropriate.

2. run Method (Line 50): This abstract method is appropriately defined, forcing subclasses to provide their own implementation. It ensures that all actions can be executed in a consistent manner.

3. executeInCurrentEditor Method (Lines 62-62): This method provides a convenient way to execute actions without needing to pass the editor instance repeatedly. It also handles the case where the action might not be registered, which is a good safety feature.

4. register Method (Lines 76-80): The method includes error handling to prevent an action from being registered multiple times. This is an important check that prevents runtime errors.

5. Error Handling: The class includes comprehensive error handling, particularly in the register method, which is crucial for maintaining stability.

6. Documentation: The comments and documentation within the class are helpful and enhance readability and maintainability.

Overall, the TextEditorAction class is well-implemented and aligns with the goals of the refactor to provide a more abstract and flexible way to handle editor actions. It should serve as a robust base for extending the editor's functionality.

src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.ts (1)

Line range hint 17-470: Comprehensive Review of MarkdownEditorMonacoComponent

The MarkdownEditorMonacoComponent is a complex component that integrates text editing functionalities into the Monaco editor. Here are some specific observations and suggestions:

1. Action Integration (Lines 146-156, 160-162, 167, 170, 173, 176): The component correctly initializes and manages various text editing actions. The use of the new TextEditorAction classes is consistent and helps reduce direct dependencies on the Monaco editor.

2. File Upload Handling (Lines 470-471): The methods for handling file uploads are well-implemented. However, consider adding more error handling for cases where the file upload service might fail.

3. Color Selector Integration (Lines 38-40): The integration of the color selector is a nice touch that enhances the usability of the markdown editor. The method for opening the color selector and handling color selection is appropriately implemented.

4. Resizing and Layout Adjustments: The component handles resizing and layout adjustments effectively. Methods like adjustEditorDimensions and onContentHeightChanged are crucial for ensuring that the editor behaves responsively.

5. Event Handling: The component handles various events such as text changes, file uploads, and drag-and-drop actions. These handlers are crucial for the functionality of the markdown editor and are implemented correctly.

6. Performance Considerations: Given the complexity of the component, consider reviewing the performance, especially in scenarios with large markdown content or frequent actions. Optimizing the handling of change detection and updates could enhance performance.

Overall, the MarkdownEditorMonacoComponent is well-crafted and integrates the new action classes effectively. It serves as a key part of the markdown editing functionality and aligns with the goals of the refactor.

[SimonEntholzer]

Works nicely on Ts4

[coderabbitai]

Actionable comments posted: 2

Outside diff range comments (2)

src/main/webapp/app/shared/monaco-editor/model/actions/quiz/insert-short-answer-option.action.ts (1)

Line range hint 13-55: Refactor of InsertShortAnswerOptionAction to Use Generalized Text Editor Interface

The refactor of InsertShortAnswerOptionAction to use the TextEditor interface instead of being tightly coupled with the Monaco editor is a significant improvement in terms of modularity and adaptability. Here are some detailed observations and suggestions:

1. Class Name and ID Change: The simplification of the class name and the update of the static ID are good practices that enhance clarity and maintainability.

2. Constructor and Method Adjustments: The adjustments in the constructor to use the new ID and the changes in the run method to handle text insertion and selection more generically are well-implemented. These changes ensure that the action can be used with any text editor that conforms to the TextEditor interface.

3. Text Insertion Logic: The logic to insert text based on the provided arguments and to handle default text insertion is robust. However, ensure that the handling of optional parameters is thoroughly tested, especially in scenarios where parameters might be omitted.

Overall, the changes are well thought out and significantly enhance the action's usability across different editor implementations. However, ensure that these changes are thoroughly tested, particularly the new text range handling functionality.

Would you like assistance in writing unit tests for the new text range handling functionality?

src/main/webapp/app/shared/monaco-editor/model/actions/communication/exercise-reference.action.ts (1)

Line range hint 13-59: Well-Structured Class for Exercise References

The ExerciseReferenceAction class is well-implemented with clear responsibilities. It uses the adapter pattern effectively to reduce dependencies on the monaco-editor API. The methods are well-named, and the logic within the register and run methods is robust, handling different scenarios efficiently.

Consider adding unit tests to cover various scenarios such as inserting different types of exercise references and ensuring that edge cases are handled correctly.

Would you like help in setting up unit tests for this class?

[coolchock]

tested on ts4, actions and shortcuts work as expected, found no issues.

[JohannesWt]

Code 👍

[az108]

Code 👍

[coderabbitai]

Actionable comments posted: 1

Outside diff range comments (1)

src/test/javascript/spec/component/programming-exercise/programming-exercise-instruction-analysis.component.spec.ts (1)

Line range hint 1-1: Approved: The TODO comment is a good reminder to add tests.

Tracking missing tests using TODO comments is a good practice. Let's ensure we address this soon.

Do you want me to open a GitHub issue to track the task of adding the missing tests?