General: Add sidebar for exercise and lecture view
#8246
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
tests
client
WalkthroughThe recent updates encompass a comprehensive overhaul of the Angular application, focusing on enhancing the course exercises and lectures components, introducing a refined sidebar functionality with new components and services, and improving the user interface and interaction. These changes streamline data handling, improve navigation, and enrich the visual presentation, ensuring a more organized and user-friendly experience. Additionally, the introduction of new components and services fortifies the application's architecture, making it more robust and maintainable. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
1
to
23
| import { ComponentFixture, TestBed, async } from '@angular/core/testing'; | ||
| import { SidebarComponent } from './sidebar.component'; | ||
|
|
||
| describe('SidebarComponent', () => { | ||
| let component: SidebarComponent; | ||
| let fixture: ComponentFixture<SidebarComponent>; | ||
|
|
||
| beforeEach(async(() => { | ||
| TestBed.configureTestingModule({ | ||
| declarations: [SidebarComponent], | ||
| }).compileComponents(); | ||
| })); | ||
|
|
||
| beforeEach(() => { | ||
| fixture = TestBed.createComponent(SidebarComponent); | ||
| component = fixture.componentInstance; | ||
| fixture.detectChanges(); | ||
| }); | ||
|
|
||
| it('should create', () => { | ||
| expect(component).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test setup for SidebarComponent follows Angular's testing conventions well, initializing the component and testing module correctly. However, consider adding more test cases to cover the component's functionality thoroughly, such as its interaction with inputs, outputs, and any public methods it may have.
Would you like suggestions on additional test cases to cover?
Comment on lines
7
to
20
| transform(array: any[] | undefined, fields: string[], value: string): any[] { | ||
| if (!Array.isArray(array)) { | ||
| return []; | ||
| } | ||
| return array?.filter((element) => { | ||
| if (!element) { | ||
| return false; | ||
| } | ||
| return fields.some((field) => { | ||
| const elementValue = element[field]; | ||
| return elementValue && typeof elementValue === 'string' ? elementValue.toLowerCase().includes(value.toLowerCase()) : false; | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The implementation of SearchFilterPipe is efficient and follows Angular's guidelines for creating pipes. However, consider adding type annotations for the parameters and return type of the transform method to improve code readability and type safety.
- transform(array: any[] | undefined, fields: string[], value: string): any[] {
+ transform(array: Array<{[key: string]: any}> | undefined, fields: string[], value: string): Array<{[key: string]: any}> {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.
Suggested change
| transform(array: any[] | undefined, fields: string[], value: string): any[] { | |
| if (!Array.isArray(array)) { | |
| return []; | |
| } | |
| return array?.filter((element) => { | |
| if (!element) { | |
| return false; | |
| } | |
| return fields.some((field) => { | |
| const elementValue = element[field]; | |
| return elementValue && typeof elementValue === 'string' ? elementValue.toLowerCase().includes(value.toLowerCase()) : false; | |
| }); | |
| }); | |
| } | |
| transform(array: Array<{[key: string]: any}> | undefined, fields: string[], value: string): Array<{[key: string]: any}> { | |
| if (!Array.isArray(array)) { | |
| return []; | |
| } | |
| return array?.filter((element) => { | |
| if (!element) { | |
| return false; | |
| } | |
| return fields.some((field) => { | |
| const elementValue = element[field]; | |
| return elementValue && typeof elementValue === 'string' ? elementValue.toLowerCase().includes(value.toLowerCase()) : false; | |
| }); | |
| }); | |
| } |
Comment on lines
1
to
3
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | ||
| import { ArtemisTestModule } from 'src/test/javascript/spec/test.module'; | ||
| import { SidebarAccordionComponent } from './sidebar-accordion.component'; |
There was a problem hiding this comment.
The test suite uses Jest for testing, but there seems to be an issue with recognizing Jest functions such as describe, beforeEach, and afterEach. Ensure that the appropriate type definitions for Jest are installed and configured in the project's tsconfig.spec.json file.
+ // Ensure to include Jest type definitions in your tsconfig.spec.json
+ "types": ["jest"],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.
Suggested change
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | |
| import { ArtemisTestModule } from 'src/test/javascript/spec/test.module'; | |
| import { SidebarAccordionComponent } from './sidebar-accordion.component'; | |
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | |
| import { ArtemisTestModule } from 'src/test/javascript/spec/test.module'; | |
| import { SidebarAccordionComponent } from './sidebar-accordion.component'; |
Comment on lines
1
to
27
| import { Component, EventEmitter, Input, Output } from '@angular/core'; | ||
| import { FormControl, FormGroup } from '@angular/forms'; | ||
| import { faMagnifyingGlass, faTimes } from '@fortawesome/free-solid-svg-icons'; | ||
|
|
||
| @Component({ | ||
| selector: 'jhi-search-filter', | ||
| templateUrl: './search-filter.component.html', | ||
| styleUrls: ['./search-filter.component.scss'], | ||
| }) | ||
| export class SearchFilterComponent { | ||
| @Input() | ||
| searchText: string; | ||
|
|
||
| faMagnifyingGlass = faMagnifyingGlass; | ||
| faTimes = faTimes; | ||
|
|
||
| @Output() newSearchEvent = new EventEmitter<string>(); | ||
|
|
||
| filterForm: FormGroup = new FormGroup({ | ||
| searchFilter: new FormControl<string>(''), | ||
| }); | ||
|
|
||
| setSearchValue(searchValue: string) { | ||
| this.newSearchEvent.emit(searchValue); | ||
| } | ||
| resetSearchValue() { | ||
| this.filterForm.get('searchFilter')?.reset(); | ||
| this.newSearchEvent.emit(''); | ||
| } | ||
| } |
There was a problem hiding this comment.
The SearchFilterComponent is well-implemented, with clear inputs and outputs. The use of FormGroup for managing the search input is a good practice. Consider adding type annotations to the @Input() and @Output() properties for enhanced type safety and readability.
- @Input() searchText: string;
+ @Input() searchText: string = '';
- @Output() newSearchEvent = new EventEmitter<string>();
+ @Output() newSearchEvent: EventEmitter<string> = new EventEmitter<string>();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.
Suggested change
| import { Component, EventEmitter, Input, Output } from '@angular/core'; | |
| import { FormControl, FormGroup } from '@angular/forms'; | |
| import { faMagnifyingGlass, faTimes } from '@fortawesome/free-solid-svg-icons'; | |
| @Component({ | |
| selector: 'jhi-search-filter', | |
| templateUrl: './search-filter.component.html', | |
| styleUrls: ['./search-filter.component.scss'], | |
| }) | |
| export class SearchFilterComponent { | |
| @Input() | |
| searchText: string; | |
| faMagnifyingGlass = faMagnifyingGlass; | |
| faTimes = faTimes; | |
| @Output() newSearchEvent = new EventEmitter<string>(); | |
| filterForm: FormGroup = new FormGroup({ | |
| searchFilter: new FormControl<string>(''), | |
| }); | |
| setSearchValue(searchValue: string) { | |
| this.newSearchEvent.emit(searchValue); | |
| } | |
| resetSearchValue() { | |
| this.filterForm.get('searchFilter')?.reset(); | |
| this.newSearchEvent.emit(''); | |
| } | |
| } | |
| import { Component, EventEmitter, Input, Output } from '@angular/core'; | |
| import { FormControl, FormGroup } from '@angular/forms'; | |
| import { faMagnifyingGlass, faTimes } from '@fortawesome/free-solid-svg-icons'; | |
| @Component({ | |
| selector: 'jhi-search-filter', | |
| templateUrl: './search-filter.component.html', | |
| styleUrls: ['./search-filter.component.scss'], | |
| }) | |
| export class SearchFilterComponent { | |
| @Input() searchText: string = ''; | |
| faMagnifyingGlass = faMagnifyingGlass; | |
| faTimes = faTimes; | |
| @Output() newSearchEvent: EventEmitter<string> = new EventEmitter<string>(); | |
| filterForm: FormGroup = new FormGroup({ | |
| searchFilter: new FormControl<string>(''), | |
| }); | |
| setSearchValue(searchValue: string) { | |
| this.newSearchEvent.emit(searchValue); | |
| } | |
| resetSearchValue() { | |
| this.filterForm.get('searchFilter')?.reset(); | |
| this.newSearchEvent.emit(''); | |
| } | |
| } |
Comment on lines
32
to
37
| path: 'about', | ||
| loadChildren: () => import('./core/about-us/artemis-about-us.module').then((module) => module.ArtemisAboutUsModule), | ||
| }, | ||
| { | ||
| path: 'courses/:courseId/lectures/:lectureId', | ||
| loadChildren: () => import('./overview/course-lectures/course-lecture-details.module').then((m) => m.ArtemisCourseLectureDetailsModule), | ||
| }, | ||
| { | ||
| // TODO: check that the LTI integration still works correctly (if not, we should implement it differently) | ||
| path: 'courses/:courseId/exercises/:exerciseId', | ||
| loadChildren: () => import('./overview/exercise-details/course-exercise-details.module').then((m) => m.CourseExerciseDetailsModule), | ||
| }, | ||
| { | ||
| path: 'courses/:courseId/competencies/:competencyId', | ||
| loadChildren: () => import('./overview/course-competencies/course-competencies-details.module').then((m) => m.ArtemisCourseCompetenciesDetailsModule), |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has revealed that while the new route for course competencies is present in the application's routing configuration, there are still numerous navigation elements that have not been updated to reflect the removal of routes related to course lectures and exercises. This finding supports the concern about ensuring all links and navigation elements are updated accordingly to reflect these routing changes.
Analysis chain
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
The restructuring of the application's routing configuration, including the removal of specific routes related to course lectures and exercises and the addition of a new route for course competencies, reflects significant changes in how content is organized and accessed within the application. It's important to ensure that these changes align with the overall goals for user experience and content management. Additionally, it's crucial to verify that all links and navigation elements within the application have been updated accordingly to reflect these routing changes.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that all navigation elements have been updated to reflect the new routing structure.
rg --type html 'routerLink=".*lectures.*|.*exercises.*"'
# Verify the new route for course competencies is correctly implemented and accessible.
rg --type ts 'loadChildren.*course-competencies-details.module'
Length of output: 29994
milljoniaer
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
bensofficial
requested changes
Mar 25, 2024
milljoniaer
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
GitHub Actions
Inactive
Hialus
requested changes
Mar 25, 2024
There was a problem hiding this comment.
The exercise content can overflow to the right. This happens at 100% scaling for me:
I think the collapsed sidebar should at least show some indication what it is, instead of just being empty:
The Ctrl+X shortcut may override the default "Cut text" action from Windows. This would interfere with the communication feature if you write a message there. Please check this in a windows device. Ctrl+Y of the other sidebar may also override the "Redo" function, but not sure here.
Without collapsing one or both sidebars there is very little space for the actual exercise. Is there a plan for this in place to maybe automatically collapse the sidebars? I think a lot of students would not immediatley notice the small icons for that.
Especially on smaller screens you first have to collapse the sidebars before the page even becomes usable.
On mobile phones the page is effectively completely unusable now:
Even though there is now a mobile app it is not yet fully featured, so sometimes it may be required to access Artemis from a phone of table, so this should be fixed.
Also having the sidebars being completely fixed in position and size is not very responsive in general, as these issues showcase very well.
On the lecture page the "Linked competencies" button is too small for the icon:
When I click on the left sidebar and then press escape it just completely disappears:
When I collapse both sidebars and then reload the page, the left sidebar is expanded again and the right one stays collapsed. I would actually expect both to remember this or having this swapped, as the left sidebar is very usable in a collapsed state, while the right one is not.
rstief
reviewed
Mar 25, 2024
There was a problem hiding this comment.
On this exercise the page shows 5 scrollbars, though this might be a problem with the programming exercise
The Strg + x shortcut also disables the normal function (cut the text)
The color coding for the difficulty is also a bit confusing to me: I thought this meant that it is a new exercise.
Also the build is not running, we should look into that 😅
|
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. |
rabeatwork
force-pushed
the
enhancement/create-card-sidebar-component
branch
from
e4fbe4e
to
53dc399
Compare
Comment on lines
+22
to
+23
| import { Subscription } from 'rxjs'; | ||
| import { ProfileService } from 'app/shared/layouts/profiles/profile.service'; |
There was a problem hiding this comment.
Consider importing takeUntil from rxjs/operators and using it in your subscriptions to ensure they are properly unsubscribed when the component is destroyed, preventing potential memory leaks.
import { finalize, tap, takeUntil } from 'rxjs/operators';Then, in your subscriptions, you can add:
this.profileService.getProfileInfo()?.subscribe((profileInfo) => {
this.isProduction = profileInfo.inProduction;
this.isTestServer = profileInfo.testServer ?? false;
+ }).pipe(takeUntil(this.destroy$)).subscribe();Remember to initialize destroy$ as a Subject and call this.destroy$.next() in ngOnDestroy.
Comment on lines
81
to
85
| this.profileService.getProfileInfo()?.subscribe((profileInfo) => { | ||
| this.isProduction = profileInfo.inProduction; | ||
| this.isTestServer = profileInfo.testServer ?? false; | ||
| }); |
There was a problem hiding this comment.
Ensure localization for user-facing strings to support internationalization.
this.profileService.getProfileInfo()?.subscribe((profileInfo) => {
- this.isProduction = profileInfo.inProduction;
- this.isTestServer = profileInfo.testServer ?? false;
+ this.isProduction = this.translateService.instant(profileInfo.inProduction);
+ this.isTestServer = this.translateService.instant(profileInfo.testServer ?? false);
});You'll need to inject TranslateService from @ngx-translate/core in the constructor.
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.
Suggested change
| this.profileService.getProfileInfo()?.subscribe((profileInfo) => { | |
| this.isProduction = profileInfo.inProduction; | |
| this.isTestServer = profileInfo.testServer ?? false; | |
| }); | |
| this.profileService.getProfileInfo()?.subscribe((profileInfo) => { | |
| this.isProduction = this.translateService.instant(profileInfo.inProduction); | |
| this.isTestServer = this.translateService.instant(profileInfo.testServer ?? false); | |
| }); |
Comment on lines
+212
to
+214
| if (this.isCommunicationPage) { | ||
| this.metisConversationService.markAsRead(this.channel.id!); | ||
| } |
There was a problem hiding this comment.
Consider using the filter operator to manage the conditional logic within the observable chain, enhancing readability and potentially improving performance by limiting unnecessary executions of the subsequent operators.
- this.messages.changes.pipe(takeUntil(this.ngUnsubscribe)).subscribe(() => {
- this.handleScrollOnNewMessage();
- if (this.isCommunicationPage) {
- this.metisConversationService.markAsRead(this.channel.id!);
- }
- });
+ this.messages.changes.pipe(
+ filter(() => this.isCommunicationPage),
+ takeUntil(this.ngUnsubscribe)
+ ).subscribe(() => {
+ this.handleScrollOnNewMessage();
+ this.metisConversationService.markAsRead(this.channel.id!);
+ });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.
Suggested change
| if (this.isCommunicationPage) { | |
| this.metisConversationService.markAsRead(this.channel.id!); | |
| } | |
| this.messages.changes.pipe( | |
| filter(() => this.isCommunicationPage), | |
| takeUntil(this.ngUnsubscribe) | |
| ).subscribe(() => { | |
| this.handleScrollOnNewMessage(); | |
| this.metisConversationService.markAsRead(this.channel.id!); | |
| }); |
Comment on lines
1
to
37
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | ||
| import { ArtemisTestModule } from 'src/test/javascript/spec/test.module'; | ||
| import { SidebarAccordionComponent } from './sidebar-accordion.component'; | ||
|
|
||
| describe('SidebarAccordionComponent', () => { | ||
| let component: SidebarAccordionComponent; | ||
| let fixture: ComponentFixture<SidebarAccordionComponent>; | ||
|
|
||
| beforeEach(() => { | ||
| TestBed.configureTestingModule({ | ||
| imports: [ArtemisTestModule], | ||
| declarations: [SidebarAccordionComponent], | ||
| }).compileComponents(); | ||
| }); | ||
|
|
||
| beforeEach(() => { | ||
| fixture = TestBed.createComponent(SidebarAccordionComponent); | ||
| component = fixture.componentInstance; | ||
| fixture.detectChanges(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('should create', () => { | ||
| expect(component).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('should expand the first item when clicked', () => { | ||
| const firstItemHeader = fixture.nativeElement.querySelector('#test-accordion-item-header'); | ||
| firstItemHeader.click(); | ||
| fixture.detectChanges(); | ||
| const firstItemContent = fixture.nativeElement.querySelector('#test-accordion-item-content'); | ||
| expect(firstItemContent.classList.contains('expanded')).toBe(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Ensure Jest type definitions are included in your tsconfig.spec.json to avoid issues with recognizing Jest functions such as describe, beforeEach, and afterEach.
+ // Ensure to include Jest type definitions in your tsconfig.spec.json
+ "types": ["jest"],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.
Suggested change
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | |
| import { ArtemisTestModule } from 'src/test/javascript/spec/test.module'; | |
| import { SidebarAccordionComponent } from './sidebar-accordion.component'; | |
| describe('SidebarAccordionComponent', () => { | |
| let component: SidebarAccordionComponent; | |
| let fixture: ComponentFixture<SidebarAccordionComponent>; | |
| beforeEach(() => { | |
| TestBed.configureTestingModule({ | |
| imports: [ArtemisTestModule], | |
| declarations: [SidebarAccordionComponent], | |
| }).compileComponents(); | |
| }); | |
| beforeEach(() => { | |
| fixture = TestBed.createComponent(SidebarAccordionComponent); | |
| component = fixture.componentInstance; | |
| fixture.detectChanges(); | |
| }); | |
| afterEach(() => { | |
| jest.restoreAllMocks(); | |
| }); | |
| it('should create', () => { | |
| expect(component).toBeTruthy(); | |
| }); | |
| it('should expand the first item when clicked', () => { | |
| const firstItemHeader = fixture.nativeElement.querySelector('#test-accordion-item-header'); | |
| firstItemHeader.click(); | |
| fixture.detectChanges(); | |
| const firstItemContent = fixture.nativeElement.querySelector('#test-accordion-item-content'); | |
| expect(firstItemContent.classList.contains('expanded')).toBe(true); | |
| }); | |
| }); | |
| import { ComponentFixture, TestBed } from '@angular/core/testing'; | |
| import { ArtemisTestModule } from 'src/test/javascript/spec/test.module'; | |
| import { SidebarAccordionComponent } from './sidebar-accordion.component'; | |
| describe('SidebarAccordionComponent', () => { | |
| let component: SidebarAccordionComponent; | |
| let fixture: ComponentFixture<SidebarAccordionComponent>; | |
| beforeEach(() => { | |
| TestBed.configureTestingModule({ | |
| imports: [ArtemisTestModule], | |
| declarations: [SidebarAccordionComponent], | |
| }).compileComponents(); | |
| }); | |
| beforeEach(() => { | |
| fixture = TestBed.createComponent(SidebarAccordionComponent); | |
| component = fixture.componentInstance; | |
| fixture.detectChanges(); | |
| }); | |
| afterEach(() => { | |
| jest.restoreAllMocks(); | |
| }); | |
| it('should create', () => { | |
| expect(component).toBeTruthy(); | |
| }); | |
| it('should expand the first item when clicked', () => { | |
| const firstItemHeader = fixture.nativeElement.querySelector('#test-accordion-item-header'); | |
| firstItemHeader.click(); | |
| fixture.detectChanges(); | |
| const firstItemContent = fixture.nativeElement.querySelector('#test-accordion-item-content'); | |
| expect(firstItemContent.classList.contains('expanded')).toBe(true); | |
| }); | |
| }); |
Comment on lines
52
to
79
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('exercise'); | ||
| this.parentParamSubscription = this.route.parent!.params.subscribe((params) => { | ||
| this.courseId = parseInt(params.courseId, 10); | ||
| }); | ||
|
|
||
| this.course = this.courseStorageService.getCourse(this.courseId); | ||
| this.onCourseLoad(); | ||
| this.prepareSidebarData(); | ||
|
|
||
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | ||
| this.course = course; | ||
| this.prepareSidebarData(); | ||
| this.onCourseLoad(); | ||
| }); | ||
|
|
||
| this.translateSubscription = this.translateService.onLangChange.subscribe(() => { | ||
| this.applyFiltersAndOrder(); | ||
| }); | ||
|
|
||
| this.exerciseForGuidedTour = this.guidedTourService.enableTourForCourseExerciseComponent(this.course, courseExerciseOverviewTour, true); | ||
| const upcomingExercise = this.courseOverviewService.getUpcomingExercise(this.course?.exercises); | ||
| this.paramSubscription = this.route.params.subscribe((params) => { | ||
| const exerciseId = parseInt(params.exerciseId, 10); | ||
| // If no exercise is selected navigate to the upcoming exercise | ||
| if (!exerciseId && upcomingExercise) { | ||
| this.router.navigate([upcomingExercise.id], { relativeTo: this.route }); | ||
| } else { | ||
| this.exerciseSelected = exerciseId ? true : false; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add error handling for the subscription to route parameters in case of invalid courseId or exerciseId values.
| } | ||
|
|
||
| mapExercisesToSidebarCardElements(exercises: Exercise[]) { | ||
| return exercises.map((exercise) => this.mapLectureToSidebarCardElement(exercise)); |
There was a problem hiding this comment.
There seems to be a typo in the method mapExercisesToSidebarCardElements. It calls this.mapLectureToSidebarCardElement(exercise) instead of this.mapExerciseToSidebarCardElement(exercise).
- return exercises.map((exercise) => this.mapLectureToSidebarCardElement(exercise));
+ return exercises.map((exercise) => this.mapExerciseToSidebarCardElement(exercise));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.
Suggested change
| return exercises.map((exercise) => this.mapLectureToSidebarCardElement(exercise)); | |
| return exercises.map((exercise) => this.mapExerciseToSidebarCardElement(exercise)); |
Comment on lines
56
to
68
| expandGroupWithSelectedItem() { | ||
| const routeParamKey = Object.keys(this.routeParams)[0]; | ||
| if (this.routeParams[routeParamKey] && this.groupedData) { | ||
| const groupWithSelectedItem = Object.entries(this.groupedData).find((groupedItem) => | ||
| groupedItem[1].entityData.some((entityItem: SidebarCardElement) => entityItem.id === Number(this.routeParams[routeParamKey])), | ||
| ); | ||
| if (groupWithSelectedItem) { | ||
| const groupName = groupWithSelectedItem[0]; | ||
| this.collapseState[groupName] = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Optimize the expandGroupWithSelectedItem method by directly accessing the route parameter instead of iterating over all keys.
- const routeParamKey = Object.keys(this.routeParams)[0];
- if (this.routeParams[routeParamKey] && this.groupedData) {
+ const lectureId = this.routeParams['lectureId'];
+ if (lectureId && this.groupedData) {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.
Suggested change
| expandGroupWithSelectedItem() { | |
| const routeParamKey = Object.keys(this.routeParams)[0]; | |
| if (this.routeParams[routeParamKey] && this.groupedData) { | |
| const groupWithSelectedItem = Object.entries(this.groupedData).find((groupedItem) => | |
| groupedItem[1].entityData.some((entityItem: SidebarCardElement) => entityItem.id === Number(this.routeParams[routeParamKey])), | |
| ); | |
| if (groupWithSelectedItem) { | |
| const groupName = groupWithSelectedItem[0]; | |
| this.collapseState[groupName] = false; | |
| } | |
| } | |
| expandGroupWithSelectedItem() { | |
| const lectureId = this.routeParams['lectureId']; | |
| if (lectureId && this.groupedData) { | |
| const groupWithSelectedItem = Object.entries(this.groupedData).find((groupedItem) => | |
| groupedItem[1].entityData.some((entityItem: SidebarCardElement) => entityItem.id === Number(lectureId)), | |
| ); | |
| if (groupWithSelectedItem) { | |
| const groupName = groupWithSelectedItem[0]; | |
| this.collapseState[groupName] = false; | |
| } | |
| } |
There was a problem hiding this comment.
Could be also 'exerciseId'
There was a problem hiding this comment.
@rabeatwork, you're right. The expandGroupWithSelectedItem method should be flexible enough to handle both 'lectureId' and 'exerciseId' as potential route parameters. Here's an updated approach that checks for both:
expandGroupWithSelectedItem() {
const contentId = this.routeParams['lectureId'] || this.routeParams['exerciseId'];
if (contentId && this.groupedData) {
const groupWithSelectedItem = Object.entries(this.groupedData).find((groupedItem) =>
groupedItem[1].entityData.some((entityItem: SidebarCardElement) => entityItem.id === Number(contentId)),
);
if (groupWithSelectedItem) {
const groupName = groupWithSelectedItem[0];
this.collapseState[groupName] = false;
}
}
}This modification ensures that the method can dynamically respond to the type of content being navigated to, whether it's a lecture or an exercise, by attempting to retrieve 'lectureId' first and then 'exerciseId' if 'lectureId' is not present.
Comment on lines
1
to
101
| ngOnDestroy(): void { | ||
| this.translateSubscription.unsubscribe(); | ||
| this.courseUpdatesSubscription.unsubscribe(); | ||
| this.paramSubscription.unsubscribe(); | ||
| } | ||
|
|
||
| /** | ||
| * Loads the sorting order from local storage | ||
| */ | ||
| private loadSortingOrder() { | ||
| const orderInStorage = this.localStorage.retrieve(SortFilterStorageKey.ORDER); | ||
| const parsedOrderInStorage = Object.keys(LectureSortingOrder).find((exerciseOrder) => exerciseOrder === orderInStorage); | ||
| this.sortingOrder = parsedOrderInStorage ? (+parsedOrderInStorage as LectureSortingOrder) : LectureSortingOrder.ASC; | ||
| updateSidebarData() { | ||
| this.sidebarData = { | ||
| groupByCategory: true, | ||
| storageId: 'lecture', | ||
| groupedData: this.accordionLectureGroups, | ||
| ungroupedData: this.sidebarLectures, | ||
| }; | ||
| } | ||
|
|
||
| private onCourseLoad() { | ||
| this.groupLectures(); | ||
| toggleSidebar() { | ||
| this.isCollapsed = !this.isCollapsed; | ||
| this.courseOverviewService.setSidebarCollapseState('lectures', this.isCollapsed); | ||
| } | ||
|
|
||
| /** | ||
| * Reorders all displayed lectures | ||
| */ | ||
| flipOrder() { | ||
| this.sortingOrder = this.sortingOrder === this.ASC ? this.DESC : this.ASC; | ||
| this.localStorage.store(SortFilterStorageKey.ORDER, this.sortingOrder.toString()); | ||
| this.groupLectures(); | ||
| } | ||
|
|
||
| public groupLectures(): void { | ||
| this.weeklyLecturesGrouped = {}; | ||
| this.weeklyIndexKeys = []; | ||
| const groupedLectures = {}; | ||
| const indexKeys: string[] = []; | ||
| const courseLectures = this.course?.lectures ? [...this.course!.lectures] : []; | ||
| const sortedLectures = this.sortLectures(courseLectures, this.sortingOrder); | ||
| const notAssociatedLectures: Lecture[] = []; | ||
| sortedLectures.forEach((lecture) => { | ||
| const dateValue = lecture.startDate ? dayjs(lecture.startDate) : undefined; | ||
| if (!dateValue) { | ||
| notAssociatedLectures.push(lecture); | ||
| return; | ||
| } | ||
| const dateIndex = dateValue ? dayjs(dateValue).startOf('week').format('YYYY-MM-DD') : 'NoDate'; | ||
| if (!groupedLectures[dateIndex]) { | ||
| indexKeys.push(dateIndex); | ||
| if (dateValue) { | ||
| groupedLectures[dateIndex] = { | ||
| start: dayjs(dateValue).startOf('week'), | ||
| end: dayjs(dateValue).endOf('week'), | ||
| isCollapsed: dateValue.isBefore(dayjs(), 'week') || dateValue.isAfter(dayjs(), 'week'), | ||
| isCurrentWeek: dateValue.isSame(dayjs(), 'week'), | ||
| lectures: [], | ||
| }; | ||
| } else { | ||
| groupedLectures[dateIndex] = { | ||
| isCollapsed: false, | ||
| isCurrentWeek: false, | ||
| lectures: [], | ||
| }; | ||
| } | ||
| } | ||
| groupedLectures[dateIndex].lectures.push(lecture); | ||
| }); | ||
| if (notAssociatedLectures.length > 0) { | ||
| this.weeklyLecturesGrouped = { | ||
| ...groupedLectures, | ||
| noDate: { | ||
| label: this.translateService.instant('artemisApp.courseOverview.exerciseList.noExerciseDate'), | ||
| isCollapsed: false, | ||
| isCurrentWeek: false, | ||
| lectures: notAssociatedLectures, | ||
| }, | ||
| }; | ||
| this.weeklyIndexKeys = [...indexKeys, 'noDate']; | ||
| } else { | ||
| this.weeklyLecturesGrouped = groupedLectures; | ||
| this.weeklyIndexKeys = indexKeys; | ||
| } | ||
| } | ||
|
|
||
| private sortLectures(lectures: Lecture[], selectedOrder: number): Lecture[] { | ||
| return lectures.sort((a, b) => { | ||
| const aValue = a.startDate ? a.startDate.valueOf() : dayjs().valueOf(); | ||
| const bValue = b.startDate ? b.startDate.valueOf() : dayjs().valueOf(); | ||
|
|
||
| return selectedOrder * (aValue - bValue); | ||
| }); | ||
| ngOnDestroy(): void { | ||
| this.courseUpdatesSubscription?.unsubscribe(); | ||
| this.paramSubscription?.unsubscribe(); |
There was a problem hiding this comment.
Ensure that all subscriptions are properly unsubscribed in the ngOnDestroy lifecycle hook to prevent memory leaks.
+ private subscriptions: Subscription[] = [];
ngOnInit() {
+ this.subscriptions.push(this.paramSubscription);
+ this.subscriptions.push(this.courseUpdatesSubscription);
}
ngOnDestroy(): void {
+ this.subscriptions.forEach(subscription => subscription.unsubscribe());
}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.
Suggested change
| import { Component, OnDestroy, OnInit } from '@angular/core'; | |
| import { Course } from 'app/entities/course.model'; | |
| import { ActivatedRoute } from '@angular/router'; | |
| import { Subject, Subscription } from 'rxjs'; | |
| import { TranslateService } from '@ngx-translate/core'; | |
| import dayjs from 'dayjs/esm'; | |
| import { ActivatedRoute, Router } from '@angular/router'; | |
| import { Subscription } from 'rxjs'; | |
| import { Lecture } from 'app/entities/lecture.model'; | |
| import { LocalStorageService } from 'ngx-webstorage'; | |
| import { ExerciseService } from 'app/exercises/shared/exercise/exercise.service'; | |
| import { faAngleDown, faAngleUp, faSortNumericDown, faSortNumericUp } from '@fortawesome/free-solid-svg-icons'; | |
| import { BarControlConfiguration, BarControlConfigurationProvider } from 'app/shared/tab-bar/tab-bar'; | |
| import { CourseStorageService } from 'app/course/manage/course-storage.service'; | |
| import { AccordionGroups, SidebarCardElement, SidebarData } from 'app/types/sidebar'; | |
| import { CourseOverviewService } from '../course-overview.service'; | |
| export enum LectureSortingOrder { | |
| ASC = 1, | |
| DESC = -1, | |
| } | |
| enum SortFilterStorageKey { | |
| ORDER = 'artemis.course.lectures.order', | |
| } | |
| const DEFAULT_UNIT_GROUPS: AccordionGroups = { | |
| future: { entityData: [] }, | |
| current: { entityData: [] }, | |
| past: { entityData: [] }, | |
| noDate: { entityData: [] }, | |
| }; | |
| @Component({ | |
| selector: 'jhi-course-lectures', | |
| templateUrl: './course-lectures.component.html', | |
| styleUrls: ['../course-overview.scss'], | |
| }) | |
| export class CourseLecturesComponent implements OnInit, OnDestroy, AfterViewInit, BarControlConfigurationProvider { | |
| export class CourseLecturesComponent implements OnInit, OnDestroy { | |
| private courseId: number; | |
| private paramSubscription: Subscription; | |
| private courseUpdatesSubscription: Subscription; | |
| private translateSubscription: Subscription; | |
| public course?: Course; | |
| public weeklyIndexKeys: string[]; | |
| public weeklyLecturesGrouped: object; | |
| public exerciseCountMap: Map<string, number>; | |
| readonly ASC = LectureSortingOrder.ASC; | |
| readonly DESC = LectureSortingOrder.DESC; | |
| // Icons | |
| faSortNumericDown = faSortNumericDown; | |
| faSortNumericUp = faSortNumericUp; | |
| faAngleUp = faAngleUp; | |
| faAngleDown = faAngleDown; | |
| course?: Course; | |
| sortingOrder: LectureSortingOrder; | |
| // The extracted controls template from our template to be rendered in the top bar of "CourseOverviewComponent" | |
| @ViewChild('controls', { static: false }) private controls: TemplateRef<any>; | |
| // Provides the control configuration to be read and used by "CourseOverviewComponent" | |
| public readonly controlConfiguration: BarControlConfiguration = { | |
| subject: new Subject<TemplateRef<any>>(), | |
| }; | |
| lectureSelected: boolean = true; | |
| sidebarData: SidebarData; | |
| accordionLectureGroups: AccordionGroups = DEFAULT_UNIT_GROUPS; | |
| sortedLectures: Lecture[] = []; | |
| sidebarLectures: SidebarCardElement[] = []; | |
| isCollapsed: boolean = false; | |
| constructor( | |
| private courseStorageService: CourseStorageService, | |
| private translateService: TranslateService, | |
| private exerciseService: ExerciseService, | |
| private route: ActivatedRoute, | |
| private localStorage: LocalStorageService, | |
| private router: Router, | |
| private courseOverviewService: CourseOverviewService, | |
| ) {} | |
| ngOnInit() { | |
| this.exerciseCountMap = new Map<string, number>(); | |
| this.loadSortingOrder(); | |
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('lectures'); | |
| this.paramSubscription = this.route.parent!.params.subscribe((params) => { | |
| this.courseId = parseInt(params['courseId'], 10); | |
| this.courseId = parseInt(params.courseId, 10); | |
| }); | |
| this.course = this.courseStorageService.getCourse(this.courseId); | |
| this.onCourseLoad(); | |
| this.prepareSidebarData(); | |
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | |
| this.course = course; | |
| this.onCourseLoad(); | |
| this.prepareSidebarData(); | |
| }); | |
| this.translateSubscription = this.translateService.onLangChange.subscribe(() => { | |
| this.groupLectures(); | |
| const upcomingLecture = this.courseOverviewService.getUpcomingLecture(this.course?.lectures); | |
| this.paramSubscription = this.route.params.subscribe((params) => { | |
| const lectureId = parseInt(params.lectureId, 10); | |
| // If no exercise is selected navigate to the upcoming exercise | |
| if (!lectureId && upcomingLecture) { | |
| this.router.navigate([upcomingLecture.id], { relativeTo: this.route }); | |
| } else { | |
| this.lectureSelected = lectureId ? true : false; | |
| } | |
| }); | |
| } | |
| ngAfterViewInit(): void { | |
| // Send our controls template to parent so it will be rendered in the top bar | |
| if (this.controls) { | |
| this.controlConfiguration.subject!.next(this.controls); | |
| prepareSidebarData() { | |
| if (!this.course?.lectures) { | |
| return; | |
| } | |
| this.sortedLectures = this.courseOverviewService.sortLectures(this.course.lectures); | |
| this.sidebarLectures = this.courseOverviewService.mapLecturesToSidebarCardElements(this.sortedLectures); | |
| this.accordionLectureGroups = this.courseOverviewService.groupLecturesByStartDate(this.sortedLectures); | |
| this.updateSidebarData(); | |
| } | |
| ngOnDestroy(): void { | |
| this.translateSubscription.unsubscribe(); | |
| this.courseUpdatesSubscription.unsubscribe(); | |
| this.paramSubscription.unsubscribe(); | |
| } | |
| /** | |
| * Loads the sorting order from local storage | |
| */ | |
| private loadSortingOrder() { | |
| const orderInStorage = this.localStorage.retrieve(SortFilterStorageKey.ORDER); | |
| const parsedOrderInStorage = Object.keys(LectureSortingOrder).find((exerciseOrder) => exerciseOrder === orderInStorage); | |
| this.sortingOrder = parsedOrderInStorage ? (+parsedOrderInStorage as LectureSortingOrder) : LectureSortingOrder.ASC; | |
| updateSidebarData() { | |
| this.sidebarData = { | |
| groupByCategory: true, | |
| storageId: 'lecture', | |
| groupedData: this.accordionLectureGroups, | |
| ungroupedData: this.sidebarLectures, | |
| }; | |
| } | |
| private onCourseLoad() { | |
| this.groupLectures(); | |
| toggleSidebar() { | |
| this.isCollapsed = !this.isCollapsed; | |
| this.courseOverviewService.setSidebarCollapseState('lectures', this.isCollapsed); | |
| } | |
| /** | |
| * Reorders all displayed lectures | |
| */ | |
| flipOrder() { | |
| this.sortingOrder = this.sortingOrder === this.ASC ? this.DESC : this.ASC; | |
| this.localStorage.store(SortFilterStorageKey.ORDER, this.sortingOrder.toString()); | |
| this.groupLectures(); | |
| } | |
| public groupLectures(): void { | |
| this.weeklyLecturesGrouped = {}; | |
| this.weeklyIndexKeys = []; | |
| const groupedLectures = {}; | |
| const indexKeys: string[] = []; | |
| const courseLectures = this.course?.lectures ? [...this.course!.lectures] : []; | |
| const sortedLectures = this.sortLectures(courseLectures, this.sortingOrder); | |
| const notAssociatedLectures: Lecture[] = []; | |
| sortedLectures.forEach((lecture) => { | |
| const dateValue = lecture.startDate ? dayjs(lecture.startDate) : undefined; | |
| if (!dateValue) { | |
| notAssociatedLectures.push(lecture); | |
| return; | |
| } | |
| const dateIndex = dateValue ? dayjs(dateValue).startOf('week').format('YYYY-MM-DD') : 'NoDate'; | |
| if (!groupedLectures[dateIndex]) { | |
| indexKeys.push(dateIndex); | |
| if (dateValue) { | |
| groupedLectures[dateIndex] = { | |
| start: dayjs(dateValue).startOf('week'), | |
| end: dayjs(dateValue).endOf('week'), | |
| isCollapsed: dateValue.isBefore(dayjs(), 'week') || dateValue.isAfter(dayjs(), 'week'), | |
| isCurrentWeek: dateValue.isSame(dayjs(), 'week'), | |
| lectures: [], | |
| }; | |
| } else { | |
| groupedLectures[dateIndex] = { | |
| isCollapsed: false, | |
| isCurrentWeek: false, | |
| lectures: [], | |
| }; | |
| } | |
| } | |
| groupedLectures[dateIndex].lectures.push(lecture); | |
| }); | |
| if (notAssociatedLectures.length > 0) { | |
| this.weeklyLecturesGrouped = { | |
| ...groupedLectures, | |
| noDate: { | |
| label: this.translateService.instant('artemisApp.courseOverview.exerciseList.noExerciseDate'), | |
| isCollapsed: false, | |
| isCurrentWeek: false, | |
| lectures: notAssociatedLectures, | |
| }, | |
| }; | |
| this.weeklyIndexKeys = [...indexKeys, 'noDate']; | |
| } else { | |
| this.weeklyLecturesGrouped = groupedLectures; | |
| this.weeklyIndexKeys = indexKeys; | |
| } | |
| } | |
| private sortLectures(lectures: Lecture[], selectedOrder: number): Lecture[] { | |
| return lectures.sort((a, b) => { | |
| const aValue = a.startDate ? a.startDate.valueOf() : dayjs().valueOf(); | |
| const bValue = b.startDate ? b.startDate.valueOf() : dayjs().valueOf(); | |
| return selectedOrder * (aValue - bValue); | |
| }); | |
| ngOnDestroy(): void { | |
| this.courseUpdatesSubscription?.unsubscribe(); | |
| this.paramSubscription?.unsubscribe(); | |
| private subscriptions: Subscription[] = []; | |
| ngOnInit() { | |
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('lectures'); | |
| this.paramSubscription = this.route.parent!.params.subscribe((params) => { | |
| this.courseId = parseInt(params.courseId, 10); | |
| }); | |
| this.subscriptions.push(this.paramSubscription); | |
| this.course = this.courseStorageService.getCourse(this.courseId); | |
| this.prepareSidebarData(); | |
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | |
| this.course = course; | |
| this.prepareSidebarData(); | |
| }); | |
| this.subscriptions.push(this.courseUpdatesSubscription); | |
| const upcomingLecture = this.courseOverviewService.getUpcomingLecture(this.course?.lectures); | |
| this.paramSubscription = this.route.params.subscribe((params) => { | |
| const lectureId = parseInt(params.lectureId, 10); | |
| // If no exercise is selected navigate to the upcoming exercise | |
| if (!lectureId && upcomingLecture) { | |
| this.router.navigate([upcomingLecture.id], { relativeTo: this.route }); | |
| } else { | |
| this.lectureSelected = lectureId ? true : false; | |
| } | |
| }); | |
| } | |
| ngOnDestroy(): void { | |
| this.subscriptions.forEach(subscription => subscription.unsubscribe()); | |
| } |
Comment on lines
43
to
67
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('lectures'); | ||
| this.paramSubscription = this.route.parent!.params.subscribe((params) => { | ||
| this.courseId = parseInt(params['courseId'], 10); | ||
| this.courseId = parseInt(params.courseId, 10); | ||
| }); | ||
|
|
||
| this.course = this.courseStorageService.getCourse(this.courseId); | ||
| this.onCourseLoad(); | ||
|
|
||
| this.prepareSidebarData(); | ||
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | ||
| this.course = course; | ||
| this.onCourseLoad(); | ||
| this.prepareSidebarData(); | ||
| }); | ||
|
|
||
| this.translateSubscription = this.translateService.onLangChange.subscribe(() => { | ||
| this.groupLectures(); | ||
| const upcomingLecture = this.courseOverviewService.getUpcomingLecture(this.course?.lectures); | ||
| this.paramSubscription = this.route.params.subscribe((params) => { | ||
| const lectureId = parseInt(params.lectureId, 10); | ||
| // If no exercise is selected navigate to the upcoming exercise | ||
| if (!lectureId && upcomingLecture) { | ||
| this.router.navigate([upcomingLecture.id], { relativeTo: this.route }); | ||
| } else { | ||
| this.lectureSelected = lectureId ? true : false; | ||
| } |
There was a problem hiding this comment.
Consider refactoring the nested subscription pattern to use RxJS operators like switchMap for cleaner and more efficient code.
Comment on lines
+22
to
+25
| it('should create', () => { | ||
| expect(component).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
Consider adding more test cases to cover the component's functionality thoroughly, such as its interaction with inputs and outputs.
Would you like me to help by adding more comprehensive test cases?
There was a problem hiding this comment.
Sure, to quote Jay-Z: Show me what you got
There was a problem hiding this comment.
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { SidebarCardItemComponent } from 'app/shared/sidebar/sidebar-card-item/sidebar-card-item.component';
import { ArtemisTestModule } from '../../../test.module';
import { MockComponent } from 'ng-mocks';
// Assume SidebarCardItemComponent has an input 'item' and emits an 'itemSelected' event as an output for this example.
describe('SidebarCardItemComponent', () => {
let component: SidebarCardItemComponent;
let fixture: ComponentFixture<SidebarCardItemComponent>;
beforeEach(async () => {
await TestBed.configureTestingModule({
imports: [ArtemisTestModule],
declarations: [SidebarCardItemComponent, MockComponent(SidebarCardItemComponent)],
}).compileComponents();
});
beforeEach(() => {
fixture = TestBed.createComponent(SidebarCardItemComponent);
component = fixture.componentInstance;
fixture.detectChanges();
});
it('should create', () => {
expect(component).toBeTruthy();
});
it('should display item title', () => {
const testItem = { title: 'Test Title', content: 'Test Content' };
component.item = testItem;
fixture.detectChanges();
const compiled = fixture.debugElement.nativeElement;
expect(compiled.querySelector('.sidebar-card-title').textContent).toContain(testItem.title);
});
it('should emit itemSelected event on click', () => {
spyOn(component.itemSelected, 'emit');
const testItem = { id: 1, title: 'Test Title', content: 'Test Content' };
component.item = testItem;
fixture.detectChanges();
const compiled = fixture.debugElement.nativeElement;
compiled.querySelector('.sidebar-card').click();
expect(component.itemSelected.emit).toHaveBeenCalledWith(testItem.id);
});
});This code snippet provides additional test cases for the SidebarCardItemComponent, focusing on its interaction with an assumed input item and an output event itemSelected. Adjust the properties and event names according to the actual implementation of your component.
src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts
Outdated
Show resolved
Hide resolved
src/test/javascript/spec/component/shared/sidebar/sidebar-accordion.component.spec.ts
Outdated
Show resolved
Hide resolved
src/test/javascript/spec/component/shared/sidebar/sidebar-card.component.spec.ts
Outdated
Show resolved
Hide resolved
src/test/javascript/spec/component/shared/sidebar/sidebar.component.spec.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/overview/course-lectures/course-lectures.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/overview/course-lectures/course-lectures.component.ts
Outdated
Show resolved
Hide resolved
src/test/javascript/spec/component/course/course-overview.service.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/component/course/course-overview.component.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/component/course/course-overview.component.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/component/course/course-overview.component.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/component/course/course-overview.component.spec.ts
Show resolved
Hide resolved
Comment on lines
73
to
85
| beforeEach(async () => { | ||
| const participationServiceMock = { | ||
| downloadArtifact: jest.fn(), | ||
| }; | ||
| global.URL.createObjectURL = jest.fn(() => 'blob:test-url'); | ||
| global.URL.revokeObjectURL = jest.fn(); | ||
| await TestBed.configureTestingModule({ | ||
| imports: [ArtemisTestModule, NgbTooltipMocksModule], | ||
| declarations: [ResultComponent, TranslatePipeMock, MockPipe(ArtemisDatePipe), MockPipe(ArtemisTimeAgoPipe)], | ||
| providers: [{ provide: NgbModal, useClass: MockNgbModalService }], | ||
| providers: [ | ||
| { provide: NgbModal, useClass: MockNgbModalService }, | ||
| { provide: ParticipationService, useValue: participationServiceMock }, | ||
| ], |
There was a problem hiding this comment.
The test setup correctly isolates the component by mocking dependencies. However, ensure that mocked objects correctly implement their interfaces to avoid type mismatches.
- comp.badge = { tooltip: 'Example Tooltip' };
+ comp.badge = { tooltip: 'Example Tooltip', class: '', text: '' };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.
Suggested change
| beforeEach(async () => { | |
| const participationServiceMock = { | |
| downloadArtifact: jest.fn(), | |
| }; | |
| global.URL.createObjectURL = jest.fn(() => 'blob:test-url'); | |
| global.URL.revokeObjectURL = jest.fn(); | |
| await TestBed.configureTestingModule({ | |
| imports: [ArtemisTestModule, NgbTooltipMocksModule], | |
| declarations: [ResultComponent, TranslatePipeMock, MockPipe(ArtemisDatePipe), MockPipe(ArtemisTimeAgoPipe)], | |
| providers: [{ provide: NgbModal, useClass: MockNgbModalService }], | |
| providers: [ | |
| { provide: NgbModal, useClass: MockNgbModalService }, | |
| { provide: ParticipationService, useValue: participationServiceMock }, | |
| ], | |
| beforeEach(async () => { | |
| const participationServiceMock = { | |
| downloadArtifact: jest.fn(), | |
| }; | |
| global.URL.createObjectURL = jest.fn(() => 'blob:test-url'); | |
| global.URL.revokeObjectURL = jest.fn(); | |
| await TestBed.configureTestingModule({ | |
| imports: [ArtemisTestModule, NgbTooltipMocksModule], | |
| declarations: [ResultComponent, TranslatePipeMock, MockPipe(ArtemisDatePipe), MockPipe(ArtemisTimeAgoPipe)], | |
| providers: [ | |
| { provide: NgbModal, useClass: MockNgbModalService }, | |
| { provide: ParticipationService, useValue: participationServiceMock }, | |
| ], |
Comment on lines
106
to
133
| it('should download build result when participation ID is provided', () => { | ||
| // Arrange | ||
| const fakeArtifact = { | ||
| fileContent: new Blob(['test'], { type: 'text/plain' }), | ||
| fileName: 'test.txt', | ||
| }; | ||
| const mockLink = document.createElement('a'); | ||
| jest.spyOn(document, 'createElement').mockReturnValue(mockLink); | ||
| jest.spyOn(document.body, 'appendChild').mockImplementation((child) => child); | ||
| jest.spyOn(document.body, 'removeChild').mockImplementation((child) => child); | ||
|
|
||
| const urlSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:test-url'); | ||
|
|
||
| participationService.downloadArtifact.mockReturnValue(of(fakeArtifact)); | ||
|
|
||
| // Act | ||
| comp.downloadBuildResult(123); | ||
|
|
||
| // Assert | ||
| expect(participationService.downloadArtifact).toHaveBeenCalledWith(123); | ||
| expect(document.createElement).toHaveBeenCalledWith('a'); | ||
| expect(urlSpy).toHaveBeenCalledWith(fakeArtifact.fileContent); | ||
| expect(mockLink.download).toBe(fakeArtifact.fileName); | ||
| expect(mockLink.href).toBe('blob:test-url'); | ||
| expect(document.body.appendChild).toHaveBeenCalledWith(mockLink); | ||
| // Cleanup to avoid memory leaks | ||
| URL.revokeObjectURL(mockLink.href); | ||
| }); |
There was a problem hiding this comment.
Ensure that mocked methods correctly simulate their behavior. The downloadArtifact method should return an observable that emits the expected value.
- participationService.downloadArtifact.mockReturnValue(of(fakeArtifact));
+ jest.spyOn(participationService, 'downloadArtifact').mockReturnValue(of(fakeArtifact));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.
Suggested change
| it('should download build result when participation ID is provided', () => { | |
| // Arrange | |
| const fakeArtifact = { | |
| fileContent: new Blob(['test'], { type: 'text/plain' }), | |
| fileName: 'test.txt', | |
| }; | |
| const mockLink = document.createElement('a'); | |
| jest.spyOn(document, 'createElement').mockReturnValue(mockLink); | |
| jest.spyOn(document.body, 'appendChild').mockImplementation((child) => child); | |
| jest.spyOn(document.body, 'removeChild').mockImplementation((child) => child); | |
| const urlSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:test-url'); | |
| participationService.downloadArtifact.mockReturnValue(of(fakeArtifact)); | |
| // Act | |
| comp.downloadBuildResult(123); | |
| // Assert | |
| expect(participationService.downloadArtifact).toHaveBeenCalledWith(123); | |
| expect(document.createElement).toHaveBeenCalledWith('a'); | |
| expect(urlSpy).toHaveBeenCalledWith(fakeArtifact.fileContent); | |
| expect(mockLink.download).toBe(fakeArtifact.fileName); | |
| expect(mockLink.href).toBe('blob:test-url'); | |
| expect(document.body.appendChild).toHaveBeenCalledWith(mockLink); | |
| // Cleanup to avoid memory leaks | |
| URL.revokeObjectURL(mockLink.href); | |
| }); | |
| it('should download build result when participation ID is provided', () => { | |
| // Arrange | |
| const fakeArtifact = { | |
| fileContent: new Blob(['test'], { type: 'text/plain' }), | |
| fileName: 'test.txt', | |
| }; | |
| const mockLink = document.createElement('a'); | |
| jest.spyOn(document, 'createElement').mockReturnValue(mockLink); | |
| jest.spyOn(document.body, 'appendChild').mockImplementation((child) => child); | |
| jest.spyOn(document.body, 'removeChild').mockImplementation((child) => child); | |
| const urlSpy = jest.spyOn(URL, 'createObjectURL').mockReturnValue('blob:test-url'); | |
| jest.spyOn(participationService, 'downloadArtifact').mockReturnValue(of(fakeArtifact)); | |
| // Act | |
| comp.downloadBuildResult(123); | |
| // Assert | |
| expect(participationService.downloadArtifact).toHaveBeenCalledWith(123); | |
| expect(document.createElement).toHaveBeenCalledWith('a'); | |
| expect(urlSpy).toHaveBeenCalledWith(fakeArtifact.fileContent); | |
| expect(mockLink.download).toBe(fakeArtifact.fileName); | |
| expect(mockLink.href).toBe('blob:test-url'); | |
| expect(document.body.appendChild).toHaveBeenCalledWith(mockLink); | |
| // Cleanup to avoid memory leaks | |
| URL.revokeObjectURL(mockLink.href); | |
| }); |
Comment on lines
211
to
230
| it('should call showDetails only when isInSidebarCard is false', () => { | ||
| comp.result = mockResult; | ||
| const detailsSpy = jest.spyOn(comp, 'showDetails'); | ||
|
|
||
| comp.isInSidebarCard = false; | ||
| comp.resultIconClass = faTimesCircle; | ||
| comp.exercise = { type: ExerciseType.PROGRAMMING }; | ||
| comp.result = mockResult; | ||
| comp.resultIconClass = faTimesCircle; | ||
| comp.templateStatus = ResultTemplateStatus.HAS_RESULT; | ||
| fixture.detectChanges(); | ||
| const resultElement = fixture.debugElement.query(By.css('#result-score')); | ||
| resultElement.triggerEventHandler('click', null); | ||
| expect(detailsSpy).toHaveBeenCalledWith(mockResult); | ||
|
|
||
| detailsSpy.mockClear(); | ||
| comp.isInSidebarCard = true; | ||
| fixture.detectChanges(); | ||
| resultElement.triggerEventHandler('click', null); | ||
| expect(detailsSpy).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
Refactor the test case to use describe blocks for different isInSidebarCard values for better organization and readability.
- it('should call showDetails only when isInSidebarCard is false', () => {
+ describe('when isInSidebarCard is false', () => {
+ it('should call showDetails', () => {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.
Suggested change
| it('should call showDetails only when isInSidebarCard is false', () => { | |
| comp.result = mockResult; | |
| const detailsSpy = jest.spyOn(comp, 'showDetails'); | |
| comp.isInSidebarCard = false; | |
| comp.resultIconClass = faTimesCircle; | |
| comp.exercise = { type: ExerciseType.PROGRAMMING }; | |
| comp.result = mockResult; | |
| comp.resultIconClass = faTimesCircle; | |
| comp.templateStatus = ResultTemplateStatus.HAS_RESULT; | |
| fixture.detectChanges(); | |
| const resultElement = fixture.debugElement.query(By.css('#result-score')); | |
| resultElement.triggerEventHandler('click', null); | |
| expect(detailsSpy).toHaveBeenCalledWith(mockResult); | |
| detailsSpy.mockClear(); | |
| comp.isInSidebarCard = true; | |
| fixture.detectChanges(); | |
| resultElement.triggerEventHandler('click', null); | |
| expect(detailsSpy).not.toHaveBeenCalled(); | |
| describe('when isInSidebarCard is false', () => { | |
| it('should call showDetails', () => { | |
| comp.result = mockResult; | |
| const detailsSpy = jest.spyOn(comp, 'showDetails'); | |
| comp.isInSidebarCard = false; | |
| comp.resultIconClass = faTimesCircle; | |
| comp.exercise = { type: ExerciseType.PROGRAMMING }; | |
| comp.result = mockResult; | |
| comp.resultIconClass = faTimesCircle; | |
| comp.templateStatus = ResultTemplateStatus.HAS_RESULT; | |
| fixture.detectChanges(); | |
| const resultElement = fixture.debugElement.query(By.css('#result-score')); | |
| resultElement.triggerEventHandler('click', null); | |
| expect(detailsSpy).toHaveBeenCalledWith(mockResult); | |
| detailsSpy.mockClear(); | |
| comp.isInSidebarCard = true; | |
| fixture.detectChanges(); | |
| resultElement.triggerEventHandler('click', null); | |
| expect(detailsSpy).not.toHaveBeenCalled(); |
rstief
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
Comment on lines
+1
to
+102
| this.paramSubscription.unsubscribe(); | ||
| updateSidebarData() { | ||
| this.sidebarData = { | ||
| groupByCategory: true, | ||
| storageId: 'lecture', | ||
| groupedData: this.accordionLectureGroups, | ||
| ungroupedData: this.sidebarLectures, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Loads the sorting order from local storage | ||
| */ | ||
| private loadSortingOrder() { | ||
| const orderInStorage = this.localStorage.retrieve(SortFilterStorageKey.ORDER); | ||
| const parsedOrderInStorage = Object.keys(LectureSortingOrder).find((exerciseOrder) => exerciseOrder === orderInStorage); | ||
| this.sortingOrder = parsedOrderInStorage ? (+parsedOrderInStorage as LectureSortingOrder) : LectureSortingOrder.ASC; | ||
| toggleSidebar() { | ||
| this.isCollapsed = !this.isCollapsed; | ||
| this.courseOverviewService.setSidebarCollapseState('lecture', this.isCollapsed); | ||
| } | ||
|
|
||
| private onCourseLoad() { | ||
| this.groupLectures(); | ||
| getLastSelectedLecture(): string | null { | ||
| return sessionStorage.getItem('sidebar.lastSelectedItem.' + 'lecture'); | ||
| } | ||
|
|
||
| /** | ||
| * Reorders all displayed lectures | ||
| */ | ||
| flipOrder() { | ||
| this.sortingOrder = this.sortingOrder === this.ASC ? this.DESC : this.ASC; | ||
| this.localStorage.store(SortFilterStorageKey.ORDER, this.sortingOrder.toString()); | ||
| this.groupLectures(); | ||
| } | ||
|
|
||
| public groupLectures(): void { | ||
| this.weeklyLecturesGrouped = {}; | ||
| this.weeklyIndexKeys = []; | ||
| const groupedLectures = {}; | ||
| const indexKeys: string[] = []; | ||
| const courseLectures = this.course?.lectures ? [...this.course!.lectures] : []; | ||
| const sortedLectures = this.sortLectures(courseLectures, this.sortingOrder); | ||
| const notAssociatedLectures: Lecture[] = []; | ||
| sortedLectures.forEach((lecture) => { | ||
| const dateValue = lecture.startDate ? dayjs(lecture.startDate) : undefined; | ||
| if (!dateValue) { | ||
| notAssociatedLectures.push(lecture); | ||
| return; | ||
| } | ||
| const dateIndex = dateValue ? dayjs(dateValue).startOf('week').format('YYYY-MM-DD') : 'NoDate'; | ||
| if (!groupedLectures[dateIndex]) { | ||
| indexKeys.push(dateIndex); | ||
| if (dateValue) { | ||
| groupedLectures[dateIndex] = { | ||
| start: dayjs(dateValue).startOf('week'), | ||
| end: dayjs(dateValue).endOf('week'), | ||
| isCollapsed: dateValue.isBefore(dayjs(), 'week') || dateValue.isAfter(dayjs(), 'week'), | ||
| isCurrentWeek: dateValue.isSame(dayjs(), 'week'), | ||
| lectures: [], | ||
| }; | ||
| } else { | ||
| groupedLectures[dateIndex] = { | ||
| isCollapsed: false, | ||
| isCurrentWeek: false, | ||
| lectures: [], | ||
| }; | ||
| } | ||
| } | ||
| groupedLectures[dateIndex].lectures.push(lecture); | ||
| }); | ||
| if (notAssociatedLectures.length > 0) { | ||
| this.weeklyLecturesGrouped = { | ||
| ...groupedLectures, | ||
| noDate: { | ||
| label: this.translateService.instant('artemisApp.courseOverview.exerciseList.noExerciseDate'), | ||
| isCollapsed: false, | ||
| isCurrentWeek: false, | ||
| lectures: notAssociatedLectures, | ||
| }, | ||
| }; | ||
| this.weeklyIndexKeys = [...indexKeys, 'noDate']; | ||
| } else { | ||
| this.weeklyLecturesGrouped = groupedLectures; | ||
| this.weeklyIndexKeys = indexKeys; | ||
| } | ||
| } | ||
|
|
||
| private sortLectures(lectures: Lecture[], selectedOrder: number): Lecture[] { | ||
| return lectures.sort((a, b) => { | ||
| const aValue = a.startDate ? a.startDate.valueOf() : dayjs().valueOf(); | ||
| const bValue = b.startDate ? b.startDate.valueOf() : dayjs().valueOf(); | ||
|
|
||
| return selectedOrder * (aValue - bValue); | ||
| }); | ||
| ngOnDestroy(): void { | ||
| this.courseUpdatesSubscription?.unsubscribe(); | ||
| this.paramSubscription?.unsubscribe(); | ||
| this.parentParamSubscription?.unsubscribe(); |
There was a problem hiding this comment.
Ensure all subscriptions are properly unsubscribed in the ngOnDestroy lifecycle hook to prevent memory leaks, as previously suggested.
+ private subscriptions: Subscription[] = [];
ngOnInit() {
+ this.subscriptions.push(this.paramSubscription);
+ this.subscriptions.push(this.courseUpdatesSubscription);
+ this.subscriptions.push(this.parentParamSubscription);
}
ngOnDestroy(): void {
+ this.subscriptions.forEach(subscription => subscription.unsubscribe());
}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.
Suggested change
| import { Component, OnDestroy, OnInit } from '@angular/core'; | |
| import { Course } from 'app/entities/course.model'; | |
| import { ActivatedRoute } from '@angular/router'; | |
| import { Subject, Subscription } from 'rxjs'; | |
| import { TranslateService } from '@ngx-translate/core'; | |
| import dayjs from 'dayjs/esm'; | |
| import { ActivatedRoute, Router } from '@angular/router'; | |
| import { Subscription } from 'rxjs'; | |
| import { Lecture } from 'app/entities/lecture.model'; | |
| import { LocalStorageService } from 'ngx-webstorage'; | |
| import { ExerciseService } from 'app/exercises/shared/exercise/exercise.service'; | |
| import { faAngleDown, faAngleUp, faSortNumericDown, faSortNumericUp } from '@fortawesome/free-solid-svg-icons'; | |
| import { BarControlConfiguration, BarControlConfigurationProvider } from 'app/shared/tab-bar/tab-bar'; | |
| import { CourseStorageService } from 'app/course/manage/course-storage.service'; | |
| import { AccordionGroups, SidebarCardElement, SidebarData } from 'app/types/sidebar'; | |
| import { CourseOverviewService } from '../course-overview.service'; | |
| export enum LectureSortingOrder { | |
| ASC = 1, | |
| DESC = -1, | |
| } | |
| enum SortFilterStorageKey { | |
| ORDER = 'artemis.course.lectures.order', | |
| } | |
| const DEFAULT_UNIT_GROUPS: AccordionGroups = { | |
| future: { entityData: [] }, | |
| current: { entityData: [] }, | |
| past: { entityData: [] }, | |
| noDate: { entityData: [] }, | |
| }; | |
| @Component({ | |
| selector: 'jhi-course-lectures', | |
| templateUrl: './course-lectures.component.html', | |
| styleUrls: ['../course-overview.scss'], | |
| }) | |
| export class CourseLecturesComponent implements OnInit, OnDestroy, AfterViewInit, BarControlConfigurationProvider { | |
| export class CourseLecturesComponent implements OnInit, OnDestroy { | |
| private courseId: number; | |
| private paramSubscription: Subscription; | |
| private parentParamSubscription: Subscription; | |
| private courseUpdatesSubscription: Subscription; | |
| private translateSubscription: Subscription; | |
| public course?: Course; | |
| public weeklyIndexKeys: string[]; | |
| public weeklyLecturesGrouped: object; | |
| public exerciseCountMap: Map<string, number>; | |
| readonly ASC = LectureSortingOrder.ASC; | |
| readonly DESC = LectureSortingOrder.DESC; | |
| // Icons | |
| faSortNumericDown = faSortNumericDown; | |
| faSortNumericUp = faSortNumericUp; | |
| faAngleUp = faAngleUp; | |
| faAngleDown = faAngleDown; | |
| course?: Course; | |
| sortingOrder: LectureSortingOrder; | |
| // The extracted controls template from our template to be rendered in the top bar of "CourseOverviewComponent" | |
| @ViewChild('controls', { static: false }) private controls: TemplateRef<any>; | |
| // Provides the control configuration to be read and used by "CourseOverviewComponent" | |
| public readonly controlConfiguration: BarControlConfiguration = { | |
| subject: new Subject<TemplateRef<any>>(), | |
| }; | |
| lectureSelected: boolean = true; | |
| sidebarData: SidebarData; | |
| accordionLectureGroups: AccordionGroups = DEFAULT_UNIT_GROUPS; | |
| sortedLectures: Lecture[] = []; | |
| sidebarLectures: SidebarCardElement[] = []; | |
| isCollapsed: boolean = false; | |
| constructor( | |
| private courseStorageService: CourseStorageService, | |
| private translateService: TranslateService, | |
| private exerciseService: ExerciseService, | |
| private route: ActivatedRoute, | |
| private localStorage: LocalStorageService, | |
| private router: Router, | |
| private courseOverviewService: CourseOverviewService, | |
| ) {} | |
| ngOnInit() { | |
| this.exerciseCountMap = new Map<string, number>(); | |
| this.loadSortingOrder(); | |
| this.paramSubscription = this.route.parent!.params.subscribe((params) => { | |
| this.courseId = parseInt(params['courseId'], 10); | |
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('lectures'); | |
| this.parentParamSubscription = this.route.parent!.params.subscribe((params) => { | |
| this.courseId = parseInt(params.courseId, 10); | |
| }); | |
| this.course = this.courseStorageService.getCourse(this.courseId); | |
| this.onCourseLoad(); | |
| this.prepareSidebarData(); | |
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | |
| this.course = course; | |
| this.onCourseLoad(); | |
| this.prepareSidebarData(); | |
| }); | |
| this.translateSubscription = this.translateService.onLangChange.subscribe(() => { | |
| this.groupLectures(); | |
| const upcomingLecture = this.courseOverviewService.getUpcomingLecture(this.course?.lectures); | |
| const lastSelectedLecture = this.getLastSelectedLecture(); | |
| this.paramSubscription = this.route.params.subscribe((params) => { | |
| const lectureId = parseInt(params.lectureId, 10); | |
| // If no exercise is selected navigate to the upcoming exercise | |
| if (!lectureId && lastSelectedLecture) { | |
| this.router.navigate([lastSelectedLecture], { relativeTo: this.route, replaceUrl: true }); | |
| } else if (!lectureId && upcomingLecture) { | |
| this.router.navigate([upcomingLecture.id], { relativeTo: this.route, replaceUrl: true }); | |
| } else { | |
| this.lectureSelected = lectureId ? true : false; | |
| } | |
| }); | |
| } | |
| ngAfterViewInit(): void { | |
| // Send our controls template to parent so it will be rendered in the top bar | |
| if (this.controls) { | |
| this.controlConfiguration.subject!.next(this.controls); | |
| prepareSidebarData() { | |
| if (!this.course?.lectures) { | |
| return; | |
| } | |
| this.sortedLectures = this.courseOverviewService.sortLectures(this.course.lectures); | |
| this.sidebarLectures = this.courseOverviewService.mapLecturesToSidebarCardElements(this.sortedLectures); | |
| this.accordionLectureGroups = this.courseOverviewService.groupLecturesByStartDate(this.sortedLectures); | |
| this.updateSidebarData(); | |
| } | |
| ngOnDestroy(): void { | |
| this.translateSubscription.unsubscribe(); | |
| this.courseUpdatesSubscription.unsubscribe(); | |
| this.paramSubscription.unsubscribe(); | |
| updateSidebarData() { | |
| this.sidebarData = { | |
| groupByCategory: true, | |
| storageId: 'lecture', | |
| groupedData: this.accordionLectureGroups, | |
| ungroupedData: this.sidebarLectures, | |
| }; | |
| } | |
| /** | |
| * Loads the sorting order from local storage | |
| */ | |
| private loadSortingOrder() { | |
| const orderInStorage = this.localStorage.retrieve(SortFilterStorageKey.ORDER); | |
| const parsedOrderInStorage = Object.keys(LectureSortingOrder).find((exerciseOrder) => exerciseOrder === orderInStorage); | |
| this.sortingOrder = parsedOrderInStorage ? (+parsedOrderInStorage as LectureSortingOrder) : LectureSortingOrder.ASC; | |
| toggleSidebar() { | |
| this.isCollapsed = !this.isCollapsed; | |
| this.courseOverviewService.setSidebarCollapseState('lecture', this.isCollapsed); | |
| } | |
| private onCourseLoad() { | |
| this.groupLectures(); | |
| getLastSelectedLecture(): string | null { | |
| return sessionStorage.getItem('sidebar.lastSelectedItem.' + 'lecture'); | |
| } | |
| /** | |
| * Reorders all displayed lectures | |
| */ | |
| flipOrder() { | |
| this.sortingOrder = this.sortingOrder === this.ASC ? this.DESC : this.ASC; | |
| this.localStorage.store(SortFilterStorageKey.ORDER, this.sortingOrder.toString()); | |
| this.groupLectures(); | |
| } | |
| public groupLectures(): void { | |
| this.weeklyLecturesGrouped = {}; | |
| this.weeklyIndexKeys = []; | |
| const groupedLectures = {}; | |
| const indexKeys: string[] = []; | |
| const courseLectures = this.course?.lectures ? [...this.course!.lectures] : []; | |
| const sortedLectures = this.sortLectures(courseLectures, this.sortingOrder); | |
| const notAssociatedLectures: Lecture[] = []; | |
| sortedLectures.forEach((lecture) => { | |
| const dateValue = lecture.startDate ? dayjs(lecture.startDate) : undefined; | |
| if (!dateValue) { | |
| notAssociatedLectures.push(lecture); | |
| return; | |
| } | |
| const dateIndex = dateValue ? dayjs(dateValue).startOf('week').format('YYYY-MM-DD') : 'NoDate'; | |
| if (!groupedLectures[dateIndex]) { | |
| indexKeys.push(dateIndex); | |
| if (dateValue) { | |
| groupedLectures[dateIndex] = { | |
| start: dayjs(dateValue).startOf('week'), | |
| end: dayjs(dateValue).endOf('week'), | |
| isCollapsed: dateValue.isBefore(dayjs(), 'week') || dateValue.isAfter(dayjs(), 'week'), | |
| isCurrentWeek: dateValue.isSame(dayjs(), 'week'), | |
| lectures: [], | |
| }; | |
| } else { | |
| groupedLectures[dateIndex] = { | |
| isCollapsed: false, | |
| isCurrentWeek: false, | |
| lectures: [], | |
| }; | |
| } | |
| } | |
| groupedLectures[dateIndex].lectures.push(lecture); | |
| }); | |
| if (notAssociatedLectures.length > 0) { | |
| this.weeklyLecturesGrouped = { | |
| ...groupedLectures, | |
| noDate: { | |
| label: this.translateService.instant('artemisApp.courseOverview.exerciseList.noExerciseDate'), | |
| isCollapsed: false, | |
| isCurrentWeek: false, | |
| lectures: notAssociatedLectures, | |
| }, | |
| }; | |
| this.weeklyIndexKeys = [...indexKeys, 'noDate']; | |
| } else { | |
| this.weeklyLecturesGrouped = groupedLectures; | |
| this.weeklyIndexKeys = indexKeys; | |
| } | |
| } | |
| private sortLectures(lectures: Lecture[], selectedOrder: number): Lecture[] { | |
| return lectures.sort((a, b) => { | |
| const aValue = a.startDate ? a.startDate.valueOf() : dayjs().valueOf(); | |
| const bValue = b.startDate ? b.startDate.valueOf() : dayjs().valueOf(); | |
| return selectedOrder * (aValue - bValue); | |
| }); | |
| ngOnDestroy(): void { | |
| this.courseUpdatesSubscription?.unsubscribe(); | |
| this.paramSubscription?.unsubscribe(); | |
| this.parentParamSubscription?.unsubscribe(); | |
| import { Component, OnDestroy, OnInit } from '@angular/core'; | |
| import { Course } from 'app/entities/course.model'; | |
| import { ActivatedRoute, Router } from '@angular/router'; | |
| import { Subscription } from 'rxjs'; | |
| import { Lecture } from 'app/entities/lecture.model'; | |
| import { CourseStorageService } from 'app/course/manage/course-storage.service'; | |
| import { AccordionGroups, SidebarCardElement, SidebarData } from 'app/types/sidebar'; | |
| import { CourseOverviewService } from '../course-overview.service'; | |
| const DEFAULT_UNIT_GROUPS: AccordionGroups = { | |
| future: { entityData: [] }, | |
| current: { entityData: [] }, | |
| past: { entityData: [] }, | |
| noDate: { entityData: [] }, | |
| }; | |
| @Component({ | |
| selector: 'jhi-course-lectures', | |
| templateUrl: './course-lectures.component.html', | |
| styleUrls: ['../course-overview.scss'], | |
| }) | |
| export class CourseLecturesComponent implements OnInit, OnDestroy { | |
| private courseId: number; | |
| private paramSubscription: Subscription; | |
| private parentParamSubscription: Subscription; | |
| private courseUpdatesSubscription: Subscription; | |
| course?: Course; | |
| lectureSelected: boolean = true; | |
| sidebarData: SidebarData; | |
| accordionLectureGroups: AccordionGroups = DEFAULT_UNIT_GROUPS; | |
| sortedLectures: Lecture[] = []; | |
| sidebarLectures: SidebarCardElement[] = []; | |
| isCollapsed: boolean = false; | |
| private subscriptions: Subscription[] = []; | |
| constructor( | |
| private courseStorageService: CourseStorageService, | |
| private route: ActivatedRoute, | |
| private router: Router, | |
| private courseOverviewService: CourseOverviewService, | |
| ) {} | |
| ngOnInit() { | |
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('lectures'); | |
| this.parentParamSubscription = this.route.parent!.params.subscribe((params) => { | |
| this.courseId = parseInt(params.courseId, 10); | |
| }); | |
| this.course = this.courseStorageService.getCourse(this.courseId); | |
| this.prepareSidebarData(); | |
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | |
| this.course = course; | |
| this.prepareSidebarData(); | |
| }); | |
| const upcomingLecture = this.courseOverviewService.getUpcomingLecture(this.course?.lectures); | |
| const lastSelectedLecture = this.getLastSelectedLecture(); | |
| this.paramSubscription = this.route.params.subscribe((params) => { | |
| const lectureId = parseInt(params.lectureId, 10); | |
| // If no exercise is selected navigate to the upcoming exercise | |
| if (!lectureId && lastSelectedLecture) { | |
| this.router.navigate([lastSelectedLecture], { relativeTo: this.route, replaceUrl: true }); | |
| } else if (!lectureId && upcomingLecture) { | |
| this.router.navigate([upcomingLecture.id], { relativeTo: this.route, replaceUrl: true }); | |
| } else { | |
| this.lectureSelected = lectureId ? true : false; | |
| } | |
| }); | |
| this.subscriptions.push(this.paramSubscription); | |
| this.subscriptions.push(this.courseUpdatesSubscription); | |
| this.subscriptions.push(this.parentParamSubscription); | |
| } | |
| prepareSidebarData() { | |
| if (!this.course?.lectures) { | |
| return; | |
| } | |
| this.sortedLectures = this.courseOverviewService.sortLectures(this.course.lectures); | |
| this.sidebarLectures = this.courseOverviewService.mapLecturesToSidebarCardElements(this.sortedLectures); | |
| this.accordionLectureGroups = this.courseOverviewService.groupLecturesByStartDate(this.sortedLectures); | |
| this.updateSidebarData(); | |
| } | |
| updateSidebarData() { | |
| this.sidebarData = { | |
| groupByCategory: true, | |
| storageId: 'lecture', | |
| groupedData: this.accordionLectureGroups, | |
| ungroupedData: this.sidebarLectures, | |
| }; | |
| } | |
| toggleSidebar() { | |
| this.isCollapsed = !this.isCollapsed; | |
| this.courseOverviewService.setSidebarCollapseState('lecture', this.isCollapsed); | |
| } | |
| getLastSelectedLecture(): string | null { | |
| return sessionStorage.getItem('sidebar.lastSelectedItem.' + 'lecture'); | |
| } | |
| ngOnDestroy(): void { | |
| this.subscriptions.forEach(subscription => subscription.unsubscribe()); | |
| } |
Comment on lines
+44
to
+67
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('lectures'); | ||
| this.parentParamSubscription = this.route.parent!.params.subscribe((params) => { | ||
| this.courseId = parseInt(params.courseId, 10); | ||
| }); | ||
|
|
||
| this.course = this.courseStorageService.getCourse(this.courseId); | ||
| this.onCourseLoad(); | ||
|
|
||
| this.prepareSidebarData(); | ||
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | ||
| this.course = course; | ||
| this.onCourseLoad(); | ||
| this.prepareSidebarData(); | ||
| }); | ||
|
|
||
| this.translateSubscription = this.translateService.onLangChange.subscribe(() => { | ||
| this.groupLectures(); | ||
| const upcomingLecture = this.courseOverviewService.getUpcomingLecture(this.course?.lectures); | ||
| const lastSelectedLecture = this.getLastSelectedLecture(); | ||
| this.paramSubscription = this.route.params.subscribe((params) => { | ||
| const lectureId = parseInt(params.lectureId, 10); | ||
| // If no exercise is selected navigate to the upcoming exercise | ||
| if (!lectureId && lastSelectedLecture) { | ||
| this.router.navigate([lastSelectedLecture], { relativeTo: this.route, replaceUrl: true }); | ||
| } else if (!lectureId && upcomingLecture) { | ||
| this.router.navigate([upcomingLecture.id], { relativeTo: this.route, replaceUrl: true }); | ||
| } else { | ||
| this.lectureSelected = lectureId ? true : false; | ||
| } |
There was a problem hiding this comment.
Refactor the nested subscription pattern to use RxJS operators like switchMap for cleaner and more efficient code, as previously suggested.
Comment on lines
+1
to
+118
| return CourseExercisesComponent.exerciseDueDate(exercise); | ||
| } else { | ||
| return exercise.releaseDate; | ||
| } | ||
| } | ||
|
|
||
| private loadOrderAndAttributeForSorting() { | ||
| const orderInStorage = this.localStorage.retrieve(SortFilterStorageKey.ORDER); | ||
| const parsedOrderInStorage = Object.keys(ExerciseSortingOrder).find((exerciseOrder) => exerciseOrder === orderInStorage); | ||
| this.sortingOrder = parsedOrderInStorage ? (+parsedOrderInStorage as ExerciseSortingOrder) : ExerciseSortingOrder.ASC; | ||
|
|
||
| const attributeInStorage = this.localStorage.retrieve(SortFilterStorageKey.ATTRIBUTE); | ||
| const parsedAttributeInStorage = Object.keys(SortingAttribute).find((exerciseOrder) => exerciseOrder === attributeInStorage); | ||
| this.sortingAttribute = parsedAttributeInStorage ? (+parsedAttributeInStorage as SortingAttribute) : SortingAttribute.DUE_DATE; | ||
| toggleSidebar() { | ||
| this.isCollapsed = !this.isCollapsed; | ||
| this.courseOverviewService.setSidebarCollapseState('exercise', this.isCollapsed); | ||
| } | ||
|
|
||
| private increaseExerciseCounter(exercise: Exercise) { | ||
| if (!this.exerciseCountMap.has(exercise.type!)) { | ||
| this.exerciseCountMap.set(exercise.type!, 1); | ||
| } else { | ||
| let exerciseCount = this.exerciseCountMap.get(exercise.type!)!; | ||
| this.exerciseCountMap.set(exercise.type!, ++exerciseCount); | ||
| } | ||
| getLastSelectedExercise(): string | null { | ||
| return sessionStorage.getItem('sidebar.lastSelectedItem.' + 'exercise'); | ||
| } | ||
|
|
||
| private updateUpcomingExercises(upcomingExercises: Exercise[]) { | ||
| const sortedExercises = (this.sortExercises(upcomingExercises, ExerciseSortingOrder.ASC, SortingAttribute.DUE_DATE) || []).map((exercise) => { | ||
| return { exercise, dueDate: CourseExercisesComponent.exerciseDueDate(exercise) }; | ||
| }); | ||
|
|
||
| if (upcomingExercises.length <= 5) { | ||
| this.upcomingExercises = sortedExercises; | ||
| } else { | ||
| // sort after selected date and take the first 5 elements | ||
| this.upcomingExercises = sortedExercises.slice(0, 5); | ||
| prepareSidebarData() { | ||
| if (!this.course?.exercises) { | ||
| return; | ||
| } | ||
| this.sortedExercises = this.courseOverviewService.sortExercises(this.course.exercises); | ||
| this.sidebarExercises = this.courseOverviewService.mapExercisesToSidebarCardElements(this.sortedExercises); | ||
| this.accordionExerciseGroups = this.courseOverviewService.groupExercisesByDueDate(this.sortedExercises); | ||
| this.updateSidebarData(); | ||
| } | ||
|
|
||
| public static exerciseDueDate(exercise: Exercise): dayjs.Dayjs | undefined { | ||
| return getExerciseDueDate(exercise, CourseExercisesComponent.studentParticipation(exercise)); | ||
| updateSidebarData() { | ||
| this.sidebarData = { | ||
| groupByCategory: true, | ||
| sidebarType: 'exercise', | ||
| storageId: 'exercise', | ||
| groupedData: this.accordionExerciseGroups, | ||
| ungroupedData: this.sidebarExercises, | ||
| }; | ||
| } | ||
|
|
||
| private static studentParticipation(exercise: Exercise): StudentParticipation | undefined { | ||
| return exercise.studentParticipations && exercise.studentParticipations.length > 0 ? exercise.studentParticipations[0] : undefined; | ||
| } | ||
|
|
||
| private updateCourseInformationWidget(exercises?: Exercise[]) { | ||
| this.exerciseCountMap = new Map<string, number>(); | ||
| const upcomingExercises: Exercise[] = []; | ||
| exercises?.forEach((exercise) => { | ||
| const dateValue = CourseExercisesComponent.getSortingAttributeFromExercise(exercise, SortingAttribute.DUE_DATE); | ||
| if (exercise.dueDate && !dayjs().isAfter(dateValue, 'day')) { | ||
| upcomingExercises.push(exercise); | ||
| } | ||
|
|
||
| this.increaseExerciseCounter(exercise); | ||
| }); | ||
|
|
||
| this.updateUpcomingExercises(upcomingExercises); | ||
| this.calcNumberOfExercises(); | ||
| private onCourseLoad() { | ||
| this.programmingSubmissionService.initializeCacheForStudent(this.course?.exercises, true); | ||
| } | ||
|
|
||
| private sortExercises(exercises?: Exercise[], sortOrder?: ExerciseSortingOrder, sortAttribute?: SortingAttribute) { | ||
| const sortingAttribute = sortAttribute ?? this.sortingAttribute; | ||
| const sortingOrder = sortOrder ?? this.sortingOrder; | ||
|
|
||
| return exercises?.sort((exerciseA, exerciseB) => { | ||
| const sortingAttributeA = CourseExercisesComponent.getSortingAttributeFromExercise(exerciseA, sortingAttribute); | ||
| const sortingAttributeB = CourseExercisesComponent.getSortingAttributeFromExercise(exerciseB, sortingAttribute); | ||
| const aValue = sortingAttributeA ? sortingAttributeA.second(0).millisecond(0).valueOf() : dayjs().valueOf(); | ||
| const bValue = sortingAttributeB ? sortingAttributeB.second(0).millisecond(0).valueOf() : dayjs().valueOf(); | ||
| const titleSortValue = exerciseA.title && exerciseB.title ? exerciseA.title.localeCompare(exerciseB.title) : 0; | ||
| return sortingOrder.valueOf() * (aValue - bValue === 0 ? titleSortValue : aValue - bValue); | ||
| }); | ||
| ngOnDestroy(): void { | ||
| this.courseUpdatesSubscription?.unsubscribe(); | ||
| this.paramSubscription?.unsubscribe(); | ||
| this.parentParamSubscription?.unsubscribe(); |
There was a problem hiding this comment.
Ensure all subscriptions are properly unsubscribed in the ngOnDestroy lifecycle hook to prevent memory leaks, as previously suggested.
+ private subscriptions: Subscription[] = [];
ngOnInit() {
+ this.subscriptions.push(this.paramSubscription);
+ this.subscriptions.push(this.courseUpdatesSubscription);
+ this.subscriptions.push(this.parentParamSubscription);
}
ngOnDestroy(): void {
+ this.subscriptions.forEach(subscription => subscription.unsubscribe());
}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.
Suggested change
| import { Component, OnDestroy, OnInit } from '@angular/core'; | |
| import { Course } from 'app/entities/course.model'; | |
| import { ActivatedRoute } from '@angular/router'; | |
| import { Subject, Subscription } from 'rxjs'; | |
| import { TranslateService } from '@ngx-translate/core'; | |
| import dayjs from 'dayjs/esm'; | |
| import { flatten, maxBy, sum } from 'lodash-es'; | |
| import { ActivatedRoute, Router } from '@angular/router'; | |
| import { Subscription } from 'rxjs'; | |
| import { GuidedTourService } from 'app/guided-tour/guided-tour.service'; | |
| import { courseExerciseOverviewTour } from 'app/guided-tour/tours/course-exercise-overview-tour'; | |
| import { isOrion } from 'app/shared/orion/orion'; | |
| import { ProgrammingSubmissionService } from 'app/exercises/programming/participate/programming-submission.service'; | |
| import { LocalStorageService } from 'ngx-webstorage'; | |
| import { Exercise, ExerciseType, IncludedInOverallScore } from 'app/entities/exercise.model'; | |
| import { QuizExercise } from 'app/entities/quiz/quiz-exercise.model'; | |
| import { getExerciseDueDate, hasExerciseDueDatePassed } from 'app/exercises/shared/exercise/exercise.utils'; | |
| import { faAngleDown, faAngleUp, faFilter, faMagnifyingGlass, faPlayCircle, faSortNumericDown, faSortNumericUp, faXmark } from '@fortawesome/free-solid-svg-icons'; | |
| import { StudentParticipation } from 'app/entities/participation/student-participation.model'; | |
| import { BarControlConfiguration, BarControlConfigurationProvider } from 'app/shared/tab-bar/tab-bar'; | |
| import { ExerciseFilter as ExerciseFilterModel } from 'app/entities/exercise-filter.model'; | |
| import { Exercise } from 'app/entities/exercise.model'; | |
| import { CourseStorageService } from 'app/course/manage/course-storage.service'; | |
| import { CourseUnenrollmentModalComponent } from 'app/overview/course-unenrollment-modal.component'; | |
| import { AccordionGroups, SidebarCardElement, SidebarData } from 'app/types/sidebar'; | |
| import { CourseOverviewService } from '../course-overview.service'; | |
| export enum ExerciseFilter { | |
| OVERDUE = 'OVERDUE', | |
| NEEDS_WORK = 'NEEDS_WORK', | |
| UNRELEASED = 'UNRELEASED', | |
| OPTIONAL = 'OPTIONAL', | |
| } | |
| export enum ExerciseSortingOrder { | |
| ASC = 1, | |
| DESC = -1, | |
| } | |
| enum SortFilterStorageKey { | |
| FILTER = 'artemis.course.exercises.filter', | |
| ORDER = 'artemis.course.exercises.order', | |
| ATTRIBUTE = 'artemis.course.exercises.attribute', | |
| } | |
| export enum SortingAttribute { | |
| DUE_DATE = 0, | |
| RELEASE_DATE = 1, | |
| } | |
| export interface ExerciseWithDueDate { | |
| exercise: Exercise; | |
| dueDate?: dayjs.Dayjs; | |
| } | |
| const DEFAULT_UNIT_GROUPS: AccordionGroups = { | |
| future: { entityData: [] }, | |
| current: { entityData: [] }, | |
| past: { entityData: [] }, | |
| noDate: { entityData: [] }, | |
| }; | |
| @Component({ | |
| selector: 'jhi-course-exercises', | |
| templateUrl: './course-exercises.component.html', | |
| styleUrls: ['../course-overview.scss'], | |
| }) | |
| export class CourseExercisesComponent implements OnInit, OnDestroy, AfterViewInit, BarControlConfigurationProvider { | |
| export class CourseExercisesComponent implements OnInit, OnDestroy { | |
| private courseId: number; | |
| private paramSubscription: Subscription; | |
| private parentParamSubscription: Subscription; | |
| private courseUpdatesSubscription: Subscription; | |
| private translateSubscription: Subscription; | |
| public course?: Course; | |
| public upcomingExercises: ExerciseWithDueDate[] = []; | |
| public exerciseCountMap: Map<string, number>; | |
| readonly ASC = ExerciseSortingOrder.ASC; | |
| readonly DESC = ExerciseSortingOrder.DESC; | |
| readonly DUE_DATE = SortingAttribute.DUE_DATE; | |
| readonly RELEASE_DATE = SortingAttribute.RELEASE_DATE; | |
| readonly filterType = ExerciseFilter; | |
| sortingOrder: ExerciseSortingOrder; | |
| activeFilters: Set<ExerciseFilter>; | |
| numberOfExercises: number; | |
| course?: Course; | |
| sortedExercises?: Exercise[]; | |
| exerciseForGuidedTour?: Exercise; | |
| sortingAttribute: SortingAttribute; | |
| searchExercisesInput: string; | |
| exerciseFilter: ExerciseFilterModel; | |
| filteredAndSortedExercises: Exercise[] | undefined; | |
| showExercisesGroupedByTimeframe: boolean = true; | |
| lastAppliedSearchString: string | undefined; | |
| // Icons | |
| faPlayCircle = faPlayCircle; | |
| faFilter = faFilter; | |
| faAngleUp = faAngleUp; | |
| faAngleDown = faAngleDown; | |
| faSortNumericUp = faSortNumericUp; | |
| faSortNumericDown = faSortNumericDown; | |
| faMagnifyingGlass = faMagnifyingGlass; | |
| faXmark = faXmark; | |
| // The extracted controls template from our template to be rendered in the top bar of "CourseOverviewComponent" | |
| @ViewChild('controls', { static: false }) private controls: TemplateRef<any>; | |
| // Provides the control configuration to be read and used by "CourseOverviewComponent" | |
| public readonly controlConfiguration: BarControlConfiguration = { | |
| subject: new Subject<TemplateRef<any>>(), | |
| }; | |
| exerciseSelected: boolean = true; | |
| accordionExerciseGroups: AccordionGroups = DEFAULT_UNIT_GROUPS; | |
| sidebarData: SidebarData; | |
| sidebarExercises: SidebarCardElement[] = []; | |
| isCollapsed: boolean = false; | |
| constructor( | |
| private courseStorageService: CourseStorageService, | |
| private translateService: TranslateService, | |
| private route: ActivatedRoute, | |
| private guidedTourService: GuidedTourService, | |
| private programmingSubmissionService: ProgrammingSubmissionService, | |
| private localStorage: LocalStorageService, | |
| private modalService: NgbModal, | |
| private router: Router, | |
| private courseOverviewService: CourseOverviewService, | |
| ) {} | |
| ngOnInit() { | |
| this.exerciseCountMap = new Map<string, number>(); | |
| this.exerciseFilter = new ExerciseFilterModel(); | |
| this.numberOfExercises = 0; | |
| this.searchExercisesInput = ''; | |
| const filters = this.localStorage.retrieve(SortFilterStorageKey.FILTER); | |
| const filtersInStorage = filters | |
| ? filters | |
| .split(',') | |
| .map((filter: string) => ExerciseFilter[filter]) | |
| .filter(Boolean) | |
| : []; | |
| this.activeFilters = new Set(filtersInStorage); | |
| this.loadOrderAndAttributeForSorting(); | |
| this.paramSubscription = this.route.parent!.params.subscribe((params) => { | |
| this.courseId = parseInt(params['courseId'], 10); | |
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('exercise'); | |
| this.parentParamSubscription = this.route.parent!.params.subscribe((params) => { | |
| this.courseId = parseInt(params.courseId, 10); | |
| }); | |
| this.course = this.courseStorageService.getCourse(this.courseId); | |
| this.onCourseLoad(); | |
| this.prepareSidebarData(); | |
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | |
| this.course = course; | |
| this.prepareSidebarData(); | |
| this.onCourseLoad(); | |
| }); | |
| this.translateSubscription = this.translateService.onLangChange.subscribe(() => { | |
| this.applyFiltersAndOrder(); | |
| }); | |
| this.exerciseForGuidedTour = this.guidedTourService.enableTourForCourseExerciseComponent(this.course, courseExerciseOverviewTour, true); | |
| const upcomingExercise = this.courseOverviewService.getUpcomingExercise(this.course?.exercises); | |
| const lastSelectedExercise = this.getLastSelectedExercise(); | |
| this.paramSubscription = this.route.params.subscribe((params) => { | |
| const exerciseId = parseInt(params.exerciseId, 10); | |
| // If no exercise is selected navigate to the lastSelected or upcoming exercise | |
| if (!exerciseId && lastSelectedExercise) { | |
| this.router.navigate([lastSelectedExercise], { relativeTo: this.route, replaceUrl: true }); | |
| } else if (!exerciseId && upcomingExercise) { | |
| this.router.navigate([upcomingExercise.id], { relativeTo: this.route, replaceUrl: true }); | |
| } else { | |
| this.exerciseSelected = exerciseId ? true : false; | |
| } | |
| }); | |
| } | |
| ngAfterViewInit(): void { | |
| // Send our controls template to parent so it will be rendered in the top bar | |
| if (this.controls) { | |
| this.controlConfiguration.subject!.next(this.controls); | |
| } | |
| } | |
| setSortingAttribute(attribute: SortingAttribute) { | |
| this.sortingAttribute = attribute; | |
| this.localStorage.store(SortFilterStorageKey.ATTRIBUTE, this.sortingAttribute.toString()); | |
| this.applyFiltersAndOrder(); | |
| } | |
| ngOnDestroy(): void { | |
| this.translateSubscription?.unsubscribe(); | |
| this.courseUpdatesSubscription?.unsubscribe(); | |
| this.paramSubscription?.unsubscribe(); | |
| } | |
| private onCourseLoad() { | |
| this.programmingSubmissionService.initializeCacheForStudent(this.course!.exercises, true); | |
| this.applyFiltersAndOrder(); | |
| } | |
| private calcNumberOfExercises() { | |
| this.numberOfExercises = sum(Array.from(this.exerciseCountMap.values())); | |
| } | |
| /** | |
| * Reorders all displayed exercises | |
| */ | |
| flipOrder() { | |
| this.sortingOrder = this.sortingOrder === this.ASC ? this.DESC : this.ASC; | |
| this.localStorage.store(SortFilterStorageKey.ORDER, this.sortingOrder.toString()); | |
| this.applyFiltersAndOrder(); | |
| } | |
| /** | |
| * Filters all displayed exercises by applying the selected activeFilters | |
| * @param filters The filters which should be applied | |
| */ | |
| toggleFilters(filters: ExerciseFilter[]) { | |
| filters.forEach((filter) => (this.activeFilters.has(filter) ? this.activeFilters.delete(filter) : this.activeFilters.add(filter))); | |
| this.localStorage.store(SortFilterStorageKey.FILTER, Array.from(this.activeFilters).join(',')); | |
| this.applyFiltersAndOrder(); | |
| } | |
| isVisibleToStudents(exercise: Exercise): boolean | undefined { | |
| return !this.activeFilters.has(ExerciseFilter.UNRELEASED) || (exercise as QuizExercise)?.visibleToStudents; | |
| } | |
| setShowExercisesGroupedByDueDateCategory(updatedShowExercisesGroupedByDueDateCategory: boolean) { | |
| this.showExercisesGroupedByTimeframe = updatedShowExercisesGroupedByDueDateCategory; | |
| } | |
| /** | |
| * Method is called when enter key is pressed on search input or search button is clicked | |
| */ | |
| onSearch() { | |
| this.searchExercisesInput = this.searchExercisesInput.trim(); | |
| this.exerciseFilter = new ExerciseFilterModel(this.searchExercisesInput); | |
| this.applyFiltersAndOrder(); | |
| this.lastAppliedSearchString = this.exerciseFilter.exerciseNameSearch; | |
| } | |
| get canUnenroll(): boolean { | |
| return !!this.course?.unenrollmentEnabled && dayjs().isBefore(this.course?.unenrollmentEndDate); | |
| } | |
| /** | |
| * Method is called when unenroll button is clicked | |
| */ | |
| onUnenroll() { | |
| const modalRef = this.modalService.open(CourseUnenrollmentModalComponent, { size: 'xl' }); | |
| modalRef.componentInstance.course = this.course; | |
| } | |
| /** | |
| * Checks if the given exercise still needs work, i.e. wasn't even started yet or is not graded with 100% | |
| * @param exercise The exercise which should get checked | |
| */ | |
| private static needsWork(exercise: Exercise): boolean { | |
| const latestResult = maxBy(flatten(exercise.studentParticipations?.map((participation) => participation.results)), 'completionDate'); | |
| return !latestResult || !latestResult.score || latestResult.score < 100; | |
| } | |
| /** | |
| * Applies all selected activeFilters and orders and groups the user's exercises | |
| */ | |
| private applyFiltersAndOrder() { | |
| let filtered = this.course?.exercises?.filter((exercise) => CourseExercisesComponent.fulfillsCurrentFilter(exercise, this.activeFilters)); | |
| filtered = filtered?.filter((exercise) => this.exerciseFilter.matchesExercise(exercise)); | |
| this.filteredAndSortedExercises = this.sortExercises(filtered); | |
| this.updateCourseInformationWidget(filtered); | |
| } | |
| /** | |
| * Check if the given exercise fulfills the currently selected filters | |
| * @param exercise to check | |
| * @param activeFilters that are applied | |
| * @return true if the given exercise fulfills the currently selected filters; false otherwise | |
| */ | |
| public static fulfillsCurrentFilter(exercise: Exercise, activeFilters: Set<ExerciseFilter>): boolean { | |
| const needsWorkFilterActive = activeFilters.has(ExerciseFilter.NEEDS_WORK); | |
| const overdueFilterActive = activeFilters.has(ExerciseFilter.OVERDUE); | |
| const unreleasedFilterActive = activeFilters.has(ExerciseFilter.UNRELEASED); | |
| const optionalFilterActive = activeFilters.has(ExerciseFilter.OPTIONAL); | |
| const participation = CourseExercisesComponent.studentParticipation(exercise); | |
| return !!( | |
| (!needsWorkFilterActive || CourseExercisesComponent.needsWork(exercise)) && | |
| (!exercise.dueDate || !overdueFilterActive || !hasExerciseDueDatePassed(exercise, participation)) && | |
| (!exercise.releaseDate || !unreleasedFilterActive || (exercise as QuizExercise)?.visibleToStudents) && | |
| (!optionalFilterActive || exercise.includedInOverallScore !== IncludedInOverallScore.NOT_INCLUDED) && | |
| (!isOrion || exercise.type === ExerciseType.PROGRAMMING) | |
| ); | |
| } | |
| public static getSortingAttributeFromExercise(exercise: Exercise, sortingAttribute?: SortingAttribute): dayjs.Dayjs | undefined { | |
| if (sortingAttribute === SortingAttribute.DUE_DATE) { | |
| return CourseExercisesComponent.exerciseDueDate(exercise); | |
| } else { | |
| return exercise.releaseDate; | |
| } | |
| } | |
| private loadOrderAndAttributeForSorting() { | |
| const orderInStorage = this.localStorage.retrieve(SortFilterStorageKey.ORDER); | |
| const parsedOrderInStorage = Object.keys(ExerciseSortingOrder).find((exerciseOrder) => exerciseOrder === orderInStorage); | |
| this.sortingOrder = parsedOrderInStorage ? (+parsedOrderInStorage as ExerciseSortingOrder) : ExerciseSortingOrder.ASC; | |
| const attributeInStorage = this.localStorage.retrieve(SortFilterStorageKey.ATTRIBUTE); | |
| const parsedAttributeInStorage = Object.keys(SortingAttribute).find((exerciseOrder) => exerciseOrder === attributeInStorage); | |
| this.sortingAttribute = parsedAttributeInStorage ? (+parsedAttributeInStorage as SortingAttribute) : SortingAttribute.DUE_DATE; | |
| toggleSidebar() { | |
| this.isCollapsed = !this.isCollapsed; | |
| this.courseOverviewService.setSidebarCollapseState('exercise', this.isCollapsed); | |
| } | |
| private increaseExerciseCounter(exercise: Exercise) { | |
| if (!this.exerciseCountMap.has(exercise.type!)) { | |
| this.exerciseCountMap.set(exercise.type!, 1); | |
| } else { | |
| let exerciseCount = this.exerciseCountMap.get(exercise.type!)!; | |
| this.exerciseCountMap.set(exercise.type!, ++exerciseCount); | |
| } | |
| getLastSelectedExercise(): string | null { | |
| return sessionStorage.getItem('sidebar.lastSelectedItem.' + 'exercise'); | |
| } | |
| private updateUpcomingExercises(upcomingExercises: Exercise[]) { | |
| const sortedExercises = (this.sortExercises(upcomingExercises, ExerciseSortingOrder.ASC, SortingAttribute.DUE_DATE) || []).map((exercise) => { | |
| return { exercise, dueDate: CourseExercisesComponent.exerciseDueDate(exercise) }; | |
| }); | |
| if (upcomingExercises.length <= 5) { | |
| this.upcomingExercises = sortedExercises; | |
| } else { | |
| // sort after selected date and take the first 5 elements | |
| this.upcomingExercises = sortedExercises.slice(0, 5); | |
| prepareSidebarData() { | |
| if (!this.course?.exercises) { | |
| return; | |
| } | |
| this.sortedExercises = this.courseOverviewService.sortExercises(this.course.exercises); | |
| this.sidebarExercises = this.courseOverviewService.mapExercisesToSidebarCardElements(this.sortedExercises); | |
| this.accordionExerciseGroups = this.courseOverviewService.groupExercisesByDueDate(this.sortedExercises); | |
| this.updateSidebarData(); | |
| } | |
| public static exerciseDueDate(exercise: Exercise): dayjs.Dayjs | undefined { | |
| return getExerciseDueDate(exercise, CourseExercisesComponent.studentParticipation(exercise)); | |
| updateSidebarData() { | |
| this.sidebarData = { | |
| groupByCategory: true, | |
| sidebarType: 'exercise', | |
| storageId: 'exercise', | |
| groupedData: this.accordionExerciseGroups, | |
| ungroupedData: this.sidebarExercises, | |
| }; | |
| } | |
| private static studentParticipation(exercise: Exercise): StudentParticipation | undefined { | |
| return exercise.studentParticipations && exercise.studentParticipations.length > 0 ? exercise.studentParticipations[0] : undefined; | |
| } | |
| private updateCourseInformationWidget(exercises?: Exercise[]) { | |
| this.exerciseCountMap = new Map<string, number>(); | |
| const upcomingExercises: Exercise[] = []; | |
| exercises?.forEach((exercise) => { | |
| const dateValue = CourseExercisesComponent.getSortingAttributeFromExercise(exercise, SortingAttribute.DUE_DATE); | |
| if (exercise.dueDate && !dayjs().isAfter(dateValue, 'day')) { | |
| upcomingExercises.push(exercise); | |
| } | |
| this.increaseExerciseCounter(exercise); | |
| }); | |
| this.updateUpcomingExercises(upcomingExercises); | |
| this.calcNumberOfExercises(); | |
| private onCourseLoad() { | |
| this.programmingSubmissionService.initializeCacheForStudent(this.course?.exercises, true); | |
| } | |
| private sortExercises(exercises?: Exercise[], sortOrder?: ExerciseSortingOrder, sortAttribute?: SortingAttribute) { | |
| const sortingAttribute = sortAttribute ?? this.sortingAttribute; | |
| const sortingOrder = sortOrder ?? this.sortingOrder; | |
| return exercises?.sort((exerciseA, exerciseB) => { | |
| const sortingAttributeA = CourseExercisesComponent.getSortingAttributeFromExercise(exerciseA, sortingAttribute); | |
| const sortingAttributeB = CourseExercisesComponent.getSortingAttributeFromExercise(exerciseB, sortingAttribute); | |
| const aValue = sortingAttributeA ? sortingAttributeA.second(0).millisecond(0).valueOf() : dayjs().valueOf(); | |
| const bValue = sortingAttributeB ? sortingAttributeB.second(0).millisecond(0).valueOf() : dayjs().valueOf(); | |
| const titleSortValue = exerciseA.title && exerciseB.title ? exerciseA.title.localeCompare(exerciseB.title) : 0; | |
| return sortingOrder.valueOf() * (aValue - bValue === 0 ? titleSortValue : aValue - bValue); | |
| }); | |
| ngOnDestroy(): void { | |
| this.courseUpdatesSubscription?.unsubscribe(); | |
| this.paramSubscription?.unsubscribe(); | |
| this.parentParamSubscription?.unsubscribe(); | |
| import { Component, OnDestroy, OnInit } from '@angular/core'; | |
| import { Course } from 'app/entities/course.model'; | |
| import { ActivatedRoute, Router } from '@angular/router'; | |
| import { Subscription } from 'rxjs'; | |
| import { GuidedTourService } from 'app/guided-tour/guided-tour.service'; | |
| import { courseExerciseOverviewTour } from 'app/guided-tour/tours/course-exercise-overview-tour'; | |
| import { ProgrammingSubmissionService } from 'app/exercises/programming/participate/programming-submission.service'; | |
| import { Exercise } from 'app/entities/exercise.model'; | |
| import { CourseStorageService } from 'app/course/manage/course-storage.service'; | |
| import { AccordionGroups, SidebarCardElement, SidebarData } from 'app/types/sidebar'; | |
| import { CourseOverviewService } from '../course-overview.service'; | |
| const DEFAULT_UNIT_GROUPS: AccordionGroups = { | |
| future: { entityData: [] }, | |
| current: { entityData: [] }, | |
| past: { entityData: [] }, | |
| noDate: { entityData: [] }, | |
| }; | |
| @Component({ | |
| selector: 'jhi-course-exercises', | |
| templateUrl: './course-exercises.component.html', | |
| styleUrls: ['../course-overview.scss'], | |
| }) | |
| export class CourseExercisesComponent implements OnInit, OnDestroy { | |
| private courseId: number; | |
| private paramSubscription: Subscription; | |
| private parentParamSubscription: Subscription; | |
| private courseUpdatesSubscription: Subscription; | |
| private subscriptions: Subscription[] = []; | |
| course?: Course; | |
| sortedExercises?: Exercise[]; | |
| exerciseForGuidedTour?: Exercise; | |
| exerciseSelected: boolean = true; | |
| accordionExerciseGroups: AccordionGroups = DEFAULT_UNIT_GROUPS; | |
| sidebarData: SidebarData; | |
| sidebarExercises: SidebarCardElement[] = []; | |
| isCollapsed: boolean = false; | |
| constructor( | |
| private courseStorageService: CourseStorageService, | |
| private route: ActivatedRoute, | |
| private guidedTourService: GuidedTourService, | |
| private programmingSubmissionService: ProgrammingSubmissionService, | |
| private router: Router, | |
| private courseOverviewService: CourseOverviewService, | |
| ) {} | |
| ngOnInit() { | |
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('exercise'); | |
| this.parentParamSubscription = this.route.parent!.params.subscribe((params) => { | |
| this.courseId = parseInt(params.courseId, 10); | |
| }); | |
| this.course = this.courseStorageService.getCourse(this.courseId); | |
| this.onCourseLoad(); | |
| this.prepareSidebarData(); | |
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | |
| this.course = course; | |
| this.prepareSidebarData(); | |
| this.onCourseLoad(); | |
| }); | |
| this.exerciseForGuidedTour = this.guidedTourService.enableTourForCourseExerciseComponent(this.course, courseExerciseOverviewTour, true); | |
| const upcomingExercise = this.courseOverviewService.getUpcomingExercise(this.course?.exercises); | |
| const lastSelectedExercise = this.getLastSelectedExercise(); | |
| this.paramSubscription = this.route.params.subscribe((params) => { | |
| const exerciseId = parseInt(params.exerciseId, 10); | |
| // If no exercise is selected navigate to the lastSelected or upcoming exercise | |
| if (!exerciseId && lastSelectedExercise) { | |
| this.router.navigate([lastSelectedExercise], { relativeTo: this.route, replaceUrl: true }); | |
| } else if (!exerciseId && upcomingExercise) { | |
| this.router.navigate([upcomingExercise.id], { relativeTo: this.route, replaceUrl: true }); | |
| } else { | |
| this.exerciseSelected = exerciseId ? true : false; | |
| } | |
| }); | |
| this.subscriptions.push(this.paramSubscription); | |
| this.subscriptions.push(this.courseUpdatesSubscription); | |
| this.subscriptions.push(this.parentParamSubscription); | |
| } | |
| toggleSidebar() { | |
| this.isCollapsed = !this.isCollapsed; | |
| this.courseOverviewService.setSidebarCollapseState('exercise', this.isCollapsed); | |
| } | |
| getLastSelectedExercise(): string | null { | |
| return sessionStorage.getItem('sidebar.lastSelectedItem.' + 'exercise'); | |
| } | |
| prepareSidebarData() { | |
| if (!this.course?.exercises) { | |
| return; | |
| } | |
| this.sortedExercises = this.courseOverviewService.sortExercises(this.course.exercises); | |
| this.sidebarExercises = this.courseOverviewService.mapExercisesToSidebarCardElements(this.sortedExercises); | |
| this.accordionExerciseGroups = this.courseOverviewService.groupExercisesByDueDate(this.sortedExercises); | |
| this.updateSidebarData(); | |
| } | |
| updateSidebarData() { | |
| this.sidebarData = { | |
| groupByCategory: true, | |
| sidebarType: 'exercise', | |
| storageId: 'exercise', | |
| groupedData: this.accordionExerciseGroups, | |
| ungroupedData: this.sidebarExercises, | |
| }; | |
| } | |
| private onCourseLoad() { | |
| this.programmingSubmissionService.initializeCacheForStudent(this.course?.exercises, true); | |
| } | |
| ngOnDestroy(): void { | |
| this.subscriptions.forEach(subscription => subscription.unsubscribe()); | |
| this.courseUpdatesSubscription?.unsubscribe(); | |
| this.paramSubscription?.unsubscribe(); | |
| this.parentParamSubscription?.unsubscribe(); |
Comment on lines
+51
to
+79
| this.isCollapsed = this.courseOverviewService.getSidebarCollapseStateFromStorage('exercise'); | ||
| this.parentParamSubscription = this.route.parent!.params.subscribe((params) => { | ||
| this.courseId = parseInt(params.courseId, 10); | ||
| }); | ||
|
|
||
| this.course = this.courseStorageService.getCourse(this.courseId); | ||
| this.onCourseLoad(); | ||
| this.prepareSidebarData(); | ||
|
|
||
| this.courseUpdatesSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course: Course) => { | ||
| this.course = course; | ||
| this.prepareSidebarData(); | ||
| this.onCourseLoad(); | ||
| }); | ||
|
|
||
| this.translateSubscription = this.translateService.onLangChange.subscribe(() => { | ||
| this.applyFiltersAndOrder(); | ||
| }); | ||
|
|
||
| this.exerciseForGuidedTour = this.guidedTourService.enableTourForCourseExerciseComponent(this.course, courseExerciseOverviewTour, true); | ||
| const upcomingExercise = this.courseOverviewService.getUpcomingExercise(this.course?.exercises); | ||
| const lastSelectedExercise = this.getLastSelectedExercise(); | ||
| this.paramSubscription = this.route.params.subscribe((params) => { | ||
| const exerciseId = parseInt(params.exerciseId, 10); | ||
| // If no exercise is selected navigate to the lastSelected or upcoming exercise | ||
| if (!exerciseId && lastSelectedExercise) { | ||
| this.router.navigate([lastSelectedExercise], { relativeTo: this.route, replaceUrl: true }); | ||
| } else if (!exerciseId && upcomingExercise) { | ||
| this.router.navigate([upcomingExercise.id], { relativeTo: this.route, replaceUrl: true }); | ||
| } else { | ||
| this.exerciseSelected = exerciseId ? true : false; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Refactor the nested subscription pattern to use RxJS operators like switchMap for cleaner and more efficient code, as previously suggested.
src/main/webapp/app/overview/course-exercises/course-exercises.component.ts
Show resolved
Hide resolved
rabeatwork
force-pushed
the
enhancement/create-card-sidebar-component
branch
from
0c7db4b
to
6dd0003
Compare
src/main/webapp/app/shared/sidebar/sidebar-card/sidebar-card.component.scss
Show resolved
Hide resolved
|
|
||
| .scrollable-content { | ||
| height: calc(100vh - $footer-height-prod - $header-height - $search-height); | ||
| &.content-height-dev { |
There was a problem hiding this comment.
Add an empty line before the nested rule for better readability.
+ 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.
Suggested change
| &.content-height-dev { | |
| &.content-height-dev { | |
| &.content-height-dev { | ||
| height: calc(100vh - $footer-height-dev - $header-height - $search-height); | ||
| } | ||
| overflow-y: auto; |
There was a problem hiding this comment.
Add an empty line before the declaration for better readability.
+ 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.
Suggested change
| overflow-y: auto; | |
| overflow-y: auto; |
| .sidebar { | ||
| background-color: var(--module-bg); | ||
| height: calc(100vh - $footer-height-prod - $header-height); | ||
| &.sidebar-height-dev { |
There was a problem hiding this comment.
Add an empty line before the nested rule for better readability.
+ 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.
Suggested change
| &.sidebar-height-dev { | |
| &.sidebar-height-dev { | |
| margin-left: $menu-width-closed !important; | ||
| } | ||
| } | ||
| @media screen and (max-width: 960px) { |
There was a problem hiding this comment.
Add an empty line before the at-rule for better readability.
+ 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.
Suggested change
| @media screen and (max-width: 960px) { | |
| @media screen and (max-width: 960px) { |
| .sidebar-container { | ||
| width: $menu-width-closed !important; | ||
| } | ||
| .auto-collapse { |
There was a problem hiding this comment.
Add an empty line before the rule for better readability.
+ 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.
Suggested change
| .auto-collapse { | |
| .auto-collapse { |
| .auto-collapse { | ||
| display: none; | ||
| } | ||
| .newMessage:after { |
There was a problem hiding this comment.
Add an empty line before the rule for better readability.
+ 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.
Suggested change
| .newMessage:after { | |
| .newMessage:after { |
|
|
||
| .btn-sidebar-collapse { | ||
| background-color: var(--link-item-bg); | ||
| &:hover { |
There was a problem hiding this comment.
Add an empty line before the rule for better readability.
+ 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.
Suggested change
| &:hover { | |
| &:hover { |
| background-color: var(--gray-200); | ||
| color: var(--primary); | ||
| } | ||
| &:focus { |
There was a problem hiding this comment.
Add an empty line before the rule for better readability.
+ 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.
Suggested change
| &:focus { | |
| &:focus { |
rstief
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
General
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
This Pull Request marks the second of many steps toward the new design of Artemis
Description
A collapsable sidebar with a search functionality was added to the exercise and lecture view.
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Test Coverage
Client
�
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
New Features
SidebarCardComponentfor managing sidebar selections.Enhancements
Refactor
Style
Documentation
Bug Fixes