Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there are still a few core requirements not met that can break tests and task compliance. First, App.jsx is importing Welcome, Header, and Article as named imports while the components are meant to be export default; you need to switch to default imports like import Welcome from './components/Welcome/Welcome'; so the imports match the exports and tests can resolve them correctly. Second, the JSX structure in App.jsx currently renders <Welcome />, <Header />, and <Article /> as siblings and then has a self-closing <main className="app" /> after them; the requirement is that these components are wrapped inside <main className="app"> ... </main>, and you should also remove the unnecessary {' '} fragments. Third, App.css still contains commented-out, component-specific styles (Header, Article, Navigation) instead of having them fully moved into their respective component CSS files; only global rules (like *, html, body, iframe) should remain in App.css. Finally, navigation-related styles are still inside Header.css rather than a dedicated Navigation component folder and CSS, which the task explicitly asks you to create. Once you fix the imports, wrap the components correctly in <main className="app">, move all remaining component styles out of App.css, and separate navigation into its own component/CSS, your solution will align well with the requirements—your existing component structure and per-component CSS are otherwise on the right track.
✨ 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/App.jsx
Outdated
| import { Welcome } from './components/Welcome/Welcome'; | ||
| import { Header } from './components/Header/Header'; | ||
| import { Article } from './components/Article/Article'; |
There was a problem hiding this comment.
You're importing the components with named imports ({ Welcome }, { Header }, { Article }). The task's Technical Constraints say: "Tests expect you to use export default for all the components." If those component files export default, import them as default imports, e.g. import Welcome from './components/Welcome/Welcome'.
src/App.jsx
Outdated
| <div> | ||
| {' '} | ||
| <Welcome /> <Header /> <Article /> <main className="app" />{' '} | ||
| </div> |
There was a problem hiding this comment.
The current JSX places <Welcome />, <Header />, and <Article /> as siblings and uses a self-closing <main className="app" /> after them. The Core Functional Requirements say: "Split the App.js into components based on CSS blocks." and the Implementation Details say: "App.js must be decomposed so that the existing Article, Header, and Welcome folders inside ./src/components/ with required files are used." Wrap these components inside <main className="app"> ... </main> so the app container (and related CSS block) actually contains these components. Also remove the unnecessary {' '} fragments.
src/App.css
Outdated
| } | ||
|
|
||
| .header { | ||
| /* .header { |
There was a problem hiding this comment.
This file still contains the .header block commented out here. The task requires that you "Split the CSS code too, corresponding to the decomposed components." Move header-related styles into the Header component folder (for example ./src/components/Header/Header.css).
This violates checklist item #2: "Create separate folder per component, where you can put all files (styles, components, and so on)."
src/App.css
Outdated
| .navigation__link:focus { | ||
| opacity: 1; | ||
| } | ||
| } */ |
There was a problem hiding this comment.
This closing comment indicates several component rules are commented together inside App.css. Rather than leaving component styles commented in App.css, extract them into component-level CSS files and import them from the corresponding components. Keep App.css for global styles only.
src/App.css
Outdated
| } */ | ||
|
|
||
| .article { | ||
| /* .article { |
There was a problem hiding this comment.
The .article block starts commented here — move these .article rules into the Article component's CSS (for example ./src/components/Article/Article.css) to satisfy the requirement to split CSS per component and folder.
src/App.css
Outdated
| margin: 3.5em auto; | ||
| } | ||
| } | ||
| } */ |
There was a problem hiding this comment.
This closes the commented .article block. After moving the article styles out, remove these commented blocks so App.css contains only global rules (e.g., iframe, *, html, body).
src/components/Welcome/Welcome.jsx
Outdated
|
|
||
| // Add a default export statement for Welcome component to use it in the other files | ||
|
|
||
| export const Welcome = () => { |
There was a problem hiding this comment.
This file uses a named export: export const Welcome = () => { ... }. The task's Technical Constraints state that tests expect export default for all components — define the component as const Welcome = () => { ... } and export it as the default to avoid import mismatches.
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.
This file contains navigation-related styles (.navigation__link) which violates the project requirement to "Split the CSS code too, corresponding to the decomposed components." Move navigation styles into a separate Navigation CSS file associated with a Navigation component (create a Navigation folder under ./src/components/).
src/components/Header/Header.css
Outdated
| .navigation__link:hover, | ||
| .navigation__link:focus { | ||
| opacity: 1; |
There was a problem hiding this comment.
Hover/focus rules for .navigation__link are also part of navigation styling and should be moved along with the base .navigation__link rules to the Navigation component's CSS to satisfy: "Move the navigation into a Navigation component (you may also create a folder for the Navigation component and move navigation there)."
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because all core requirements appear to be implemented correctly and there are no critical blockers. You’ve split App.jsx into separate Welcome, Header, Article, and Navigation components, each in its own folder, and each component correctly imports its own CSS and uses a default export. The Navigation styles have been moved into their own Navigation.css, and Header.jsx properly composes the Navigation component instead of mixing navigation markup/styles directly into the header. The JSX structure in App.jsx is valid, main wraps the components, and there are no obvious syntax or runtime issues in the reviewed files.
The remaining points are non-blocking but worth addressing on your own: make sure you’ve actually run npm test and check that all tests pass, and update the PR description with the correct demo link if your instructions require it. You can also optionally clean up instructional comments at the top of component files and consider using more consistent units in Welcome.css (e.g., rem instead of pt). Overall, this is a solid, clean implementation of the component decomposition and per-component CSS requirements—well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.