Iris: Improved general UI changes for Iris
#12045
Conversation
…pdated design tokens for Iris
…tyling on client ui
github-actions
bot
added
server
WalkthroughAdds Iris (chat) to course overview: new IRIS route and component, sidebar integration with analytics, chat UI/UX enhancements (copy, new-session handling, history grouping), theme variable consolidation, and new ScienceEventType values for Iris analytics. Changes
Sequence Diagram(s)mermaid User->>Sidebar: click Iris sidebar item Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/webapp/app/core/course/overview/course-overview/course-overview.component.ts (1)
299-308: Fix dashboard flag logic to prevent unintended dashboard activation.When
studentCourseAnalyticsDashboardEnabledis false butirisEnabledInCourseis true, the OR condition passes true togetStudentDefaultItems, causing the dashboard item to be added even though analytics are disabled. Pass onlystudentCourseAnalyticsDashboardEnabledas the first parameter:Fix
const defaultItems = this.sidebarItemService.getStudentDefaultItems( - currentCourse?.studentCourseAnalyticsDashboardEnabled || currentCourse?.irisEnabledInCourse, + currentCourse?.studentCourseAnalyticsDashboardEnabled, currentCourse?.trainingEnabled, );
🤖 Fix all issues with AI agents
In
`@src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.html`:
- Around line 200-205: The ngFor track uses the loop variable "line" which can
produce duplicate keys; update the ngFor in the iris-base-chatbot template that
iterates over (content | as: IrisTextMessageContent).textContent.split('\n') to
use a unique index key (track $index) instead of track line so each rendered
line has a stable unique key and avoids DOM reuse/NG0955 warnings.
- Around line 216-239: The three icon-only buttons for copy, thumbs-up, and
thumbs-down in iris-base-chatbot.component.html (the buttons invoking
copyMessage(message, i), rateMessage(message, true) and rateMessage(message,
false)) need accessible labels: add both aria-label and title attributes to each
button using the artemisTranslate pipe with keys `artemisApp.iris.chat.copy`,
`artemisApp.iris.chat.thumbsUp`, and `artemisApp.iris.chat.thumbsDown`
respectively so screen readers and tooltips receive the translated text.
In `@src/main/webapp/app/iris/overview/course-iris/course-iris.component.scss`:
- Around line 1-8: The :host selector contains two conflicting min-height
declarations (min-height: 100% and min-height: 0); remove the incorrect/unused
one so only the intended value remains (likely keep min-height: 0 for flex
behavior) by editing the :host block in course-iris.component.scss to delete the
duplicate min-height declaration.
In `@src/main/webapp/content/scss/themes/_dark-variables.scss`:
- Around line 633-640: The iris theme variables in _dark-variables.scss (e.g.,
$iris-primary-background, $iris-secondary-background, $iris-tertiary-background,
$iris-accent-background, $iris-btn-primary, $iris-label-color,
$iris-accent-border-color) incorrectly reference non-existent CSS custom
properties via var(--...), causing runtime styling failures; fix this by either
(preferred) replacing each var(...) usage with the corresponding SCSS variable
(for example use $neutral-dark, $neutral-dark-l-5, $neutral-dark-l-10,
$neutral-dark-l-15, $primary, $gray-500) or alternatively ensure those CSS
custom properties are defined in the theme :root; update the lines that assign
the $iris-* variables accordingly so they reference existing symbols in the SCSS
variable set.
In `@src/main/webapp/content/scss/themes/_default-variables.scss`:
- Around line 568-574: The SCSS variables $iris-secondary-background,
$iris-tertiary-background, $iris-accent-background, $iris-accent-border-color,
$iris-label-color and $iris-btn-primary are referencing undefined CSS custom
properties; update them to use the existing SCSS tokens instead (e.g. replace
var(--gray-100)/var(--gray-200)/var(--gray-300)/var(--gray-500)/var(--gray-600)
with the corresponding $gray-100/$gray-200/$gray-300/$gray-500/$gray-600 tokens
and replace var(--primary) with the existing $primary token) so the variables
resolve correctly in _default-variables.scss.
🧹 Nitpick comments (7)
src/main/webapp/app/iris/overview/exercise-chatbot/exercise-chatbot-button.component.html (1)
12-13: Remove commented-out code or restore functionality.The commented-out translation span appears to be dead code. If the "see full" translation is no longer needed, remove the comment entirely. If it's intended to be restored, consider creating a tracking issue.
♻️ Proposed fix
`@if` (isOverflowing()) { <div class="read-more"> - <!-- <span jhiTranslate="artemisApp.iris.chatBubble.seeFull"></span> --> <fa-icon [icon]="faArrowRight" class="read-more-icon" /> </div> }src/main/webapp/app/iris/overview/course-chatbot/course-chatbot.component.scss (1)
8-9: Remove commented-out code.The commented border line should be removed rather than left as dead code. If this was an intentional design change for the Iris UI update, keeping the comment adds noise without value.
♻️ Proposed fix
- // border: 1px solid var(--border-color); border-radius: 8px;src/main/webapp/app/iris/overview/base-chatbot/chat-status-bar/chat-status-bar.component.ts (1)
30-31: Remove unusedopenTimeoutvariable.The
openTimeoutvariable is declared and cleared in cleanup (line 80) but never assigned. Since the immediate closure behavior removed its usage, this is now dead code.♻️ Proposed fix
// Timeouts are not signals since they're not used in the template - private openTimeout: ReturnType<typeof setTimeout> | undefined; private styleTimeout: ReturnType<typeof setTimeout> | undefined;Also update the cleanup:
onCleanup(() => { - clearTimeout(this.openTimeout); clearTimeout(this.styleTimeout); });And remove the now-unnecessary clear at line 54:
if (firstUnfinished) { - clearTimeout(this.openTimeout); clearTimeout(this.styleTimeout);src/main/webapp/app/iris/overview/base-chatbot/chat-history-item/chat-history-item.component.ts (1)
20-20: Consider maintainability for additional language support.The hardcoded
NEW_CHAT_TITLESset couples this component to specific translation strings. If additional languages are added (beyond English and German), this set will require updates. Consider documenting this coupling or extracting these values to a shared constant if they're used elsewhere.src/main/webapp/app/iris/overview/course-iris/course-iris.component.ts (1)
19-28: PreferundefinedovernullforcourseIdParam.Line 19 sets
initialValue: null; map the param toundefinedto keep the signal type consistent and avoidnull. As per coding guidelines, preferundefinedovernull.🔧 Suggested update
- private readonly courseIdParam = toSignal((this.route.parent?.paramMap ?? of(convertToParamMap({}))).pipe(map((params) => params.get('courseId'))), { initialValue: null }); + private readonly courseIdParam = toSignal( + (this.route.parent?.paramMap ?? of(convertToParamMap({}))).pipe(map((params) => params.get('courseId') ?? undefined)), + { initialValue: undefined }, + );src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.html (1)
16-75: Precompute session buckets instead of calling helpers repeatedly in the template.Lines 26–66 call
getSessionsBetween(...)multiple times per change detection; consider computed signals (e.g.,todaySessions,yesterdaySessions, etc.) to avoid repeated filtering/date math.src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.ts (1)
71-71: Avoid relying on localized titles for “new chat” detection.Line 71 and Lines 533–575 hard-code EN/DE titles; other locales or backend title changes will misclassify sessions. Prefer a stable flag/enum on
IrisSessionDTO(or a backend-provided marker).Also applies to: 533-575
src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.html
Show resolved
Hide resolved
src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.html
Show resolved
Hide resolved
src/main/webapp/app/iris/overview/course-iris/course-iris.component.scss
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
|
@thynguyentumde Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.html`:
- Around line 201-205: The template renders user-provided text lines via
[innerHTML]="line" (in the iris-base-chatbot component loop over (content | as:
IrisTextMessageContent).textContent.split('\n')), which risks XSS; change it to
use safe text interpolation (replace [innerHTML]="line" with {{ line }} so
Angular escapes user text) or, if you intend to allow HTML from user messages,
apply the same sanitization pipe used for LLM messages (e.g., [innerHTML]="line
| htmlForMarkdown") to keep treatment consistent with LLM outputs.
🧹 Nitpick comments (4)
src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.html (4)
96-114: Remove commented-out code before merging.This block contains ~20 lines of commented HTML that clutters the template. If this functionality may be needed later, consider tracking it in an issue rather than leaving it in the codebase.
178-186: Usetrack $indexfor suggestion strings to avoid potential duplicate key warnings.Line 178 uses
track suggestionwheresuggestionis a string value. If two suggestions happen to have identical text, Angular will produce NG0955 warnings due to duplicate track keys.🔧 Suggested fix
- `@for` (suggestion of suggestions(); track suggestion) { + `@for` (suggestion of suggestions(); track $index) {
301-324: Extract duplicated disabled logic to a computed property.The same complex disabled condition appears in both the textarea (lines 301-306) and send button (lines 318-324). This duplication makes maintenance error-prone.
Consider extracting to a computed signal in the component:
readonly isInputDisabled = computed(() => this.isLoading() || !this.active() || (this.rateLimitInfo()?.rateLimit && this.rateLimitInfo()?.currentMessageCount === this.rateLimitInfo()?.rateLimit) || this.hasActiveStage() );Then in template:
[disabled]="isInputDisabled()"
16-74: Convert session grouping methods to computed signals to avoid repeated evaluation on every change detection cycle.The template calls
getNewChatSessions()andgetSessionsBetween()multiple times. While@letprevents repetition within a single template execution, it does not cache results across change detection cycles—these methods re-execute on every update. Converting tocomputedsignals will memoize results until the underlying data (chatSessions()orsearchValue()) actually changes.Example:
readonly newChatSessions = computed(() => this.getNewChatSessions()); readonly todaySessions = computed(() => this.getSessionsBetween(0, 0)); readonly yesterdaySessions = computed(() => this.getSessionsBetween(1, 1)); // ... etcThen use these computed signals directly in the template instead of calling the methods.
| <span class="bubble-right"> | ||
| @for (line of (content | as: IrisTextMessageContent).textContent.split('\n'); track $index) { | ||
| <div [innerHTML]="line"></div> | ||
| } | ||
| </span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for textContent usage in iris directory
rg -n "textContent" "src/main/webapp/app/iris/" -A 2 -B 2 | head -100Repository: ls1intum/Artemis
Length of output: 11371
🏁 Script executed:
# Also check the component TypeScript file
fd "iris-base-chatbot.component.ts" "src/main/webapp/app/iris/"Repository: ls1intum/Artemis
Length of output: 137
🏁 Script executed:
# Look at the surrounding context in the HTML file around line 203
sed -n '195,215p' "src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.html" | cat -nRepository: ls1intum/Artemis
Length of output: 1806
🏁 Script executed:
# Find the htmlForMarkdown pipe definition
fd "htmlForMarkdown" "src/main/webapp/"Repository: ls1intum/Artemis
Length of output: 42
🏁 Script executed:
# Search for the pipe implementation
rg -n "htmlForMarkdown" "src/main/webapp/app/iris/" -B 5 -A 10 | head -80Repository: ls1intum/Artemis
Length of output: 5077
🏁 Script executed:
# Check if there are any comments or documentation about security considerations
rg -n "innerHTML|sanitize|XSS" "src/main/webapp/app/iris/overview/base-chatbot/" -B 2 -A 2Repository: ls1intum/Artemis
Length of output: 2494
🏁 Script executed:
# Find htmlForMarkdown pipe in shared pipes or common locations
find "src/main/webapp" -name "*.ts" -type f | xargs grep -l "htmlForMarkdown" | grep -i pipeRepository: ls1intum/Artemis
Length of output: 118
🏁 Script executed:
# Search for the pipe declaration or export
rg -n "htmlForMarkdown|HtmlForMarkdown" "src/main/webapp" -t ts -B 3 -A 15 | head -120Repository: ls1intum/Artemis
Length of output: 12855
🏁 Script executed:
# Read the htmlForMarkdown pipe implementation
cat -n "src/main/webapp/app/shared/pipes/html-for-markdown.pipe.ts"Repository: ls1intum/Artemis
Length of output: 1531
🏁 Script executed:
# Find the ArtemisMarkdownService to understand what safeHtmlForMarkdown does
rg -n "safeHtmlForMarkdown" "src/main/webapp/app/shared/service/markdown.service.ts" -A 20 | head -60Repository: ls1intum/Artemis
Length of output: 1232
🏁 Script executed:
# Also check if there's any documentation or comments about user input handling
cat -n "src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.ts" | sed -n '265,280p'Repository: ls1intum/Artemis
Length of output: 972
🏁 Script executed:
# Check if user messages ever need HTML rendering or if they should be plain text
rg -n "IrisTextMessageContent|textContent" "src/main/webapp/app/iris/" -B 3 -A 3 | grep -A 5 -B 5 "USER"Repository: ls1intum/Artemis
Length of output: 998
🏁 Script executed:
# Check the component imports to see if htmlForMarkdown is available
sed -n '1,50p' "src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.ts" | cat -nRepository: ls1intum/Artemis
Length of output: 2990
🏁 Script executed:
# Check if the user can input arbitrary HTML or if it's controlled
rg -n "textContent|IrisUserMessage" "src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.ts" -B 2 -A 5 | grep -A 8 -B 8 "sendMessage"Repository: ls1intum/Artemis
Length of output: 860
🏁 Script executed:
# Check how messages are sent from the client
fd "iris-chat.service.ts" "src/main/webapp/app/iris/" -exec cat -n {} \; | sed -n '1,100p'Repository: ls1intum/Artemis
Length of output: 290
🏁 Script executed:
# Check the iris-chat.service.ts file
cat "src/main/webapp/app/iris/overview/services/iris-chat.service.ts" | head -150Repository: ls1intum/Artemis
Length of output: 6895
🏁 Script executed:
# Check if there's any HTML sanitization on the client side when processing user input
rg -n "DomSanitizer|sanitize" "src/main/webapp/app/iris/" -B 2 -A 2Repository: ls1intum/Artemis
Length of output: 42
Apply sanitization or text interpolation for user message content on line 203.
User-provided textContent is rendered directly via [innerHTML] without the htmlForMarkdown sanitization pipe applied to LLM messages (line 213). This creates an inconsistency in security treatment and exposes user input to potential XSS if malicious HTML is injected.
Since user messages are plain text, use text interpolation instead:
- <div [innerHTML]="line"></div>
+ <div>{{ line }}</div>Alternatively, if HTML formatting is intentional, apply the same htmlForMarkdown pipe used for LLM messages.
📝 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.
| <span class="bubble-right"> | |
| @for (line of (content | as: IrisTextMessageContent).textContent.split('\n'); track $index) { | |
| <div [innerHTML]="line"></div> | |
| } | |
| </span> | |
| <span class="bubble-right"> | |
| `@for` (line of (content | as: IrisTextMessageContent).textContent.split('\n'); track $index) { | |
| <div>{{ line }}</div> | |
| } | |
| </span> |
🤖 Prompt for AI Agents
In
`@src/main/webapp/app/iris/overview/base-chatbot/iris-base-chatbot.component.html`
around lines 201 - 205, The template renders user-provided text lines via
[innerHTML]="line" (in the iris-base-chatbot component loop over (content | as:
IrisTextMessageContent).textContent.split('\n')), which risks XSS; change it to
use safe text interpolation (replace [innerHTML]="line" with {{ line }} so
Angular escapes user text) or, if you intend to allow HTML from user messages,
apply the same sanitization pipe used for LLM messages (e.g., [innerHTML]="line
| htmlForMarkdown") to keep treatment consistent with LLM outputs.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
|
@thynguyentumde Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
MaximilianAnzinger
left a comment
MaximilianAnzinger
left a comment
There was a problem hiding this comment.
science endpoint related changes look good. Minor remarks by Codacy should be considered.
| flex: 1; | ||
| min-height: 0; /* allow child to shrink inside flex container */ | ||
| min-height: 0; | ||
| /* allow child to shrink inside flex container */ |
There was a problem hiding this comment.
Codacy comment
| .messages-container { | ||
| max-height: calc(90vh - 11.25rem); /* account for header and footer on mobile */ | ||
| max-height: calc(90vh - 11.25rem); | ||
| /* account for header and footer on mobile */ |
There was a problem hiding this comment.
codacy comment
Summary
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
Improved the Iris UI layout and behavior in course and widget contexts, ensuring consistent sizing, alignment, and usability across different pages and view modes. Ensured the color contrast for the Iris client and chatbot interface. Added a dedicated sidebar entry point for the Iris so that students can easily find Iris from the global sidebar.
Description
iris_entrypoint_impressionandiris_openedto track the open rate students click on "Iris" from the global sidebar.Steps for Testing
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
Performance Tests
Test Coverage
Warning: Client tests failed. Coverage could not be fully measured. Please check the workflow logs.
Server
Last updated: 2026-01-27 14:07:45 UTC
Screenshots
FIX IRIS-80, IRIS-51, IRIS-9
Summary by CodeRabbit
New Features
Improvements
Tests
Translations
✏️ Tip: You can customize this high-level summary in your review settings.