-
Notifications
You must be signed in to change notification settings - Fork 314
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
base: develop
Are you sure you want to change the base?
Conversation
Communication
: Select an Emoji from a suggestion box while typing
WalkthroughThis 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
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
Suggested labels
Suggested reviewers
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
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
Documentation and Community
|
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: 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
⛔ 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/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. Usingmousedown
instead ofclick
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.
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.spec.ts
Show resolved
Hide resolved
src/main/webapp/app/communication/emoji/emoji-suggestion-dropdown.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts
Show resolved
Hide resolved
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts
Outdated
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
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.
some important comments
(mousedown)="onSelect.emit(s)" | ||
(mouseenter)="activeIndex = i" | ||
(mouseleave)="activeIndex = -1" | ||
style="display: flex; align-items: center; padding: 1px 0; cursor: pointer" |
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.
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> |
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.
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> |
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.
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); |
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.
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; |
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.
same for all below, no px please rather rem
providers: [EmojiSearch, EmojiService], | ||
}) | ||
export class EmojiSuggestionDropdownComponent { | ||
@Input() suggestions: { name: string; emoji: string }[] = []; |
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.
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 }>(); |
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.
here please use @output
@@ -1,4 +1,4 @@ | |||
<div class="col-12"> | |||
<div class="col-12 position-relative" style="height: 100%"> |
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.
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 }[] = []; |
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.
try to turn as many fields as possible to signals here
src/main/webapp/app/communication/posting-markdown-editor/posting-markdown-editor.component.ts
Show resolved
Hide resolved
…age with public methods.
…dropdown component styling
…ture/emoji-shortcut
…nent.scss for better style separation.
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: 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
andEmojiSearch
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 publicgetScrolledVisiblePosition
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 containerrole="option"
on each suggestion itemaria-selected="true"
on the active itemaria-label
describing the dropdown purposeWould 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 usesignal()
and updating their usage accordingly throughout the component.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 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/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
andgetScrolledVisiblePosition
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 ingetScrolledVisiblePosition
.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.
.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; | ||
} |
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.
🛠️ 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.
@Input() suggestions: { name: string; emoji: string }[] = []; | ||
@Input() dropdownStyle: { [key: string]: string } = {}; | ||
@Output() onSelect = new EventEmitter<{ name: string; emoji: string }>(); | ||
@Input() activeIndex: number = 0; |
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.
🛠️ 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.
if (editor && (editor as any)._editor) { | ||
(editor as any)._editor.onKeyDown((event: any) => { | ||
const domEvent = event.browserEvent as KeyboardEvent; | ||
this.onKeyDown(domEvent); | ||
}); | ||
} |
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.
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.
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.
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 }); | ||
} |
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.
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.
End-to-End (E2E) Test Results Summary
|
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. |
Checklist
General
Server
Client
authorities
to all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
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.
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
Exam Mode Test
Performance Tests
Test Coverage
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style
Tests