638-feat: Add registration end date to the course pages #828
Conversation
satrija87
requested review from
andron13,
dzmitry-varabei,
SpaNb4,
Quiddlee,
ansivgit and
natanchik
as code owners
github-actions
bot
changed the title
π WalkthroughWalkthroughThe updates introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant CourseCard
participant ShortInfoPanel
participant DateSimple
CourseCard->>ShortInfoPanel: Render with props (may include mentoring props)
ShortInfoPanel->>DateSimple: Render date(s) based on view (normal/upcoming/mentoring)
ShortInfoPanel-->>CourseCard: Rendered info panel (with language, icons, etc.)
Suggested reviewers
Note β‘οΈ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note β‘οΈ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure π Recent review detailsConfiguration used: .coderabbit.yaml π Files selected for processing (5)
π§ Files skipped from review as they are similar to previous changes (5)
β° Context from checks skipped due to timeout of 90000ms (2)
β¨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. πͺ§ TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
π§Ή Nitpick comments (1)
src/shared/ui/short-info-panel/short-info-panel.tsx (1)
58-58: Inconsistent test IDThe data-testid "course-language1" is inconsistent with the pattern used elsewhere in the file ("course-language").
- <span className={cx('language')} data-testid="course-language1"> + <span className={cx('language')} data-testid="course-language">
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
π Files selected for processing (5)
.env.example(0 hunks)src/entities/course/ui/course-card/course-card.tsx(1 hunks)src/shared/constants.ts(1 hunks)src/shared/ui/short-info-panel/short-info-panel.test.tsx(1 hunks)src/shared/ui/short-info-panel/short-info-panel.tsx(1 hunks)
π€ Files with no reviewable changes (1)
- .env.example
π Additional comments (2)
src/shared/constants.ts (1)
9-9: New constant added for registration end date labelThis addition is necessary for implementing the registration end date feature on course pages.
src/shared/ui/short-info-panel/short-info-panel.test.tsx (1)
43-47: Test updated to verify multiple note iconsThe test now correctly checks all instances of note-icon elements, which is necessary since we're adding another DateSimple component that will include an additional note icon.
|
run visual now |
|
I haven't added end registration date to upcoming-card items in the main page because ,in my opinion, it will overload the cards in this place and it will be better in short version here. |
|
Lighthouse Report:
|
|
Lighthouse Report:
|
I think it should be there too because it's important information for this section. We could place it in line with start date, for example |
|
Please, fix the name of the PR in accordance with the name of the task - |
satrija87
changed the title
I found a solution to our card size issue. |
But it is not clear - when the course starts. Is it first date or second date? Is registration done in advance before the course starts and closed when it begins, or still possible to register after the course has already started? Or another case? What date is more important for students - start date, or registration end date? Should be another naming, which can clarify this case with two dates in a right way ? Can we name in a such way: "Course starts on: Apr 22 - Apr 30, 2025 English"? |
You're right. Let's put it on two lines then. We can just add a line with the registration end date below. |
There was a problem hiding this comment.
Actionable comments posted: 0
β»οΈ Duplicate comments (1)
src/shared/ui/short-info-panel/short-info-panel.tsx (1)
95-97: π οΈ Refactor suggestionAdd fallback for personalMentoringEndDate
When personalMentoringStartDate is present but personalMentoringEndDate is null, no fallback is provided, which could lead to display issues.
<DateSimple label={LABELS.MENTOR_ACTIVITIES} startDate={personalMentoringStartDate || TO_BE_DETERMINED} - endDate={personalMentoringStartDate ? personalMentoringEndDate : null} + endDate={personalMentoringStartDate ? (personalMentoringEndDate || TO_BE_DETERMINED) : null} labelSeparator={LABELS.MENTOR_ACTIVITIES_SEPARATOR} showMentoringStartDate={true} >
π§Ή Nitpick comments (3)
src/shared/ui/short-info-panel/short-info-panel.tsx (3)
40-46: Consider explicit view mode prop instead of implicit determinationThe view mode is currently determined implicitly from props, which makes the component's behavior less predictable and testable.
Consider accepting an explicit
viewprop with these checks as fallbacks:- let view: ShortInfoPanelView = 'normal'; - - if (showMentoringStartDate) { - view = 'mentoring'; - } else if (!label) { - view = 'upcoming'; - } + const determineView = (): ShortInfoPanelView => { + if (showMentoringStartDate) return 'mentoring'; + if (!label) return 'upcoming'; + return 'normal'; + }; + + const view = determineView();
59-64: Use self-closing tag for empty DateSimple componentThe DateSimple component has empty children elements that can be simplified.
<DateSimple label={LABELS.REGISTRATION_END} startDate={registrationEndDate} showMentoringStartDate={false} -> -</DateSimple> +/>
100-100: Use self-closing tag for DateSimple component hereSimilar to the previous instance, this DateSimple component has empty children elements.
showMentoringStartDate={true} -> -</DateSimple> +/>
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
π Files selected for processing (4)
src/shared/ui/date-simple/date-simple.test.tsx(6 hunks)src/shared/ui/date-simple/date-simple.tsx(3 hunks)src/shared/ui/short-info-panel/short-info-panel.test.tsx(1 hunks)src/shared/ui/short-info-panel/short-info-panel.tsx(2 hunks)
β Files skipped from review due to trivial changes (1)
- src/shared/ui/date-simple/date-simple.test.tsx
π§ Files skipped from review as they are similar to previous changes (2)
- src/shared/ui/date-simple/date-simple.tsx
- src/shared/ui/short-info-panel/short-info-panel.test.tsx
π§° Additional context used
𧬠Code Graph Analysis (1)
src/shared/ui/short-info-panel/short-info-panel.tsx (4)
src/shared/types.ts (1)
Language(6-6)src/shared/constants.ts (2)
LABELS(7-15)TO_BE_DETERMINED(4-4)src/shared/ui/date-simple/date-simple.tsx (1)
DateSimple(30-66)src/shared/helpers/getCourseDate.ts (1)
calculateFreshDate(13-18)
π Additional comments (3)
src/shared/ui/short-info-panel/short-info-panel.tsx (3)
14-14: Good use of type for view modesCreating a specific type for the panel views improves code readability and type safety.
23-25: Props added for mentoring functionalityThe new optional props support the mentoring view mode requirements effectively.
110-117: Good use of switch statement for view selectionThe switch statement clearly selects the appropriate view based on the computed view value, making the code more maintainable.
Please , can you check the first variant with one line and two dates ? If it looks messy in code, i will fix and rewrite it with two lines. I would like you to evaluate how it looks. |
|
@satrija87, please resolve the conflicts so we can merge this PR |
There was a problem hiding this comment.
Actionable comments posted: 4
β»οΈ Duplicate comments (1)
src/shared/ui/short-info-panel/short-info-panel.tsx (1)
96-98: Add fallback for personalMentoringEndDateWhen personalMentoringStartDate is present but personalMentoringEndDate is null, no fallback is provided.
<DateSimple label={LABELS.MENTOR_ACTIVITIES} startDate={personalMentoringStartDate || TO_BE_DETERMINED} - endDate={personalMentoringStartDate ? personalMentoringEndDate : null} + endDate={personalMentoringStartDate ? (personalMentoringEndDate || TO_BE_DETERMINED) : null} labelSeparator={LABELS.MENTOR_ACTIVITIES_SEPARATOR} showMentoringStartDate={true} >
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
π Files selected for processing (4)
src/shared/constants.ts(1 hunks)src/shared/ui/date-simple/date-simple.test.tsx(6 hunks)src/shared/ui/date-simple/date-simple.tsx(1 hunks)src/shared/ui/short-info-panel/short-info-panel.tsx(2 hunks)
β Files skipped from review due to trivial changes (1)
- src/shared/constants.ts
π§ Files skipped from review as they are similar to previous changes (1)
- src/shared/ui/date-simple/date-simple.test.tsx
π§° Additional context used
𧬠Code Graph Analysis (1)
src/shared/ui/date-simple/date-simple.tsx (2)
src/shared/constants.ts (1)
TO_BE_DETERMINED(6-6)src/shared/helpers/day-js.ts (1)
dayJS(8-8)
πͺ GitHub Check: Build rs.school
src/shared/ui/date-simple/date-simple.tsx
[failure] 7-7:
Cannot find module '@/shared/helpers/dayJS' or its corresponding type declarations.
src/shared/ui/short-info-panel/short-info-panel.tsx
[failure] 7-7:
Cannot find module '@/shared/helpers/getCourseDate' or its corresponding type declarations.
πͺ GitHub Check: CI
src/shared/ui/date-simple/date-simple.tsx
[failure] 66-66:
Newline required at end of file but not found
[failure] 9-9:
./date-simple.module.scss import should occur before import of @/shared/helpers/dayJS
[failure] 7-7:
Unable to resolve path to module '@/shared/helpers/dayJS'
[failure] 7-7:
There should be no empty line within import group
[failure] 6-6:
There should be at least one empty line between import groups
src/shared/ui/short-info-panel/short-info-panel.tsx
[failure] 7-7:
There should be no empty line within import group
[failure] 6-6:
There should be at least one empty line between import groups
πͺ ESLint
src/shared/ui/date-simple/date-simple.tsx
[error] 6-6: There should be at least one empty line between import groups
(import/order)
[error] 7-7: There should be no empty line within import group
(import/order)
[error] 9-9: ./date-simple.module.scss import should occur before import of @/shared/helpers/dayJS
(import/order)
[error] 66-66: Newline required at end of file but not found.
(@stylistic/eol-last)
src/shared/ui/short-info-panel/short-info-panel.tsx
[error] 6-6: There should be at least one empty line between import groups
(import/order)
[error] 7-7: There should be no empty line within import group
(import/order)
[error] 7-7: @/shared/helpers/getCourseDate import should occur after import of ./short-info-panel.module.scss
(import/order)
πͺ GitHub Actions: Preview
src/shared/ui/date-simple/date-simple.tsx
[error] 7-7: TypeScript error TS2307: Cannot find module '@/shared/helpers/dayJS' or its corresponding type declarations.
πͺ GitHub Actions: Pull Request
src/shared/ui/date-simple/date-simple.tsx
[error] 6-6: ESLint: There should be at least one empty line between import groups (import/order)
π Additional comments (7)
src/shared/ui/date-simple/date-simple.tsx (2)
19-19: New prop added for mentoring date supportThe
showMentoringStartDateboolean prop allows the component to handle different date formatting needs.
36-37: Properly implemented date formatting logicThe component now correctly conditionally formats dates based on the context they're displayed in, using raw dates for mentoring view and formatted dates otherwise.
Also applies to: 41-41, 53-53
src/shared/ui/short-info-panel/short-info-panel.tsx (5)
15-15: Good use of a union type for panel viewsUsing a union type for the possible panel views improves type safety and makes the component's capabilities clear.
24-26: Added props for mentoring functionalityThe new props allow the component to display mentoring-specific information.
41-47: Clear view selection logicThe logic for determining which view to display is straightforward and easy to understand.
49-74: Well-structured view componentsSplitting the different view implementations into separate variables makes the code cleaner and easier to maintain.
Also applies to: 76-90, 92-109
111-118: Clean switch statement for view selectionUsing a switch statement to select the appropriate view makes the code easy to read and maintain.
satrija87
force-pushed
the
feat/638-add-data-reg
branch
from
00d6af3 to
6c073aa
Compare
|
Lighthouse Report:
|
What type of PR is this? (select all that apply)
Description
Added registration end date to the course pages and course cards
Related Tickets & Documents
Screenshots, Recordings
Added/updated tests?
[optional] Are there any post deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit
New Features
Refactor
Style
Tests