www #4265
www #4265
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great job on decomposing the application into Welcome, Header, and Article components! The project structure is well-organized and most of the requirements have been met.
However, this solution requires changes because a key part of the decomposition was missed. The task description suggested creating a separate Navigation component, but the navigation logic and styles currently remain inside the Header component.
To complete the task, please:
- Create a new
Navigationcomponent with its ownNavigation.jsxandNavigation.cssfiles. - Move the
<nav>block fromHeader.jsxintoNavigation.jsx. - Move the related navigation styles from
Header.csstoNavigation.css. - Import and use the new
<Navigation />component insideHeader.jsx.
Making this change will fully align your work with the task's goal of creating modular, reusable components. You're very close to a perfect solution!
✨ 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
src/components/Header/Header.css
Outdated
| .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; | ||
| } |
There was a problem hiding this comment.
These styles are specific to the navigation links. The task description suggests that the navigation block could be its own separate component. To improve component modularity, it would be better to move these styles (along with the :hover/:focus styles below) into a dedicated Navigation.css file.
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.
The task description suggests creating a separate Navigation component. It would be a good practice to extract this <nav> block into its own Navigation.jsx file and then import and use it here like <Navigation />. This would better follow the principle of component decomposition.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving this submission as an educational override to help you progress, but please review the feedback below to fully complete the task. You've done a fantastic job creating the Navigation component and separating its styles, which was the main goal!
There are just two key fixes needed to make your solution perfect:
- Component Placement: The
<Navigation />component should be rendered inside theHeadercomponent. To fix this, please move the import forNavigationfromsrc/App.jsxtosrc/components/Header/Header.jsxand place the component tag within theHeader's JSX. - Missing Export: The application isn't building because the
Navigationcomponent is not being exported. Please addexport default Navigation;to the end of yoursrc/components/Navigation/Navigation.jsxfile.
Making these changes will resolve the test failures and align your code with the project requirements. Excellent effort on this task!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| import Article from './components/Article/Article'; | ||
| import Header from './components/Header/Header'; | ||
| import Welcome from './components/Welcome/Welcome'; | ||
| import Navigation from './components/Navigation/Navigation'; |
There was a problem hiding this comment.
This import should be moved to Header.jsx. The Navigation component is part of the header and should be imported where it is used.
| </article> | ||
| <Welcome /> | ||
| <Header /> | ||
| <Navigation /> |
There was a problem hiding this comment.
According to the task requirements, the Navigation component should be rendered inside the Header component, not directly in App.jsx. Please move this component into Header.jsx.
| </a> | ||
| </nav> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
This component needs to be exported to be used in other parts of the application. The task requirements specify that you should use a default export for all components.
| const Header = () => { | ||
| return ( | ||
| <header className="header"> | ||
| <h1 className="header__title">Site Name</h1> |
There was a problem hiding this comment.
To fulfill the task requirements, you need to import the Navigation component into this file and render it here, inside the <header> element.
2 participants
[DEMO LINK](https://shymkivvasyl.github.io/react_decompose/