style(apps/frontend-manage): update messages shown on creation editors and mark past courses #4300
Conversation
…s and mark past courses
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 updates to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/frontend-manage/src/components/courses/CourseListButton.tsx (2)
35-35: LGTM: NewisPastvariable added with correct logic.The
isPastvariable correctly determines if a course has ended by comparing theendDatewith the current date. The use of optional chaining is appropriate.Consider memoizing this value if the component re-renders frequently:
const isPast = React.useMemo(() => endDate ? dayjs(endDate).isBefore(dayjs()) : false, [endDate] );This optimization is optional and depends on the component's re-render frequency and performance needs.
64-72: LGTM: Badge rendering logic updated correctly.The changes improve the organization of badges and correctly implement the new "ended" badge based on the
isPastcondition. The use of icons and translations is appropriate.For consistency, consider applying the same hover effect to the "archived" badge as used in the "ended" badge:
- {isArchived && <Badge>{t('shared.generic.archived')}</Badge>} + {isArchived && ( + <Badge className="hover:bg-gray-700"> + {t('shared.generic.archived')} + </Badge> + )}This change would provide a consistent user experience across all badges.
packages/i18n/messages/en.ts (1)
214-214: LGTM! Consider adding lowercase variant for flexibility.The addition of
ended: 'Ended'is appropriate and well-placed in thegenericsection. It provides a clear status indicator for ended activities or events.For consistency with other status indicators in this file (e.g.,
running: 'Running'), consider adding a lowercase variant:ended: 'Ended', +endedLowercase: 'ended',This addition would provide flexibility for different UI contexts where a lowercase version might be preferred.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- apps/frontend-manage/src/components/courses/CourseListButton.tsx (3 hunks)
- apps/frontend-manage/src/pages/courses/index.tsx (1 hunks)
- packages/i18n/messages/de.ts (4 hunks)
- packages/i18n/messages/en.ts (4 hunks)
🧰 Additional context used
🔇 Additional comments (7)
apps/frontend-manage/src/components/courses/CourseListButton.tsx (2)
2-2: LGTM: New icon import added correctly.The
faCheckicon is imported from the appropriate package and is used later in the component for the new "ended" badge.
Line range hint
1-78: Overall, the changes to CourseListButton.tsx are well-implemented.The modifications enhance the component's functionality by introducing the "ended" course status and improving the organization of badges. The code is clean, and the logic is sound. The minor suggestions provided for optimization and consistency can further refine the implementation, but they are not critical.
apps/frontend-manage/src/pages/courses/index.tsx (1)
66-66: Clarify the reasoning for changing course sorting logicThe sorting logic has been modified to use
endDateinstead ofstartDatefor ordering courses. While this change is technically correct, it may have implications on user experience and expectations.
- Can you provide the rationale behind this change? Understanding the motivation will help ensure it aligns with the overall product goals.
- Consider the user experience implications:
- Will users expect to see courses ending soon at the top of the list?
- How does this affect the visibility of upcoming courses?
- If this change is intentional and approved, please ensure that:
- Any relevant documentation is updated to reflect this new behavior.
- The product team is aware of this change for any necessary updates to user guides or FAQs.
To ensure consistency across the application, let's check if this change affects other parts of the codebase:
packages/i18n/messages/de.ts (4)
214-214: LGTM: New translation added correctly.The addition of
ended: 'Beendet'in thegenericsection is appropriate and consistent with the expected changes. This new translation will be useful for indicating completed states in the German version of the application.
1031-1033: Excellent update to the microlearning guidance.The revised text for
microLearningNoCourseprovides clearer instructions to users. It now specifies that microlearnings must be associated with an "ongoing" course and offers the additional option of extending an existing course. This change enhances user understanding and provides more flexibility in course management.
1137-1139: Well-implemented consistency in practice quiz guidance.The update to
practiceQuizNoCoursemirrors the improvements made to the microlearning section. It now clearly states that practice quizzes must be associated with an ongoing course and provides the options of creating a new course or extending an existing one. This change maintains consistency across different activity types and improves user guidance.
1206-1208: Excellent enhancement to group activity requirements.The update to
groupActivityNoCoursesignificantly improves the guidance for creating group activities. It now clearly specifies the need for an ongoing course with both gamification and group formation enabled. The added instruction to verify the existence of a suitable course before proceeding is particularly helpful. This change provides users with more precise requirements, potentially reducing errors in group activity setup.
klicker-uzh
|
||||||||||||||||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Run status |
|
| Run duration | 10m 59s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
2
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
45
|
Tests for review
E-course-workflow.cy.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Test course creation and editing functionalities > Test the assignment of random groups in the seeded test course |
Test Replay
Screenshots
|
|
F-live-quiz-workflow.cy.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Different live-quiz workflows > creates a live quiz without questions and tests the feedback mechanisms and deletes the completed live session |
Test Replay
Screenshots
|
|
cypress/e2e/K-group-activity-workflow.cy.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| Create and solve a group activity > create a group activity and edit it |
Screenshots
|
|
klicker-uzh
|
||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Run status |
|
| Run duration | 09m 56s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
0
|
|
Flaky
|
1
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
47
|
Summary by CodeRabbit
Release Notes
New Features
Localization