Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because the core requirements are now met: the app is decomposed into Welcome, Header, Article, and Navigation components, each with its own CSS file, and all components (including App) use export default as required. The navigation has been correctly moved into a dedicated Navigation component under src/components/Navigation/Navigation.jsx with Navigation.css, and it is integrated via the Header component, which aligns with the task’s intent. The CSS is properly split per component (Header.css, Article.css, Welcome.css, Navigation.css), and there are no obvious runtime or structural issues in the provided code.
To strengthen your work further, just make sure all new files are committed and that you run npm test locally to confirm everything passes, since test output isn’t visible here. As a small best-practice improvement, you can consider adding accessibility enhancements like an aria-label on the <nav> element and avoiding removing :focus-visible outlines on links. Overall, you’ve implemented the requested refactor cleanly and your component structure and styling organization look good.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| import React from 'react'; | ||
| import './App.css'; | ||
| // Move each BEM block to a separate component (file) and import them here | ||
| import Header from './components/Header/Header'; |
There was a problem hiding this comment.
The task explicitly requires: "Move the navigation into a Navigation component (may create a folder for it and move navigation there)" but I don't see any Navigation import or <Navigation /> rendered in App. If navigation was in this file originally, create ./src/components/Navigation/Navigation.jsx (and a CSS file) and import and render it here (probably above Header).
| import './App.css'; | ||
| // Move each BEM block to a separate component (file) and import them here | ||
| import Header from './components/Header/Header'; | ||
| import Article from './components/Article/Article'; |
There was a problem hiding this comment.
The requirements state: "CSS code should be split too (corresponding to the decomposed components)." This file still imports ./App.css (line 2), which suggests component CSS may not be split yet. Move styles into per-component CSS files (e.g. Header.css, Article.css, Welcome.css, Navigation.css) and import them inside each component instead of keeping everything in App.css.
| diam. | ||
| </p> | ||
| </article> | ||
| <Welcome /> |
There was a problem hiding this comment.
Technical constraint: "Tests expect you to use export default for all the components." App uses export default (line 17) — good. Make sure the imported components (Welcome, Header, Article, and the new Navigation) also use export default in their files so tests pass.
2 participants
DEMO LINK