Skip to content
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

532-refactor: FSD: Widget Course main #543

Merged
merged 18 commits into from
Oct 18, 2024

Conversation

SkarabeyDM
Copy link
Collaborator

@SkarabeyDM SkarabeyDM commented Sep 23, 2024

What type of PR is this? (select all that apply)

  • πŸ• Feature
  • πŸ› Bug Fix
  • 🚧 Breaking Change
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ“ Documentation Update

Description

@/widgets/course-main

  1. move course-main.test.tsx, course-main.tsx, course-main.module.scss to ui folder
  2. replace interface with type
  3. change JSX classes declaration to use cx (example in entities folder components)
  4. rename utils folder to helpers
  5. move courseLoader to entities/course/api.
  6. rename courseLoader -> loader including file name
  7. re-export loader in the entities/course/index.ts file

Related Tickets & Documents

Screenshots, Recordings

Added/updated tests?

  • πŸ‘Œ Yes
  • πŸ™…β€β™‚οΈ No, because they aren't needed
  • πŸ™‹β€β™‚οΈ No, because I need help

[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

  • New Features

    • Introduced localization support for course enrollment with English and Russian options.
    • Added a new HeroCourse component, replacing the previous CourseMain component across various course pages.
  • Improvements

    • Enhanced MainTitle component with additional size variants for better styling options.
  • Refactor

    • Updated routing to use a more generic loader for course-related pages.
  • Chores

    • Removed the CourseMain component directory and its associated tests, streamlining the codebase.
    • Added comprehensive test coverage for the new HeroCourse component and its functionalities.

@SkarabeyDM SkarabeyDM self-assigned this Sep 23, 2024
@SkarabeyDM SkarabeyDM changed the title Widget CourseMain to FSD and cx 532-refactor: Widget CourseMain to FSD and cx Sep 23, 2024
@SkarabeyDM SkarabeyDM changed the title 532-refactor: Widget CourseMain to FSD and cx FSD: Widget Course main Sep 23, 2024
Copy link

Lighthouse Report:

  • Performance: 95 / 100
  • Accessibility: 98 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

@SkarabeyDM SkarabeyDM changed the title FSD: Widget Course main 532-refactor: FSD: Widget Course main Sep 23, 2024
Copy link

Lighthouse Report:

  • Performance: 93 / 100
  • Accessibility: 98 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link

Lighthouse Report:

  • Performance: 97 / 100
  • Accessibility: 98 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link

Lighthouse Report:

  • Performance: 93 / 100
  • Accessibility: 98 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

@SkarabeyDM SkarabeyDM requested a review from KristiBo September 24, 2024 08:05
@Quiddlee Quiddlee requested a review from natanchik as a code owner October 16, 2024 15:47
Copy link
Contributor

coderabbitai bot commented Oct 16, 2024

πŸ“ Walkthrough

Walkthrough

The pull request introduces several changes, including the addition of a new constant heroCourseLocalized for localized strings in multiple languages, the renaming and restructuring of the CourseMain component to HeroCourse, and updates to the MainTitle component to support size variants. Additionally, the courseLoader has been renamed to a more generic loader, affecting various route definitions. The course-main directory has been removed, and a new hero-course directory has been created with corresponding tests and styles.

Changes

Files Change Summary
dev-data/hero-course.data.ts, dev-data/index.ts Added constant heroCourseLocalized for localized strings and exported it from index.ts.
src/app/routes.tsx, src/entities/course/api/loader.ts, src/entities/course/index.ts Replaced courseLoader with loader, updated exports accordingly.
src/pages/angular.tsx, src/pages/aws-developer.tsx, src/pages/aws-devops.tsx, src/pages/aws-fundamentals.tsx, src/pages/javascript-en.tsx, src/pages/javascript-preschool-ru.tsx, src/pages/javascript-ru.tsx, src/pages/nodejs.tsx, src/pages/react.tsx Replaced CourseMain with HeroCourse across multiple files.
src/shared/ui/main-title/main-title.module.scss, src/shared/ui/main-title/main-title.tsx Updated MainTitle component to include a size variant with corresponding styles.
src/widgets/course-main/course-main.test.tsx, src/widgets/course-main/index.ts, src/widgets/hero-course/index.ts, src/widgets/hero-course/ui/hero-course.module.scss, src/widgets/hero-course/ui/hero-course.tsx, src/widgets/hero-course/ui/hero-course.test.tsx Removed CourseMain, added HeroCourse with new styles and tests.

Assessment against linked issues

Objective Addressed Explanation
Move course-main.test.tsx, course-main.tsx, course-main.module.scss to ui folder (#532) βœ…
Replace interface with type (#532) ❌ No interface was found to replace.
Change JSX classes declaration to use cx (#532) ❌ No specific cx usage was observed.
Rename courseLoader to loader and re-export in entities/course/index.ts (#532) βœ…

Possibly related issues

Possibly related PRs

Suggested reviewers

  • dzmitry-varabei
  • andron13
  • natanchik
  • Quiddlee
  • SpaNb4

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?

❀️ Share
πŸͺ§ Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Outside diff range and nitpick comments (8)
dev-data/hero-course.data.ts (1)

1-4: LGTM. Consider future scalability.

The constant is well-structured and uses as const appropriately. For future scalability, consider using a separate localization library if the number of strings grows significantly.

src/widgets/hero-course/ui/hero-course.module.scss (1)

23-24: Good responsive design adjustment.

Consider using CSS variables for consistent padding values across breakpoints.

src/shared/ui/main-title/main-title.tsx (2)

11-19: LGTM with a minor suggestion.

The mainTitleVariants constant now supports size variants, enhancing flexibility. Consider adding a large size for future-proofing.

 const mainTitleVariants = cva(cx('main-title'), {
   variants: {
     size: {
       small: cx('small'),
       medium: '',
+      large: cx('large'),
     },
   },
   defaultVariants: { size: 'medium' },
 });

21-29: Looks good. Consider type refinement.

The MainTitle component now correctly uses the size prop. For better type safety, consider explicitly defining the size prop type.

-export const MainTitle = ({ children, className, size }: MainTitleProps) => {
+export const MainTitle = ({ children, className, size }: MainTitleProps & { size?: 'small' | 'medium' }) => {
dev-data/index.ts (1)

24-24: LGTM. Consider alphabetical ordering.

The new export fits well. For consistency, consider placing it alphabetically.

src/widgets/hero-course/ui/hero-course.test.tsx (3)

17-40: Test data setup is comprehensive.

The mock course objects cover different scenarios effectively. Consider extracting the common properties of the mock courses into a separate object to reduce duplication.

const baseMockedCourse = {
  language: ['English'],
  type: 'Mentoring Program',
  mode: 'online',
  secondaryIcon: MOCKED_IMAGE_PATH,
};

const mockedCourse = {
  ...baseMockedCourse,
  title: 'Node.js',
  enroll: 'https://wearecommunity.io/events/nodejs-rs-2024q1',
  startDate: dayJS().subtract(2, 'month').format('D MMM, YYYY'),
};

// Use baseMockedCourse for other mocked courses as well

42-72: Component rendering tests are well-structured.

The tests cover the main aspects of the component. Consider adding a test for the course type rendering:

it('renders the course type correctly', async () => {
  const typeElement = await screen.findByText('Mentoring Program');
  expect(typeElement).toBeVisible();
});

74-86: Course label tests are appropriate.

The tests cover "available" and "upcoming" states. Add a test for the "planned" state:

it('renders the section with correct label "PLANNED"', () => {
  renderWithRouter(<HeroCourse courseName="Node.js" />);
  expect(screen.getByText('planned')).toBeVisible();
});
πŸ“œ Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

πŸ“₯ Commits

Files that changed from the base of the PR and between 21d26c6 and 401edb1.

πŸ“’ Files selected for processing (22)
  • dev-data/hero-course.data.ts (1 hunks)
  • dev-data/index.ts (1 hunks)
  • src/app/routes.tsx (10 hunks)
  • src/entities/course/api/loader.ts (1 hunks)
  • src/entities/course/index.ts (1 hunks)
  • src/pages/angular.tsx (2 hunks)
  • src/pages/aws-developer.tsx (2 hunks)
  • src/pages/aws-devops.tsx (2 hunks)
  • src/pages/aws-fundamentals.tsx (2 hunks)
  • src/pages/javascript-en.tsx (2 hunks)
  • src/pages/javascript-preschool-ru.tsx (2 hunks)
  • src/pages/javascript-ru.tsx (2 hunks)
  • src/pages/nodejs.tsx (2 hunks)
  • src/pages/react.tsx (2 hunks)
  • src/shared/ui/main-title/main-title.module.scss (1 hunks)
  • src/shared/ui/main-title/main-title.tsx (1 hunks)
  • src/widgets/course-main/course-main.test.tsx (0 hunks)
  • src/widgets/course-main/index.ts (0 hunks)
  • src/widgets/hero-course/index.ts (1 hunks)
  • src/widgets/hero-course/ui/hero-course.module.scss (1 hunks)
  • src/widgets/hero-course/ui/hero-course.test.tsx (1 hunks)
  • src/widgets/hero-course/ui/hero-course.tsx (3 hunks)
πŸ’€ Files with no reviewable changes (2)
  • src/widgets/course-main/course-main.test.tsx
  • src/widgets/course-main/index.ts
βœ… Files skipped from review due to trivial changes (1)
  • src/widgets/hero-course/index.ts
🧰 Additional context used
πŸ““ Learnings (1)
src/pages/react.tsx (1)
Learnt from: Quiddlee
PR: rolling-scopes/site#572
File: src/pages/home.tsx:25-25
Timestamp: 2024-09-30T12:07:39.137Z
Learning: The `Courses` component in `src/widgets/courses/other-courses.test.tsx`, `src/pages/courses.tsx`, and `dev-data/courses-path.data.ts` is a different component from the one replaced in `src/pages/home.tsx`.
πŸ”‡ Additional comments (29)
src/entities/course/api/loader.ts (1)

3-5: Approved: Function renamed as per refactoring objectives.

The change from courseLoader to loader is in line with the PR goals.

Let's check for any lingering references to the old name:

βœ… Verification successful

Action Required: Update search command to include 'tsx' files

The previous search command did not recognize tsx files. Let's update the script to include both .ts and .tsx file types correctly:


Verified: No remaining references to 'courseLoader' found.

The function has been successfully renamed without impacting other parts of the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining references to 'courseLoader'
rg 'courseLoader' --type ts --type tsx

Length of output: 66


Script:

#!/bin/bash
# Search for any remaining references to 'courseLoader' in .ts and .tsx files
rg 'courseLoader' --type-add 'tsx:*.tsx' --type ts --type tsx

Length of output: 61

src/entities/course/index.ts (1)

4-4: Loader export added correctly.

The new export statement for the loader function is in line with the PR objectives.

βœ… Verification successful

Loader implementation verified successfully.

The loader function is correctly implemented and aligns with the project requirements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the loader implementation
ast-grep --lang typescript --pattern 'export const loader = $_' src/entities/course/api/loader.ts

Length of output: 264

src/widgets/hero-course/ui/hero-course.module.scss (1)

6-12: Good use of flexbox and gap.

The layout implementation is clean and follows modern CSS practices.

src/shared/ui/main-title/main-title.module.scss (2)

9-16: Good use of SCSS nesting and variables.

The .small class is well-structured and uses appropriate properties for a smaller title variant. The use of SCSS variables ensures consistency across the stylesheet.


17-22: Responsive design implemented correctly.

The media query for tablets is well-placed and makes appropriate adjustments for smaller screens. This ensures good readability across different devices.

src/pages/react.tsx (3)

5-5: Import updated to reflect component rename.

The import statement has been correctly updated to use HeroCourse instead of CourseMain.


15-15: Component usage updated consistently.

The HeroCourse component is now used in place of CourseMain, maintaining consistency with the import change.


Line range hint 1-24: Overall structure remains intact.

The rest of the file, including other component imports and usage, remains unchanged. This is consistent with the focused nature of the refactoring described in the PR objectives.

src/pages/aws-devops.tsx (2)

5-5: Import statement updated correctly.

The HeroCourse component is now imported from the correct location.


16-16: Component usage updated correctly.

The HeroCourse component is now used instead of CourseMain, maintaining the courseName prop.

Let's verify if this change is consistent across the project:

src/pages/nodejs.tsx (2)

6-6: Import statement updated correctly.

The import statement has been updated to reflect the new component name and path.


17-17: Component usage updated correctly.

The CourseMain component has been replaced with HeroCourse, maintaining the same prop.

Verify the consistency of the changes:

βœ… Verification successful

Please execute the following scripts to verify references and usage:


Component usage correctly updated.

All references to CourseMain have been removed, and HeroCourse is used consistently across the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining references to CourseMain
rg --type typescript 'CourseMain'
# Check for correct usage of HeroCourse
rg --type typescript 'HeroCourse'

Length of output: 137


Script:

#!/bin/bash
# Check for any remaining references to CourseMain in .ts and .tsx files
rg 'CourseMain' --glob '*.ts' --glob '*.tsx'
# Check for correct usage of HeroCourse in .ts and .tsx files
rg 'HeroCourse' --glob '*.ts' --glob '*.tsx'

Length of output: 2288

src/pages/aws-developer.tsx (2)

5-5: Import statement updated correctly.

The HeroCourse component is now imported from the correct path.


17-17: Component usage updated consistently.

HeroCourse replaces CourseMain with the same prop.

src/pages/aws-fundamentals.tsx (2)

5-5: Import statement updated correctly.

The import has been changed from CourseMain to HeroCourse, aligning with the refactoring objectives.


16-16: Component usage updated correctly.

The CourseMain component has been replaced with HeroCourse as intended. The courseName prop is still being passed correctly.

Verify that the HeroCourse component's interface matches the previous CourseMain component:

βœ… Verification successful

Unable to verify prop types of CourseMain and HeroCourse based on current script results.


Unable to verify prop types of CourseMain and HeroCourse based on current script results.


Unable to verify prop types of CourseMain and HeroCourse based on current script results.


Component usage updated correctly.

The CourseMain component has been replaced with HeroCourse. The courseName prop is still being passed correctly, and the new HeroCourse component includes additional props (lang, type) as expected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Compare the prop types of CourseMain and HeroCourse components

# Search for CourseMain component definition
echo "CourseMain component:"
ast-grep --lang typescript --pattern 'type CourseMainProps = { $$$1 }' || ast-grep --lang typescript --pattern 'interface CourseMainProps { $$$1 }'

# Search for HeroCourse component definition
echo "HeroCourse component:"
ast-grep --lang typescript --pattern 'type HeroCourseProps = { $$$1 }' || ast-grep --lang typescript --pattern 'interface HeroCourseProps { $$$1 }'

Length of output: 251


Script:

#!/bin/bash
# Description: Extract prop definitions from CourseMain and HeroCourse components

# Find prop definitions in CourseMain
echo "CourseMain props:"
ast-grep --lang typescript --pattern 'const CourseMain = ($_props) =>' || ast-grep --lang typescript --pattern 'function CourseMain($_props)'

# Find prop definitions in HeroCourse
echo "HeroCourse props:"
ast-grep --lang typescript --pattern 'const HeroCourse = ($_props) =>' || ast-grep --lang typescript --pattern 'function HeroCourse($_props)'

Length of output: 925


Script:

#!/bin/bash
# Description: Extract prop type definitions using ripgrep

# Search for CourseMainProps type or interface
echo "CourseMainProps definition:"
rg -A 5 "type\s+CourseMainProps\s+=\s+{|interface\s+CourseMainProps\s+{" src/

# Search for HeroCourseProps type or interface
echo "HeroCourseProps definition:"
rg -A 5 "type\s+HeroCourseProps\s+=\s+{|interface\s+HeroCourseProps\s+{" src/

Length of output: 639


Script:

#!/bin/bash
# Description: Extract prop type definitions using ripgrep with corrected regex

# Search for CourseMainProps type or interface
echo "CourseMainProps definition:"
rg -A 5 "(type\s+CourseMainProps\s+=\s+\{|interface\s+CourseMainProps\s+\{)" src/

# Search for HeroCourseProps type or interface
echo "HeroCourseProps definition:"
rg -A 5 "(type\s+HeroCourseProps\s+=\s+\{|interface\s+HeroCourseProps\s+\{)" src/

Length of output: 654

src/pages/javascript-en.tsx (2)

6-6: Import statement updated correctly.

The import statement has been updated to reflect the new component name and path.


18-18: Component usage updated correctly.

The HeroCourse component is now used with the courseName prop. This change aligns with the refactoring objectives.

src/pages/javascript-preschool-ru.tsx (2)

6-6: Import statement updated.

The HeroCourse component is now imported from @/widgets/hero-course.


20-20: CourseMain replaced with HeroCourse.

The CourseMain component has been replaced with HeroCourse, maintaining the same props.

βœ… Verification successful

HeroCourse Component Replacement Verified

The replacement of CourseMain with HeroCourse has been successfully verified across the codebase with consistent usage observed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the usage of HeroCourse component across the codebase
rg --type typescript --type tsx "HeroCourse" -g '!src/pages/javascript-preschool-ru.tsx'

Length of output: 123


Script:

#!/bin/bash
# Verify the usage of HeroCourse component across the codebase
rg --type tsx "HeroCourse" -g '!src/pages/javascript-preschool-ru.tsx'

Length of output: 98


Script:

#!/bin/bash
# Verify the usage of HeroCourse component across the codebase
rg "HeroCourse" --glob "*.tsx" --glob "!src/pages/javascript-preschool-ru.tsx"

Length of output: 1999

src/pages/angular.tsx (2)

6-6: Import statement added for HeroCourse.

The import is correctly formatted and aligns with the component usage.


19-19: CourseMain replaced with HeroCourse.

The component replacement is correct and maintains the existing prop usage.

Verify the component usage:

βœ… Verification successful

Component Replacement Verified

<CourseMain> has been successfully replaced with <HeroCourse>, and no remaining instances of <CourseMain> were found. <HeroCourse> is correctly used throughout the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining usage of CourseMain
rg --type typescript --type tsx 'CourseMain'

# Verify HeroCourse usage
rg --type typescript --type tsx 'HeroCourse'

Length of output: 159


Script:

#!/bin/bash
# Check for any remaining usage of CourseMain
rg --type ts --type tsx 'CourseMain'

# Verify HeroCourse usage
rg --type ts --type tsx 'HeroCourse'

Length of output: 129


Script:

#!/bin/bash
# Check for any remaining usage of CourseMain in .ts and .tsx files
rg 'CourseMain' --glob '*.ts' --glob '*.tsx'

# Verify HeroCourse usage in .ts and .tsx files
rg 'HeroCourse' --glob '*.ts' --glob '*.tsx'

Length of output: 2288

src/pages/javascript-ru.tsx (2)

6-6: Import change approved.

The HeroCourse component is correctly imported from the new location.


20-20: Component replacement approved. Verify prop types.

The CourseMain component has been correctly replaced with HeroCourse. The same props are passed, which is good.

Ensure that the HeroCourse component accepts the same prop types as CourseMain:

src/widgets/hero-course/ui/hero-course.test.tsx (1)

1-15: Imports and mocks are well-structured.

The necessary dependencies are imported, and the mocks for use-course-by-title and react-router-dom are correctly set up.

src/app/routes.tsx (3)

3-3: Updated import for loader

The import has been changed from a specific courseLoader to a generic loader. This change aligns with the PR objectives.


18-18: Consistent use of generic loader

The loader is now used consistently across all course routes. This change improves code consistency and maintainability.

Also applies to: 27-27, 36-36, 45-45, 54-54, 63-63, 72-72, 81-81, 90-90


3-3: Verify loader functionality

Ensure the new generic loader maintains the same functionality as the previous courseLoader for all affected routes.

Also applies to: 18-18, 27-27, 36-36, 45-45, 54-54, 63-63, 72-72, 81-81, 90-90

βœ… Verification successful

Loader functionality verified successfully

The new generic loader is correctly implemented and exported in the codebase, maintaining the intended functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation of the new loader function

# Test: Check the implementation of the new loader
ast-grep --lang typescript --pattern 'export const loader = $_'

# Test: Verify that the loader is exported correctly
rg --type typescript 'export.*loader' src/entities/course/index.ts

Length of output: 334


Script:

#!/bin/bash
# Description: Verify the implementation of the new loader function with corrected ripgrep usage

# Test: Check the implementation of the new loader
ast-grep --lang typescript --pattern 'export const loader = $_'

# Test: Verify that the loader is exported correctly using file extensions
rg --ext ts --ext tsx 'export.*loader' src/entities/course/index.ts

Length of output: 730


Script:

#!/bin/bash
# Description: Verify the implementation of the new loader function with corrected ripgrep usage

# Test: Check the implementation of the new loader
ast-grep --lang typescript --pattern 'export const loader = $_'

# Test: Verify that the loader is exported correctly using file extensions
rg -g '*.ts' -g '*.tsx' 'export.*loader' src/entities/course/index.ts

Length of output: 341

src/widgets/hero-course/ui/hero-course.tsx (1)

Line range hint 27-62: Component refactoring aligns with objectives

The renaming and restructuring of the component to HeroCourse is consistent with the refactoring goals. The use of cx for class name binding enhances styling management.

src/widgets/hero-course/ui/hero-course.module.scss Outdated Show resolved Hide resolved
src/widgets/hero-course/ui/hero-course.test.tsx Outdated Show resolved Hide resolved
src/widgets/hero-course/ui/hero-course.tsx Show resolved Hide resolved
src/widgets/hero-course/ui/hero-course.tsx Show resolved Hide resolved
src/shared/ui/main-title/main-title.tsx Outdated Show resolved Hide resolved
src/widgets/hero-course/ui/hero-course.test.tsx Outdated Show resolved Hide resolved
src/widgets/hero-course/ui/hero-course.test.tsx Outdated Show resolved Hide resolved
Copy link

Lighthouse Report:

  • Performance: 97 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)
src/widgets/hero-course/ui/hero-course.tsx (1)

14-14: Consider using an absolute import path.

The import for heroCourseLocalized might benefit from using an absolute path for consistency.

- import { heroCourseLocalized } from 'data';
+ import { heroCourseLocalized } from '@/data';
src/widgets/hero-course/ui/hero-course.module.scss (1)

13-16: Review necessity of both width and min-width

Since width and min-width are set to fixed pixel values, setting both may be unnecessary. Consider retaining only width for clarity.

πŸ“œ Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

πŸ“₯ Commits

Files that changed from the base of the PR and between 401edb1 and 9c8de8e.

πŸ“’ Files selected for processing (2)
  • src/widgets/hero-course/ui/hero-course.module.scss (1 hunks)
  • src/widgets/hero-course/ui/hero-course.tsx (3 hunks)
🧰 Additional context used
πŸ”‡ Additional comments (5)
src/widgets/hero-course/ui/hero-course.tsx (5)

16-18: LGTM!

The style import update and classNames initialization improve styling management.


26-26: LGTM!

The component renaming and props type update align with the refactoring objectives.


42-45: LGTM!

The structural changes improve semantics and styling consistency.


47-47: LGTM!

The MainTitle usage with size prop improves component flexibility.


55-57: LGTM!

The localized link label and fixed HTML structure improve the component.

Copy link

Lighthouse Report:

  • Performance: 84 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link

Lighthouse Report:

  • Performance: 92 / 100
  • Accessibility: 100 / 100
  • Best Practices: 100 / 100
  • SEO: 100 / 100

View detailed report

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (5)
src/widgets/hero-course/helpers/has-day.ts (1)

1-2: Consider adding input validation.

The function assumes a specific date format. Add checks for empty strings or inputs without commas.

 export function hasDayInDate(date: string) {
+  if (!date || !date.includes(',')) return false;
   const dateWithoutDay = date.split(',')[0];
src/widgets/hero-course/helpers/has-day.test.ts (1)

9-16: Test suite is well-structured and covers all scenarios.

Minor suggestion: Consider making the test description more specific, e.g., "should return true for dates with day and false for dates without day or empty strings".

src/widgets/hero-course/helpers/get-course-status.ts (2)

3-3: Good use of constants for configurability.

Importing constants improves maintainability. Consider grouping related imports.

 import { COURSE_STALE_AFTER_DAYS, COURSE_UPCOMING_PERIOD_MONTHS } from '@/app/const';
 import type { CourseStatus } from '@/entities/course';
-import { courseStatus } from 'data';
+import { courseStatus } from '@/data';

Also applies to: 5-5


21-21: Improved status assignment using courseStatus object.

Good use of courseStatus for consistency. Consider simplifying the function:

-  let label: CourseStatus = courseStatus.planned;
-
   if (isAvailable) {
     return courseStatus.available;
   }
@@ -26,7 +24,5 @@ export function getCourseStatus(courseStartDate: string): CourseStatus {
   if (isUpcoming) {
     return courseStatus.upcoming;
   }
-
-  return label;
+  return courseStatus.planned;
 }

Also applies to: 24-24, 28-28

src/widgets/hero-course/ui/hero-course.tsx (1)

20-24: Consider making the type property more flexible.

The type property is currently restricted to 'Pre-school RU'. If there's a possibility of more course types in the future, consider using a union type or making it a string.

πŸ“œ Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

πŸ“₯ Commits

Files that changed from the base of the PR and between 3f21175 and c83b4c1.

πŸ“’ Files selected for processing (9)
  • dev-data/hero-course.data.ts (1 hunks)
  • dev-data/index.ts (1 hunks)
  • src/app/const/index.ts (1 hunks)
  • src/widgets/hero-course/helpers/get-course-status.test.ts (1 hunks)
  • src/widgets/hero-course/helpers/get-course-status.ts (2 hunks)
  • src/widgets/hero-course/helpers/has-day.test.ts (1 hunks)
  • src/widgets/hero-course/helpers/has-day.ts (1 hunks)
  • src/widgets/hero-course/ui/hero-course.test.tsx (3 hunks)
  • src/widgets/hero-course/ui/hero-course.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • dev-data/hero-course.data.ts
  • dev-data/index.ts
🧰 Additional context used
πŸ““ Learnings (1)
src/widgets/hero-course/ui/hero-course.tsx (1)
Learnt from: Quiddlee
PR: rolling-scopes/site#543
File: src/widgets/hero-course/ui/hero-course.tsx:14-14
Timestamp: 2024-10-16T18:09:58.731Z
Learning: In this TypeScript project, `'data'` is a valid import path for module imports due to TypeScript path aliases configuration.
πŸ”‡ Additional comments (22)
src/widgets/hero-course/helpers/has-day.ts (2)

1-5: Function implementation looks good.

The function is concise and focused on its task of checking for digits in the date string before the comma.


4-4: Efficient use of regular expression.

The regex /\d/ efficiently checks for any digit in the string.

src/widgets/hero-course/helpers/has-day.test.ts (2)

1-1: Import looks good.


3-7: Test data is well-structured and covers various scenarios.

src/app/const/index.ts (1)

5-7: New constants added for course timing.

The new constants COURSE_STALE_AFTER_DAYS and COURSE_UPCOMING_PERIOD_MONTHS are consistent with the existing structure.

Please confirm that 14 days and 3 months are the correct values for these constants.

src/widgets/hero-course/helpers/get-course-status.ts (2)

14-14: Improved configurability for 'Available' status.

Using COURSE_STALE_AFTER_DAYS enhances flexibility and readability.


17-18: Enhanced 'Upcoming' status logic.

Good use of COURSE_UPCOMING_PERIOD_MONTHS and ensuring mutual exclusivity with 'Available' status.

src/widgets/hero-course/helpers/get-course-status.test.ts (4)

1-3: Imports look good.

Necessary dependencies and types are correctly imported.


5-16: Well-structured test data.

Test data covers various scenarios, including dates with and without day information.


18-26: Effective parameterized testing.

The test case efficiently checks course status for various dates using it.each.


28-36: Comprehensive test coverage.

This test case complements the first by checking dates without day information.

src/widgets/hero-course/ui/hero-course.tsx (6)

1-1: Import changes look good.

The new imports and path updates align with the refactoring objectives.

Also applies to: 3-3, 12-12, 14-14


16-18: Style setup is appropriate.

The style import update and classNames setup are consistent with the component renaming.


26-26: Component renaming is consistent.

The component renaming and props type update align with the PR objectives.


42-47: Component structure improvements are good.

The changes to semantic HTML, class naming, and title component usage are appropriate.


49-51: Hero subtitle rendering is well-implemented.

The conditional rendering and consistent class naming for the subtitle are good additions.


55-55: Localized link label is a good improvement.

Using heroCourseLocalized for the link label supports better internationalization.

src/widgets/hero-course/ui/hero-course.test.tsx (5)

2-4: Import statements updated correctly.

The changes align with the refactoring objectives.


23-37: Mocked course object expanded appropriately.

The changes address the previous feedback about including all properties in the mocked object.


40-50: Additional mocked course objects improve test coverage.

The mockedCourseAvailable and mockedCourseUpcoming objects enhance the test suite.


52-54: Test suite structure updated correctly.

The changes reflect the component renaming from CourseMain to HeroCourse.


Line range hint 57-81: Test cases updated to match new component structure.

The changes align with the new HeroCourse component. Consider adding tests for edge cases and error handling as suggested in the previous review.

@ansivgit ansivgit merged commit 58e7c6d into main Oct 18, 2024
4 checks passed
@ansivgit ansivgit deleted the refactor/532-fsd-widget-course-main branch October 18, 2024 08:42
@coderabbitai coderabbitai bot mentioned this pull request Dec 20, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FSD: Widget Course main
6 participants