Skip to content

Communication: Select an Emoji from a suggestion box while typing #10976

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

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

Predixx
Copy link

@Predixx Predixx commented Jun 3, 2025

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) and too complex database calls.
  • I strictly followed the principle of data economy for all database calls.
  • I strictly followed the server coding and design guidelines.
  • I added multiple integration tests (Spring) related to the features (with a high test coverage).
  • I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
  • I documented the Java code using JavaDoc style.

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 principle of data economy for all client-server REST calls.
  • 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.
  • I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
  • I documented the TypeScript code using JSDoc style.
  • I added multiple screenshots/screencasts of my UI changes.
  • I translated all newly inserted strings into English and German.

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).
  • I tested all changes and their related features with all corresponding user types on a test server configured with LocalVC and Jenkins.

Motivation and Context

When writing a text quickly I want to type emojis from the keyboard without having to interupt my flow and having to use the trackpad/mouse.

Description

I have added a suggestion box for emojis that detects when the user starts to type the name of an emoji and helps him find the correct one.

image

Steps for Testing

Prerequisites:

  • 1 Instructor/Student
  • 1 Course with a communication channel
  1. Log in to Artemis
  2. Navigate to any Communication channel
  3. Start typing an emoji name (e.g. ":joy")
  4. Test the new suggestion box for functionality

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

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance even for very large courses with more than 2000 students.
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance even for very large courses with more than 2000 students.

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Exam Mode Test

  • Test 1
  • Test 2

Performance Tests

  • Test 1
  • Test 2

Test Coverage

Screenshots

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Added emoji autocomplete in the markdown editor, triggered by typing colon-prefixed emoji codes.
    • Introduced an interactive emoji suggestion dropdown with keyboard navigation and clickable selection.
    • Emoji suggestions are displayed near the cursor and update dynamically as you type.
  • Style

    • Enhanced visual styling for the emoji suggestion dropdown for improved usability and appearance.
    • Updated editor container styling to support dropdown positioning.
  • Tests

    • Added unit tests to ensure correct emoji suggestion functionality and component behavior.

@github-project-automation github-project-automation bot moved this to Work In Progress in Artemis Development Jun 3, 2025
@github-actions github-actions bot added client Pull requests that update TypeScript code. (Added Automatically!) communication Pull requests that affect the corresponding module labels Jun 3, 2025
@Predixx Predixx changed the title Select an Emoji from a suggestion box while typing Communication: Select an Emoji from a suggestion box while typing Jun 3, 2025
@Predixx Predixx marked this pull request as ready for review June 3, 2025 19:26
@Predixx Predixx requested review from a team and krusche as code owners June 3, 2025 19:26
Copy link
Contributor

coderabbitai bot commented Jun 3, 2025

Walkthrough

This change introduces an emoji suggestion dropdown feature to the markdown editor. It adds a new Angular component for displaying emoji suggestions, integrates it into the editor UI, and implements logic for detecting emoji codes, showing suggestions, handling keyboard navigation, and inserting selected emojis. Associated styles, utility functions, and tests are included. The Monaco editor component is extended to expose keydown event handling and position calculation methods.

Changes

File(s) Change Summary
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.html
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts
Added a new Angular standalone component for emoji suggestion dropdown with template, styles, inputs, outputs, and a utility function to fetch emoji suggestions from the emoji mart library.
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.spec.ts Added unit tests for the emoji suggestion dropdown component and the emoji suggestion utility function, covering component instantiation and various query scenarios for emoji suggestions.
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.html Modified the markdown editor template to add the CSS class position-relative and conditionally render the emoji suggestion dropdown component with appropriate bindings.
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.scss Added a style rule setting height to 100% for elements with classes col-12 and position-relative.
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts Extended the markdown editor component to integrate emoji suggestion dropdown: added state management for suggestions, dropdown visibility, styling, active index, and last matched query; implemented lifecycle hooks to add/remove window resize listener; hooked into Monaco editor keydown events; enhanced content update to detect emoji queries and position dropdown; added keyboard navigation and selection handling for emoji suggestions.
src/main/webapp/app/shared/monaco-editor/monaco-editor.component.ts Added two public methods to expose Monaco editor internals: onKeyDown to register keydown event listeners, and getScrolledVisiblePosition to get the visible position of a cursor location, aiding dropdown positioning and keyboard handling in the markdown editor.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PostingMarkdownEditorComponent
    participant EmojiSuggestionDropdownComponent
    participant EmojiSuggestionUtil

    User->>PostingMarkdownEditorComponent: Types text (e.g., ":smil")
    PostingMarkdownEditorComponent->>EmojiSuggestionUtil: getEmojiSuggestions(":smil")
    EmojiSuggestionUtil-->>PostingMarkdownEditorComponent: Return emoji suggestions
    PostingMarkdownEditorComponent->>EmojiSuggestionDropdownComponent: Show dropdown with suggestions and position
    User->>EmojiSuggestionDropdownComponent: Navigate/select suggestion (mouse/keyboard)
    EmojiSuggestionDropdownComponent->>PostingMarkdownEditorComponent: Emit selection event
    PostingMarkdownEditorComponent->>PostingMarkdownEditorComponent: Replace code with selected emoji
    PostingMarkdownEditorComponent->>EmojiSuggestionDropdownComponent: Hide dropdown
Loading

Suggested labels

feature, ready for review

Suggested reviewers

  • asliayk
  • krusche
  • HawKhiem

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

npm error Exit handler never called!
npm error This is an error with npm itself. Please report this error at:
npm error https://github.com/npm/cli/issues
npm error A complete log of this run can be found in: /.npm/_logs/2025-06-04T20_49_08_296Z-debug-0.log

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ 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.
    • Explain this complex logic.
    • 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 explain this code block.
    • @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 explain its main purpose.
    • @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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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 generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (5)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss (1)

16-23: Fix CSS rule ordering and formatting issues.

The static analysis tool has identified valid issues with CSS organization that should be addressed for better maintainability.

Apply this diff to fix the formatting and ordering issues:

+
 .emoji-suggestion-item.active,
 .emoji-suggestion-item:hover {
     background: var(--dropdown-link-hover-bg, var(--bs-secondary-bg, #e9ecef));
     color: var(--bs-body-color, #222);
     transition:
         background 0.15s,
         color 0.15s;
 }
-
-.emoji-suggestion-item {
-    border-radius: 4px;
-}

And move the base .emoji-suggestion-item rule before its variants:

 }
+
+.emoji-suggestion-item {
+    border-radius: 4px;
+}
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis

[notice] 16-16: src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss#L16
Expected empty line before rule (rule-empty-line-before)

src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.html (1)

10-13: Consider moving inline styles to SCSS for better maintainability.

While the inline styles work correctly, moving them to the component's SCSS file would improve code organization and reusability.

Consider adding these styles to the SCSS file:

.emoji-suggestion-item {
    display: flex;
    align-items: center;
    padding: 1px 0;
    cursor: pointer;
    border-radius: 4px;

    .emoji-char {
        font-size: 1.1em;
        width: 1.7em;
    }

    .emoji-name {
        margin-left: 0.4em;
        font-size: 0.93em;
    }
}

Then update the template:

 <div
     class="emoji-suggestion-item"
     [ngClass]="{ active: i === activeIndex }"
     (mousedown)="onSelect.emit(s)"
     (mouseenter)="activeIndex = i"
     (mouseleave)="activeIndex = -1"
-    style="display: flex; align-items: center; padding: 1px 0; cursor: pointer"
 >
-    <span style="font-size: 1.1em; width: 1.7em">{{ s.emoji }}</span>
-    <span style="margin-left: 0.4em; font-size: 0.93em">:{{ s.name }}:</span>
+    <span class="emoji-char">{{ s.emoji }}</span>
+    <span class="emoji-name">:{{ s.name }}:</span>
 </div>
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1)

20-20: Remove the unnecessary empty constructor.

The empty constructor serves no purpose and can be safely removed.

Apply this diff to remove the empty constructor:

-    constructor() {}
🧰 Tools
🪛 Biome (1.9.4)

[error] 20-20: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (2)

323-323: Use optional chaining for cleaner code.

The null check can be simplified using optional chaining.

Apply this diff to use optional chaining:

-        if (this.markdownEditor && this.markdownEditor.monacoEditor && (this.markdownEditor.monacoEditor as any)._editor) {
+        if (this.markdownEditor?.monacoEditor && (this.markdownEditor.monacoEditor as any)._editor) {
🧰 Tools
🪛 Biome (1.9.4)

[error] 323-323: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


244-279: Extract emoji pattern detection logic for better maintainability.

The emoji pattern detection and dropdown positioning logic is complex and embedded within the updateField method. Consider extracting this into separate methods for better testability and maintainability.

Consider refactoring into smaller methods:

private detectEmojiPattern(text: string): { query: string; index: number } | null {
    const matches = text.match(/:([a-zA-Z0-9_+-]*)/g);
    if (!matches) return null;
    
    const lastMatch = matches[matches.length - 1];
    const lastIndex = text.lastIndexOf(lastMatch);
    const query = lastMatch.slice(1);
    
    return query.length > 0 ? { query, index: lastIndex } : null;
}

private calculateDropdownPosition(text: string, matchIndex: number): { left: string; top: string } | null {
    // Extract position calculation logic here
}

private showEmojiSuggestions(suggestions: Array<{ name: string; emoji: string }>, position: { left: string; top: string }) {
    // Extract dropdown display logic here
}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee6d6e3 and ec2b6df.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (6)
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.html (1 hunks)
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss (1 hunks)
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.spec.ts (1 hunks)
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1 hunks)
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.html (2 hunks)
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/main/webapp/**/*.html`: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.

  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.html
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.html
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...

src/main/webapp/**/*.ts: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.spec.ts
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts
🧬 Code Graph Analysis (2)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1)
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (1)
  • Component (55-371)
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (1)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1)
  • getEmojiSuggestions (31-53)
🪛 GitHub Check: Codacy Static Code Analysis
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss

[notice] 16-16: src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss#L16
Expected empty line before rule (rule-empty-line-before)


[warning] 25-25: src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss#L25
Expected selector ".emoji-suggestion-item" to come before selector ".emoji-suggestion-item:hover" (no-descending-specificity)

🪛 Biome (1.9.4)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts

[error] 20-20: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts

[error] 323-323: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
  • GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
  • GitHub Check: client-tests-selected
  • GitHub Check: client-tests
  • GitHub Check: server-tests
  • GitHub Check: Analyse
🔇 Additional comments (6)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss (1)

1-15: LGTM! Good theming and positioning approach.

The dropdown styling uses proper CSS custom properties for theming support and includes appropriate positioning with z-index management. The fallback values ensure compatibility across different themes.

src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.html (2)

1-1: Good positioning setup for dropdown integration.

Adding position-relative class and height styling creates the proper positioning context for the absolutely positioned emoji dropdown.


40-47: Excellent integration of emoji dropdown component.

The conditional rendering uses the new @if syntax as required by the coding guidelines, and the component bindings are comprehensive, covering all necessary inputs and event handlers for proper functionality.

src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.html (2)

2-3: Excellent use of new Angular control flow syntax.

The template correctly uses the new @if and @for syntax as required by the coding guidelines, replacing the older *ngIf and *ngFor directives.


7-9: Good event handling pattern for interactive dropdown.

The mouse event handlers provide proper user interaction: mousedown for selection, mouseenter/mouseleave for hover states. Using mousedown instead of click prevents issues with focus changes.

src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.spec.ts (1)

23-63: Excellent utility function test coverage.

The tests for getEmojiSuggestions are comprehensive and well-structured, covering edge cases, parameter validation, result structure verification, and common usage scenarios. This provides confidence in the function's reliability.

@github-project-automation github-project-automation bot moved this from Work In Progress to Ready For Review in Artemis Development Jun 3, 2025
Copy link

github-actions bot commented Jun 3, 2025

End-to-End (E2E) Test Results Summary

TestsPassed ☑️Skipped ⚠️Failed ❌️Time ⏱
End-to-End (E2E) Test Report201 ran197 passed3 skipped1 failed55m 35s 94ms
TestResultTime ⏱
End-to-End (E2E) Test Report
e2e/exercise/quiz-exercise/QuizExerciseParticipation.spec.ts
ts.Quiz Exercise Participation › DnD Quiz participation › Student can participate in DnD Quiz❌ failure2m 4s 663ms

Copy link
Contributor

@az108 az108 left a comment

Choose a reason for hiding this comment

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

some important comments

(mousedown)="onSelect.emit(s)"
(mouseenter)="activeIndex = i"
(mouseleave)="activeIndex = -1"
style="display: flex; align-items: center; padding: 1px 0; cursor: pointer"
Copy link
Contributor

Choose a reason for hiding this comment

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

please do not use any styling in the html and move it to the scss

style="display: flex; align-items: center; padding: 1px 0; cursor: pointer"
>
<span style="font-size: 1.1em; width: 1.7em">{{ s.emoji }}</span>
<span style="margin-left: 0.4em; font-size: 0.93em">:{{ s.name }}:</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

same here no styling please

(mouseleave)="activeIndex = -1"
style="display: flex; align-items: center; padding: 1px 0; cursor: pointer"
>
<span style="font-size: 1.1em; width: 1.7em">{{ s.emoji }}</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

same here no styling please

.emoji-suggestion-dropdown {
background: var(--dropdown-bg, #222);
color: var(--bs-body-color, #eee);
outline: 1px solid var(--dropdown-border-color, gray);
Copy link
Contributor

Choose a reason for hiding this comment

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

please dont use px, rather use rem

background: var(--dropdown-bg, #222);
color: var(--bs-body-color, #eee);
outline: 1px solid var(--dropdown-border-color, gray);
border-radius: 4px;
Copy link
Contributor

Choose a reason for hiding this comment

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

same for all below, no px please rather rem

providers: [EmojiSearch, EmojiService],
})
export class EmojiSuggestionDropdownComponent {
@Input() suggestions: { name: string; emoji: string }[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

please use input. and not @input as we want to move towards signals, do for all below please

export class EmojiSuggestionDropdownComponent {
@Input() suggestions: { name: string; emoji: string }[] = [];
@Input() dropdownStyle: { [key: string]: string } = {};
@Output() onSelect = new EventEmitter<{ name: string; emoji: string }>();
Copy link
Contributor

Choose a reason for hiding this comment

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

here please use @output

@@ -1,4 +1,4 @@
<div class="col-12">
<div class="col-12 position-relative" style="height: 100%">
Copy link
Contributor

Choose a reason for hiding this comment

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

please no styling here in html

@@ -104,6 +106,14 @@ export class PostingMarkdownEditorComponent implements OnInit, ControlValueAcces
protected readonly MarkdownEditorHeight = MarkdownEditorHeight;
private overlay = inject(Overlay);

// --- Emoji dropdown state ---
emojiSuggestions: { name: string; emoji: string }[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

try to turn as many fields as possible to signals here

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (3)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1)

6-8: Excellent performance optimization with module-level caching.

The caching of EmojiService and EmojiSearch instances at the module level addresses the previous performance concern about creating new instances on every function call.

src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (2)

293-298: Replace private API usage with public method.

This code is still accessing Monaco's private _editor API for positioning. Use the public getScrolledVisiblePosition method consistently.

Apply this diff:

-            if (suggestions.length > 0 && this.markdownEditor && this.markdownEditor.monacoEditor && (this.markdownEditor.monacoEditor as any)._editor) {
-                const editor = (this.markdownEditor.monacoEditor as any)._editor;
+            if (suggestions.length > 0 && this.markdownEditor?.monacoEditor) {
                 const lines = newValue.substring(0, lastIndex).split('\n');
                 const line = lines.length;
                 const column = lines[lines.length - 1].length + 1;
-                const coords = editor.getScrolledVisiblePosition({ lineNumber: line, column });
+                const coords = this.markdownEditor.monacoEditor.getScrolledVisiblePosition({ lineNumber: line, column });

338-354: Consider adding accessibility attributes for screen readers.

The emoji suggestion dropdown keyboard navigation works well, but it lacks proper ARIA attributes for accessibility. Screen reader users won't know about the available suggestions or the currently selected item.

The dropdown component template should include:

  • role="listbox" on the dropdown container
  • role="option" on each suggestion item
  • aria-selected="true" on the active item
  • aria-label describing the dropdown purpose

Would you like me to help implement the accessibility improvements in the dropdown component template?

🧹 Nitpick comments (2)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1)

24-24: Remove unnecessary constructor.

The empty constructor serves no purpose and can be removed for cleaner code.

Apply this diff:

-    constructor() {}
🧰 Tools
🪛 Biome (1.9.4)

[error] 24-24: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (1)

110-116: Consider migrating emoji dropdown state to signals.

Based on the coding guidelines preference for signals, consider converting the emoji dropdown state variables to signals for better performance and reactivity.

This would involve converting fields like emojiSuggestions, showEmojiDropdown, emojiDropdownStyle, etc. to use signal() and updating their usage accordingly throughout the component.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec2b6df and fb883c9.

📒 Files selected for processing (7)
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.html (1 hunks)
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss (1 hunks)
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1 hunks)
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.html (2 hunks)
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.scss (1 hunks)
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (7 hunks)
  • src/main/webapp/app/shared/monaco-editor/monaco-editor.component.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.scss
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.html
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.html
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalC...

src/main/webapp/**/*.ts: angular_style:https://angular.io/guide/styleguide;methods_in_html:false;lazy_loading:true;code_reuse:true;tests:meaningful;types:PascalCase;enums:PascalCase;funcs:camelCase;props:camelCase;no_priv_prefix:true;strings:single_quotes;localize:true;btns:functionality;links:navigation;icons_text:newline;labels:associate;code_style:arrow_funcs,curly_braces,open_braces_same_line,indent_4;memory_leak_prevention:true;routes:naming_schema;chart_framework:ngx-charts;responsive_layout:true

  • src/main/webapp/app/shared/monaco-editor/monaco-editor.component.ts
  • src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts
  • src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts
🧠 Learnings (1)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10147
File: src/main/webapp/app/shared/components/code-button/code-button.component.ts:1-1
Timestamp: 2025-01-18T08:49:30.693Z
Learning: In Angular 19+ projects, component inputs should use the new input() function from @angular/core instead of the @Input() decorator, as it's part of the new signals architecture that improves performance and type safety.
🧬 Code Graph Analysis (2)
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (1)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1)
  • getEmojiSuggestions (35-53)
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts (1)
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (1)
  • Component (55-371)
🪛 Biome (1.9.4)
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts

[error] 323-323: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts

[error] 24-24: This constructor is unnecessary.

Unsafe fix: Remove the unnecessary constructor.

(lint/complexity/noUselessConstructor)

🪛 GitHub Check: Codacy Static Code Analysis
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss

[notice] 16-16: src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss#L16
Expected empty line before rule (rule-empty-line-before)


[warning] 25-25: src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss#L25
Expected selector ".emoji-suggestion-item" to come before selector ".emoji-suggestion-item:hover" (no-descending-specificity)

⏰ Context from checks skipped due to timeout of 90000ms (7)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: client-tests-selected
  • GitHub Check: client-tests
  • GitHub Check: server-tests
  • GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
  • GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
  • GitHub Check: Analyse
🔇 Additional comments (3)
src/main/webapp/app/shared/monaco-editor/monaco-editor.component.ts (1)

511-527: Well-implemented public API methods.

The new onKeyDown and getScrolledVisiblePosition methods provide clean, type-safe access to Monaco editor functionality. The implementation follows the existing patterns in the component and properly handles the undefined case in getScrolledVisiblePosition.

src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts (2)

264-264: Excellent use of the new public API.

Great work using the new getScrolledVisiblePosition public method instead of accessing Monaco's private internals.


244-279: Solid emoji suggestion implementation with good performance.

The emoji detection logic using regex and integration with the getEmojiSuggestions utility function is well-implemented. The positioning logic correctly calculates cursor coordinates and the extraction of the magic number to a named constant improves readability.

Comment on lines +16 to +31
.emoji-suggestion-item.active,
.emoji-suggestion-item:hover {
background: var(--dropdown-link-hover-bg, var(--bs-secondary-bg, #e9ecef));
color: var(--bs-body-color, #222);
transition:
background 0.15s,
color 0.15s;
}

.emoji-suggestion-item {
display: flex;
align-items: center;
padding: 0.04rem 0;
cursor: pointer;
border-radius: 0.18rem;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix CSS specificity ordering and add missing empty line.

The CSS selectors are not in the optimal order for specificity, and there's a missing empty line before the rule.

Apply this diff to fix the specificity ordering and formatting:

+
 .emoji-suggestion-item.active,
 .emoji-suggestion-item:hover {
     background: var(--dropdown-link-hover-bg, var(--bs-secondary-bg, #e9ecef));
     color: var(--bs-body-color, #222);
     transition:
         background 0.15s,
         color 0.15s;
 }

 .emoji-suggestion-item {
     display: flex;
     align-items: center;
     padding: 0.04rem 0;
     cursor: pointer;
     border-radius: 0.18rem;
 }

This ensures the base selector comes before the more specific hover/active selectors, which is the recommended CSS pattern for maintainability and clarity.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis

[notice] 16-16: src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss#L16
Expected empty line before rule (rule-empty-line-before)


[warning] 25-25: src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss#L25
Expected selector ".emoji-suggestion-item" to come before selector ".emoji-suggestion-item:hover" (no-descending-specificity)

🤖 Prompt for AI Agents
In
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.scss
between lines 16 and 31, reorder the CSS rules so that the base selector
.emoji-suggestion-item appears before the more specific
.emoji-suggestion-item.active and .emoji-suggestion-item:hover selectors. Also,
add a missing empty line before the .emoji-suggestion-item rule to improve
readability and maintain the recommended CSS pattern for specificity and
clarity.

Comment on lines +19 to +22
@Input() suggestions: { name: string; emoji: string }[] = [];
@Input() dropdownStyle: { [key: string]: string } = {};
@Output() onSelect = new EventEmitter<{ name: string; emoji: string }>();
@Input() activeIndex: number = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Migrate to Angular signals for better performance and type safety.

Based on the coding guidelines and retrieved learnings, Angular 19+ projects should use the new signals architecture with input() and output() functions instead of decorators.

Apply this diff to migrate to signals:

-    @Input() suggestions: { name: string; emoji: string }[] = [];
-    @Input() dropdownStyle: { [key: string]: string } = {};
-    @Output() onSelect = new EventEmitter<{ name: string; emoji: string }>();
-    @Input() activeIndex: number = 0;
+    suggestions = input<{ name: string; emoji: string }[]>([]);
+    dropdownStyle = input<{ [key: string]: string }>({});
+    onSelect = output<{ name: string; emoji: string }>();
+    activeIndex = input<number>(0);

Don't forget to add the required imports at the top:

-import { Component, EventEmitter, Input, Output } from '@angular/core';
+import { Component, input, output } from '@angular/core';
🤖 Prompt for AI Agents
In
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts
around lines 19 to 22, the component uses Angular decorators @Input and @Output
for properties and events. To improve performance and type safety, replace these
decorators with Angular signals by using the input() function for inputs and
output() function for outputs. Update the property declarations accordingly and
add the necessary imports for input and output from '@angular/core/signals' at
the top of the file.

Comment on lines +155 to +160
if (editor && (editor as any)._editor) {
(editor as any)._editor.onKeyDown((event: any) => {
const domEvent = event.browserEvent as KeyboardEvent;
this.onKeyDown(domEvent);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use the new public API instead of accessing private Monaco internals.

The code is accessing Monaco's private _editor API, but you've added a public onKeyDown method to MonacoEditorComponent that should be used instead.

Apply this diff to use the public API:

-        if (editor && (editor as any)._editor) {
-            (editor as any)._editor.onKeyDown((event: any) => {
-                const domEvent = event.browserEvent as KeyboardEvent;
-                this.onKeyDown(domEvent);
-            });
-        }
+        if (editor) {
+            editor.onKeyDown((event: monaco.IKeyboardEvent) => {
+                const domEvent = event.browserEvent as KeyboardEvent;
+                this.onKeyDown(domEvent);
+            });
+        }
📝 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
if (editor && (editor as any)._editor) {
(editor as any)._editor.onKeyDown((event: any) => {
const domEvent = event.browserEvent as KeyboardEvent;
this.onKeyDown(domEvent);
});
}
// Use public onKeyDown API instead of digging into Monaco's private _editor
if (editor) {
editor.onKeyDown((event: monaco.IKeyboardEvent) => {
const domEvent = event.browserEvent as KeyboardEvent;
this.onKeyDown(domEvent);
});
}
🤖 Prompt for AI Agents
In
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts
around lines 155 to 160, the code accesses the private _editor property of the
Monaco editor instance, which is discouraged. Instead, replace this with the
public onKeyDown method provided by the MonacoEditorComponent by calling
editor.onKeyDown directly and passing the event handler. This avoids using
private internals and uses the intended public API.

Comment on lines +323 to +331
if (this.markdownEditor && this.markdownEditor.monacoEditor && (this.markdownEditor.monacoEditor as any)._editor) {
const editor = (this.markdownEditor.monacoEditor as any)._editor;
const model = this.markdownEditor.monacoEditor.getModel();
if (editor && model) {
model.setValue(newText);
const pos = before.length + selected.emoji.length;
const { lineNumber, column } = model.getPositionAt(pos);
editor.setPosition({ lineNumber, column });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use optional chaining and avoid private API access.

Apply the static analysis suggestion to use optional chaining and replace private API usage.

Apply this diff:

-        if (this.markdownEditor && this.markdownEditor.monacoEditor && (this.markdownEditor.monacoEditor as any)._editor) {
-            const editor = (this.markdownEditor.monacoEditor as any)._editor;
+        if (this.markdownEditor?.monacoEditor) {
             const model = this.markdownEditor.monacoEditor.getModel();
-            if (editor && model) {
+            if (model) {
                 model.setValue(newText);
                 const pos = before.length + selected.emoji.length;
                 const { lineNumber, column } = model.getPositionAt(pos);
-                editor.setPosition({ lineNumber, column });
+                this.markdownEditor.monacoEditor.setPosition({ lineNumber, column });
             }
         }

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome (1.9.4)

[error] 323-323: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🤖 Prompt for AI Agents
In
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts
around lines 323 to 331, replace the direct access to the private _editor
property with safe optional chaining to avoid accessing private APIs. Refactor
the code to use optional chaining on markdownEditor and monacoEditor properties
and use only public API methods to get the editor instance and model. This will
improve code safety and maintainability by avoiding reliance on internal private
properties.

Copy link

github-actions bot commented Jun 4, 2025

End-to-End (E2E) Test Results Summary

TestsPassed ☑️Skipped ⚠️Failed ❌️Time ⏱
End-to-End (E2E) Test Report201 ran197 passed3 skipped1 failed1h 2m 58s 835ms
TestResultTime ⏱
End-to-End (E2E) Test Report
e2e/exercise/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts
ts.Static code analysis tests › Configures SCA grading and makes a successful submission with SCA errors❌ failure2m 26s 945ms

Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jun 12, 2025
@helios-aet helios-aet bot temporarily deployed to artemis-test1.artemis.cit.tum.de June 13, 2025 10:44 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) communication Pull requests that affect the corresponding module stale
Projects
Status: Ready For Review
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants