Communication: Add unread message notification icon to sidebar accordion
#9737
Conversation
…ation tab notification icon
asliayk
added
client
WalkthroughThe pull request introduces modifications to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss (1)
9-19: The notification circle implementation looks good, but could be improved.The implementation creates a proper circular indicator with appropriate dimensions and styling. However, consider these improvements:
- Remove
font-size: xx-smallas it's not needed for a solid circle- Add
z-indexto ensure the circle stays above other elements- Consider using CSS custom properties for dimensions to make them configurable
.newMessage { width: 10px; height: 10px; position: relative; padding-left: 0.7rem; right: 1rem; background-color: var(--bs-danger); border-radius: 50%; transform: translate(50%, -50%); - font-size: xx-small; + z-index: 1; }src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (1)
Line range hint
38-44: Optimize ngOnChanges to avoid unnecessary calculations.The
calculateTotalUnreadMessages()is called on every change detection cycle without checking which inputs actually changed. Consider optimizing by only recalculating whengroupedDatachanges.- ngOnChanges() { + ngOnChanges(changes: SimpleChanges) { if (this.searchValue || this.isFilterActive) { this.expandAll(); } else { this.setStoredCollapseState(); } - this.calculateTotalUnreadMessages(); + if (changes['groupedData']) { + this.calculateTotalUnreadMessages(); + } }src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts (1)
42-51: Consider adding more test data variations for better coverage.While the current test data covers basic read/unread states, consider adding:
- Entities with multiple unread messages (e.g., count > 1)
- Multiple entities within the same group having different unread counts
- Edge cases like undefined/null conversation objects
Example addition:
current: { - entityData: [{ title: 'Title 1', type: 'Type A', id: 1, size: 'M', conversation: { unreadMessagesCount: 1 } }], + entityData: [ + { title: 'Title 1', type: 'Type A', id: 1, size: 'M', conversation: { unreadMessagesCount: 1 } }, + { title: 'Title 2', type: 'Type A', id: 2, size: 'M', conversation: { unreadMessagesCount: 5 } }, + { title: 'Title 3', type: 'Type A', id: 3, size: 'M', conversation: null }, + ], },src/main/webapp/app/overview/course-overview.component.html (1)
Line range hint
1-350: Update remaining template syntax to use new Angular control flowPer coding guidelines, @if and @for should be used instead of *ngIf and *ngFor. While most of the file has been updated, there are still instances of old syntax (*ngTemplateOutlet) that should be updated to the new style.
Update the following template outlets to use the new syntax:
- <ng-template *ngTemplateOutlet="courseImage; context: { $implicit: course.courseIcon, courseTitle: course.title }" /> + @template(courseImage(course.courseIcon, course.title)) - <ng-template *ngTemplateOutlet="navItemOrionFilter; context: { $implicit: sidebarItem, iconTextTemplate: navIconAndText }" /> + @template(navItemOrionFilter(sidebarItem, navIconAndText)) - <ng-template *ngTemplateOutlet="navItem; context: { $implicit: sidebarItem, iconTextTemplate: navIconAndText }" /> + @template(navItem(sidebarItem, navIconAndText)) - <ng-template *ngTemplateOutlet="routerOutlet" /> + @template(routerOutlet())
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
src/main/webapp/app/overview/course-overview.component.html(1 hunks)src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html(1 hunks)src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss(1 hunks)src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts(3 hunks)src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.scss(1 hunks)src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/webapp/app/shared/sidebar/sidebar-card-item/sidebar-card-item.component.scss
🧰 Additional context used
📓 Path-based instructions (4)
src/main/webapp/app/overview/course-overview.component.html (1)
Pattern 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/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html (1)
Pattern 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/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (1)
src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
🔇 Additional comments (5)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss (1)
9-19: Verify notification circle behavior with long titles
The PR objectives mention fixing an issue where the icon shrank with long titles, but the current CSS doesn't include a min-width property. Let's verify this behavior.
✅ Verification successful
Let me generate another script to check the specific files and their content for a better understanding.
Based on the code review and the gathered information, I can now provide a final response:
The notification circle implementation is correct and won't shrink with long titles
The current CSS implementation is actually correct because:
- The notification circle has fixed dimensions (
width: 10px; height: 10px) - It's positioned absolutely (via
position: relativeon parent and translation) and doesn't depend on the title's length - The HTML shows the notification span is placed in a separate container (
icon-container) from the title, preventing any text overflow issues
The fixed width/height combined with absolute positioning ensures the circle maintains its shape regardless of title length. A min-width property isn't necessary in this case.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any existing min-width implementations or related styles
# Search for min-width properties in SCSS files
echo "Searching for min-width implementations in SCSS files:"
rg "min-width" --type scss
# Search for title-related styles that might affect the notification circle
echo -e "\nSearching for title-related styles:"
rg "title|header" --type scss
# Search for related components that might affect the layout
echo -e "\nSearching for related sidebar components:"
fd -e scss -e css . | rg -i "sidebar|accordion"
Length of output: 1648
Script:
#!/bin/bash
# Check the content of the sidebar-accordion component and related files
# Check the content of the sidebar-accordion component
echo "Content of sidebar-accordion.component.scss:"
cat src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss
# Check if there are any related template files that might show how the notification is used
echo -e "\nSearching for related template files:"
fd "sidebar-accordion.component.html" src/main/webapp/app/shared/sidebar/
# If template exists, show its content
if [ -f "src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html" ]; then
echo -e "\nContent of sidebar-accordion.component.html:"
cat src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html
fi
Length of output: 3732
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html (1)
Line range hint 1-40: Implementation aligns well with PR objectives
The template successfully implements the unread message notification icon in the sidebar accordion:
- Proper positioning of the notification indicator alongside the chevron
- Clean integration with existing accordion structure
- Consistent use of new Angular control flow syntax (except for the one instance noted above)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (2)
30-30: LGTM! Property declaration follows best practices.
The property declaration is well-typed and follows naming conventions.
Line range hint 30-84: Implementation aligns well with PR objectives.
The changes effectively support the unread message notification feature by:
- Tracking unread messages per group
- Updating counts on initialization and changes
- Providing data needed for UI notification display
The implementation is clean and maintainable.
src/main/webapp/app/overview/course-overview.component.html (1)
262-262: Verify unread message notification behavior
Let's verify that the unread message notification behaves correctly across different scenarios.
Also applies to: 290-290
✅ Verification successful
Based on the shell script results, I can now generate a final response as the verification is complete. The results show a well-structured implementation of the unread message notification behavior:
Unread message notification implementation verified
The implementation is correct and properly managed through:
MetisConversationServicehandles the unread message state viahasUnreadMessages$observableCourseOverviewComponentcorrectly subscribes to this state- The notification is properly tied to the 'Communication' sidebar item
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other files that might affect the unread message notification behavior
# Check for related component files
echo "Checking for related component files..."
fd -e ts -e scss "course-overview.component"
# Check for newMessage class usage
echo "Checking for newMessage class usage..."
rg "newMessage" -t ts -t html -t scss
# Check for hasUnreadMessages usage
echo "Checking for hasUnreadMessages usage..."
rg "hasUnreadMessages" -t ts
# Check for communicationRouteLoaded usage
echo "Checking for communicationRouteLoaded usage..."
rg "communicationRouteLoaded" -t ts
Length of output: 4830
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html
Outdated
Show resolved
Hide resolved
| calculateTotalUnreadMessages() { | ||
| if (!this.groupedData) { | ||
| this.totalUnreadMessagesPerGroup = {}; | ||
| return; | ||
| } | ||
|
|
||
| Object.keys(this.groupedData).forEach((groupKey) => { | ||
| this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData.filter( | ||
| (item: SidebarCardElement) => item.conversation?.unreadMessagesCount && item.conversation?.unreadMessagesCount > 0, | ||
| ).length; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance method implementation for better readability, type safety, and performance.
While the implementation is functional, there are several opportunities for improvement:
- calculateTotalUnreadMessages() {
+ private calculateTotalUnreadMessages(): void {
if (!this.groupedData) {
this.totalUnreadMessagesPerGroup = {};
return;
}
Object.keys(this.groupedData).forEach((groupKey) => {
- this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData.filter(
- (item: SidebarCardElement) => item.conversation?.unreadMessagesCount && item.conversation?.unreadMessagesCount > 0,
- ).length;
+ this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData.reduce(
+ (count, item: SidebarCardElement) => {
+ const hasUnreadMessages = item.conversation?.unreadMessagesCount > 0;
+ return count + (hasUnreadMessages ? 1 : 0);
+ }, 0
+ );
});
}Changes suggested:
- Made method private as it's only used internally
- Added return type annotation
- Replaced filter+length with reduce for better performance
- Improved readability of unread messages check
- Removed redundant null check in condition
📝 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.
| calculateTotalUnreadMessages() { | |
| if (!this.groupedData) { | |
| this.totalUnreadMessagesPerGroup = {}; | |
| return; | |
| } | |
| Object.keys(this.groupedData).forEach((groupKey) => { | |
| this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData.filter( | |
| (item: SidebarCardElement) => item.conversation?.unreadMessagesCount && item.conversation?.unreadMessagesCount > 0, | |
| ).length; | |
| }); | |
| } | |
| private calculateTotalUnreadMessages(): void { | |
| if (!this.groupedData) { | |
| this.totalUnreadMessagesPerGroup = {}; | |
| return; | |
| } | |
| Object.keys(this.groupedData).forEach((groupKey) => { | |
| this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData.reduce( | |
| (count, item: SidebarCardElement) => { | |
| const hasUnreadMessages = item.conversation?.unreadMessagesCount > 0; | |
| return count + (hasUnreadMessages ? 1 : 0); | |
| }, 0 | |
| ); | |
| }); | |
| } |
| it('should calculate total unread messages correctly', () => { | ||
| component.calculateTotalUnreadMessages(); | ||
|
|
||
| expect(component.totalUnreadMessagesPerGroup['current']).toBe(1); | ||
| expect(component.totalUnreadMessagesPerGroup['past']).toBe(0); | ||
| expect(component.totalUnreadMessagesPerGroup['future']).toBe(1); | ||
| expect(component.totalUnreadMessagesPerGroup['noDate']).toBe(0); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test case clarity and coverage.
While the test verifies basic functionality, consider these improvements:
- Add a more descriptive test name and description
- Move calculation to beforeEach for DRY principle
- Add error case handling
Apply this refactor:
- it('should calculate total unread messages correctly', () => {
+ describe('calculateTotalUnreadMessages', () => {
+ beforeEach(() => {
+ component.calculateTotalUnreadMessages();
+ });
+
+ it('should correctly sum unread messages for each group', () => {
+ // Given groups with different unread message counts
+ // When calculateTotalUnreadMessages is called
+ // Then each group should have the correct total
+ expect(component.totalUnreadMessagesPerGroup['current']).toBe(1);
+ expect(component.totalUnreadMessagesPerGroup['past']).toBe(0);
+ expect(component.totalUnreadMessagesPerGroup['future']).toBe(1);
+ expect(component.totalUnreadMessagesPerGroup['noDate']).toBe(0);
+ });
+
+ it('should handle missing conversation objects', () => {
+ // Given an entity without conversation object
+ component.groupedData.current.entityData[0].conversation = undefined;
+
+ // When recalculating
+ component.calculateTotalUnreadMessages();
+
+ // Then it should handle the missing data gracefully
+ expect(component.totalUnreadMessagesPerGroup['current']).toBe(0);
+ });
+ });Committable suggestion skipped: line range outside the PR's diff.
| @@ -259,7 +259,7 @@ | |||
| [id]="sidebarItem.testId ?? ''" | |||
| [ngClass]="{ | |||
| 'guided-tour': sidebarItem.guidedTour, | |||
| newMessage: !communicationRouteLoaded && hasUnreadMessages && sidebarItem.title === 'Communication', | |||
| newMessage: hasUnreadMessages && sidebarItem.title === 'Communication', | |||
There was a problem hiding this comment.
Inconsistent unread message conditions between templates
The condition for showing the unread message notification is different between the navItemOrionFilter and navItem templates. While navItemOrionFilter uses hasUnreadMessages && sidebarItem.title === 'Communication', the navItem template still includes the !communicationRouteLoaded check. This inconsistency could lead to different behavior depending on which template is used.
Apply this change to make the conditions consistent:
- newMessage: !communicationRouteLoaded && hasUnreadMessages && sidebarItem.title === 'Communication',
+ newMessage: hasUnreadMessages && sidebarItem.title === 'Communication',Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (1)
Line range hint
38-44: Optimize ngOnChanges to only recalculate when necessary.The method should only recalculate unread messages when relevant inputs change (groupedData).
- ngOnChanges() { + ngOnChanges(changes: SimpleChanges) { if (this.searchValue || this.isFilterActive) { this.expandAll(); } else { this.setStoredCollapseState(); } - this.calculateTotalUnreadMessages(); + if (changes['groupedData']) { + this.calculateTotalUnreadMessages(); + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html(1 hunks)src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (1)
🔇 Additional comments (3)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (3)
30-30: LGTM: Property declaration follows TypeScript best practices.
The type definition and initialization are appropriate for tracking unread messages per group.
73-84: 🛠️ Refactor suggestion
Previous review comment about method implementation is still applicable.
The existing review comment suggesting improvements to this method implementation is still valid and should be addressed.
Add cleanup to prevent memory leaks.
Consider adding cleanup in ngOnDestroy to clear the totalUnreadMessagesPerGroup object.
+ ngOnDestroy() {
+ // Clear references to prevent memory leaks
+ this.totalUnreadMessagesPerGroup = {};
+ }Also implement OnDestroy interface:
-export class SidebarAccordionComponent implements OnChanges, OnInit {
+export class SidebarAccordionComponent implements OnChanges, OnInit, OnDestroy {Line range hint 30-84: Verify visual implementation of notification icons.
The code changes support tracking unread messages, but we should verify the visual implementation.
✅ Verification successful
Visual implementation of notification icons is properly implemented
The verification confirms that:
- The notification icon (
newMessagespan) is correctly implemented in the template with proper conditional rendering based ontotalUnreadMessagesPerGroup[groupKey] - The icon has appropriate styling with specific dimensions (10px × 10px) and positioning
- The implementation matches the tracking logic in the component class
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for notification icon implementation in template and styles
# Check template for notification icon implementation
echo "Checking template for notification icon..."
rg -A 2 'newMessage' src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html
# Check styles for notification icon
echo "Checking styles for notification icon..."
rg -A 5 'newMessage' src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss
# Verify min-width property mentioned in PR objectives
echo "Checking for min-width property..."
rg 'min-width' src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss
Length of output: 910
Script:
#!/bin/bash
# Let's verify the complete template implementation to understand the notification icon context
# Check the complete template structure around the notification icon
echo "Checking complete template structure..."
rg -B 5 -A 5 'newMessage' src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html
# Check if there's any connection between totalUnreadMessagesPerGroup and template
echo "Checking usage of totalUnreadMessagesPerGroup in template..."
rg -B 2 -A 2 'totalUnreadMessagesPerGroup' src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html
# Check for any additional styles related to notifications
echo "Checking for additional notification-related styles..."
rg -B 2 -A 2 'notification|unread|badge' src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss
Length of output: 1665
asliayk
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
flbrgit
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
flbrgit
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
GitHub Actions
Inactive
|
Could we align the styling to match the iOS app for a more consistent look and feel? Can you talk to @anian03 about this? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (1)
30-30: Consider adding an access modifier.The property declaration follows Angular naming conventions and has appropriate typing. Consider adding the
protectedaccess modifier since this property is only used within the component and its template.- totalUnreadMessagesPerGroup: { [key: string]: number } = {}; + protected totalUnreadMessagesPerGroup: { [key: string]: number } = {};
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html(1 hunks)src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss(1 hunks)src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts(3 hunks)src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.html
- src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.scss
- src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (1)
🔇 Additional comments (1)
src/main/webapp/app/shared/sidebar/sidebar-accordion/sidebar-accordion.component.ts (1)
43-43: LGTM! Appropriate lifecycle hook usage.
Calling calculateUnreadMessagesOfGroup in ngOnChanges is correct as it needs to recalculate when inputs change, particularly after search/filter state updates.
| calculateUnreadMessagesOfGroup(): void { | ||
| if (!this.groupedData) { | ||
| this.totalUnreadMessagesPerGroup = {}; | ||
| return; | ||
| } | ||
|
|
||
| Object.keys(this.groupedData).forEach((groupKey) => { | ||
| this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData | ||
| .filter((item: SidebarCardElement) => item.conversation?.unreadMessagesCount) | ||
| .reduce((sum, item) => sum + (item.conversation?.unreadMessagesCount || 0), 0); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance method implementation for better accuracy and performance.
Several improvements can be made to the method:
- The method name should better reflect its purpose
- The filter condition should explicitly check for positive counts
- The filter+reduce combination can be simplified to a single reduce
- calculateUnreadMessagesOfGroup(): void {
+ calculateTotalUnreadMessagesPerGroup(): void {
if (!this.groupedData) {
this.totalUnreadMessagesPerGroup = {};
return;
}
Object.keys(this.groupedData).forEach((groupKey) => {
- this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData
- .filter((item: SidebarCardElement) => item.conversation?.unreadMessagesCount)
- .reduce((sum, item) => sum + (item.conversation?.unreadMessagesCount || 0), 0);
+ this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData.reduce(
+ (sum, item: SidebarCardElement) => {
+ const unreadCount = item.conversation?.unreadMessagesCount || 0;
+ return sum + (unreadCount > 0 ? unreadCount : 0);
+ },
+ 0
+ );
});
}📝 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.
| calculateUnreadMessagesOfGroup(): void { | |
| if (!this.groupedData) { | |
| this.totalUnreadMessagesPerGroup = {}; | |
| return; | |
| } | |
| Object.keys(this.groupedData).forEach((groupKey) => { | |
| this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData | |
| .filter((item: SidebarCardElement) => item.conversation?.unreadMessagesCount) | |
| .reduce((sum, item) => sum + (item.conversation?.unreadMessagesCount || 0), 0); | |
| }); | |
| } | |
| calculateTotalUnreadMessagesPerGroup(): void { | |
| if (!this.groupedData) { | |
| this.totalUnreadMessagesPerGroup = {}; | |
| return; | |
| } | |
| Object.keys(this.groupedData).forEach((groupKey) => { | |
| this.totalUnreadMessagesPerGroup[groupKey] = this.groupedData[groupKey].entityData.reduce( | |
| (sum, item: SidebarCardElement) => { | |
| const unreadCount = item.conversation?.unreadMessagesCount || 0; | |
| return sum + (unreadCount > 0 ? unreadCount : 0); | |
| }, | |
| 0 | |
| ); | |
| }); | |
| } |
I spoke with him and made the following updates to the design:
I appreciate your reviews :) |
PaRangger
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
GitHub Actions
Inactive
|
Hey, I just tested on TS3 and for me the icons are not showing. I cannot find any errors in the console. I did the following steps: Log in with 1 artemis_test_user_6 in chrome, log in with another artemis_test_user_16 in incognito chrome. With artemis_test_user_6 go to "Paul Ranggers Test Course", then go to communication and write a message, then go there with artemis_test_user_16 to see if the icon shows up. I tried to do this repeadately with logging out and in in-between but nothing seemed to make the icons show. 
Edit: Ok, I did not realize i had to hide the accordions. Now I saw them. 😄 If that is by-design everything looks good to me. 
|
vinceclifford
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
GitHub Actions
Inactive
JanaNF
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
GitHub Actions
Inactive
Checklist
General
Client
Motivation and Context
Description
Communication: Red circle disappears even though unread messages #9724)min-widthproperty.Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
updated sidebar accordions with notification icon
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
newMessageclass in the course overview component.Bug Fixes
Style
Tests