556-feat: Widget not found #667
Conversation
github-actions
bot
changed the title
|
run visual now |
|
run visual now |
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
KristiBo
requested review from
andron13,
dzmitry-varabei,
Quiddlee,
ansivgit and
natanchik
as code owners
📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve the removal of the existing mentorship page functionality and the introduction of a new mentorship course route within a Next.js application. The previous Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 6
🧹 Outside diff range and nitpick comments (3)
src/views/not-found/not-found.module.scss (1)
1-18: Extract magic numbers into variablesConsider extracting magic numbers into semantic variables for better maintainability.
+ $spacing-large: 30px; + $image-max-width: 170px; + $content-max-width: 480px; .not-found { display: flex; flex-direction: column; - gap: 30px; + gap: $spacing-large; min-height: 100vh; } .not-found-image { - max-width: 170px; + max-width: $image-max-width; height: auto; } .not-found-paragraph { - max-width: 480px; + max-width: $content-max-width;src/app/mentorship/page.tsx (1)
5-9: Consider enhancing metadata for SEO.The metadata could include additional SEO-friendly properties like description, keywords, and OpenGraph tags.
export async function generateMetadata(): Promise<Metadata> { const title = `Mentorship · The Rolling Scopes School`; - - return { title }; + return { + title, + description: 'Explore mentorship opportunities at The Rolling Scopes School', + openGraph: { + title, + description: 'Explore mentorship opportunities at The Rolling Scopes School', + }, + }; }src/app/mentorship/[course]/page.tsx (1)
18-31: Improve course matching precision.Using
includesfor path matching could lead to false positives. Consider using exact matching.const mentorshipCourse = - mentorshipCourses.find((item) => item.detailsUrl.includes(`/${course}`)) + mentorshipCourses.find((item) => + item.detailsUrl.split('/').pop() === course) || mentorshipCoursesDefault;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
src/app/[...mentorship]/page.tsx(0 hunks)src/app/mentorship/[course]/page.tsx(1 hunks)src/app/mentorship/page.tsx(1 hunks)src/app/not-found.tsx(1 hunks)src/shared/__tests__/visual/not-found.spec.ts(1 hunks)src/views/not-found.tsx(0 hunks)src/views/not-found/not-found.module.scss(1 hunks)src/views/not-found/not-found.tsx(1 hunks)src/widgets/not-found/index.ts(0 hunks)src/widgets/not-found/ui/not-found.scss(0 hunks)src/widgets/not-found/ui/not-found.tsx(0 hunks)
💤 Files with no reviewable changes (5)
- src/widgets/not-found/index.ts
- src/views/not-found.tsx
- src/widgets/not-found/ui/not-found.tsx
- src/widgets/not-found/ui/not-found.scss
- src/app/[...mentorship]/page.tsx
🔇 Additional comments (7)
src/app/not-found.tsx (1)
2-2: LGTM: Import path updated correctly
The import path change reflects the new file structure, aligning with the PR objective of reorganizing the not-found component.
src/views/not-found/not-found.tsx (3)
1-9: LGTM! Well-organized imports and proper CSS modules setup.
11-28: Great implementation with semantic HTML and accessibility!
The component follows best practices with:
- Semantic
maintag - Descriptive alt text
- Proper test ID for E2E testing
7-7: Verify CSS module implementation
Let's check if the CSS module follows the requirements of removing parent selectors.
Also applies to: 13-18
✅ Verification successful
CSS module implementation is correct
The CSS module follows best practices with no parent selectors. It uses flat class names and includes proper media query usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check CSS module for parent selectors
cat src/views/not-found/not-found.module.scss
Length of output: 291
src/app/mentorship/page.tsx (2)
1-3: LGTM! Imports are well-structured.
11-13: LGTM! Component follows Next.js best practices.
src/app/mentorship/[course]/page.tsx (1)
1-3: LGTM! Imports are properly organized.
|
Lighthouse Report:
|
What type of PR is this? (select all that apply)
Description
Related Tickets & Documents
Screenshots, Recordings
Please replace this line with any relevant images for UI changes.
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
Bug Fixes
Documentation
Chores