`Development`: Migrate the MonacoEditorModule to standalone components #9407

pzdr7 · 2024-10-01T20:19:39Z

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.
Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.

Changes affecting Programming Exercises

High priority: I tested all changes and their related features with all corresponding user types on a test server configured with the integrated lifecycle setup (LocalVC and LocalCI).

Motivation and Context

According to our new client coding guidelines, we want to move away from modules and @Input/@Output. The Monaco Editor still uses a module (for registering its custom languages) and non-signal based inputs and outputs.

Description

Deleted the MonacoEditorModule
Made the components standalone and implement the OnPush change detection strategy.
Replaced the inputs/outputs with signals
Added the new MonacoEditorService that replaces the module by
- registering the custom language
- implementing the theme switch (now with signals)
Adjusted other components, tests, and modules to import the components directly rather than the module

Steps for Testing

Prerequisites:

1 Programming exercise
1 Instructor

Log in and navigate to course management > your course > your programming exercise > Edit
Verify that when you switch between light and dark mode, the problem statement editor also switches between light and dark mode.
Verify that the problem statement still has its markdown syntax highlighting (and is not plain text)

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-diff-editor.component.ts	96.49%	✅
monaco-editor.component.ts	97.01%	✅
monaco-editor.service.ts	100%	✅

Screenshots

No UI changes

Summary by CodeRabbit

New Features
- Enhanced the MonacoDiffEditorComponent to function independently with improved reactivity.
- Updated component properties for better performance and usability.
Bug Fixes
- Streamlined the component's constructor for improved readability and functionality.
Tests
- Enhanced test coverage for MonacoDiffEditorComponent with updated assertions and property handling.
- Improved test setup by removing unnecessary module imports and focusing on component imports.

This reverts commit da4ea63.

pzdr7 · 2024-10-01T20:52:35Z

...spec/component/programming-exercise/programming-exercise-custom-build-plan.component.spec.ts

+        comp.editor = TestBed.createComponent(MonacoEditorComponent).componentInstance;
        expect(comp.editor).toBeDefined();


If this looks odd to you: comp.editor is actually a get/set pair. We will remove this when refactoring the build plan components

pzdr7 · 2024-10-01T20:53:43Z

src/main/webapp/app/shared/monaco-editor/monaco-editor.service.ts

+    }
+
+    applyTheme(artemisTheme: Theme): void {
+        monaco.editor.setTheme(artemisTheme === Theme.LIGHT ? 'vs' : 'vs-dark');


vs-light (from the other components) was actually wrong. The light theme (which is also the fallback for incorrect themes 😅) is vs

coderabbitai · 2024-10-02T06:36:46Z

Walkthrough

The changes in this pull request focus on significant updates to the MonacoDiffEditorComponent in monaco-diff-editor.component.ts. The component is now standalone and utilizes an OnPush change detection strategy. Input and output properties have been refactored to use new decorators, enhancing reactivity. The constructor has been simplified by directly injecting dependencies. Additionally, an effect has been introduced for reactive updates to editor options, and theme management has been delegated to the MonacoEditorService.

Changes

File	Change Summary
`src/main/webapp/app/shared/monaco-editor/monaco-diff-editor.component.ts`	Updated `MonacoDiffEditorComponent` to be standalone, modified change detection strategy to `OnPush`, refactored input/output properties to use new decorators, streamlined constructor by injecting dependencies directly, and introduced an effect for reactive updates to editor options.

Possibly related PRs

Programming exercises: Fix the diff editor layout #8863: This PR enhances the MonacoDiffEditorComponent, which is directly related to the changes made in the main PR that also modifies this component.
Programming exercises: Improve the diff viewer with Monaco's built-in code diff #8540: This PR introduces improvements to the diff viewer using Monaco, which aligns with the updates made to the MonacoDiffEditorComponent in the main PR.
Programming exercises: Change the problem statement editor to Monaco #8856: This PR changes the problem statement editor to use Monaco, which is relevant as it involves the same Monaco editor framework being updated in the main PR.
Programming exercises: Change the build plan editors to Monaco #8978: This PR replaces the Ace editor with Monaco in the build plan editor, which is related to the overall transition to Monaco editors in the project, similar to the changes in the main PR.

Suggested labels

enhancement, documentation, ready to merge

Suggested reviewers

florian-glombik
egekurt123
coolchock
Jan-Thurner

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

🪧 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>, please review it.
- 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @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.

az108

Tested on TS6, works as expected and editor highlightings also transfer correctly between dark and light mode

SimonEntholzer

Re-approve

BBesrour

Tested on TS6, lgtm!

pzdr7 added 16 commits September 22, 2024 21:38

Make Monaco standalone

5902a58

Use inject function

f6e0e26

Add monaco editor service

1c4cbfc

Use input and output signals

4ea9f24

Use input and output signals, part 2

dc16948

Reorganize attributes

89cf3fd

Reorganize attributes, part 2

aaf368c

Fix aeolus tests

5380595

Fix monaco tests

e5fe810

Fix build error

b3a6e45

Delete MonacoEditorModule

5c04869

WIP themes

da4ea63

Revert "WIP themes"

d378c63

This reverts commit da4ea63.

Merge branch 'develop' into chore/programming-exercises/monaco-themes

badfb0f

Move theme apply to service

2ec48ba

Replace runInInjectionContext

b44d1c0

github-actions bot assigned pzdr7 Oct 1, 2024

github-actions bot added tests client Pull requests that update TypeScript code. (Added Automatically!) labels Oct 1, 2024

Adapt change detection strategy

984cbcf

pzdr7 commented Oct 1, 2024

View reviewed changes

pzdr7 added the deploy:artemis-test5 label Oct 2, 2024

pzdr7 temporarily deployed to artemis-test5.artemis.cit.tum.de October 2, 2024 06:22 — with GitHub Actions Inactive

github-actions bot added lock:artemis-test5 and removed deploy:artemis-test5 labels Oct 2, 2024

pzdr7 removed the lock:artemis-test5 label Oct 2, 2024

pzdr7 marked this pull request as ready for review October 2, 2024 06:36

pzdr7 requested a review from a team as a code owner October 2, 2024 06:36

pzdr7 requested a review from JohannesWt October 2, 2024 06:36

github-actions bot added the lock:artemis-test6 label Oct 6, 2024

az108 removed the lock:artemis-test6 label Oct 6, 2024

az108 approved these changes Oct 6, 2024

View reviewed changes

SimonEntholzer approved these changes Oct 6, 2024

View reviewed changes

pzdr7 added the deploy:artemis-test6 label Oct 7, 2024

pzdr7 temporarily deployed to artemis-test6.artemis.cit.tum.de October 7, 2024 07:09 — with GitHub Actions Inactive

github-actions bot added lock:artemis-test6 and removed deploy:artemis-test6 labels Oct 7, 2024

pzdr7 removed the lock:artemis-test6 label Oct 7, 2024

BBesrour approved these changes Oct 7, 2024

View reviewed changes

krusche added this to the 7.6.0 milestone Oct 7, 2024

krusche merged commit 1b12f3e into develop Oct 7, 2024
46 of 51 checks passed

krusche deleted the chore/programming-exercises/monaco-standalone branch October 7, 2024 18:56

This was referenced Oct 20, 2024

Communication: Fix an issue with link previews #9532

Merged

Development: Upgrade markdown library to markdown-it #9354

Merged

Communication: Allow file uploads in messages #9580

Merged

Communication: Add emoji support to messages #9595

Merged

This was referenced Nov 13, 2024

Development: Update theme switcher to use Angular 18 practices #9250

Merged

Development: Migrate text module to use inject #9772

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Development`: Migrate the MonacoEditorModule to standalone components #9407

`Development`: Migrate the MonacoEditorModule to standalone components #9407

pzdr7 commented Oct 1, 2024 •

edited

Loading

pzdr7 Oct 1, 2024

pzdr7 Oct 1, 2024

coderabbitai bot commented Oct 2, 2024 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

az108 left a comment

SimonEntholzer left a comment

BBesrour left a comment

		comp.editor = TestBed.createComponent(MonacoEditorComponent).componentInstance;
		expect(comp.editor).toBeDefined();

Development: Migrate the MonacoEditorModule to standalone components #9407

Development: Migrate the MonacoEditorModule to standalone components #9407

Conversation

pzdr7 commented Oct 1, 2024 • edited Loading

Checklist

General

Client

Changes affecting Programming Exercises

Motivation and Context

Description

Steps for Testing

Testserver States

Review Progress

Code Review

Manual Tests

Test Coverage

Client

Screenshots

Summary by CodeRabbit

Summary by CodeRabbit

pzdr7 Oct 1, 2024

Choose a reason for hiding this comment

pzdr7 Oct 1, 2024

Choose a reason for hiding this comment

coderabbitai bot commented Oct 2, 2024 • edited Loading

Walkthrough

Changes

Possibly related PRs

Suggested labels

Suggested reviewers

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

az108 left a comment

Choose a reason for hiding this comment

SimonEntholzer left a comment

Choose a reason for hiding this comment

BBesrour left a comment

Choose a reason for hiding this comment

`Development`: Migrate the MonacoEditorModule to standalone components #9407

`Development`: Migrate the MonacoEditorModule to standalone components #9407

pzdr7 commented Oct 1, 2024 •

edited

Loading

coderabbitai bot commented Oct 2, 2024 •

edited

Loading