-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exam mode
: Fix navigation to exams list and add no-exam hint
#7834
Conversation
WalkthroughThe recent updates involve enhancing user interaction with a course overview component by changing how the "Exams" link functions. Instead of navigating via a router link, a click event now handles the transition to the exams page. Additionally, the display logic for exam sections has been refined, and a new icon has been introduced to the exams component for better visual representation. Changes
Poem
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? TipsChat with CodeRabbit Bot (
|
Exam mode
: Fix a navigation issue on the course cardExam mode
: Fix navigation to exams list and add no-exam hint in course card
Exam mode
: Fix navigation to exams list and add no-exam hint in course cardExam mode
: Fix navigation to exams list and add no-exam hint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- src/main/webapp/i18n/de/exam.json
- src/main/webapp/i18n/en/exam.json
Files selected for processing (4)
- src/main/webapp/app/overview/course-card.component.html (1 hunks)
- src/main/webapp/app/overview/course-card.component.ts (1 hunks)
- src/main/webapp/app/overview/course-exams/course-exams.component.html (2 hunks)
- src/main/webapp/app/overview/course-exams/course-exams.component.ts (2 hunks)
Additional comments: 6
src/main/webapp/app/overview/course-exams/course-exams.component.html (3)
1-10: The implementation for displaying a message when no exams are present is correct. It uses proper localization and conditional rendering based on the length of
realExamsOfCourse
andtestExamsOfCourse
. The navigation button is also correctly implemented with[routerLink]
.1-15: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [12-26]
The structure and logic for displaying the real exams and the beginning of the test exams section are consistent and follow Angular best practices. The conditions for rendering are clear and the separation of concerns is maintained.
- 24-30: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [27-32]
The use of
trackBy
inngFor
is a performance optimization that helps Angular identify unique items for DOM updates. The expand/collapse functionality is implemented correctly with appropriate icons and click event binding. The visual separation usinghr
tags is also appropriate.src/main/webapp/app/overview/course-card.component.html (1)
- 31-37: The change from
[routerLink]
to(click)
for the "Exams" link is implemented correctly. It uses an event binding to call thenavigateToExams
method, which is expected to handle the navigation imperatively. This should resolve the navigation issue described in the PR objectives.src/main/webapp/app/overview/course-card.component.ts (1)
- 104-112: The
navigateToExams
method is implemented correctly. It usesRouter
to navigate to the exams page and callsevent.stopPropagation()
to prevent event bubbling, which is necessary due to the nested click events in the component.src/main/webapp/app/overview/course-exams/course-exams.component.ts (1)
- 7-13: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [7-32]
The FontAwesome icons are imported and assigned to properties correctly. The addition of the
faListAlt
icon is consistent with its usage in thecourse-exams.component.html
file for the no-exam state message.
Checklist
General
Client
Motivation and Context
This PR addresses two primary issues in the Artemis platform. First, there was a navigation problem in the course card component, particularly when trying to access the exams list. This bug was significant as it hindered efficient and intuitive navigation within the platform. Secondly, due to recent changes (PR #7826), users could navigate to the exam list even when no exams were present, which could lead to confusion. This PR aims to resolve these issues, thereby improving the user experience.
Description
The PR introduces a change in the navigation method from the course card to the exams list by replacing the nested router link with imperative navigation. This approach resolves the navigation issue. Additionally, a new UI element is added to the exams page, which displays a helpful message and a navigation button when no exams are available. This feature enhances user understanding and navigation in scenarios where the exam list is empty.
Steps for Testing
Prerequisites:
Steps:
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
Screenshots
Summary by CodeRabbit
Bug Fixes
New Features
Style