-
Notifications
You must be signed in to change notification settings - Fork 14
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
style(apps/frontend-manage): display notification if course description is missing #4313
Conversation
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
📝 WalkthroughWalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/frontend-manage/src/pages/courses/[id].tsx (1)
98-108
: LGTM: Notification for missing course description implemented correctlyThe conditional rendering for the course description is well-implemented and aligns with the PR objective. It properly handles cases where the description is missing by displaying a notification with an icon.
Consider adding an
aria-label
to theFontAwesomeIcon
component for improved accessibility. For example:<FontAwesomeIcon icon={faTriangleExclamation} className="text-orange-600" + aria-label={t('manage.course.warningIconLabel')} />
Don't forget to add the corresponding translation key in your localization files.
packages/i18n/messages/en.ts (1)
1568-1568
: Consider renaming the key to 'noDescription' for consistencyIn the existing codebase, similar keys follow the pattern
noEntity
, such asnoSessions
,noPracticeQuizzes
, andnoMicrolearnings
. RenamingnoDescriptionNotification
tonoDescription
would maintain consistency and improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- apps/frontend-manage/src/pages/courses/[id].tsx (2 hunks)
- packages/i18n/messages/de.ts (1 hunks)
- packages/i18n/messages/en.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
apps/frontend-manage/src/pages/courses/[id].tsx (1)
3-6
: LGTM: Icon import added correctlyThe addition of the
faTriangleExclamation
icon import is consistent with the PR objective and follows best practices.packages/i18n/messages/de.ts (1)
1588-1588
: Excellent addition to improve user feedback!The new notification message for missing course descriptions is a valuable addition. It enhances the user interface by explicitly informing users when a description is not available, improving overall user experience.
klicker-uzh Run #3239
Run Properties:
|
Project |
klicker-uzh
|
Run status |
Passed #3239
|
Run duration | 11m 28s |
Commit |
1eab0c357e ℹ️: Merge 758e23c2b45fcb6e4a21e76c6d698f4f28b1bb9b into e402d7ec44ccbc5ceb5078b8cfcd...
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
138
|
klicker-uzh Run #3240
Run Properties:
|
Project |
klicker-uzh
|
Run status |
Passed #3240
|
Run duration | 11m 23s |
Commit |
e1a27ac924: style(apps/frontend-manage): display notification if course description is missi...
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
1
|
Pending |
0
|
Skipped |
0
|
Passing |
138
|
Summary by CodeRabbit
New Features
Localization Updates