723-feat: Mentorship Page Redesign #740
Conversation
* feat: 716 - add mentorship courses section * feat: 716 - show courses only on main page * feat: 716 - add unit tests * fix: 716 - adaptive issue * fix: 716 - type confusion * fix: 716 - import issue * fix: 716 - export issue * refactor: 716 - re-arrange classes * refactor: 716 - change class naming
Quiddlee
changed the title
|
Lighthouse Report:
|
* feat: 718 - add video playlist * test: video playlist * fix: 718 - apply eslint * fix: 718 - player styles * refactor: 718 - hide iframe in percy * fix: 718 - player styles
|
Lighthouse Report:
|
* feat: 721 - del image and add social icon * feat: 721 - add icon-link telegramm en-ru all course * feat: 721 - remove gap * feat: 721 - del max-width in styles * feat: 721 - return back max width
|
Lighthouse Report:
|
* fix: missing env variable * feat: use dev and prod env vars
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
* feat: 715 - add activity card * feat: 715 - add data & types * fix: 715 - change type * feat: 715 - add component * feat: 715 - add tests
|
Lighthouse Report:
|
Quiddlee
requested review from
SpaNb4,
andron13,
dzmitry-varabei,
ansivgit and
natanchik
as code owners
|
Lighthouse Report:
|
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive redesign of the Mentorship page, focusing on restructuring components, adding new features, and improving the overall user experience. The changes span multiple files across the project, introducing new components, updating styles, and refactoring existing code to support a more modular and maintainable mentorship section. Changes
Assessment against linked issues
Possibly related issues
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
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
🔭 Outside diff range comments (1)
.github/workflows/production.yml (1)
Line range hint
8-8: Update Node.js version to a stable release.Node.js 22.x is not yet released. Consider using the current LTS version 20.x.
- NODE_VERSION: '22.x' # set this to the node version to use + NODE_VERSION: '20.x' # set this to the node version to use
🧹 Nitpick comments (33)
src/widgets/mentors-docs/ui/doc-detail/ui/doc-detail.tsx (1)
27-27: Consider merging custom styles with existing classes.
If you want to combineprops.classNamewith local styles, you could usecx(['doc-detail-wrapper', props.className])for more consistent styling.<Paragraph className={cx('doc-detail-wrapper')}> {`${props.textBeforeLink} `} <LinkCustom - className={props.className} + className={cx(props.className)} href={props.linkDocs} external > {props.textLink} </LinkCustom> {` ${props.textAfterLink}`} </Paragraph>src/views/mentorship/helpers/transformCoursesToMentorship.ts (1)
5-11: Consider fallback handling.If a title is missing in
mentorshipLinks, a fallback or error might be safer than assigningundefined.src/shared/ui/slider/slider.tsx (1)
9-14: Improve clarity of prop definitions
TheSliderPropsinterface is straightforward, but providing JSDoc or comments detailing each prop’s purpose will help future maintainers.src/views/mentorship/ui/mentorship-courses/mentorship-courses.tsx (2)
14-15: Potential error handling needed for data fetch.
Currently,getCourses()is awaited without any fallback or error handling. If an error occurs, this could lead to an unhandled promise rejection. Consider adding a try/catch or a fallback for a more robust experience.-export const MentorshipCourses = async () => { - const coursesWithMentorship = transformCoursesToMentorship(await getCourses()); +export const MentorshipCourses = async () => { + try { + const result = await getCourses(); + const coursesWithMentorship = transformCoursesToMentorship(result); + // proceed + } catch (error) { + // handle fallback or logging + }
17-29: Consider a loading state while fetching data.
For a smoother user experience, you might present a loading indicator while fetching. This helps avoid rendering a blank space if the network connection is slow.src/views/mentorship/ui/mentor-activities/ui/mentor-activities/mentor-activities.tsx (1)
16-37: Optional fallback for empty activities.
Consider rendering a message like “No activities available” ifactivitiesis empty. This ensures a more user-friendly experience.+{!activities.length && ( + <p className={cx('no-activities')}>No activities available</p> +)}src/views/mentorship/mentorship-hero/ui/mentorship-hero.test.tsx (3)
1-2: Consolidate imports
Consider grouping similar imports from testing libraries (@testing-library/react,vitest) to enhance clarity.
13-21: Accurate suite description
The suite name references "HeroPage component" but the file name ismentorship-hero.test.tsx. Consider aligning them so it’s clear the tests relate directly to the MentorshipHero component.
27-36: Complete content coverage
The test verifies main title and subtitles. Consider adding a test for negative cases or fallback states (e.g., when data is empty or missing).src/views/mentorship/ui/mentor-activities/ui/activity-card/activity-card.tsx (1)
1-3: Unused import note
If there's no dynamic binding needed,classnames/bindcould be replaced by a standard import to reduce overhead.src/views/mentorship/ui/details/details.tsx (1)
18-37: Optional fallback props
langdefaults to'en'. Consider makingdescriptionoptional or handle an empty array to avoid errors when no data is passed.src/views/mentorship/ui/details/details.test.tsx (2)
7-18: Consider testing edge cases for your details data.
You might want to include cases whereinfohas more than two words or is empty, so thesplit(' ')logic doesn't break.
39-46: Revisit string splitting approach for better robustness.
Relying on a two-word split may fail if you change the text. A more flexible approach or direct text matching could ensure the test remains stable.src/views/mentorship/mentorship-hero/ui/mentorship-hero.tsx (1)
23-45: Handle potential multiple subtitle elements.
Currently, we only display the first element fromsubTitle. If there's more data, consider mapping each item to ensure consistent usage of the array.src/views/mentorship/ui/mentor-activities/ui/activity-card/activity-card.test.tsx (1)
39-43: Prefer using text content assertions instead of innerHTML.
UsetoHaveTextContent()with RTL to avoid potential pitfalls with HTML tags or nested elements.src/views/mentorship/ui/mentor-activities/ui/mentor-activities/mentor-activities.test.tsx (1)
37-56: Clear and concise testing.
Verifies visual elements, counts, and text content for a complete snapshot of the component’s core functionality. Could consider future negative or edge-case tests for greater coverage.src/entities/mentor/ui/mentor-feedback-card/mentor-feedback-card.tsx (2)
17-18: Consider externalizing the magic number.
Using a config could make modifications easier.
30-66: Component layout is well-organized.
Extracting subcomponents might be an option if it grows.src/shared/ui/video-playlist-with-player/playlist/playlist.test.tsx (1)
16-22: Consider testing edge cases
Great checks for the number of thumbnails and titles. You might also add a test for an empty or null videos array to ensure graceful handling.src/shared/ui/modal/modal.tsx (3)
28-47: Outside click detection
Usingdocument.addEventListener('mousedown', handleMouseDown)for outside clicks might also catch events on child elements. Consider bounding or ignoring children inside the dialog.
57-65: Handling showModal logic
The effect that callsshowModal()is straightforward and ensures the dialog is visible. Remember to also consider keyboard accessibility, e.g., enabling ESC to close.
70-96: Portal usage and structure
Creating a portal todocument.bodyis a common approach for modals. Consider adding ARIA attributes for accessibility (e.g.,aria-modal="true").dev-data/index.ts (1)
69-69: Confirm Usage of Feedback DataCheck that
mentorsFeedbackDatahas sufficient coverage in tests and is used appropriately in the application to avoid unused exports.src/shared/ui/video-playlist-with-player/video-playlist-with-player.tsx (1)
27-63: Fetching YouTube PlaylistThe fetch logic appears standard. Consider:
• Handling partial responses if the playlist lacks items.
• Logging or fallback behavior ifdata.itemsis empty.src/shared/ui/video-playlist-with-player/video-playlist-with-player.test.tsx (2)
23-32: Consider simplifying render logic.
You could render outsidewaitForand then await the error indicator directly.
34-56: Magic string for "3 Video Feedbacks".
Consider using the length ofMOCKED_VIDEOSto form the displayed text to avoid hardcoding.src/shared/ui/video-playlist-with-player/video-playlist-with-player.module.scss (2)
1-9: LGTM! Consider adding fallback for gap property.The flexbox layout with gap looks good, but older browsers might need fallback spacing.
.video-player-container { position: relative; display: flex; gap: 24px; + /* Fallback for browsers that don't support gap */ + & > * + * { + margin-left: 24px; + } + @supports (gap: 24px) { + & > * + * { + margin-left: 0; + } + } }
16-20: Add overflow handling for iframe border radius.Border radius on iframes can cause content overflow in some browsers.
.video-player { + overflow: hidden; + border-radius: 12px; iframe { - border-radius: 12px; + width: 100%; + height: 100%; } }src/views/mentorship/mentorship-hero/ui/mentorship-hero.module.scss (2)
1-4: Consider using a variable for the background image path.Hardcoding the image path makes it less maintainable.
+$hero-bg-image: '../../../../shared/assets/mentorship-hero.webp'; + .hero { min-height: 440px; - background: url('../../../../shared/assets/mentorship-hero.webp') center / cover no-repeat; + background: url($hero-bg-image) center / cover no-repeat;
5-35: Consider using BEM methodology to reduce nesting.Deep nesting makes styles harder to maintain and increases specificity.
-.hero { - &-content { - .hero-info { - .title-main { +.hero-content { + display: flex; +} +.hero-info { + display: flex; + flex-direction: column; + /* ... other styles ... */ +} +.hero-title-main { + /* ... title styles ... */ +}src/shared/ui/modal/modal.module.scss (2)
1-25: Consider using CSS custom properties for reusable values.Values like max-width, border-radius, and padding could be defined as CSS variables for better maintainability.
+:root { + --modal-max-width: 889px; + --modal-padding: 32px; + --modal-border-radius: 12px; +} .modal { max-width: var(--modal-max-width); - padding: 0 32px 32px 32px; + padding: 0 var(--modal-padding) var(--modal-padding) var(--modal-padding); - border-radius: 12px; + border-radius: var(--modal-border-radius);
60-74: Add hover and focus states for better accessibility.The close button should have visible states for interaction.
.modal-close-button { cursor: pointer; + transition: opacity 0.2s; + + &:hover, + &:focus-visible { + opacity: 0.7; + } + + &:focus-visible { + outline: 2px solid currentColor; + outline-offset: 2px; + }src/entities/mentor/ui/mentor-feedback-card/mentor-feedback-card.module.scss (1)
74-75: Consider using design tokens for line height.Extract line-height value to a variable for better maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/shared/assets/mentor-register.svgis excluded by!**/*.svgsrc/shared/assets/svg/best-practices-icon.svgis excluded by!**/*.svgsrc/shared/assets/svg/close.svgis excluded by!**/*.svgsrc/shared/assets/svg/code-review-icon.svgis excluded by!**/*.svgsrc/shared/assets/svg/conducting-tech-interview-icon.svgis excluded by!**/*.svgsrc/shared/assets/svg/mentor-sloths/1.svgis excluded by!**/*.svgsrc/shared/assets/svg/mentor-sloths/2.svgis excluded by!**/*.svgsrc/shared/assets/svg/supervising-team-assignment-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (82)
.github/workflows/main.yml(1 hunks).github/workflows/preview-create.yml(1 hunks).github/workflows/production.yml(2 hunks).github/workflows/visual-testing-on-comment.yml(1 hunks).github/workflows/visual-testing-on-push.yml(1 hunks)dev-data/about-mentors.data.ts(1 hunks)dev-data/courses-data.types.ts(1 hunks)dev-data/hero-page.data.ts(1 hunks)dev-data/index.ts(2 hunks)dev-data/mentor-docs.data.ts(2 hunks)dev-data/mentors-feedback.data.ts(1 hunks)dev-data/mentors-register.data.ts(1 hunks)dev-data/mentorship-data.types.ts(3 hunks)dev-data/mentorship.data.ts(14 hunks)env.d.ts(1 hunks)package.json(2 hunks)readme/styleguide-design.md(1 hunks)src/app/mentorship/[course]/page.tsx(1 hunks)src/app/mentorship/page.tsx(2 hunks)src/core/base-layout/components/header/header.module.scss(1 hunks)src/core/base-layout/components/header/header.tsx(1 hunks)src/core/styles/_constants.scss(3 hunks)src/core/styles/index.scss(2 hunks)src/entities/course/ui/course-card/course-card.test.tsx(2 hunks)src/entities/course/ui/course-card/course-card.tsx(4 hunks)src/entities/mentor/index.ts(1 hunks)src/entities/mentor/types.ts(1 hunks)src/entities/mentor/ui/mentor-feedback-card/mentor-feedback-card.module.scss(1 hunks)src/entities/mentor/ui/mentor-feedback-card/mentor-feedback-card.test.tsx(1 hunks)src/entities/mentor/ui/mentor-feedback-card/mentor-feedback-card.tsx(1 hunks)src/shared/__tests__/constants.ts(2 hunks)src/shared/__tests__/setup-tests.tsx(2 hunks)src/shared/types.ts(1 hunks)src/shared/ui/modal/index.ts(1 hunks)src/shared/ui/modal/modal.module.scss(1 hunks)src/shared/ui/modal/modal.test.tsx(1 hunks)src/shared/ui/modal/modal.tsx(1 hunks)src/shared/ui/slider/index.ts(1 hunks)src/shared/ui/slider/slider.tsx(1 hunks)src/shared/ui/subtitle/subtitle.module.scss(1 hunks)src/shared/ui/subtitle/subtitle.tsx(1 hunks)src/shared/ui/video-player/index.ts(1 hunks)src/shared/ui/video-player/video-player.module.scss(1 hunks)src/shared/ui/video-player/video-player.tsx(1 hunks)src/shared/ui/video-playlist-with-player/index.ts(1 hunks)src/shared/ui/video-playlist-with-player/playlist/index.ts(1 hunks)src/shared/ui/video-playlist-with-player/playlist/playlist.module.scss(1 hunks)src/shared/ui/video-playlist-with-player/playlist/playlist.test.tsx(1 hunks)src/shared/ui/video-playlist-with-player/playlist/playlist.tsx(1 hunks)src/shared/ui/video-playlist-with-player/video-playlist-with-player.module.scss(1 hunks)src/shared/ui/video-playlist-with-player/video-playlist-with-player.test.tsx(1 hunks)src/shared/ui/video-playlist-with-player/video-playlist-with-player.tsx(1 hunks)src/views/mentorship/constants.ts(1 hunks)src/views/mentorship/helpers/transformCoursesToMentorship.ts(1 hunks)src/views/mentorship/mentorship-hero/index.ts(1 hunks)src/views/mentorship/mentorship-hero/ui/mentorship-hero.module.scss(1 hunks)src/views/mentorship/mentorship-hero/ui/mentorship-hero.test.tsx(1 hunks)src/views/mentorship/mentorship-hero/ui/mentorship-hero.tsx(1 hunks)src/views/mentorship/mentorship.tsx(2 hunks)src/views/mentorship/ui/details/details.module.scss(1 hunks)src/views/mentorship/ui/details/details.test.tsx(1 hunks)src/views/mentorship/ui/details/details.tsx(1 hunks)src/views/mentorship/ui/mentor-activities/index.ts(1 hunks)src/views/mentorship/ui/mentor-activities/ui/activity-card/activity-card.module.scss(1 hunks)src/views/mentorship/ui/mentor-activities/ui/activity-card/activity-card.test.tsx(1 hunks)src/views/mentorship/ui/mentor-activities/ui/activity-card/activity-card.tsx(1 hunks)src/views/mentorship/ui/mentor-activities/ui/mentor-activities/mentor-activities.module.scss(1 hunks)src/views/mentorship/ui/mentor-activities/ui/mentor-activities/mentor-activities.test.tsx(1 hunks)src/views/mentorship/ui/mentor-activities/ui/mentor-activities/mentor-activities.tsx(1 hunks)src/views/mentorship/ui/mentorship-courses/mentorship-courses.module.scss(1 hunks)src/views/mentorship/ui/mentorship-courses/mentorship-courses.test.tsx(1 hunks)src/views/mentorship/ui/mentorship-courses/mentorship-courses.tsx(1 hunks)src/widgets/about-video/ui/about-video.module.scss(0 hunks)src/widgets/about-video/ui/about-video.test.tsx(1 hunks)src/widgets/about-video/ui/about-video.tsx(1 hunks)src/widgets/hero-page/ui/hero-page.module.scss(0 hunks)src/widgets/hero-page/ui/hero-page.tsx(1 hunks)src/widgets/member-activity/ui/member-activity.tsx(1 hunks)src/widgets/mentors-docs/ui/doc-detail/ui/doc-detail.tsx(2 hunks)src/widgets/mentors-docs/ui/mentors-docs/mentors-docs.module.scss(1 hunks)src/widgets/mentors-docs/ui/mentors-docs/mentors-docs.tsx(1 hunks)src/widgets/mentors-feedback/index.ts(1 hunks)
⛔ Files not processed due to max files limit (15)
- src/widgets/mentors-feedback/ui/mentors-feedback.module.scss
- src/widgets/mentors-feedback/ui/mentors-feedback.test.tsx
- src/widgets/mentors-feedback/ui/mentors-feedback.tsx
- src/widgets/mentors-register/ui/mentors-register.module.scss
- src/widgets/mentors-register/ui/mentors-register.test.tsx
- src/widgets/mentors-register/ui/mentors-register.tsx
- src/widgets/numbers/ui/info-cell/info-cell.module.scss
- src/widgets/numbers/ui/info-cell/info-cell.test.tsx
- src/widgets/numbers/ui/info-cell/info-cell.tsx
- src/widgets/numbers/ui/info-grid/info-grid.module.scss
- src/widgets/numbers/ui/info-grid/info-grid.tsx
- src/widgets/numbers/ui/numbers/numbers.module.scss
- src/widgets/numbers/ui/numbers/numbers.tsx
- tsconfig.json
- vitest.config.ts
💤 Files with no reviewable changes (2)
- src/widgets/hero-page/ui/hero-page.module.scss
- src/widgets/about-video/ui/about-video.module.scss
✅ Files skipped from review due to trivial changes (16)
- dev-data/about-mentors.data.ts
- src/shared/ui/video-playlist-with-player/index.ts
- src/shared/ui/modal/index.ts
- src/shared/ui/video-playlist-with-player/playlist/index.ts
- dev-data/mentors-register.data.ts
- src/shared/ui/slider/index.ts
- src/shared/ui/video-player/index.ts
- src/views/mentorship/mentorship-hero/index.ts
- src/widgets/mentors-feedback/index.ts
- dev-data/mentor-docs.data.ts
- src/views/mentorship/ui/details/details.module.scss
- src/views/mentorship/ui/mentor-activities/ui/mentor-activities/mentor-activities.module.scss
- src/views/mentorship/ui/mentor-activities/index.ts
- src/shared/ui/video-player/video-player.module.scss
- src/views/mentorship/ui/mentorship-courses/mentorship-courses.module.scss
- src/views/mentorship/ui/mentor-activities/ui/activity-card/activity-card.module.scss
🔇 Additional comments (152)
src/widgets/mentors-docs/ui/doc-detail/ui/doc-detail.tsx (1)
15-15: Optional className prop is a good enhancement.
DefiningclassNameas optional provides flexibility for consumers of this component to apply custom styling. No issues here.src/widgets/hero-page/ui/hero-page.tsx (1)
Line range hint
52-64: Streamlined approach
Removing the conditional class for mentorship simplifies the layout logic. Just confirm that you’ve either repurposed or removed any leftover mentorship-specific styling or references elsewhere in the code.src/app/mentorship/[course]/page.tsx (1)
3-3: Confirm correct import pathThe import path has changed to
'@/views/mentorship/mentorship'. Kindly verify that this folder structure exists and the resulting compiled output is correct.src/shared/ui/subtitle/subtitle.tsx (5)
25-28: Nice addition of the weight variant.
This cleanly maps thenormalandboldvariants to distinct classes, ensuring straightforward usage.
33-33: Practical default variant.
Settingweightto'normal'as a default provides a sensible baseline for most use cases.
37-37: Elegant structuring of props.
Incorporatingweightinto the prop signature aligns well with the variant approach, maintaining a clear API.
43-43: Seamless variant usage.
Passingweightdirectly intosubtitleVariantskeeps styling logic concise and consistent.
47-47: Props spreading confirmed.
Spreading additional HTML attributes is a good practice here, offering flexibility without clutter.src/shared/ui/subtitle/subtitle.module.scss (2)
62-64: Good alignment with design tokens.
Using$font-weight-mediumunder the.normalclass matches conventional naming and consistency.
66-68: Clear definition for bold styling.
Mapping.boldto$font-weight-boldis straightforward and easy to maintain.env.d.ts (1)
4-4: Placement of the environment variable looks correct.
No issues at a glance.src/entities/mentor/index.ts (1)
1-2: Good modular export.
Nicely organizes the feedback type and card.src/entities/mentor/types.ts (1)
1-8: Clear typed structure.
Definition effectively covers the needed fields for mentor feedback.src/shared/types.ts (1)
8-12: Video type is well-defined.
Properties suitably represent typical video data.src/app/mentorship/page.tsx (2)
3-3: Ensure consistent import patterns.Using the more explicit path is fine, but confirm all imports remain consistent across the codebase.
13-13: Validate the boolean prop usage.Passing
coursesas a boolean is acceptable ifMentorshipexpects it. Double-check that it aligns with the intended component logic.src/shared/ui/video-player/video-player.tsx (5)
1-2: Client-side mode confirmed.Marking this component as client-side is appropriate for video playback.
3-4: Good use of external libraries.
classnames/bindwithreact-youtubeis straightforward and clean.
6-9: SCSS modules and prop extension are clear.The typed props ensure easy pass-through to the YouTube component.
10-11: Efficient class binding strategy.
cxsimplifies dynamic class binding, keeping the code concise.
12-18: VideoPlayer structure looks solid.The container approach enforces the 16:9 ratio nicely. No issues here.
src/views/mentorship/helpers/transformCoursesToMentorship.ts (1)
1-4: Imports well-organized.No issues found with these imports.
src/views/mentorship/constants.ts (2)
4-9: Array definition is clear.All included titles match up with the intended mentorship scope.
11-16: Maintain sync between titles and links.This object perfectly mirrors the titles array. Ensure any future additions remain consistent.
src/shared/ui/slider/slider.tsx (2)
1-2: Consider the rendering approach for Next.js applications
You’ve declared'use client'at the top, which is appropriate for client-side rendering. Ensure no server-side logic is required in this component before proceeding.
16-31: Overall component implementation looks solid
TheSwiperusage with pagination and A11y modules is clear, and the mapping ofslidesintoSwiperSlideis concise.dev-data/hero-page.data.ts (1)
24-27: Renaming to ‘Mentorship’ aligns with the redesign
The updated text and cleared fields streamline the branding. Looks good and consistent with the PR objectives.src/widgets/about-video/ui/about-video.test.tsx (1)
25-25: YouTube embed test is correct and necessary
Verifying the iframe source withRS_INTRO_URLensures the correct video link is rendered.dev-data/courses-data.types.ts (2)
4-4: Refined import from MentorActivities to MentorActivity
Switching to a singular type reduces confusion about the data structure.
9-9: Array of MentorActivity fosters clarity
ReplacingMentorActivities[]withMentorActivity[]improves type specificity and maintainability.src/views/mentorship/ui/mentorship-courses/mentorship-courses.tsx (1)
1-13: Imports and bindings look correct.
Everything appears properly defined and consistent.src/widgets/about-video/ui/about-video.tsx (1)
20-28: Unconditional rendering of iframe looks solid.
No major concerns. Thehide-in-percyclass is helpful for visual regression tests. Ensure you trust the source ofRS_INTRO_URL.src/views/mentorship/ui/mentorship-courses/mentorship-courses.test.tsx (1)
1-29: Validate the testing approach for an async component.
You're awaitingMentorshipCourses()and passing the returned JSX torenderWithRouter. Verify thatgetCoursesis adequately mocked, or the test may rely on network calls. Otherwise, it can lead to flakiness or slow test runs.✅ Verification successful
Test setup is properly configured with mocked data
ThegetCoursesfunction is correctly mocked insetup-tests.tsx, returning static test data without making network calls.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # If you haven't already, search for any mock or real calls to getCourses: rg "getCourses" -A 5Length of output: 7267
src/views/mentorship/mentorship-hero/ui/mentorship-hero.test.tsx (1)
4-7: Verify the data source import
Ensure theheroPageDataandPAGE_NAMES.MENTORSHIPreferences are accessible and valid across the project to prevent potential runtime errors.src/views/mentorship/ui/mentor-activities/ui/activity-card/activity-card.tsx (1)
7-13: Props design clarity
UsingOmit<MentorActivity,'id'>is concise. Ensure there are no unforeseen side effects ifidis necessary for tracking or keys.src/views/mentorship/ui/details/details.tsx (2)
1-2: Consistent imports
classnames/bindis used for dynamic style binding. Ensure the rest of the app uses consistent patterns to maintain uniformity.
7-8: Data lookup
CallingaboutMentorsData[lang]is a flexible approach, but confirm all language keys exist or handle a fallback scenario (e.g., default to'en').src/shared/__tests__/setup-tests.tsx (3)
7-7: Mock usage
ImportingMockfromvitestis valid. Confirm the rest of the codebase consistently uses the same type if needed.
17-23: Dialog element mocks
MockingHTMLDialogElementensures tests won't break in jsdom. Great approach for coverage.
25-29: Clearer test isolation
Resetting mocks inbeforeEachis good practice. Ensure future test additions also maintain isolation.src/widgets/mentors-docs/ui/mentors-docs/mentors-docs.tsx (1)
32-32: Confirm docs alignment with new data structure.
Ensure thementorDocsLinkusage and text references match expectations after removingcourseDocsLink; re-verify if references tocourseDocsLinkare still needed in other files.src/entities/course/ui/course-card/course-card.tsx (4)
24-24: Enable custom class usage.
Including the'className'prop in the type definition is a great touch for styling flexibility.
35-35: Seamless integration of new prop.
DestructuringclassNamehere correctly leverages the new prop for easy custom styling.
45-45: Conditionally merged classes.
Usingcx('course-card', className)ensures default styles and user-defined classes coexist without conflict.
46-46: Improved testability.
Addingdata-testid="card-header"allows precise element selection in tests, making them more resilient.src/views/mentorship/ui/mentor-activities/ui/mentor-activities/mentor-activities.test.tsx (2)
1-6: Effective test setup.
Imports from@testing-library/reactandvitestaccurately provide a foundation for reliable test rendering and assertions.
7-35: Useful mock data.
Providing comprehensiveactivitiesandactivitiesTitleensures thorough coverage of real-world scenarios in the test.src/views/mentorship/mentorship.tsx (7)
1-3: Streamlined imports.
Removing unused imports and adding new ones clarifies dependencies for updated mentorship components.Also applies to: 8-10
14-14: Enhanced flexibility with optional prop.
Usingcourses?: boolean;provides a straightforward toggle without forcing consumers to supply the prop.
17-17: Destructuring default.
courses = falseconveniently avoids undefined checks inside the component body.
20-21: Refined hero and details.
Replacing older components withMentorshipHeroandDetailsMentorshipclarifies purpose and follows consistent naming.
23-24: Improved mentor activities.
Switching toMentorActivitieshelps highlight discrete activities, offering a more focused UX.
27-27: Conditional rendering.
ShowingMentorshipCourseson demand avoids clutter for users who don’t need course info at the moment.
37-37: Extra layer of feedback.
IncorporatingMentorsFeedbackeffectively collects valuable input to enhance mentorship offerings.src/shared/ui/video-playlist-with-player/playlist/playlist.tsx (5)
1-3: Logical imports.
React andclassnames/bindare well-structured for this playlist component.
4-6: Strong typing.
Using theVideotype from shared types fosters consistent structure across the codebase.
8-9: Class binding hook.
const cx = classNames.bind(styles)is a neat approach to keep style usage organized.
10-16: Flexible props design.
This well-definedPlaylistPropstype ensures all essential fields are accounted for while allowing optional callbacks and styling.
18-42: Straightforward playlist UI.
Displays a list of videos with click handling, highlights the current selection, and employs consistent data-testid usage for simpler test assertions.src/entities/course/ui/course-card/course-card.test.tsx (2)
4-4: Import statement is clear.
This import now neatly includes the type reference, enhancing clarity for the test.
45-51: Good use of data-testid for targeted styling checks.
The test asserts critical styles in a straightforward way—no issues identified.dev-data/mentorship-data.types.ts (11)
8-14: Straightforward type definition for MentorActivity.
This structure captures essential details nicely.
16-19: ActivityLink type is clear and concise.
Good for linking within or outside the platform.
22-22: CourseTitle definition is accurate.
Switching to this indexing approach is robust.
24-27: MentorshipCourseTitles extraction is logical.
It clarifies which course titles apply to mentorship.
28-28: No updates.
29-31: MentorshipLinks mapping is well-structured.
Handy for linking route definitions.
37-41: Union type usage is good.
Makes route keys explicit.
42-42: Defines MentorshipCourseRoute succinctly.
No issues found.
51-55: MentorshipDetails is well-defined.
It conveys ID, title, and info effectively.
56-56: Blank line change.
70-72: Enhanced MentorshipCourse structure.
AddingactivitiesTitleand array details refines data organization.src/entities/mentor/ui/mentor-feedback-card/mentor-feedback-card.tsx (7)
1-2: Client directive included.
Ensures Next.js client-side rendering for this component.
3-6: Imports look neat.
Standard React setup with class binding.
7-10: Type import and UI imports are coherent.
Keeps the data model in sync with the UI elements.
11-13: Scoped styles approach is appropriate.
Class binding helps avoid naming collisions.
15-16: Props typing is well-defined.
Makes the component more self-documenting.
19-29: Stateful modal logic is straightforward.
The local state approach is easy to follow.
68-78: Neat modal integration.
Reusing the same content keeps the UI consistent.src/entities/mentor/ui/mentor-feedback-card/mentor-feedback-card.test.tsx (8)
1-3: Imports are conventional.
No issues found.
4-6: Component and mock data references.
Ensures accurate test coverage.
7-16: Test data is well-structured.
Clearly separates long and short reviews.
18-30: Comprehensive rendering test.
Checks important fields properly.
32-37: Conditional rendering logic validated.
Ensures the button appears only when needed.
38-42: Short review path tested thoroughly.
Well done.
44-53: Modal open behavior confirmed.
The test ensures user interaction triggers the modal.
54-67: Modal close behavior confirmed.
Ensures the 'see more' flow is fully covered.src/shared/ui/video-playlist-with-player/playlist/playlist.test.tsx (6)
1-8: Good import structure and setup
The test utilities and mock data are imported clearly, laying a solid foundation for this test suite.
9-15: Comprehensive title validation
The test properly expects the playlist title to include the number of videos. It’s a good practice to quickly indicate total items.
23-29: Style check is well-implemented
Verifying inline styles like background color helps ensure visual requirements. All good here.
31-43: Robust interaction test
Confirming thatonSelectVideois called with the correct item is excellent coverage.
44-61: Effective highlighting test
Validating theselectedclass and the overlay ensures users can identify the currently playing video. Good job.
63-71: No-selection scenario covered
This test prevents regressions by confirming no items are highlighted. Good completeness.src/shared/ui/modal/modal.tsx (2)
20-26: Clean close handler
CallingdialogRef.current?.close()beforeonClose()helps maintain correct order of operations. This is straightforward and effective.
49-55: Proper event cleanup
Removing the event listener in the cleanup function prevents memory leaks. This is a good practice.src/widgets/member-activity/ui/member-activity.tsx (1)
11-19: Updated type references
Switching fromMentorActivities[]toMentorActivity[]is likely part of a broader refactor. Verify all references to the old type are removed or updated accordingly.src/shared/ui/modal/modal.test.tsx (7)
1-24: Test wrapper readability
TestModalWrapperneatly demonstrates how the modal behaves in real usage. Clear separation of states.
26-44: Good fixture and mock management
TheonCloseMockis reset inbeforeEach(), preventing state leakage between tests. Nicely done.
45-60: Visibility checks
VerifiesModalrendering behavior when{ isOpen: true }or{ isOpen: false }. This ensures consistent UI display logic.
61-67: Title presence test
Ensures that the title is displayed and matches text content. Straightforward and effective confirmation.
68-80: Close button validation
Verifies the dialog closes and the callback triggers. This is a key user interaction.
81-95: Overlay click
Ensures clicking the dialog element boundary triggers closure. This covers the vital outside-click scenario.
96-104: Custom class test
Applying additional class names is tested. This helps confirm flexibility for styling or theming.dev-data/index.ts (2)
11-15: Confirm All References to Updated Type NamesBe sure that occurrences of the older
MentorActivitiesreferences are updated throughout to the newMentorActivityor corresponding types. This ensures no broken references remain in the codebase.
18-18: Exporting Additional Types Looks GoodExporting
MentorshipCourseTitlesandMentorshipLinkscleanly centralizes new mentorship-related types.src/shared/ui/video-playlist-with-player/video-playlist-with-player.tsx (6)
1-17: Client-Side Initialization and Interface SetupUsing
'use client'correctly enables client-side React APIs. The prop definitions forplaylistIdandapiKeylook straightforward.
19-26: State Variables DeclarationMaintaining separate states (
videos,selectedVideo, etc.) is clean. Verify all initial states, especiallyselectedVideo, align with your UX needs.
65-79: Dynamic Resize LogicWell-structured useEffect for updating playlist height on window resize. This helps maintain a responsive layout.
81-83: Video Selection HandlerStraightforward handler to set the selected video. No issues found.
85-96: Conditional Rendering for Loading, Errors, and No VideosThe conditions are well differentiated. Confirm that the user receives enough context from error or empty states.
[approve]
97-111: Main Layout RenderingRendering
VideoPlayeralongside the playlist is intuitive. The usage of styling for dynamic heights is appreciated.src/core/base-layout/components/header/header.tsx (2)
24-24: Type for Accent ColorDefining
HeaderAccentColoras a union type is clear and ensures color safety.
29-29: Path-Based Accent ColorAssigning
'blue'for the Mentorship route is a neat approach. Double-check that other routes still receive intended styling.dev-data/mentors-feedback.data.ts (2)
1-4: Imports for Mentor Feedback and AssetsNo issues in import statements. Assets are clearly referenced.
5-49: Mentors Feedback Data StructureThis array-based feedback is straightforward. Confirm that using real mentor names and text is acceptable. If anonymization is needed, consider masking.
src/shared/ui/video-playlist-with-player/video-playlist-with-player.test.tsx (7)
1-2: Imports look good.
No issues with importing necessary methods for testing.
4-9: Mocking strategy is appropriate.
Usingvi.mockforVideoPlayeris a standard approach for isolated testing.
10-10: Global fetch mocking is correct.
Spying onglobal.fetchsimplifies controlling responses in tests.
12-15: afterEach with vi.clearAllMocks is good practice.
This ensures each test starts with a clean slate.
17-21: Loading state test is clean.
Straightforward assertion for the loading message.
58-103: Video selection test is thorough.
Verifying expected calls toVideoPlayeris sufficient.
105-143: Dynamic height test is robust.
Offset mocking and resize event coverage is well handled.src/shared/__tests__/constants.ts (2)
195-201: Mentor feedback constants look correct.
They align with the importedMentorFeedbacktype.
202-225: MOCKED_VIDEOS is well-structured.
Data shape aligns with theVideointerface.dev-data/mentorship.data.ts (10)
9-12: SVG icon imports are consistent.
No concerns with newly added icons.
31-60: activitiesContent object is well-organized.
Titles, descriptions, and icons are grouped logically.
66-75: Docs links appear accurate.
They match the new structure and use consistent naming.
103-124: details array for mentorshipCoursesDefault.
Each object is well-defined, no immediate issues.
Line range hint
128-154: Default course activities are clearly structured.
All references to activitiesContent are cohesive.
171-195: Second course links updated properly.
Inclusion of both Telegram EN and RU is good.
Line range hint
199-237: Second course activities cover multiple aspects.
The distribution of links and descriptions is sensible.
Line range hint
251-317: RU version of the course is consistent with EN.
Localization is handled; titles and descriptions match the new structure.
Line range hint
331-385: React course data is aligned.
Mixing best practices, code review, interviews all in place.
Line range hint
399-453: Angular course data also updated consistently.
Harmonized approach to activities and details.src/widgets/mentors-docs/ui/mentors-docs/mentors-docs.module.scss (1)
3-3: Column layout change.
Switching.docs-wrapperto column may significantly alter layout; confirm that it matches the new design requirements..github/workflows/visual-testing-on-push.yml (1)
9-10: Consider using a dedicated testing API key.Using a development API key in workflows could expose sensitive credentials. Consider creating a separate key for testing purposes.
.github/workflows/main.yml (1)
11-12: Same API key concern as in visual-testing workflow..github/workflows/production.yml (1)
14-15: LGTM! Environment variables are properly configured.Good practice moving API_URL to global env and adding YOUTUBE_API_KEY for video integration.
.github/workflows/visual-testing-on-comment.yml (1)
10-11: LGTM! Environment configuration is consistent.Proper use of development-specific YouTube API key for testing environment.
src/core/base-layout/components/header/header.module.scss (1)
20-20: LGTM! Header styling enhancement.The gradient background provides a modern look consistent with the redesign.
src/shared/ui/video-playlist-with-player/playlist/playlist.module.scss (2)
8-8: Consider accessibility impact of thin scrollbar.Custom scrollbar width might affect users with accessibility needs. Consider testing with various accessibility tools.
98-98: Verify color contrast ratio for overlay text.Ensure the overlay text meets WCAG contrast requirements against the semi-transparent background.
✅ Verification successful
Color contrast meets WCAG requirements
The white text (color-white) on 70% opaque black background provides sufficient contrast for the "Now Playing" overlay text. The bold weight further enhances readability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all color and opacity variables to verify contrast rg -l '(\$color|\$opacity)' src/core/styles/Length of output: 105
Script:
#!/bin/bash # Get color and opacity values echo "=== Constants File ===" cat src/core/styles/_constants.scss echo -e "\n=== Playlist Module ===" cat src/shared/ui/video-playlist-with-player/playlist/playlist.module.scssLength of output: 4348
src/entities/mentor/ui/mentor-feedback-card/mentor-feedback-card.module.scss (1)
75-75: Review whitespace handling in content.
white-space: pre-linemight cause unexpected layouts with user-generated content. Consider normalizing whitespace..github/workflows/preview-create.yml (1)
18-19: Verify GitHub secrets configuration.Ensure that both
API_URLandYOUTUBE_API_KEY_DEVELOPMENTsecrets are configured in the repository settings.src/core/styles/_constants.scss (3)
23-24: LGTM! Color variables follow the established pattern.New color variables are well-defined using HSL values and maintain consistency with the existing color system.
Also applies to: 43-44, 48-49
50-51: LGTM! Gradient definition is clear and reusable.The gradient uses the newly defined color variables, promoting consistency.
78-78: LGTM! New opacity values documented in style guide.The new opacity values (0.7 and 0.1) are properly defined and documented in the style guide.
Also applies to: 81-81
src/core/styles/index.scss (3)
8-10: LGTM! Dialog overflow handling improves UX.Prevents background scrolling when dialog is open.
183-185: LGTM! Swiper configuration is properly defined.Custom property for pagination bullet size is set at the root level.
187-208: LGTM! Percy-specific styles handle video placeholders.Visual testing accommodations are properly implemented with clear placeholder text.
package.json (1)
39-39: Verify dependency compatibility.New dependencies added:
- react-youtube for video integration
- swiper for carousel functionality
- @types/gapi.youtube for TypeScript support
Also applies to: 47-47, 60-60
✅ Verification successful
Dependencies are compatible with the project
The added packages are compatible with the project's React version (19.0.0) and other dependencies. No conflicts detected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential dependency conflicts npm ls react-youtube swiper @types/gapi.youtubeLength of output: 95
Script:
#!/bin/bash # Get full package.json content cat package.json # Check peer dependencies for react-youtube npm view react-youtube peerDependencies --json # Look for existing usage of these packages rg -l "react-youtube|swiper|gapi\.youtube" --type ts --type tsxLength of output: 3529
readme/styleguide-design.md (1)
72-72: LGTM! Documentation updated for new opacity tokens.New opacity tokens (0.7 and 0.1) are properly documented with consistent formatting.
Also applies to: 75-75
src/views/mentorship/ui/mentor-activities/ui/activity-card/activity-card.tsx
Show resolved
Hide resolved
|
Lighthouse Report:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/views/mentorship/mentorship.tsx (3)
12-15: Consider using a more descriptive prop name.The
coursesboolean prop could be more descriptive, e.g.,showMentorshipCoursesordisplayCourses, to better indicate its purpose.type MentorshipProps = { mentorshipData: MentorshipCourse; - courses?: boolean; + showMentorshipCourses?: boolean; };
17-17: Update prop name in destructuring if renamed.If you rename the
coursesprop as suggested above, update the destructuring here as well.-export const Mentorship = ({ mentorshipData, courses = false }: MentorshipProps) => { +export const Mentorship = ({ mentorshipData, showMentorshipCourses = false }: MentorshipProps) => {
22-22: Document the Benefits component conditional rendering.The condition
!mentorshipData.titlefor rendering the Benefits component is not immediately clear. Consider adding a comment explaining why this check determines whether to show benefits.+ {/* Show benefits only for general mentorship page, not for specific courses */} {!mentorshipData.title && <Benefits {...benefitMentorshipMentors} />}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/views/mentorship/mentorship.tsx(1 hunks)
🔇 Additional comments (3)
src/views/mentorship/mentorship.tsx (3)
1-10: LGTM! Well-organized imports.The imports are logically grouped and all components are utilized in the implementation.
23-26: LGTM! Clean implementation of MentorActivities.The MentorActivities component receives clear, focused props from mentorshipData.
27-28: LGTM! Logical placement of optional components.The conditional rendering of MentorshipCourses and placement of MentorsFeedback follow a natural page flow.
What type of PR is this? (select all that apply)
Description
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
Release Notes
Overview
This release introduces several enhancements to the mentorship section, including new components, improved styling, and expanded functionality across the application.
New Features
MentorshipDetailsandDetailsMentorshipcomponents for displaying mentorship informationMentorActivitiescomponent to showcase various activitiesImprovements
Bug Fixes
Technical Updates
Dependencies
react-youtubeswiperBreaking Changes
MentorActivitiesandMentorshipCourses