Add solution #4244
Add solution #4244
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
You've done a great job separating the Article and Welcome components, and the overall structure is clean. However, this submission requires a change because the navigation block was not extracted into its own component, which is a core requirement of the task. The goal is to split the application into components based on distinct CSS blocks.
Please create a new Navigation component by following these steps:
- Create a
src/components/Navigationfolder withNavigation.jsxandNavigation.cssfiles inside. - Move the
<nav>element and its contents fromHeader.jsxtoNavigation.jsx. - Move the corresponding
.navigation__linkstyles fromHeader.csstoNavigation.css. - Finally, import and use the
Navigationcomponent inside yourHeadercomponent.
This will complete the component decomposition as required. You're very close!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| .navigation__link { | ||
| display: inline-block; | ||
| outline: none; | ||
| text-decoration: none; | ||
| opacity: 0.7; | ||
| padding: 0 0.5em; | ||
| color: black; | ||
| transition: opacity 0.2s ease-in-out; | ||
| } | ||
|
|
||
| .navigation__link:hover, | ||
| .navigation__link:focus { | ||
| opacity: 1; | ||
| } |
There was a problem hiding this comment.
These styles belong to the navigation BEM block. According to the task description, you should split the application into components based on CSS blocks. It's recommended to create a separate Navigation component and move this CSS into its own file (Navigation/Navigation.css).
src/components/Header/Header.jsx
Outdated
| <nav className="navigation"> | ||
| <a className="navigation__link" href="#about"> | ||
| About | ||
| </a> | ||
| <a className="navigation__link" href="#services"> | ||
| Services | ||
| </a> | ||
| <a className="navigation__link" href="#contact"> | ||
| Contact | ||
| </a> | ||
| </nav> |
There was a problem hiding this comment.
This <nav> element and its content form a distinct 'navigation' BEM block. As per the task instructions, this should be extracted into its own Navigation component. You should create a new folder src/components/Navigation, create Navigation.jsx and Navigation.css inside it, and move this logic there.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job on correctly separating the Article and Welcome components! However, changes are needed because the core requirement to extract the navigation logic into its own component has not been met. This was also pointed out in the previous review.
To complete the task, please follow these steps:
- Create a new folder
src/components/Navigation. - Inside this folder, create
Navigation.jsxandNavigation.css. - Move the
<nav>element and its contents fromHeader.jsxintoNavigation.jsx. - Move the corresponding
.navigation__linkstyles fromHeader.cssintoNavigation.css. - Import and use the new
Navigationcomponent within theHeadercomponent.
Once the Navigation component is properly extracted, your solution will be ready. Keep up the good work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| .navigation__link { | ||
| display: inline-block; | ||
|
|
||
| padding: 0 0.5em; | ||
|
|
||
| color: black; | ||
| text-decoration: none; | ||
|
|
||
| opacity: 0.7; | ||
| outline: none; | ||
|
|
||
| transition: opacity 0.2s ease-in-out; | ||
| } | ||
|
|
||
| .navigation__link:hover, | ||
| .navigation__link:focus { | ||
| opacity: 1; | ||
| } |
There was a problem hiding this comment.
These navigation styles should be moved to the Navigation.css file, which should be created inside a new Navigation component folder. The task requires splitting the CSS along with the components.
src/components/Header/Header.jsx
Outdated
| <nav className="navigation"> | ||
| <a className="navigation__link" href="#about"> | ||
| About | ||
| </a> | ||
| <a className="navigation__link" href="#services"> | ||
| Services | ||
| </a> | ||
| <a className="navigation__link" href="#contact"> | ||
| Contact | ||
| </a> | ||
| </nav> |
There was a problem hiding this comment.
This nav element and its content should be moved to a separate Navigation component. The task requires splitting the UI into components based on CSS blocks, and navigation is a distinct block that needs its own component.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Excellent progress on this task! You've done a fantastic job creating the Navigation component and separating the logic, which was the main goal. Your project structure is much cleaner now.
I am approving your solution as an educational override so you can move forward, but please note there is one small thing to fix. The styles for the navigation links are still present in src/components/Header/Header.css. Since you've correctly moved them to Navigation.css, the old styles in Header.css are now redundant and should be removed to complete the component separation.
Keep up the great work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| .navigation__link { | ||
| display: inline-block; | ||
|
|
||
| padding: 0 0.5em; | ||
|
|
||
| color: black; | ||
| text-decoration: none; | ||
|
|
||
| opacity: 0.7; | ||
| outline: none; | ||
|
|
||
| transition: opacity 0.2s ease-in-out; | ||
| } | ||
|
|
||
| .navigation__link:hover, | ||
| .navigation__link:focus { | ||
| opacity: 1; | ||
| } |
There was a problem hiding this comment.
These styles belong to the Navigation component. Since they have already been moved to Navigation.css, this block of code is now redundant and should be removed from this file.
2 participants
No description provided.