720-refactor: After completing registration section #748
Conversation
github-actions
bot
changed the title
|
Lighthouse Report:
|
ansivgit
requested review from
andron13,
dzmitry-varabei,
SpaNb4 and
natanchik
as code owners
π WalkthroughWalkthroughThe pull request introduces a new "After Completing the Registration" section for the mentorship page. It involves creating a new component Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
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: 0
π§Ή Nitpick comments (6)
dev-data/mentors-after-register.data.ts (1)
6-19: Consider using numeric IDs and fix typo in Russian text.Two suggestions for improvement:
- Use numeric type for step IDs instead of strings since they represent sequential numbers
- Fix the spelling of "Π’Π΅Π»Π΅Π³ΡΠ°ΠΌ" (single 'ΠΌ') in Russian content
- id: '1', + id: 1, // ... similar changes for other IDs - content: 'ΠΈΠ· ΠΏΠΈΡΡΠΌΠ° ΠΊ ΡΠΎΠΎΠ±ΡΠ΅ΡΡΠ²Ρ Π²ΡΠ±ΡΠ°Π½Π½ΠΎΠ³ΠΎ ΠΊΡΡΡΠ° Π² Π’Π΅Π»Π΅Π³ΡΠ°ΠΌΠΌ ΠΈ ΠΠΈΡΠΊΠΎΡΠ΄', + content: 'ΠΈΠ· ΠΏΠΈΡΡΠΌΠ° ΠΊ ΡΠΎΠΎΠ±ΡΠ΅ΡΡΠ²Ρ Π²ΡΠ±ΡΠ°Π½Π½ΠΎΠ³ΠΎ ΠΊΡΡΡΠ° Π² Π’Π΅Π»Π΅Π³ΡΠ°ΠΌ ΠΈ ΠΠΈΡΠΊΠΎΡΠ΄',Also applies to: 26-39
src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.tsx (3)
23-24: Remove unnecessary data assignment.The assignment
const data = mentorsAfterRegisterDatais redundant.- const data = mentorsAfterRegisterData; -
40-40: Use bound className for step content.The step-content className should use the bound classNames utility like other elements.
- <p className="step-content">{content}</p> + <p className={cx('step-content')}>{content}</p>
26-46: Add accessibility attributes.Consider adding ARIA labels and roles to improve accessibility.
- <section className={cx('container')} data-testid="mentors-after-register"> + <section + className={cx('container')} + data-testid="mentors-after-register" + aria-labelledby="after-register-title" + > <div className={cx('content', 'mentors-after-register')}> - <WidgetTitle mods="asterisk">{data[lang].title}</WidgetTitle> + <WidgetTitle + id="after-register-title" + mods="asterisk" + > + {mentorsAfterRegisterData[lang].title} + </WidgetTitle> - <section className={cx('steps')}> + <section + className={cx('steps')} + role="list" + >src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.test.tsx (2)
6-7: Improve regex patterns for language validation.Current patterns are too permissive. Consider using more precise patterns that match the actual content structure.
-const latinSymbolsRegExp = /^[?!,.a-zA-Z0-9\s]+$/; -const cyrillicSymbolsRegExp = /^[?!,.Π°-ΡΠ-Π―ΡΠ0-9\s]+$/; +const latinSymbolsRegExp = /^[A-Z][a-zA-Z0-9\s.,!]+$/; +const cyrillicSymbolsRegExp = /^[Π-Π―Π][Π°-ΡΡΠ-Π―Π0-9\s.,!]+$/;
13-53: Add test for default lang prop.Add a test to verify that 'en' is used when lang prop is undefined.
it('uses "en" as default language when lang prop is undefined', () => { const { rerender } = render(<MentorsAfterRegister />); const titleWithoutLang = screen.getByTestId('widget-title'); rerender(<MentorsAfterRegister lang="en" />); const titleWithEn = screen.getByTestId('widget-title'); expect(titleWithoutLang.textContent).toBe(titleWithEn.textContent); });
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
π Files selected for processing (9)
dev-data/index.ts(1 hunks)dev-data/mentors-after-register.data.ts(1 hunks)dev-data/mentors-register.data.ts(0 hunks)src/views/mentorship.tsx(2 hunks)src/views/mentorship/ui/mentors-after-register/index.ts(1 hunks)src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.module.scss(1 hunks)src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.test.tsx(1 hunks)src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.tsx(1 hunks)src/widgets/mentors-register/ui/mentors-register.tsx(0 hunks)
π€ Files with no reviewable changes (2)
- src/widgets/mentors-register/ui/mentors-register.tsx
- dev-data/mentors-register.data.ts
β Files skipped from review due to trivial changes (2)
- src/views/mentorship/ui/mentors-after-register/index.ts
- src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.module.scss
π Additional comments (2)
src/views/mentorship.tsx (1)
2-2: LGTM!The MentorsAfterRegister component is properly integrated and positioned logically in the UI flow.
Also applies to: 35-35
dev-data/index.ts (1)
65-65: LGTM! Export follows established patterns.The new export for
mentorsAfterRegisterDatais well-placed and maintains the file's alphabetical ordering.
|
Lighthouse Report:
|
src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.tsx
Show resolved
Hide resolved
src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.test.tsx
Show resolved
Hide resolved
src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
π§Ή Nitpick comments (3)
src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.tsx (3)
16-20: Consider strengthening the type definitions.The types could be more specific:
idcould be constrained to positive integerscontentmight need to support HTML content if rich text is requiredtype StepAfterRegisterProps = { - id: number; + id: Positive<number>; subtitle: string; - content: string; + content: string | JSX.Element; };
23-23: Add type safety for the data access.Consider adding type checking for the imported data to ensure type safety at runtime.
- const data = mentorsAfterRegisterData; + const data = mentorsAfterRegisterData satisfies Record<Language, { + title: string; + steps: StepAfterRegisterProps[]; + }>;
31-43: Extract the step rendering logic.Consider extracting the step rendering logic into a separate component for better maintainability and reusability.
+const AfterRegisterStep = ({ id, subtitle, content }: StepAfterRegisterProps) => ( + <article className={cx('step')} data-testid="after-register-step"> + <Subtitle className={cx('step-subtitle')} fontSize="small" color="black"> + {`${id}. ${subtitle}`} + </Subtitle> + <p className={cx('step-content')} data-testid="step-content">{content}</p> + </article> +); <section className={cx('steps')}> - {data[lang].steps.map((step: StepAfterRegisterProps) => { - const { id, subtitle, content } = step; - - return ( - <article className={cx('step')} key={step.id} data-testid="after-register-step"> - <Subtitle className={cx('step-subtitle')} fontSize="small" color="black"> - {`${id}. ${subtitle}`} - </Subtitle> - <p className={cx('step-content')} data-testid="step-content">{content}</p> - </article> - ); - })} + {data[lang].steps.map((step) => ( + <AfterRegisterStep key={step.id} {...step} /> + ))} </section>
π Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
π Files selected for processing (3)
dev-data/mentors-after-register.data.ts(1 hunks)src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.test.tsx(1 hunks)src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.tsx(1 hunks)
π§ Files skipped from review as they are similar to previous changes (2)
- dev-data/mentors-after-register.data.ts
- src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.test.tsx
π Additional comments (2)
src/views/mentorship/ui/mentors-after-register/ui/mentors-after-register.tsx (2)
6-6: Verify the data import path.The import path 'data' seems inconsistent with the path mentioned in the summary ('dev-data/index.ts'). Ensure the import path is correct and follows the project's path aliasing conventions.
β Verification successful
Import path is correctly configured
The import path 'data' is valid as it's properly aliased to '../dev-data' in tsconfig.json.
π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the actual location of mentorsAfterRegisterData rg --type ts "export.*mentorsAfterRegisterData"Length of output: 223
Script:
#!/bin/bash # Check for path alias configurations in tsconfig files fd tsconfig.json --exec cat {}Length of output: 1174
40-40: Consider content sanitization.If the content can include HTML, ensure proper sanitization to prevent XSS attacks.
ansivgit
deleted the
refactor/720-after-completing-registration-section
branch
What type of PR is this? (select all that apply)
Description
Implement After Completing the Registration section of the new design for the mentorship page
Design file - https://www.figma.com/design/opKwNO3FhSfQqR5uUpjJQp/RS-Mentorship?node-id=0-1&p=f&t=9p19VOoIuzY2KHpn-0
Related Tickets & Documents
Screenshots, Recordings
Before
After
Added/updated tests?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests