Eveloper #4261
Eveloper #4261
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 Header, Welcome, and Article components! The separation of logic and styles is well done.
However, there's a critical issue with the project structure that needs to be fixed. Your solution code is currently in a src folder at the root of the repository, but it should be inside the decompose project folder. To fix this, please move the contents of your root src directory into decompose/src, replacing the default Vite boilerplate files. This will ensure that your components are correctly part of the application that gets built and run.
While you're making that change, it would be a good time to also remove the leftover instructional comments from files like App.jsx and Welcome.jsx to clean up the code.
✨ 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
| function App() { | ||
| const [count, setCount] = useState(0) | ||
|
|
||
| return ( | ||
| <> | ||
| <div> | ||
| <a href="https://vite.dev" target="_blank"> | ||
| <img src={viteLogo} className="logo" alt="Vite logo" /> | ||
| </a> | ||
| <a href="https://react.dev" target="_blank"> | ||
| <img src={reactLogo} className="logo react" alt="React logo" /> | ||
| </a> | ||
| </div> | ||
| <h1>Vite + React</h1> | ||
| <div className="card"> | ||
| <button onClick={() => setCount((count) => count + 1)}> | ||
| count is {count} | ||
| </button> | ||
| <p> | ||
| Edit <code>src/App.jsx</code> and save to test HMR | ||
| </p> | ||
| </div> | ||
| <p className="read-the-docs"> | ||
| Click on the Vite and React logos to learn more | ||
| </p> | ||
| </> | ||
| ) | ||
| } |
There was a problem hiding this comment.
This App component is the default starter code from the Vite template. According to the task requirements, this should be replaced with a version that imports and renders the Header, Welcome, and Article components.
src/App.jsx
Outdated
| import Article from './components/Article/Article'; | ||
| import Header from './components/Header/Header'; | ||
| import Welcome from './components/Welcome/Welcome'; | ||
| // Move each BEM block to a separate component (file) and import them here |
There was a problem hiding this comment.
This instructional comment can be removed now that the components have been successfully created and imported. It's good practice to clean up such comments from the final code.
| .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 for the navigation links. According to the task description, you could create a separate Navigation component. If you do that, it would be better to move these navigation-related styles to their own dedicated CSS file (e.g., Navigation.css) to improve modularity.
| @@ -1,5 +1,14 @@ | |||
| // import a css file containig welcome styles | |||
There was a problem hiding this comment.
This instructional comment can be removed from the final code for better readability.
| @@ -1,5 +1,14 @@ | |||
| // import a css file containig welcome styles | |||
| import './Welcome.css'; | |||
|
|
|||
| // Create a Welcome function returning the HTML of welcome block | |||
There was a problem hiding this comment.
This comment explaining the function's purpose is a bit redundant given the function's name. It can be safely removed.
| <span className="welcome__text">Sticky Header!</span> | ||
| </section> | ||
| ); | ||
| }; | ||
|
|
||
| // Add a default export statement for Welcome component to use it in the other files |
There was a problem hiding this comment.
This instructional comment can be removed as the export default statement is self-explanatory.
2 participants
https://shymkivvasyl.github.io/react_decompose/